search

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

Remove Control.Monad.Exceptable #178

Closed cocreature closed 8 years ago

cocreature commented 8 years ago

We already require transformers >= 0.4 in llvm-general-pure so there no reason to not require it in llvm-general. (The mtl bounds in llvm-general-pure also require transformers >= 0.4). The module specifically mentions that you want to keep compatibility with 0.3, but since llvm-general already has dependencies not bundled with GHC (e.g. utf8-string) you already neet cabal and at that point using cabal to get transformers-0.4 is trivial.

Apart from the fact that the best code is deleted code, this also fixes a build error for GHC-7.8 which was in that module.

bscarlet commented 8 years ago

It's going to take a bit for me to figure out if this change is reasonable. Please read all the discussion in issue #115 for context. I think the current state (not yet sure how awful it is) looks like users of llvm-general & ghc 7.8 wind up using both transformers-0.3 and transformers-0.4, and your change will break things for them. Either that or issue #115 isn't fully fixed.

cocreature commented 8 years ago

Interesting, let me try to summarize it:

Problem The ghc package, i.e., the ghc api, depends on transformers-0.3. llvm-general-pure however requires transformers-0.4. Note that the ghc api is also used by packages such as doctests (according to the discussion in #115) so it is probably not reasonable to dismiss it as an edge case.

Current status The mutually exclusive bounds mean that it’s impossible to compile a package using llvm-general-pure and ghc (or transitively depending on them).

I just tried this to confirm it and ended up with

rejecting: llvm-general-pure-3.8.0.0 (conflict: ghc =>
transformers==0.3.0.0/installed-645..., llvm-general-pure =>
transformers>=0.4.0.0)

and a failing build.

What this PR changes Nothing, now llvm-general also requires transformers>=0.4 but since it is impossible to install llvm-general without installing llvm-general-pure this bound is already incurred transitively.

Possible solutions transformers-compat provides the necessary instances and mtl-compat provides Control.Monad.Except. I think we should be able to simply drop these in and fix the build with transformers-0.4 thereby. I’ll try to adapt this PR later today.

cocreature commented 8 years ago

transformers-0.3 is now supported via mtl-compat and transformers-compat.

bscarlet commented 8 years ago

Thanks for the nice summary.

Before we replace the previous choice of hack, please take a look to see how hard it would be to weaken the bound for transformers in llvm-general-pure instead? I got convinced to go down the current path by the inimitable ekmett@, and among other things his preference for avoiding needing an mtl-compat (which I note does not seem to be of his making). In my experience his preferences often prove wise.

The whole hack can come out when we can drop support for ghc 7.8 anyway. I'd prefer to stick with the original choice than to change hacks midstream.

cocreature commented 8 years ago

I’m not sure I get why mtl-compat is a bad thing, but it was easy enough to remove. I just pushed a commit doing that.

cocreature commented 8 years ago

Also after rebasing we now finally have a green build \o/

bscarlet commented 8 years ago

I'm also not sure about mtl-compat. I was quite confused as to how you managed to remove Exceptable without mtl-compat until I found the comment on version 0.4 in the transformers-compat changelog. That version was added after the circus that was issue #115.

bscarlet commented 8 years ago

\o/ indeed.