Merge ghc-imported-from into ghc-mod #810
Conversation
Also removed the bounds on cabal-helper to build with latest Stack.
|
Struggling to get this to build on ghc-7.6.3 due to dependencies on process-streaming. Will sort it out. |
| @@ -174,9 +175,10 @@ Library | |||
| , bytestring < 0.11 | |||
| , binary < 0.9 && >= 0.5.1.0 | |||
| , containers < 0.6 | |||
| , cabal-helper < 0.8 && >= 0.7.1.0 | |||
There was a problem hiding this comment.
Don't remove the bounds please :)
There was a problem hiding this comment.
Reverted ghc-mod.cabal:
commit 875b5137a867416bfa7416493a0aac0165c91222
Author: Carlo Hamalainen <carlo@carlo-hamalainen.net>
Date: Fri Jul 15 21:09:59 2016 +1000
Revert ghc-mod.cabal to current master.
| Just ve -> ve | ||
| Nothing -> let pname' = ((head $ separateBy '-' pname) ++ ":" ++ thisModuleName) in | ||
| fromMaybe | ||
| (throw $ GMENoVisibleExports thisModuleName pname') |
There was a problem hiding this comment.
Again, monadic context -> inline that failure.
There was a problem hiding this comment.
L.H.GM.Error.liftMaybe should be applicable here.
|
Phew, long review. We're getting there. It's your bad luck I had too much time on my hands to do a thorough review. Most of the stuff is pretty nitpick though so we ought to be about done. I really have to write some code style guidelines one of these days :) So never mind the thing about merging early I said before probably best not to loose all those comments. |
|
I'm not sure what to do about the |
Avoid ```fromMaybe (throw ...)``` pattern.
| @@ -282,7 +282,7 @@ ghcPkgFindModule mod = do | |||
|
|
|||
| shortcut [ stackGhcPkgFindModule rp mod | |||
| , hcPkgFindModule rp mod | |||
| , _ghcPkgFindModule rp mod | |||
There was a problem hiding this comment.
FYI in GHC a leading underscore denotes a typed hole but you can also use it to suppress the "unused symbol" warning when you then go on to define that symbol. I'm not aware of what "python style" you refer to here.
|
@lierdakil tests for 7.10 are failing with I have no idea what's going on there. |
| @@ -552,7 +552,10 @@ refineVisibleExports getHaddockInterfaces exports = mapM f exports | |||
| qexports = qualifiedExports mexports -- e.g. ["base-4.8.2.0:GHC.Base.Just", ...] | |||
| mVisibleExportsMap <- getVisibleExports getHaddockInterfaces pname | |||
|
|
|||
| let visibleExportsMap = fromMaybe (throw $ GMEString $ "ImportedFrom: visible exports map is Nothing") mVisibleExportsMap | |||
| visibleExportsMap <- case mVisibleExportsMap of | |||
There was a problem hiding this comment.
I think you misunderstood, there are three kinds of failures:
- A pure exception
do let foo = throw "whatever"
return foo
- An excpetion where a monadic value should be
do throw "whatever"
- An ExceptT/MonadError failure
do throwError $ GMEString "whatever"
The differences are subtle but in general the first two kinds are annoying since you need to explicitly catch the exceptions whereas the last one just always gets returned as a Left by runGhcModT.
The difference between 1) and 2) is important though: throw "whatever" >> return "foo" will always crash when run, let foo = throw "whatever" in return (foo, "bar") will only crash when you touch the fst of that tuple! (lazyness and such)
The GhcModError stuff should always be returned via throwError where possible. All you really had to do was to replace fromMaybe with liftMaybe :)
There was a problem hiding this comment.
Also fail is equivalent to throwError . GMEString which is why I suggested you use that before.
|
Actually let's continue in #823, it's easier if we have a shared branch. |
Merging ghc-imported-from into ghc-mod: https://github.com/carlohamalainen/ghc-imported-from