-
Notifications
You must be signed in to change notification settings - Fork 52
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
Closure inference (again) #1294
Conversation
85ffb7e
to
06e7432
Compare
closure_args.insert(0, closure_env.clone()); | ||
|
||
let base = Term { | ||
kind: TermKind::Postcondition { item: def_id, args, params: closure_args }, | ||
ty: self.types.bool, | ||
span, | ||
}; | ||
base |
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.
I don't understand all the details, but I find quite suspicious that there is no special case here when postcond_kind
is FnMut
.
More generally, I would have expected to only generate the term when closure_kind
is equal to postcond_kind
, and rely on the various compatibility laws to let the provers deduce the other cases.
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.
That's because what matters here is mainly the underlying type of the closure:
- If it's
FnOnce
it cannot have anFnMut
- If its
Fn
then itsFnMut
instance ignores the result (since it must be equal to the initial parameter), this fact being provided by the compatibility law.
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 fact being provided by the compatibility law.
Well, then, why not relying fully on the compatibility law for the whole post-condition?
I tried using the branch, and ran into a crash. Here is a minimal example: pub fn foo() {
let my_closure = |x: Option<i32>| match x {
Some(y) => y,
None => unreachable!("unwrapped None"),
};
} This crashes with |
Does that crash happen on master? |
Yes, it does 😕 |
06e7432
to
8ed9b72
Compare
I opened #1312 to track this. |
@@ -879,11 +927,11 @@ impl<'tcx> TranslationCtx<'tcx> { | |||
let self_ = Term::var(Symbol::intern("self"), env_ty); |
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 the second definition of self_
, so this one can be removed (and the let mut csubst
just below can be moved inside the if to avoid a borrow checker error)
Maybe related to @jhjourdan's remark: it seems code using spec inference with extern crate creusot_contracts;
use creusot_contracts::*;
#[requires(f.precondition((2i32,)))]
#[ensures(exists<f_fin: F> f.postcondition_mut((2i32,), f_fin, result) && resolve(&f_fin))]
fn uses_closure_mut<F: FnMut(i32) -> i32>(mut f: F) -> i32 {
f(2)
}
#[requires(f.precondition((2i32,)))]
#[ensures(f.postcondition((2i32,), result))]
fn uses_closure<F: Fn(i32) -> i32>(mut f: F) -> i32 {
f(2)
}
fn foo() {
let f =
// #[requires(x@ + 1 <= i32::MAX@)] // works with the explicit spec
// #[ensures(result@ == x@ + 1)]
|x| x + 1;
let y = uses_closure_mut(f); // works with `uses_closure`
assert!(y == 3);
} The |
ok I'm trying to get this passing but the
|
Obvious question: have you tried to reset creusot's config ? |
Yea I tried via a new cargo creusot setup install, however it doesn't seem to work. |
Then @Armael , any idea where this could come from? |
ok I've gotten the tests building locally but I can't replicate the failure of CI on |
oof, it comes from the |
@arnaudgolfouse can you update the tests using your computer? |
3e2bc43
to
6d2a5f0
Compare
6d2a5f0
to
e4ecd45
Compare
Ok, I finally got to do that ! |
This (ideally) should not be visible to Creusot, no? |
I don't see how that can be avoided? The best thing would be to specify the target triple in the test suite so that we are all compiling proofs for x86-64. |
I'm going to go ahead and merge this and I'll work on the missing / broken cases separately. |
Adds support for inferring the specifications of closures through Coma's novel
extspec
mechanism.Using this, you can exclude a contract for your closure and Creusot will instead infer its contract as roughly the WP and SP for the pre and post conditions respectively. I've updated some of our iterator tests to use this feature.