Make Applicative a superclass of Monad
ClosedPublic

Authored by austin on Jun 9 2014, 8:23 AM.

Details

Summary

This includes pretty much all the changes needed to make Applicative
a superclass of Monad finally. There's mostly reshuffling in the
interests of avoid orphans and boot files, but luckily we can resolve
all of them, pretty much. The only catch was that
Alternative/MonadPlus also had to go into Prelude to avoid this.

Signed-off-by: Austin Seipp <austin@well-typed.com>

Test Plan

Build things, they might not explode horribly.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Lint Skipped
Unit
Unit Tests Skipped
austin updated this revision to Diff 28.Jun 9 2014, 8:23 AM
austin retitled this revision from to Make Applicative a superclass of Monad.
austin updated this object.
austin edited the test plan for this revision. (Show Details)
austin added reviewers: hvr, simonmar.
austin added a comment.Jun 9 2014, 8:37 AM

You're also going to need the latest git version of Happy from @simonmar's repo: https://github.com/simonmar/happy

hvr edited edge metadata.EditedJun 9 2014, 9:06 AM

Generally LGTM, please consider an entry for libraries/base/changelog.md and bumping the base version to 4.8.0.0.

simonpj added a subscriber: simonpj.Jun 9 2014, 9:48 AM
relrod added a subscriber: relrod.Jun 9 2014, 6:20 PM
hvr awarded a token.Jun 15 2014, 10:13 AM
hvr accepted this revision.Jun 22 2014, 10:35 AM
hvr edited edge metadata.
This revision is now accepted and ready to land.Jun 22 2014, 10:35 AM
ekmett edited edge metadata.Jul 1 2014, 9:46 AM

Looks good to me.

We should pull the trigger on this sooner rather than later as a lot of other stuff blocks on this change.

ekmett accepted this revision.Jul 1 2014, 9:47 AM
ekmett edited edge metadata.
hvr added a comment.Jul 3 2014, 2:41 AM

@simonmar if the patch to happy is sound/harmless, can we get a happy release soonish, so GHC devs can simply cabal install happy once this lands?

@austin do we need to update the check on the happy version in ./configure.ac?

simonmar edited edge metadata.Jul 7 2014, 6:09 AM

Just uploaded happy-1.19.4

glguy added a subscriber: glguy.Jul 11 2014, 6:16 PM
austin planned changes to this revision.Jul 13 2014, 9:44 PM

I have some more changes needed for this actually.

austin updated this revision to Diff 404.Aug 19 2014, 7:50 AM
austin edited edge metadata.

Good:

  • Fix all tests
  • Compiler builds and works pretty well

Bad:

  • Haddock goes into an infinite loop, yuck. :(

Ugly:

  • Some of the performance tests get worse. Not yet investigated.
This revision is now accepted and ready to land.Aug 19 2014, 7:50 AM
austin planned changes to this revision.Aug 19 2014, 7:54 AM

OK, so this patch is now effectively complete, but I'm completely stumped by a ./validate error in Haddock, which causes an infinite loop.

First, apply this diff to utils/haddock (this commit is not upstream in any way)

diff --git a/src/Haddock/Backends/LaTeX.hs b/src/Haddock/Backends/LaTeX.hs
index 7b72c03..35f379c 100644
--- a/src/Haddock/Backends/LaTeX.hs
+++ b/src/Haddock/Backends/LaTeX.hs
@@ -1,4 +1,5 @@
 {-# OPTIONS_GHC -fno-warn-name-shadowing #-}
+{-# LANGUAGE CPP #-}
 -----------------------------------------------------------------------------
 -- |
 -- Module      :  Haddock.Backends.LaTeX
@@ -31,7 +32,7 @@ import qualified Data.Map as Map
 import System.Directory
 import System.FilePath
 import Data.Char
-import Control.Monad
+import Control.Monad hiding (empty)
 import Data.Maybe
 import Data.List

Next, run ./validate, and Haddock will finish building and fail generating the docs for GHC.IntWord64 in ghc-prim - some verbosity output indicates this happens in CoreTidy, but I cannot see why!

I'll also note this makes some of the compiler performance tests worse (by 10% in some cases), but I haven't checked why. I speculate part of it is that the shuffling of interfaces has probably caused more module loading by default, which impacts startup performance for Prelude and base.

simonpj edited edge metadata.Aug 19 2014, 7:58 AM

-ddump-if-trace shows you which interface files are loaded.

In D13#26, @austin wrote:

Next, run ./validate, and Haddock will finish building and fail generating the docs for GHC.IntWord64 in ghc-prim - some verbosity output indicates this happens in CoreTidy, but I cannot see why!

What's the exact error? Anything related to Trac #8320?

In D13#31, @jstolarek wrote:
In D13#26, @austin wrote:

Next, run ./validate, and Haddock will finish building and fail generating the docs for GHC.IntWord64 in ghc-prim - some verbosity output indicates this happens in CoreTidy, but I cannot see why!

What's the exact error? Anything related to Trac #8320?

No, doesn't look like it. There's no real error; it just never finishes.

In D13#33, @austin wrote:

No, doesn't look like it. There's no real error; it just never finishes.

Now it does:

--- a/haddock-library/vendor/attoparsec-0.12.1.1/Data/Attoparsec/Internal/Types.hs
+++ b/haddock-library/vendor/attoparsec-0.12.1.1/Data/Attoparsec/Internal/Types.hs
@@ -163,14 +163,6 @@ instance Applicative (Parser i) where
     (<*>)  = apP
     {-# INLINE (<*>) #-}

-    -- These definitions are equal to the defaults, but this
-    -- way the optimizer doesn't have to work so hard to figure
-    -- that out.
-    (*>)   = (>>)
-    {-# INLINE (*>) #-}
-    x <* y = x >>= \a -> y >> return a
-    {-# INLINE (<*) #-}
-
austin added a comment.Sep 5 2014, 4:21 PM
In D13#33, @austin wrote:

No, doesn't look like it. There's no real error; it just never finishes.

Now it does:

--- a/haddock-library/vendor/attoparsec-0.12.1.1/Data/Attoparsec/Internal/Types.hs
+++ b/haddock-library/vendor/attoparsec-0.12.1.1/Data/Attoparsec/Internal/Types.hs
@@ -163,14 +163,6 @@ instance Applicative (Parser i) where
     (<*>)  = apP
     {-# INLINE (<*>) #-}

-    -- These definitions are equal to the defaults, but this
-    -- way the optimizer doesn't have to work so hard to figure
-    -- that out.
-    (*>)   = (>>)
-    {-# INLINE (*>) #-}
-    x <* y = x >>= \a -> y >> return a
-    {-# INLINE (<*) #-}
-

iiam

This Applicative instance defines (*>) as (>>), and the default Monad instance in the AMP patch points (>>) at (*>), hence the loop. I'd assume bos added this for some reason though, and it's probably worth digging through Hackage to see how common 'override default to subclass' implementations are.

In D13#36, @austin wrote:
--- a/haddock-library/vendor/attoparsec-0.12.1.1/Data/Attoparsec/Internal/Types.hs
+++ b/haddock-library/vendor/attoparsec-0.12.1.1/Data/Attoparsec/Internal/Types.hs
-    (*>)   = (>>)

iiam

austin added a comment.Sep 5 2014, 6:01 PM

This Applicative instance defines (*>) as (>>), and the default Monad instance in the AMP patch points (>>) at (*>), hence the loop. I'd assume bos added this for some reason though, and it's probably worth digging through Hackage to see how common 'override default to subclass' implementations are.

Yes, the macro was more of a joke on the diff itself (and the comment about not making the optimizer 'work so hard', hence 'it is a mystery'), not a response to the find itself. (In fact this also manifested slightly differently in an earlier version of this patch, where I had a similar error involving liftA2 I believe on accident.)

NathanHowell added a comment.EditedSep 5 2014, 6:04 PM
In D13#38, @austin wrote:

Yes, the macro was more of a joke on the diff itself (and the comment about not making the optimizer 'work so hard', hence 'it is a mystery'), not a response to the find itself.

lol, that's pretty much the same conversation that @carter and I were having when I got your update :-)

austin closed this revision.Sep 9 2014, 8:13 AM
austin updated this revision to Diff 515.

Closed by commit rGHCd94de87252d0 (authored by @austin).

nomeata added a subscriber: nomeata.Sep 9 2014, 8:37 AM

A bit late, but I wonder why >>= has a default implementation that will always be ⊥.

libraries/base/GHC/Base.lhs
416

What is the point of this default implementation if join is not a method?

austin added a comment.Sep 9 2014, 8:46 AM
In D13#43, @nomeata wrote:

A bit late, but I wonder why >>= has a default implementation that will always be ⊥.

Yoinks, I forgot to pull this change out when I revised the initial draft. This needs to be fixed.

hvr added a comment.Sep 9 2014, 8:51 AM

Guess http://ro-che.info/ccc/21 needs updating now... :)