Gabriella439 / pipes

Compositional pipelines
BSD 3-Clause "New" or "Revised" License
487 stars 72 forks source link

Pipes doesn't appear to build with GHC-9.0 #224

Closed recursion-ninja closed 3 years ago

recursion-ninja commented 3 years ago

I tried building pipes-4.3.14 with ghc-9.0-rc1 and got the following error:

Building library for pipes-4.3.14..
[3 of 6] Compiling Pipes            ( src/Pipes.hs, dist/build/Pipes.o, dist/build/Pipes.dyn_o )

src/Pipes.hs:141:9: error:
    • Couldn't match type: Proxy x'0 x0 a'0 a m0 a'0
                     with: forall x' x. Proxy x' x () a m ()
      Expected: a -> Producer' a m ()
        Actual: a -> Proxy x'0 x0 a'0 a m0 a'0
    • In the expression: respond
      In an equation for ‘yield’: yield = respond
    • Relevant bindings include
        yield :: a -> Producer' a m () (bound at src/Pipes.hs:141:1)
    |
141 | yield = respond
    |         ^^^^^^^

src/Pipes.hs:186:36: error:
    • Couldn't match type: forall x'2 x2. Proxy x'2 x2 () c m2 ()
                     with: Proxy x' x c' c m c'
      Expected: c -> Proxy x' x c' c m c'
        Actual: c -> Producer' c m2 ()
    • In the second argument of ‘for’, namely ‘yield’
      In the expression: for p yield
      When checking the rewrite rule "for p yield"
    • Relevant bindings include
        p :: Proxy x' x c' c m a' (bound at src/Pipes.hs:186:26)
    |
186 |   ; "for p yield" forall p . for p yield = p
    |                                    ^^^^^

src/Pipes.hs:640:8: error:
    • Couldn't match type: Proxy x'1 x1 () a m1 ()
                     with: forall x' x. Proxy x' x () a m ()
      Expected: f a -> Producer' a m ()
        Actual: f a -> Proxy x'1 x1 () a m1 ()
    • In the expression: foldr (\ a p -> yield a >> p) (return ())
      In an equation for ‘each’:
          each = foldr (\ a p -> yield a >> p) (return ())
    • Relevant bindings include
        each :: f a -> Producer' a m () (bound at src/Pipes.hs:640:1)
    |
640 | each = F.foldr (\a p -> yield a >> p) (return ())
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cabal: Failed to build pipes-4.3.14

I believe that there has been a subtle change to quantified types that is affecting the use of Proxy throughout the code base.

Gabriella439 commented 3 years ago

@recursion-ninja: This looks like a consequence of: https://gitlab.haskell.org/ghc/ghc/-/wikis/migration/9.0#deep-instantiation

… and based on that description it seems like the fix is to eta-expand any affected definitions. For example, instead of:

yield = respond

… we instead define:

yield x = respond x

Could you test to see if that change fixes some of the errors?

recursion-ninja commented 3 years ago

I can confirm that the following eta-expansion does remove one of the errors.

 yield :: Functor m => a -> Producer' a m ()
-yield = respond
+yield x = respond x
 {-# INLINABLE [1] yield #-}

@Gabriel439 was correct in diagnosing the cause of these compilation errors. All the compilation errors appear to be of this form.

Gabriella439 commented 3 years ago

@recursion-ninja: Is that the only error? If you can post the full list of errors I can try to fix all of them

recursion-ninja commented 3 years ago

@Gabriel439 Those three were the only errors I encounter. I could easily resolve the first and third with an obvious eta-expansion. The second error is coming from a rewrite rule, which I was having trouble understanding due to the lack of type signatures and lack of familiarity with the internals of the pipes library.

Since I couldn't resolve the second error, I don't know if there will be other compilation errors after the Pipes module builds. There's a chance the Pipes.Prelude and Pipes.Tutorial modules which depend on the Pipes module could have compilation issues. I wouldn't anticipate that to be the case, but I cannot rule it out either.

If you make a pull request fixing the three compilation errors in the Pipes module, I'm more than happy to rebuild the pull request locally with ghc-9.0-rc1 to confirm that pipes now builds successfully.

Gabriella439 commented 3 years ago

@recursion-ninja: Alright, see if this change works better: https://github.com/Gabriel439/Haskell-Pipes-Library/pull/225

recursion-ninja commented 3 years ago

@Gabriel439 Unfortunately, after correcting the three compilation errors above, a new error crops up in the Pipes module, now that yield compiles:

src/Pipes.hs:641:8: error:
    • Couldn't match type: Proxy x'0 x0 () a m0 ()
                     with: forall x' x. Proxy x' x () a m ()
      Expected: f a -> Producer' a m ()
        Actual: f a -> Proxy x'0 x0 () a m0 ()
    • In the expression: foldr (\ a p -> yield a >> p) (return ())
      In an equation for ‘each’:
          each = foldr (\ a p -> yield a >> p) (return ())
    • Relevant bindings include
        each :: f a -> Producer' a m () (bound at src/Pipes.hs:641:1)
    |
641 | each = F.foldr (\a p -> yield a >> p) (return ())
    | 

This can easily be fixed by eta expansion as well. Once this compilation error is corrected, the Pipes module will successfully compile!

However, the Pipes.Prelude module which imports Pipes no longer compiles:

src/Pipes/Prelude.hs:558:18: error:
    • Couldn't match type: forall x' x. Proxy x' x () a m0 ()
                     with: Proxy () (f a) () a m ()
      Expected: f a -> Proxy () (f a) () a m ()
        Actual: f a -> Producer' a m0 ()
    • In the second argument of ‘for’, namely ‘each’
      In the expression: for cat each
      In an equation for ‘concat’: concat = for cat each
    • Relevant bindings include
        concat :: Pipe (f a) a m r (bound at src/Pipes/Prelude.hs:558:1)
    |
558 | concat = for cat each
    |                  ^^^^

src/Pipes/Prelude.hs:562:52: error:
    • Couldn't match type: forall x'1 x1. Proxy x'1 x1 () c m1 ()
                     with: Proxy x' x () c m ()
      Expected: f c -> Proxy x' x () c m ()
        Actual: f c -> Producer' c m1 ()
    • In the second argument of ‘for’, namely ‘each’
      In the expression: for p each
      When checking the rewrite rule "p >-> concat"
    • Relevant bindings include
        p :: Proxy x' x () (f c) m a'
          (bound at src/Pipes/Prelude.hs:562:27)
    |
562 |     "p >-> concat" forall p . p >-> concat = for p each
    |                                                    ^^^^

src/Pipes/Prelude.hs:666:19: error:
    • Couldn't match type: Producer' b m ()
                     with: Proxy () a () b m ()
      Expected: ListT m b -> Proxy () a () b m ()
        Actual: ListT m b -> Producer' b m ()
    • In the first argument of ‘(.)’, namely ‘every’
      In the second argument of ‘for’, namely ‘(every . k)’
      In the expression: for cat (every . k)
    • Relevant bindings include
        k :: a -> ListT m b (bound at src/Pipes/Prelude.hs:666:6)
        loop :: (a -> ListT m b) -> Pipe a b m r
          (bound at src/Pipes/Prelude.hs:666:1)
    |
666 | loop k = for cat (every . k)
    |                   ^^^^^

src/Pipes/Prelude.hs:929:13: error:
    • Couldn't match type: Proxy x'0 x0 () c m r
                     with: forall x' x. Proxy x' x () c m r
      Expected: Producer a m r -> Producer b m r -> Producer' c m r
        Actual: Producer a m r -> Producer b m r -> Proxy x'0 x0 () c m r
    • In the expression: go
      In an equation for ‘zipWith’:
          zipWith f
            = go
            where
                go p1 p2
                  = do e1 <- lift $ next p1
                       ....
    • Relevant bindings include
        go :: forall {m :: * -> *} {b1} {x'} {x}.
              Monad m =>
              Producer a m b1 -> Producer b m b1 -> Proxy x' x () c m b1
          (bound at src/Pipes/Prelude.hs:931:5)
        f :: a -> b -> c (bound at src/Pipes/Prelude.hs:929:9)
        zipWith :: (a -> b -> c)
                   -> Producer a m r -> Producer b m r -> Producer' c m r
          (bound at src/Pipes/Prelude.hs:929:1)
    |
929 | zipWith f = go
    |

This is unfortunate, since the changed definitions seem to "bleed outside" of the Pipes module an importing module. It makes me wonder if ghc-9.0 presents a breaking change to the pipes library that might require more thought than some mechanical eta expansions in the library. Will user code importing Pipes encounter similar errors to Pipes.Prelude when using ghc-9.0 if the library were to be fixed with simple eta-expansions?

Gabriella439 commented 3 years ago

@recursion-ninja: I think the reliable fix isn't to eta-expand at all, but rather to not use the Producer' type synonym and instead use an explicit universal quantification for all affected type signatures.

Gabriella439 commented 3 years ago

@recursion-ninja: I pushed a few more fixes. Let me know if I missed anything

recursion-ninja commented 3 years ago

The pipes library successfully builds now!

The benchmark and test suite build targets do not build due to transitive dependency compilation errors.

cabal: Failed to build QuickCheck-2.7.6 (which is required by test:tests from
pipes-4.3.14). See the build log above for details.
Failed to build optparse-applicative-0.14.3.0 (which is required by
bench:prelude-benchmarks from pipes-4.3.14 and bench:lift-benchmarks from
pipes-4.3.14). See the build log above for details.
Failed to build regex-base-0.94.0.0 (which is required by test:tests from
pipes-4.3.14). See the build log above for details.

Upgrading to use optparse-applicative-0.16 will likely fix the benchmark suite's compilation error, though it might require minor code changes as part of the interface has changed. If you want backwards compatibility with older versions of optparse-applicativebefore version 16, you might have to use CPP.

I don't know why QuickCheck is failing to build, given that pipes.cabal has permissive upper bounds.

The regex-base compilation error is already a known issue https://github.com/haskell-hvr/regex-base/issues/7. Probably will have to wait for the upstream fix.

I think we should merge changes that fix the library compilation and worry about the benchmark and test suite later. This would allow (most) users pipes to build with ghc-9.0, whether pipes is a direct or transient dependency.

Gabriella439 commented 3 years ago

Yeah, the broken benchmark suite is a known issue which I still need to fix, but it's orthogonal to this

Gabriella439 commented 3 years ago

@recursion-ninja: https://github.com/Gabriel439/Haskell-Pipes-Library/pull/226 should fix the other build failures you noted