Clarifying the logic of levels in custom and constr entry rules (reopening of #13025) #17117
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
herbelin
added
kind: fix
coqbot-app
bot
removed
the
request: full CI
This was referenced Jan 14, 2023
Merged
herbelin
added a commit
to herbelin/github-coq
that referenced
this pull request
Jan 15, 2023
herbelin
force-pushed
the
master+fix-printing-custom-no-level
branch
from
1e916c8
to
c827679
Compare
coqbot-app
bot
added
the
needs: full CI
herbelin
added
request: full CI
|
@coqbot run full CI |
coqbot-app
bot
removed
the
request: full CI
SkySkimmer
reviewed
Jan 16, 2023
herbelin
added a commit
to herbelin/github-coq
that referenced
this pull request
Jan 17, 2023
herbelin
force-pushed
the
master+fix-printing-custom-no-level
branch
from
c827679
to
f696872
Compare
coqbot-app
bot
added
the
needs: full CI
|
@coqbot run full CI |
coqbot-app
bot
removed
the
needs: full CI
|
This was ready and needing a rebase before I reopened it. @proux01, you might be the best expert for assigning! |
github-actions
bot
added
the
needs: rebase
proux01
approved these changes
Feb 15, 2023
interp/constrexpr.ml
Outdated
| type notation_entry_level = InConstrEntrySomeLevel | InCustomEntryLevel of string * entry_level | ||
|
|
||
| (* A notation entry with the level where the notation lives *) | ||
| type notation_entry_level = notation_entry * entry_level |
There was a problem hiding this comment.
This would have been a good opportunity to use a record IMHO.
interp/constrexpr.ml
Outdated
| type notation_entry_level = notation_entry * entry_level | ||
|
|
||
| (* Notation subentries, to be associated to the variables of the notation *) | ||
| type notation_entry_relative_level = notation_entry * (entry_relative_level * side option) |
| | None -> raise No_match | ||
| | Some coercion -> | ||
|
|
||
| let scopes = (InConstrEntrySomeLevel, snd scopes) in | ||
| let scopes = ((InConstrEntry,(LevelSome (*??*),None)), snd scopes) in |
| let level_of_notation ntn = | ||
| if is_numeral_in_constr (decompose_notation_key ntn) then | ||
| (* A primitive notation *) | ||
| (fst ntn, 0, []) (* TODO: string notations*) |
There was a problem hiding this comment.
Would the string case be hard to complete?
|
This PR is now at risk of being auto-closed by the bot due to a rebase not happening. We should really be more proactive in merging intermediate cleanup PRs otherwise they just vanish. |
proux01
force-pushed
the
master+fix-printing-custom-no-level
branch
from
af26871
to
24fa443
Compare
proux01
pushed a commit
to herbelin/github-coq
that referenced
this pull request
May 30, 2023
coqbot-app
bot
removed
needs: rebase
github-actions
bot
added
the
needs: rebase
|
@coqbot run full ci |
terminology already in used).
informations about the subentries: we remember whether we are at any level, at the next level or at the self level. Incidentally, we also take into account option Printing Parentheses in custom entries. This is a more principled fix to coq#12775/coq#13018 than coq#13073 (printing bugs in custom entry rules with no explicit level). Compared to "subentries", the name "entry_relative" insists on the syntactic structure of the type rather than on its semantic origin, since, after all, subentries are promoted as entries in the recursive printing loop.
proux01
pushed a commit
to herbelin/github-coq
that referenced
this pull request
May 30, 2023
proux01
force-pushed
the
master+fix-printing-custom-no-level
branch
from
24fa443
to
48b5178
Compare
coqbot-app
bot
removed
the
needs: rebase
We hard-wire a coercion in constr from level 0 to the top level.
proux01
force-pushed
the
master+fix-printing-custom-no-level
branch
from
48b5178
to
43bface
Compare
|
@coqbot run full ci |
coqbot-app
bot
removed
the
needs: full CI
|
@coqbot merge now |
|
@proux01: Please take care of the following overlays:
|
herbelin
added a commit
to herbelin/github-coq
that referenced
this pull request
Jul 6, 2023
…tuples. See comment at coq#17117 (comment).
1 task
herbelin
added a commit
to herbelin/github-coq
that referenced
this pull request
Jul 6, 2023
…tuples. See comment at coq#17117 (comment).
This was referenced Jul 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Kind: cleanup + small enhancements (reopening of #13025)
Note: This PR was originally to fix #13018 and #12775 but those were finally fixed by #13073. This PR is thus now about small enhancements and a more principled code. The header has been updated.
We introduce a type
notation_subentry_levelto preserve more informations about the subentries: we remember whether we are at any level, at the next level or at the self level.We take into account option
Printing Parenthesesin custom entriesWe provide a more principled fix to custom entry without explicit level prints as constr instead #13018 and Parens not printing right in custom grammar #12775 which subsumes the quicker fix done in A temporary fix of #13018 and #12775 for branch 8.12 (bis) #13073 (the loss of the
any levelinformation was the cause of the bug; A temporary fix of #13018 and #12775 for branch 8.12 (bis) #13073 hackily fixed it by usingmax_intas the default possible highest level in a custom entry)We warn about coercions not used for printing
Added / updated test-suite
Documented API change
Change log (though it is rather anecdotical, observationally)
Synchronous overlay