-
Notifications
You must be signed in to change notification settings - Fork 37
Work around the "can't use natural in base" problem on a per-flavour basis #676
Conversation
…basis The only flavours that need the workaround are the ones that build GHC/{Natural, Num}.hs with -O0, namely 'quick', 'quickest' and 'prof'. This patches defines the necessary arguments in one place and uses them in all the aforementionned flavour definitions. This will allow us to have both quick/quickest/prof builds that come through as well as an efficient compiler when we want it (with e.g perf), which wasn't the case before my series of patches for this problem.
@@ -0,0 +1,11 @@ | |||
module Settings.Flavours.Common where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we have a module dedicated to various temporary hacks and workarounds? It could live at the top src/ToDo.hs
, and it will be the go-to place for Hadrian maintainers for introducing and -- importantly -- finding forgotten temporary fixes that can now be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hesitated, but imagine it gets empty, we delete it? And then we have new hacks, we recreate it? So I just went with the very obvious Common
, where we can put anything that's shared between flavours, hacks or not :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, let's stick with your naming. I agree, this module may be useful for other common flavour-related settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK great. I'll let you review everything now, I addressed your other bit of feedback.
src/Settings/Flavours/Common.hs
Outdated
|
||
-- See https://ghc.haskell.org/trac/ghc/ticket/15286 and | ||
-- https://phabricator.haskell.org/D4880 | ||
naturalInBaseFixArgs :: [Args] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's give it type Args
and use mconcat
? Then you will be able to use naturalInBaseFixArgs
directly without ++
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in the last commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, looks great, so merging!
The only flavours that need the workaround are the ones that build
GHC/{Natural, Num}.hs with -O0, namely 'quick', 'quickest' and 'prof'.
This patches defines the necessary arguments in one place and uses them
in all the aforementionned flavour definitions.
This will allow us to have both quick/quickest/prof builds that come through
as well as an efficient compiler when we want it (with e.g perf), which wasn't
the case before my series of patches for this problem.