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

Clean the constr-as-hint API #13139

Merged
merged 7 commits into from
Nov 6, 2020
Merged

Conversation

ppedrot
Copy link
Member

@ppedrot ppedrot commented Oct 4, 2020

This PR hides internal details of the API allowing to declare arbitrary terms as hints. Furthermore, in order to reduce the underspecified behaviours exposed to the end-user, this changes the semantics of open terms added as hints. Before the PR, it would implicitly quantify over the dangling evars in a fragile way (amongst others sensitive to the evar naming), now it simply errors out and one needs to explicitly quantify over the remaining holes. Note that I've only found one place where this (undocumented) feature was used in the whole CI, namely in fiat-crypto.

Overlays:

ppedrot added a commit to ppedrot/fiat that referenced this pull request Oct 5, 2020
ppedrot added a commit to ppedrot/fiat-crypto that referenced this pull request Oct 5, 2020
JasonGross pushed a commit to mit-plv/fiat-crypto that referenced this pull request Oct 5, 2020
ppedrot added a commit to ppedrot/Coq-Equations that referenced this pull request Oct 6, 2020
ppedrot added a commit to ppedrot/Coq-Equations that referenced this pull request Oct 14, 2020
@ppedrot ppedrot force-pushed the clean-hint-constr branch 2 times, most recently from f02ddb4 to 4eb7898 Compare October 14, 2020 09:25
@ppedrot ppedrot marked this pull request as ready for review October 14, 2020 09:34
@ppedrot ppedrot requested review from a team as code owners October 14, 2020 09:34
@ppedrot
Copy link
Member Author

ppedrot commented Oct 16, 2020

@mattam82 would you assign?

@ppedrot
Copy link
Member Author

ppedrot commented Oct 19, 2020

@mattam82 ping

1 similar comment
@ppedrot
Copy link
Member Author

ppedrot commented Nov 4, 2020

@mattam82 ping

We know statically that only global references are passed to make_resolves.
The prepare_hint function was trying to requantify over all undefined evars,
but actually this should not happen when defining generic terms as hints through
some Hint vernacular command.
We reuse the standard code path for term interpretation instead of trying
to mangle it.
ppedrot added a commit to ppedrot/fiat that referenced this pull request Nov 4, 2020
@ppedrot
Copy link
Member Author

ppedrot commented Nov 6, 2020

@SkySkimmer maybe then, since @mattam82 seems trapped in a spatio-temporal anomaly?

@SkySkimmer
Copy link
Contributor

Please link overlay PRs

@SkySkimmer SkySkimmer self-assigned this Nov 6, 2020
@SkySkimmer SkySkimmer added the kind: cleanup Code removal, deprecation, refactorings, etc. label Nov 6, 2020
@SkySkimmer SkySkimmer added this to the 8.13+beta1 milestone Nov 6, 2020
Copy link
Contributor

@SkySkimmer SkySkimmer left a comment

Choose a reason for hiding this comment

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

Seems ok

@ppedrot
Copy link
Member Author

ppedrot commented Nov 6, 2020

(Done.)

@SkySkimmer
Copy link
Contributor

@coqbot: merge now

@coqbot-app coqbot-app bot merged commit 6cebd41 into coq:master Nov 6, 2020
@coqbot-app
Copy link
Contributor

coqbot-app bot commented Nov 6, 2020

@SkySkimmer: Please take care of the following overlays:

  • 13139-ppedrot-clean-hint-constr.sh

ppedrot added a commit to mattam82/Coq-Equations that referenced this pull request Nov 6, 2020
JasonGross pushed a commit to mit-plv/fiat that referenced this pull request Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: cleanup Code removal, deprecation, refactorings, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants