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

Hashconsed arrows/abstractions in the SMT encoding must be abstracted over their free variables #3028

Merged
merged 12 commits into from
Aug 23, 2023

Conversation

mtzguido
Copy link
Member

A bug and a fix in this PR, this was previously accepted:

let mk t = unit -> Dv t
let _ = assert (mk int == mk bool)

which is clearly pretty bad. The problem was that we were encoding the abstraction univ -> Dv t as a single fresh top-level SMT term, with a has computed from its definition. But, this fails to consider that a t is free in it, and therefore all mk _ calls are provably equal by the SMT. Abstractions had a similar issue.

The fix is to make these hashconsed terms dependent on their free variables, so the encoding of unit -> Dv t is now a Term to Term function. This is pretty simple and fixes the problem. Surprisingly there was only a single regressions in the examples.

This PR also removes the pre-typing axiom for impure functions, as we think it's probably not very useful.

@mtzguido
Copy link
Member Author

Sadly this breaks HACL:

/home/guido/r/fstar/master/everest-TEMP/hacl-star/providers/evercrypt/fst/EverCrypt.DRBG.fst(70,14-78,17): (Error 276) queries-EverCrypt.DRBG-28.smt2: Unexpected output from Z3: (error "line 766902 column 0: unknown constant @x0")

looking at it.

@mtzguido mtzguido marked this pull request as ready for review August 23, 2023 05:46
@mtzguido mtzguido merged commit f3229b2 into FStarLang:master Aug 23, 2023
@mtzguido mtzguido deleted the fix-hashcons-env branch August 23, 2023 23:30
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.

1 participant