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

Refactored saw-core-coq #1928

Merged
merged 15 commits into from
Sep 2, 2023
Merged

Refactored saw-core-coq #1928

merged 15 commits into from
Sep 2, 2023

Conversation

eddywestbrook
Copy link
Contributor

This PR refactors the saw-core-coq translator so that all the local variable information is in a reader effect and not a state effect, making it clearer where in the translator code variables are bound. It also fixes #1927 by invalidating the term sharing cache whenever a binder is entered.

Eddy Westbrook added 4 commits August 31, 2023 15:05
… and pis when they are type-checked, in order to make the Coq translation work in cases where these argument types have identifiers that translate to alternate Coq definitions
… information is in a reader effect and not a state effect, making it clearer where in the translator code variables are bound; also fixed #1927
Copy link
Contributor

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the example from #1927 as a test case?


Also, there's an error in mbox_proofs.v (CI log) that needs to be addressed:

File "./mbox_proofs.v", line 67, characters 12-13:
Error: The reference e was not found in the current environment.

Where e is found here.

saw-core/src/Verifier/SAW/Term/Functor.hs Show resolved Hide resolved
saw-core-coq/src/Verifier/SAW/Translation/Coq/Term.hs Outdated Show resolved Hide resolved
saw-core-coq/src/Verifier/SAW/Translation/Coq/Term.hs Outdated Show resolved Hide resolved
saw-core-coq/src/Verifier/SAW/Translation/Coq/Term.hs Outdated Show resolved Hide resolved
saw-core-coq/src/Verifier/SAW/Translation/Coq/Term.hs Outdated Show resolved Hide resolved
@eddywestbrook
Copy link
Contributor Author

@RyanGlScott you must be using a different architecture than I am (I'm on an X86 MacBook), because I can't even compile mbox_gen.v on my machine: it's creating a very large constant for a symbol called "mem_in_use", that is created from a zero initializer in the LLVM globals. It should be a simple fix, but it will be a slightly weird addition to this PR...

@RyanGlScott
Copy link
Contributor

I don't think the architecture has anything to do with it, since I'm also using an x86-64 machine (as are the CI runners). The mbox proofs have been known to take a while to load, however—see #1843.

Eddy Westbrook added 2 commits September 1, 2023 08:10
… directly translates an LLVM zero initializer to a Heapster permission + SAW core term, so that we use repeatBVVec in the SAW term rather than a giant vector literal
@eddywestbrook
Copy link
Contributor Author

Huh. Well, it seems like that was the culprit.

@RyanGlScott
Copy link
Contributor

The heapster-tests now run within 10 minutes or so after your most recent changes—hooray! As a result, this should fix #1843.

@eddywestbrook
Copy link
Contributor Author

Do we even have any tests for the Coq translator? What we be a good way to organize them, and where should we put them?

@Ptival
Copy link
Contributor

Ptival commented Sep 1, 2023

The only "tests" I had was to re-run the Makefile in the coq directory, and then make sure all the generated Coq files compile. Not sure more tests were ever created.

@RyanGlScott
Copy link
Contributor

I was also reminded of this issue, which seems to be in very similar territory to what this PR accomplishes. Would you say that this PR resolves the issue?

@eddywestbrook
Copy link
Contributor Author

I was also reminded of this issue, which seems to be in very similar territory to what this PR accomplishes. Would you say that this PR resolves the issue?

Huh, look at that, I guess I don't ever say anything new. Yes, this PR resolves that issue.

@eddywestbrook eddywestbrook added the subsystem: saw-core-coq Issues related to converting saw-core to Gallina for use with the Coq/Rocq theorem prover label Sep 1, 2023
@eddywestbrook eddywestbrook added the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Sep 2, 2023
@eddywestbrook eddywestbrook merged commit 18ca05a into master Sep 2, 2023
@mergify mergify bot deleted the saw-core-coq/var-binding-fix branch September 2, 2023 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run subsystem: saw-core-coq Issues related to converting saw-core to Gallina for use with the Coq/Rocq theorem prover
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Coq translator incorrectly handles let-bindings in argument types
3 participants