Fix undefined GHC.Real export with integer-simple
ClosedPublic

Authored by puffnfresh on Dec 29 2014, 8:00 PM.

Details

Test Plan

Check that GHC.Real compiles without OPTIMISE_INTEGER_GCD_LCM nor MIN_VERSION_integer_gmp defined.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
puffnfresh updated this revision to Diff 2046.Dec 29 2014, 8:00 PM
puffnfresh retitled this revision from to Fix undefined GHC.Real export with integer-simple.
puffnfresh updated this object.
puffnfresh edited the test plan for this revision. (Show Details)
puffnfresh added reviewers: austin, carter, ezyang.
dfeuer requested changes to this revision.Dec 29 2014, 8:32 PM
dfeuer added a reviewer: dfeuer.
dfeuer added a subscriber: dfeuer.

Yes, this is a duplicate of D598, my own version of the same mistake. Are you fixing this now, or should I fix that?

This revision now requires changes to proceed.Dec 29 2014, 8:32 PM
puffnfresh updated this revision to Diff 2047.Dec 29 2014, 8:37 PM
puffnfresh edited edge metadata.

Fixed by making consistent with advice from D598.

In D600#16838, @dfeuer wrote:

Yes, this is a duplicate of D598, my own version of the same mistake. Are you fixing this now, or should I fix that?

Sorry, didn't see that! I've updated the diff.

Still compiling but onto stage2 so think this successfully re-enables compilation with integer-simple.

hvr added inline comments.Dec 30 2014, 6:53 AM
libraries/base/GHC/Real.hs
32–33

it's *very* hard to see what changed here :-(

erikd added a subscriber: erikd.Dec 31 2014, 12:00 AM

@hvr This patch simply removes the export of gcdInt' and gcdWord'.

hvr edited edge metadata.Dec 31 2014, 8:29 AM
In D600#16881, @erikd wrote:

@hvr This patch simply removes the export of gcdInt' and gcdWord'.

I know... my point (which I should have made more explicit) was rather that re-wrapping import lists in this way (rather than keeping this a single-line modification) is bad practice for patches, as it's hard to review, makes the output of git blame less useful, and significantly increases the risk of merge conflicts.

hvr requested changes to this revision.EditedDec 31 2014, 8:30 AM
hvr edited edge metadata.

please make this a minimal a single-line patch

This revision now requires changes to proceed.Dec 31 2014, 8:30 AM
puffnfresh updated this revision to Diff 2055.Jan 3 2015, 12:07 AM
puffnfresh edited edge metadata.

Down to a one line change.

erikd accepted this revision.Jan 3 2015, 1:28 AM
erikd added a reviewer: erikd.

Looks good to me. Exactly what @hvr requested. @puffnfresh do you have commit access?

hvr accepted this revision.Jan 3 2015, 4:42 AM
hvr edited edge metadata.

much better, thanks

hvr added a comment.Jan 3 2015, 4:44 AM

Btw, there's one validation failure: driver T9938 (normal)

any idea what's up with that?

In D600#16927, @hvr wrote:

Btw, there's one validation failure: driver T9938 (normal)

any idea what's up with that?

master just got this commit:

commit 633814f5664f135b3648e2a0a6f37e41e2b54ea0
Author: Joachim Breitner <mail@joachim-breitner.de>
Date:   Sat Jan 3 14:39:52 2015 +0100

    Mark T9938 as not broken
    
    either one of the two recent commits (d8d0031, fd97d2a) fixed it, or
    there is some nondeterminism here. See #9938.

So thankfully nothing to do with me!

dfeuer accepted this revision.Jan 6 2015, 6:03 PM
dfeuer edited edge metadata.
This revision is now accepted and ready to land.Jan 6 2015, 6:03 PM
austin accepted this revision.Jan 6 2015, 6:09 PM
austin edited edge metadata.

LGTM.

This revision was automatically updated to reflect the committed changes.
erikd added a comment.Jan 7 2015, 3:54 AM

Sorry @puffnfresh, arc land replaced you with me as the author. Not any easy way to fix it as the GHC repo has disabled git push -f.

erikd added a comment.Jan 7 2015, 4:03 AM
This comment was removed by erikd.