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

Handle delayed opaque proofs in Print Assumptions #14382

Merged
merged 4 commits into from
May 25, 2021

Conversation

ppedrot
Copy link
Member

@ppedrot ppedrot commented May 25, 2021

Instead of using Global.body_of_constant_body and tinkering with the result we roll up our own version of the accessor which is delayed-proof-aware.

Fixes #13589: Print Assumptions for vok.

Supersedes #13614.

@ppedrot ppedrot requested a review from a team as a code owner May 25, 2021 09:32
@ppedrot ppedrot force-pushed the assumptions-delayed-opaque branch from bc0c623 to 382c226 Compare May 25, 2021 09:56
@SkySkimmer
Copy link
Contributor

@SkySkimmer SkySkimmer self-assigned this May 25, 2021
@SkySkimmer SkySkimmer added the kind: fix This fixes a bug or incorrect documentation. label May 25, 2021
@SkySkimmer SkySkimmer added this to the 8.14+rc1 milestone May 25, 2021
@ppedrot
Copy link
Member Author

ppedrot commented May 25, 2021

@SkySkimmer yes, it seems like the binary called by the script doesn't like the -vok flag. Unfortunately I don't know what happens exactly in async mode, isn't it supposed to be always a call to coqc?

Instead of using Global.body_of_constant_body and tinkering with the result
we roll up our own version of the accessor which is delayed-proof-aware.

Fixes coq#13589: Print Assumptions for vok.
I find the code clearer that way, as it clearly indicates intent.
@ppedrot ppedrot force-pushed the assumptions-delayed-opaque branch from 382c226 to 060deaf Compare May 25, 2021 11:53
@ppedrot
Copy link
Member Author

ppedrot commented May 25, 2021

Let's try a bit of cargo culting to see whether it works. I mimicked the script in the vos folder, which calls the coqc binary instead of going through the variable exposed by the test-suite wrapper.

@SkySkimmer
Copy link
Contributor

I guess -vok is passed to the worker (incorrectly).

@ppedrot ppedrot force-pushed the assumptions-delayed-opaque branch from 060deaf to 198acb4 Compare May 25, 2021 11:59
@ppedrot
Copy link
Member Author

ppedrot commented May 25, 2021

Seems to be OK now.

@SkySkimmer
Copy link
Contributor

@coqbot: merge now

@coqbot-app coqbot-app bot merged commit 5149bd0 into coq:master May 25, 2021
@ppedrot ppedrot deleted the assumptions-delayed-opaque branch May 25, 2021 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: fix This fixes a bug or incorrect documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Print Assumptions for vok
2 participants