Skip to content
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

A fix in Core, revealed by FStarLang/pulse#100 #3326

Merged
merged 7 commits into from
Jun 25, 2024

Conversation

mtzguido
Copy link
Member

When checking equality via Core between x:t{p} and t, we were completely ignoring the refinement. We need to add a guard to check that it is constantly true. Also add a local regression test.

Fixes FStarLang/pulse#100

mtzguido added 7 commits June 24, 2024 22:37
This was not checking that `p` is constantly true, which is unsound. See
FStarLang/pulse#100.
The Core fix change the shape of some guards, and the goal
is slightly different. Do some normalization to get it to
how it was before and also a test without a tactic.
mtzguido added a commit to mtzguido/pulse that referenced this pull request Jun 25, 2024
mtzguido added a commit to mtzguido/pulse that referenced this pull request Jun 25, 2024
After fixing FStarLang#100 (see also FStarLang/FStar#3326) this now fails
to match a goal of shape

     A.pts_to ha.tmp (reveal #(Seq.seq U8.t) ?u)

since in the context we have

     A.pts_to ha.tmp (reveal #(Seq.lseq U8.t 32) _)

To fix this, just avoid the use of `lseq` and use pure predicates
instead. Requires a tiny change to the blake2b assumed API.
@mtzguido
Copy link
Member Author

Hi @TWal, could you maybe try this PR in your projects to see if there's an impact?

Asking since the only regression this caused (besides a tiny one in Pulse) was in the #2756 repro, which you filed. It looks like it could just be a bit of extra noise and only affecting the repro, but would be good if you can confirm.

@TWal
Copy link
Contributor

TWal commented Jun 25, 2024

My projects still work with this PR!

@cmovcc
Copy link
Contributor

cmovcc commented Jun 25, 2024

Hi, I can confirm my project also still works with this PR!

@mtzguido
Copy link
Member Author

Thank you both!

@mtzguido mtzguido merged commit 91d06ee into FStarLang:master Jun 25, 2024
2 checks passed
@mtzguido mtzguido deleted the fix_pulse_100 branch June 25, 2024 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ill-typed spec is accepted
3 participants