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

Use GHC.Base.Any for compatibility with GHC 8.2 #6023

Merged
merged 1 commit into from Nov 16, 2017

Conversation

tchajed
Copy link
Contributor

@tchajed tchajed commented Oct 25, 2017

Fixes #6022.

Thanks @bmsherman for identifying the issue!

@tchajed tchajed requested a review from letouzey October 25, 2017 20:18
@tchajed
Copy link
Contributor Author

tchajed commented Oct 25, 2017

The GHC issue (Trac 10886) doesn't really resolve the backwards compatibility issue. It looks like GHC.Base re-exports GHC.Types and GHC.Prim, which means this solution works in both 8.0.1 and 8.2.1.

I'd like to check that this works in GHC 7 and then I'm happy with this solution, even though it's somewhat unclear to me what GHC wants users to do. The alternative is to use GHC.Prim.Any in older GHC versions and GHC.Types.Any in GHC >= 8.2.

@tchajed
Copy link
Contributor Author

tchajed commented Oct 25, 2017

Also here's a test case that triggers a use of Any.

Require Import Extraction.

Inductive expr : Type -> Type :=
| bind : forall T T', expr T -> expr T'.

Extraction Language Haskell.

Extraction "expr.hs" expr.

@maximedenes
Copy link
Member

@letouzey Could you review this PR? Thanks!

@maximedenes
Copy link
Member

@letouzey ping

@maximedenes
Copy link
Member

@letouzey Would you have some time to look at this PR?

@tchajed
Copy link
Contributor Author

tchajed commented Nov 15, 2017

I actually want this patch at least in Coq master, since Arch uses GHC 8.2. Is there anybody else with some basic knowledge of Haskell to review this?

I've now tested this example on GHC 7.10.3, 8.0.2, and 8.2.1. If somebody wants to reproduce (which I imagine is the only blocker to merging), use Stack resolvers lts-6.27, lts-9.13, and nightly-2017-11-15 respectively.

@Zimmi48 Zimmi48 added the kind: fix This fixes a bug or incorrect documentation. label Nov 15, 2017
@Zimmi48 Zimmi48 added this to Request 8.7 inclusion in Coq 8.7 via automation Nov 15, 2017
@Zimmi48 Zimmi48 added this to the 8.7.1 milestone Nov 15, 2017
@Zimmi48
Copy link
Member

Zimmi48 commented Nov 15, 2017

Maxime has asked a review from @letouzey because he is kind of "the guardian of the temple" when it comes to Extraction. However, given the triviality of this patch, given that it has been pending for more than 20 days, and given that it was extensively tested by @tchajed, I say this should get merged without waiting more.

@Zimmi48 Zimmi48 removed the request for review from letouzey November 15, 2017 16:33
@Zimmi48
Copy link
Member

Zimmi48 commented Nov 15, 2017

@tchajed: BTW, @letouzey said we can't test the result of extraction to Haskell in the test-suite but we could on Travis. If you want to contribute such tests, a PR would be welcome, I guess.

@maximedenes
Copy link
Member

Sounds good, will merge then, thanks!

@maximedenes maximedenes merged commit 3a5abd4 into coq:master Nov 16, 2017
Coq 8.7 automation moved this from Request 8.7 inclusion to Waiting to be backported Nov 16, 2017
@maximedenes
Copy link
Member

Thanks for your contribution and your patience, @tchajed !

Zimmi48 added a commit to Zimmi48/coq that referenced this pull request Nov 16, 2017
@Zimmi48 Zimmi48 moved this from Waiting to be backported to Waiting for CI in Coq 8.7 Nov 16, 2017
@Zimmi48 Zimmi48 moved this from Waiting for CI to Shipped in 8.7.1 in Coq 8.7 Nov 16, 2017
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.
Projects
No open projects
Coq 8.7
  
Shipped in 8.7.1
Development

Successfully merging this pull request may close these issues.

None yet

3 participants