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

Remove custom extraction of comparison to int in ExtrOCamlInt63 #15294

Merged
merged 1 commit into from Dec 14, 2021

Conversation

Lysxia
Copy link
Contributor

@Lysxia Lysxia commented Dec 5, 2021

Fixes #15280

  • Added changelog.
  • Opened overlay pull requests.

@Lysxia Lysxia requested a review from a team as a code owner December 5, 2021 21:59
@herbelin
Copy link
Member

herbelin commented Dec 6, 2021

Cc @vbgl to be sure.

@herbelin
Copy link
Member

herbelin commented Dec 6, 2021

Cc also @mattam82, since Metacoq had to take into account in {erasure/safechecker}/theories/Extraction.v the choice made by ExtrOCamlInt63.v.

@Lysxia Lysxia force-pushed the extract-comparison branch 2 times, most recently from cce9a7f to fb814b9 Compare December 6, 2021 15:30
@herbelin
Copy link
Member

herbelin commented Dec 7, 2021

Then, I guess that it just remains to do an overlay in MetaCoq to change

Extract Constant ascii_compare =>
 "fun x y -> match Char.compare x y with 0 -> Eq | x when x < 0 -> Lt | _ -> Gt".

into

Extract Constant ascii_compare =>
 "fun x y -> match Char.compare x y with 0 -> Eq | x when x < 0 -> Lt | _ -> Gt".

or something like that (I did not double check) in files erasure/theories/Extraction.v, template-coq/theories/Extraction.v and pcuic/theories/Extraction.v. [Edited to fix code]

@herbelin
Copy link
Member

herbelin commented Dec 7, 2021

Actually, couldn't we add also a

Extract Constant compare =>
 "fun x y -> match Char.compare x y with 0 -> Eq | x when x < 0 -> Lt | _ -> Gt".

in file theories/extraction/ExtrOcamlString.v of the Coq archive.

@Lysxia
Copy link
Contributor Author

Lysxia commented Dec 12, 2021

I made the overlay for metacoq; the other CI failures seem unrelated.

Actually, couldn't we add also a
...
in file theories/extraction/ExtrOcamlString.v of the Coq archive.

Let me do that soon in another PR.

@herbelin
Copy link
Member

I made the overlay for metacoq; the other CI failures seem unrelated.

Actually, couldn't we add also a
...
in file theories/extraction/ExtrOcamlString.v of the Coq archive.

Let me do that soon in another PR.

OK, then I guess it only remains to make a PR for the overlay.

Assigning.

@herbelin herbelin self-assigned this Dec 13, 2021
@Lysxia
Copy link
Contributor Author

Lysxia commented Dec 13, 2021

MetaCoq/metacoq#621

@herbelin
Copy link
Member

OK, will merge within a couple of hours then.

@herbelin
Copy link
Member

@coqbot: merge now

@coqbot-app
Copy link
Contributor

coqbot-app bot commented Dec 13, 2021

@herbelin: You can't merge the PR because there is no kind: label and no milestone is set.

@herbelin herbelin added the part: extraction The extraction mechanism. label Dec 13, 2021
@herbelin herbelin added this to the 8.16+rc1 milestone Dec 13, 2021
@herbelin
Copy link
Member

@coqbot: merge now

@coqbot-app
Copy link
Contributor

coqbot-app bot commented Dec 14, 2021

@herbelin: You can't merge the PR because there is no kind: label.

@herbelin herbelin added the kind: fix This fixes a bug or incorrect documentation. label Dec 14, 2021
@herbelin
Copy link
Member

@coqbot: merge now

@coqbot-app coqbot-app bot merged commit 0af7ced into coq:master Dec 14, 2021
@coqbot-app
Copy link
Contributor

coqbot-app bot commented Dec 14, 2021

@herbelin: Please take care of the following overlays:

  • 15294-Lysxia-extract-comparison.sh

TheoWinterhalter added a commit to MetaCoq/metacoq that referenced this pull request Dec 14, 2021
Adapt to coq/coq#15294 (comparison is no longer extracted to int)
@vbgl
Copy link
Contributor

vbgl commented Feb 18, 2022

Would it make sense to backport this bugfix to 8.15? Seems easy and might be useful if a 8.15.1 is released before 8.16.0.

@Alizter Alizter added this to Done in Extraction May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: fix This fixes a bug or incorrect documentation. part: extraction The extraction mechanism.
Projects
Archived in project
Extraction
  
Done
Development

Successfully merging this pull request may close these issues.

Don't extract comparison to int
4 participants