containers
containers copied to clipboard
Prepare for liftA2 being exported from Prelude
CLC has approved the proposal to export liftA2 from Prelude
(https://github.com/haskell/core-libraries-committee/issues/50).
This is a PR to future proof for that change, while minimising warnings.
@googleson78 could you please fix CI?
I tried to take into account ghc 8.0.2 this time around.
@treeowl @sjakobi could you please take a look? We need this merged before proceeding with https://gitlab.haskell.org/ghc/ghc/-/merge_requests/8449
@treeowl could this please be merged?
@treeowl just a gentle reminder. Otherwise it blocks the respective GHC MR.
Import-styles
hidingand(..)should be avoided where possible. MaybeMIN_VERSION_basecan help.
I would prefer hiding to additional CPP any day, especially for a boot library, because MIN_VERSION_base is sometimes wrong in nightly GHC builds. Let's wait for @treeowl opinion.
It is somehow strange that changes are necessary at all here. The PR description isn't very explicit about what problems it addresses. Let me guess: GHC is complaining whenever liftA2 is explicitly imported from somewhere other than Prelude, because it is already implicitly imported from Prelude.
That's silly by GHC. That warning logic is simply odd. All the energy developers are spending into fixing GHC's silly unused import warnings should maybe go into fixing GHC. See https://www.snoyman.com/blog/2020/11/haskell-bad-parts-2/ :
... we all turn on
-Wall. Because we're good kids. We want to do the right thing. And this, of course, turns on unused import warnings. And because each new release of GHC and every library on Hackage likes to mess with us, we are constantly exporting new identifiers from different modules.
The amount of time I have spent adding weird hacks to account for the fact that
<>is suddenly exposed fromPrelude, and therefore myimport Data.Monoid ((<>))isn't necessary, is obscene. The introduction of fiddly CPP to work around this sucks. In fact, this all sucks. It's horrible.
In the light of this, maybe the better patch here is to add -fno-warn-unused-imports to the ghc-options of the .cabal file (resp. -Wno-unused-imports for GHC >= 8) until the oddity in GHC is fixed.
EDIT: I filed https://gitlab.haskell.org/ghc/ghc/-/issues/21879
@treeowl could you please opine on this? We are blocked for more than a month.
I agree with @andreasabel that the hiding approach isn't great. Can we avoid it without too much CPP?
@googleson78 any chance you can pick it up again? If you are busy, I can give it a try myself.
If we want to avoid the hiding personally I'd prefer just disabling the warning, as Andreas suggested in
In the light of this, maybe the better patch here is to add -fno-warn-unused-imports to the ghc-options of the .cabal file (resp. -Wno-unused-imports for GHC >= 8) until the oddity in GHC is fixed.
@treeowl how do you feel about throwing -fno-warn-unused-imports? I agree that it's probably the least evil.
We can definitely disable unused import warnings for older GHC to unstick.
I reverted the previous changes and disabled that warning globally for containers.
Will review after CI
I meant you can turn off the warnings for old versions. Why do we need to turn them off for all? Is it really too painful otherwise?
It would need to be turned on for versions newer than whatever ghc/base version ships with the liftA2 export change.
Is there a way to confine the mess to a separate module?
@treeowl @sjakobi could you please take ownership of this issue and merge any solution which suits you? The upstream GHC is blocked on this for two months. Asking a volunteer to explore completely orthogonal solutions just to be rejected on a whim strikes my as unreasonable.
I'd suggest to merge any approach (e. g., the current state of PR) to unblock GHC, and then you can polish it until containers release in whatever direction you wish.
I don't consider myself a maintainer of containers at this point, so I take no responsibility for this PR.
Based on the discussion here, I'd suggest to add {-# OPTIONS_GHC -fno-warn-unused-imports #-} and explanatory comments in each of the affected modules.
@treeowl just a kind reminder about this issue. If it's not completed within two weeks, we'll have to patch the GHC mirror of containers to unblock upstream.
Ugh. I'll do something tonight or tomorrow.
@treeowl ping.
@treeowl ping.
Time scheduled for this today.
I'm attempting an alternative approach in #852, which plays "Let's pretend liftA2 has always been in the Prelude.`
I hate the approach in #852 a bit less, so I'm doing that instead.
Thanks!