bscarlet / llvm-general

Rich LLVM bindings for Haskell (with transfer of LLVM IR to and from C++, detailed compilation pass control, etc.)
http://hackage.haskell.org/package/llvm-general
132 stars 38 forks source link

need to enable RankNTypes in Internal/DecodeAST.hs and Encode #83

Closed cartazio closed 10 years ago

cartazio commented 10 years ago

theres a breaking change in 7.8 wrt whats considered higher rank / impredicative types

src/LLVM/General/Internal/DecodeAST.hs:62:5:
    Illegal polymorphic or qualified type:
      forall r. (a -> IO r) -> IO r
    Perhaps you intended to use RankNTypes or Rank2Types
    In an expression type signature:
      forall (a :: *).
      (forall (r :: *). (a -> IO r) -> IO r) -> DecodeAST a
    In the expression:
        ghc-prim:GHC.Prim.coerce
          (anyContToM ::
             (forall (r :: *). (a -> IO r) -> IO r)
             -> AnyContT (StateT DecodeState IO) a) ::
          forall (a :: *).
          (forall (r :: *). (a -> IO r) -> IO r) -> DecodeAST a
    In an equation for ‛anyContToM’:
        anyContToM
          = ghc-prim:GHC.Prim.coerce
              (anyContToM ::
                 (forall (r :: *). (a -> IO r) -> IO r)
                 -> AnyContT (StateT DecodeState IO) a) ::
              forall (a :: *).
              (forall (r :: *). (a -> IO r) -> IO r) -> DecodeAST a

via enthropy on irc (im still sorting out mac related issues)

aavogt commented 10 years ago

Hi, I am enthropy. If I add RankNTypes, ImpredicativeTypes to DecodeAST and EncodeAST it compiles (provided I also use -fshared-llvm)

On the other hand my ghc is version 7.7.20140120

cartazio commented 10 years ago

(also @aavogt 's testing on linux, so he's not hitting any mac issues too)

cartazio commented 10 years ago

i'm using cabal 1.18 too

bscarlet commented 10 years ago

Does it really require ImpredicativeTypes? RankNTypes alone is not enough?

Is there a separate issue requiring "-fshared-llvm"?

cartazio commented 10 years ago

Yup, Both impredicative types AND rank n are needed. I tried sans impredicative. No go.

-fshared-llvm is needed to use llvm-general in ghci in 7.8. get linker errors and associated madness with the static linked version. see https://gist.github.com/cartazio/8744168

cartazio commented 10 years ago

i've opened a separate ticket for the shared-llvm issue

@aavogt did you get the linking error with compiled code or just using it in ghci?

aavogt commented 10 years ago

@cartazio the linking error was when building library itself (so I didn't explicitly call ghci, but maybe with the template-haskell used somewhere in the library it is practically involved?)

cartazio commented 10 years ago

so both! (ghci in 7.8 uses the system dylinker on x86/x86_64/ARM, and TH runs via ghci).

bscarlet commented 10 years ago

Can you offer any documentation on the change in 7.8? It looks like the central code causing the change is the stuff in AnyContT (under Control.Monad.AnyCont), which previously had RankNTypes. Why is ImpredicativeTypes now necessary, and why in modules using AnyCont rather than just in AnyCont?

cartazio commented 10 years ago

i'll test / report back in a day or so (if i don't, holler and i'll put at the top of my queue of tasks again)

iainnicol commented 10 years ago

@bscarlet, https://ghc.haskell.org/trac/ghc/ticket/8565 is plausibly related.

cartazio commented 10 years ago

yup! Thats the relevant GHC change i was thinking of. Thanks for digging that up @iainnicol :)

cartazio commented 10 years ago

ok, only ImpredicativeTypes is needed in the decode / encode modules. No need to add the rankN types flags therein

cartazio commented 10 years ago

hrmm, so based on the most recent comments on thist ticket https://ghc.haskell.org/trac/ghc/ticket/8631#comment:12, the type checker issue might be resolved on the GHC side for 7.8.

goldfirere commented 10 years ago

Yes, this triggered a regression in GHC 7.8. It's now fixed in HEAD, and it should be merged for the release of 7.8 -- you shouldn't need ImpredicativeTypes to compile the module.

bscarlet commented 10 years ago

Thank's for doing all the work! As it looks, this issue should be a non-issue before 7.8 is released, so I'm going to close it. Feel free to re-open it if necessary.