-
Notifications
You must be signed in to change notification settings - Fork 72
Update exercises #1024
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
Update exercises #1024
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,9 +6,7 @@ use creusot_contracts::*; | |
| pub fn all_zero(v: &mut Vec<u32>) { | ||
| let mut i = 0; | ||
| let old_v = snapshot! { v }; | ||
| // Until https://gitlab.inria.fr/why3/why3/-/merge_requests/667 is merged | ||
| // the following invariant is needed to allow Why3 to remember prophecies dont change | ||
| #[invariant(proph_const, ^v == ^old_v.inner())] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keep in mind, this is actually now needed again by the change to coma. At least until 0.2 comes out (soon ish).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, so that’s why I needed |
||
|
|
||
| while i < v.len() { | ||
| v[i] = 0; | ||
| i += 1; | ||
|
|
||
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.
Are you saying compilation/translation to Coma fails without this? Or did you have to add this to prove whichever spec you wanted to prove?
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.
Ok, this happened after adding the specification.
For the code above, I get the following error message:
I can also use
tgt@instead of*tgtin the specifications, but then I get an additional message aboutShallowModelnot being implemented byT.I’m on commit
917bcee1, if this is of interest.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.
Indeed, you need to add this bound. I think my original intent was to have that be part of the exercise, but its probably best to provide that for the user.
Uh oh!
There was an error while loading. Please reload this page.
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.
In fact, I started to wonder whether adding the
DeepModeltrait bound was intended to be part of the exercise once I read @sarsko’s comment 😅 I guess the actual issue is thatDeepModelis not mentioned in the guide. Otherwise I would have felt more comfortable with the exercise. And if you have the page anyway, you could use#[diagnostic::on_unimplemented]and link the respective page, which would IMO be the best experience for newcomers 😉