-
Notifications
You must be signed in to change notification settings - Fork 32
Add invariants for while loops. #1375
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
Conversation
43d59aa
to
ffcd6b5
Compare
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.
Thanks, as we discussed there's a few thing we should change (rought notes from what we cahtted about):
- make the loop invariant macro produce two different calls to hax_lib according to wether we gave it a closure or not
- add a
loop_annotation_kind type e.g.
| LoopInvariant of { index_pat: pat option } | LoopDecreaseMeasure` - remove
extract_loop_variant
andextract_loop_invariant
, instead have aextract_loop_annotation
that takes out alet _ = ..; body
and produces(expr * expr * loop_annotation_kind)
- we should accept annotations in any order
Also, why is the decrease clause nat
? You can use the <<
operator in F* too reason about decreasing of any type. Then the default decreasing clause can be ()
.
Thanks, I pushed a new version which resolves all 4 points. About |
This PR is needed for some applications. Let's get this in? |
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.
Thanks!
I have two last comments, but mainly about style and readability.
If you can improve the code there a bit that'd be nice.
Thanks, I made some style improvements where you pointed and it is indeed much nicer. Merging now. |
Two jobs failed in the merge queue:
|
Let's revisit this again tomorrow. I would like to understand the bertie failure, and if it is an atomic update issue for ML-KEM, yes we can force the merge here. |
This failed but it also uses hax-lib from main so that is expected |
The Bertie failure is a dependency issue that I documented in cryspen/bertie#151. This is unrelated, should we wait for this to be fixed or force the merge anyway? |
Ok, after reviewing Bertie failure. I agree that this can be merged. |
This PR adds invariants to
while
loops (using the macrohax_lib::loop_invariant
that is already used in for loops). It also adds a macro to prove termination (hax_lib::loop_decreases
), that takes a decreasinghax_lib::Int
.