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

Notation wish #17845: add support for notations with recursive binders involving "let" and "match" #17856

Conversation

herbelin
Copy link
Member

@herbelin herbelin commented Jul 14, 2023

This seems to work for parsing.

Recognizing instances of the recursive pattern of the notations for printing is more aleatory: it interacts with pattern-matching decompilation (done earlier in detyping) in ways difficult to control.

Fixes #17845

  • Added / updated test-suite.
  • Added changelog.

@herbelin herbelin added part: notations The notation system. kind: enhancement Enhancement to an existing user-facing feature, tactic, etc. labels Jul 14, 2023
@herbelin herbelin added this to the 8.19+rc1 milestone Jul 14, 2023
@herbelin herbelin requested review from a team as code owners July 14, 2023 09:33
@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Jul 14, 2023
herbelin added a commit to herbelin/github-coq that referenced this pull request Jul 14, 2023
@herbelin herbelin added the request: full CI Use this label when you want your next push to trigger a full CI. label Jul 14, 2023
herbelin added a commit to herbelin/github-coq that referenced this pull request Jul 14, 2023
@herbelin herbelin force-pushed the master+more-general-form-binders-recursive-notation branch from f831d98 to 80165fa Compare July 14, 2023 12:32
@coqbot-app coqbot-app bot removed request: full CI Use this label when you want your next push to trigger a full CI. needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. labels Jul 14, 2023
@herbelin herbelin added the request: full CI Use this label when you want your next push to trigger a full CI. label Jul 16, 2023
herbelin added a commit to herbelin/github-coq that referenced this pull request Jul 16, 2023
@herbelin herbelin force-pushed the master+more-general-form-binders-recursive-notation branch from 80165fa to 89481c2 Compare July 16, 2023 08:22
@coqbot-app coqbot-app bot removed the request: full CI Use this label when you want your next push to trigger a full CI. label Jul 16, 2023
herbelin added a commit to herbelin/github-coq that referenced this pull request Jul 17, 2023
@herbelin herbelin force-pushed the master+more-general-form-binders-recursive-notation branch from 89481c2 to 6ce2427 Compare July 17, 2023 09:52
@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Jul 17, 2023
@herbelin
Copy link
Member Author

I would say that even if printing does not work symmetrically due to the difficult-to-control interaction with pattern-matching decompilation, I think that this is worth to go.

Note: I added a word on the changelog to say that the support is for parsing ("not for printing" implicitly implied). Not rerunning full ci though, as it is already green.

herbelin added a commit to herbelin/github-coq that referenced this pull request Aug 5, 2023
@herbelin herbelin force-pushed the master+more-general-form-binders-recursive-notation branch from 6ce2427 to f7b49b9 Compare August 5, 2023 08:45
@herbelin
Copy link
Member Author

herbelin commented Sep 7, 2023

@coqbot run full ci

@coqbot-app coqbot-app bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Sep 7, 2023
@proux01 proux01 self-assigned this Nov 3, 2023
@proux01
Copy link
Contributor

proux01 commented Nov 3, 2023

Rerunning CI just to be safe before merging.

@coqbot run full ci

Copy link
Contributor

coqbot-app bot commented Nov 3, 2023

The job library:ci-fiat_crypto_legacy has failed in allow failure mode
ping @JasonGross

@github-actions github-actions bot added the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label Nov 13, 2023
Probably also fix a bug where the second occurrence of a same pair of
recursive binders did not check that the bodies of fun/forall were the
same.
Shadowing of variables not treated though (but it cannot arrive in
interpretation of notations).

Allow to make subst_glob_vars working on other kinds of patterns than
lam and prod (such as "match" and "let").
herbelin added a commit to herbelin/github-coq that referenced this pull request Nov 16, 2023
@herbelin herbelin force-pushed the master+more-general-form-binders-recursive-notation branch from f7b49b9 to 16b0e80 Compare November 16, 2023 15:19
@coqbot-app coqbot-app bot added needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. and removed needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. labels Nov 16, 2023
…/match.

Printing notations with complex nesting of pattern-matching that
includes variables for recursive binders would typically require to
intertwin pattern-matching decompilation with search for notations.
@proux01 proux01 force-pushed the master+more-general-form-binders-recursive-notation branch from 16b0e80 to 732a038 Compare November 17, 2023 10:06
@proux01
Copy link
Contributor

proux01 commented Nov 17, 2023

@herbelin sorry it appears I completely forget that one. Fixed the compilation issue do tu my delay.

@proux01
Copy link
Contributor

proux01 commented Nov 17, 2023

@coqbot run full ci

@coqbot-app coqbot-app bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Nov 17, 2023
@proux01
Copy link
Contributor

proux01 commented Nov 18, 2023

@coqbot merge now

@coqbot-app coqbot-app bot merged commit 257b88d into coq:master Nov 18, 2023
15 of 17 checks passed
@proux01
Copy link
Contributor

proux01 commented Nov 20, 2023

@herbelin sorry again for the delay here, I apparently just forgot to merge, feel free to "harass" me in such cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Enhancement to an existing user-facing feature, tactic, etc. part: notations The notation system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Coq does not seem to provide a way to adjust recursive binders
2 participants