-
Notifications
You must be signed in to change notification settings - Fork 64
Logical functions aliases #1346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Logical functions aliases #1346
Conversation
bb131ae to
9a174b3
Compare
9a174b3 to
ab7d097
Compare
jhjourdan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very interesting feature, howeer, there are a few important parts missing:
-
Support for aliases in traits. A simple constraint could be to force the alias to belong to the same trait. Then, we sould add a refinement check for implementations (i.e., implementors declare compatible aliases).
-
Uses in creusot_contracts, to get rid of all the
_ghostand_logicfunctions. -
Use this for
Ghost::Deref,Rc::Deref,Snapshot::Deref, etc... and get rid of the special cases for this in Creusot's code.
| } | ||
| }; | ||
| let tokens_2 = tokens.clone(); | ||
| let func = parse_macro_input!(tokens_2 as syn::ImplItemFn); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that you force the method to have a body, which forbids logical aliases for trait methods. This seems restrictive. Is this intended? A simple fix may be to use FnOrMethod here instead.
| let pat = &pat_type.pat; | ||
| quote!(#pat) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assumes that the pattern is also a term, which is not always the case.
This is probably fine in this case, but it would be better to detect this in the macro, and emit an error.
| /// | ||
| /// They can later be retrieved using [`Self::logical_alias`]. | ||
| pub(crate) fn load_logical_aliases(&mut self) { | ||
| // FIXME: what about functions from another crate? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cannot we simply reject them?
| } | ||
| _ => unreachable!(), | ||
| }; | ||
| if t1 != t2 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we check the equality of constant param names, too? It seems weird to do that for types and lifetimes, but not for constants.
| let mut program_subst = program_subst.iter().map(|arg| arg.kind()); | ||
| let mut logic_subst = logic_subst.iter().map(|arg| arg.kind()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing that directly in the loop would be clearer and shorter.
| let logic_subst = erased_identity_for_item(tcx, logic_id); | ||
| let mut program_subst = program_subst.iter().map(|arg| arg.kind()); | ||
| let mut logic_subst = logic_subst.iter().map(|arg| arg.kind()); | ||
| loop { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From itertools:
for (prog_arg, logic_arg) in program_subst.zip_longest(logic_subst) { ... }|
Perhaps we could ask Maël Coulmance to work on this? |
Example