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

Change specification of declared constant and co #822

Merged
merged 4 commits into from Jan 3, 2023

Conversation

tabareau
Copy link
Member

This PR changes the definition of declared_constant and declared_minductive
to make us of In instead of the more operational lookup_env approach.

@tabareau
Copy link
Member Author

I am not sure that the way I have adapted some proofs is optimal, in particular in Erasure, but those proofs are not very well structured, so it seems hard to organize better without a huge refactoring.

@yannl35133
Copy link
Contributor

I may be missing some context, but what is the issue with the existing definitions ? Are the _gen variants still useful ?

@tabareau
Copy link
Member Author

The idea is that the use of lookup_global in the specification was a bit to concrete, using In seems more accurate. The _gen variants are used to relate the abstract specification to presentation using a concrete or abstract lookup function. In PCUIC, we use the concrete lookup because it provides simple reasoning. In Safechecker we use an abstract lookup that is latter instantiated with an efficient lookup on AVL trees.

@tabareau
Copy link
Member Author

I may be missing some context, but what is the issue with the existing definitions ? Are the _gen variants still useful ?

@yannl35133 sorry, I wanted to assign the PR to Yannick but I misclicked.

@yannl35133
Copy link
Contributor

Right, that makes more sense

@yforster yforster removed the request for review from yannl35133 December 23, 2022 08:15
Copy link
Member

@yforster yforster left a comment

Choose a reason for hiding this comment

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

The amount of unshelve epose proof (...); eauto lets me think that there should be a primitive in Coq for something like that

@JasonGross
Copy link
Contributor

Does this conflict with #816?

@tabareau
Copy link
Member Author

I don't think it conflicts with #816 as it mainly changes basic definitions in EnvironmentTyping.v, which is almost not touched by #816. It should not be difficult to merge one after the other. We will see that beginning of January.

@tabareau tabareau merged commit 6e942ea into coq-8.16 Jan 3, 2023
@tabareau tabareau deleted the change-specification-of-declared-constant-and-co branch January 3, 2023 14:10
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.

None yet

4 participants