Compile with GHC8.10.2 #157
Conversation
|
It seems that the build failed while compiling GHC itself on |
|
And a strange error about no |
| @@ -84,7 +84,7 @@ roundTripRatioBi = | |||
| roundTripNanoBi :: Property | |||
| roundTripNanoBi = eachOf | |||
| 1000 | |||
| (MkFixed @E9 <$> Gen.integral (Range.constantFrom 0 (-1e12) 1e12)) | |||
| ((MkFixed :: Integer -> Fixed E9) <$> Gen.integral (Range.constantFrom 0 (-1e12) 1e12)) | |||
There was a problem hiding this comment.
GHC 8.6.5: MkFixed @E9
GHC 8.10.2: MkFixed @_ @E9
So I went for something longer that should work with all versions
binary/cardano-binary.cabal
Outdated
| ghc-options: -fno-warn-missing-deriving-strategies | ||
| if impl(ghc >=8.10) | ||
| ghc-options: -fno-warn-missing-safe-haskell-mode | ||
| -fno-warn-prepositive-qualified-module | ||
| -fno-warn-unused-packages |
There was a problem hiding this comment.
I prefer -Wno-X over -fno-warn-X, then it's consistent with the warning flags we use above.
There was a problem hiding this comment.
I have no opinion on this; I'm just taking the same approach as was used in cardano-preludehttps://github.com/input-output-hk/cardano-prelude/blob/master/cardano-prelude.cabal#L79
There was a problem hiding this comment.
Yeah, I saw that too. Would you mind if I pushed a commit to this PR that changed it to -Wno-X?
We use `-W` for enabling warning flags, we should use `-Wno-` for disabling warning flags. The same way extension flags have switched a while ago from `-f` to `-X`, warnings flags should switch to `-W`.
binary/cardano-binary.cabal
Outdated
| if impl(ghc >=8.8) | ||
| ghc-options: -Wno-missing-deriving-strategies | ||
| if impl(ghc >=8.10) | ||
| ghc-options: -Wno-missing-safe-haskell-mode | ||
| -Wno-prepositive-qualified-module | ||
| -Wno-unused-packages |
There was a problem hiding this comment.
Actually, are all these flags that disable warnings even needed (in the other cabal files as well)? I assume you have copied them from cardano-prelude, but there -Weverything is used. Here -Wall is used, so these extra warnings don't have to be disabled. (I didn't have to disable them in IntersectMBO/ouroboros-network#2542)
I prefer -Wall + a manual selection of flags over -Weverything, which forces you to disable some flags based on the GHC version.
There was a problem hiding this comment.
Hmm, certainly some of them were needed in some places. But it's true that after adding them to a couple of files, I pre-emptively added them in other places...
There was a problem hiding this comment.
BTW, I wouldn't disable -Wno-unused-packages, I'd just fix the warnings 🙂
There was a problem hiding this comment.
I just tried removing all those flags myself and it compiled without warnings (except for those from the C code).
There was a problem hiding this comment.
I prefer
-Wall+ a manual selection of flags over-Weverything, which forces you to disable some flags based on the GHC version.
In other words, no changes are needed to the cabal files at all.
|
I'm assuming you're planning on rebasing/squashing? It would be nice to get those two merge commits out of the history. |
This PR does not yet update CI to build with GHC 8.10, but makes various changes that bring the code into a place where it compiles with GHC 8.10. As such, downstream libraries may choose to build it with later GHC versions, and it should be easier to move to 8.10 when we decide to make the switch.