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

Adapt w.r.t coq/coq#20167. #1023

Merged
merged 2 commits into from
Feb 4, 2025
Merged

Adapt w.r.t coq/coq#20167. #1023

merged 2 commits into from
Feb 4, 2025

Conversation

ppedrot
Copy link
Member

@ppedrot ppedrot commented Jan 31, 2025

This is backwards compatible.

I copied the function here because Tok.equal in Coq is actually not the equality function, there is special-casing of KEYWORD and IDENT that used to be there to support a hack in the parser.

@ppedrot
Copy link
Member Author

ppedrot commented Feb 1, 2025

I'm not sure why the CI is still broken here, but it should really be backwards compatible, I tested it locally.

@SkySkimmer
Copy link
Contributor

The CI for master seems to have issues with the stdlib split or renaming

Error: I don't know about package coq-stdlib (passed through --only-packages)

CI for <9.0 correctly says

Error (warning 8 [partial-match]): this pattern-matching is not exhaustive.
Here is an example of a case that is not matched:
(PATTERNIDENT _, _)

@gares
Copy link
Member

gares commented Feb 2, 2025

What is Tok.equal then, and why it is called like that?

@ppedrot
Copy link
Member Author

ppedrot commented Feb 2, 2025

@gares it was introduced for backward compatibility with camlp4 quite a long time ago, and this IDENT ≤ KEYWORD rule was added to work around the fact that an indent could be turned at any point into a keyword. So that's a plain hack.

@ppedrot
Copy link
Member Author

ppedrot commented Feb 4, 2025

Anyone merging this?

@SkySkimmer
Copy link
Contributor

This branch has conflicts that must be resolved

@ppedrot
Copy link
Member Author

ppedrot commented Feb 4, 2025

Rebased.

@SkySkimmer
Copy link
Contributor

You still have the non exhaustive match issue on older Coq versions

@ppedrot
Copy link
Member Author

ppedrot commented Feb 4, 2025

Hopefully this does not trigger warnings or something.

@SkySkimmer SkySkimmer merged commit 85594dd into coq:main Feb 4, 2025
23 of 27 checks passed
@ppedrot ppedrot deleted the tok-rm-equal branch February 4, 2025 10:47
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.

4 participants