haskell / core-libraries-committee

95 stars 16 forks source link

Status of Data.List compatibility warning #269

Closed mpickering closed 1 month ago

mpickering commented 5 months ago

There is currently a warning implemented in -Wcompat which warns you when importing Data.List in a non-qualified manner.

A.hs:3:8: warning: [-Wcompat-unqualified-imports]
    To ensure compatibility with future core libraries changes
    imports to Data.List should be
    either qualified or have an explicit import list.
  |
3 | import Data.List
  |        ^^^^^^^^^
Ok, one module loaded.

GHC ticket: https://gitlab.haskell.org/ghc/ghc/-/issues/17244 CLC discussion: https://groups.google.com/g/haskell-core-libraries/c/q3zHLmzBa5E

This warning was implemented as part of the migration to making Data.List monomorphic again (and to be used like Data.Set, Data.Map etc). That doesn't seem like it happened, and I imagine that the current CLC would require a new proposal anyway in order to do that now. It's not clear in any case what "future core libraries changes" we are waiting to happen before this warning can be removed.

Therefore, I propose to remove this warning and to "start again" with a transition plan if people want to modify Data.List in some manner.

This is now motivated because ghc-internal is split from base we want to remove base as a wired-in package (so there is potential for it to be reinstalled).

Tracking GHC ticket: https://gitlab.haskell.org/ghc/ghc/-/issues/24904

thielema commented 5 months ago

On Wed, 29 May 2024, Matthew Pickering wrote:

There is currently a warning implemented in -Wcompat which warns you when importing Data.List in a non-qualified manner.

A.hs:3:8: warning: [-Wcompat-unqualified-imports] To ensure compatibility with future core libraries changes imports to Data.List should be either qualified or have an explicit import list. | 3 | import Data.List | ^^^^^^^^^ Ok, one module loaded.

GHC ticket: https://gitlab.haskell.org/ghc/ghc/-/issues/17244 CLC discussion: https://groups.google.com/g/haskell-core-libraries/c/q3zHLmzBa5E

If people import Data.List unqualified they usually rely on the assumption that the functions from Data.List are actually the same as from Prelude, so no name clashes occur. I think this is generally a bad idea and so the warning is still justified.

Bodigrim commented 5 months ago

@mpickering the latest CLC discussion on the topic is at https://github.com/haskell/core-libraries-committee/issues/22 and the decision is recorded at https://github.com/haskell/core-libraries-committee/blob/main/guides/monomorphic-data-list.md.

As I said earlier, "it would set a bad precedent if current CLC just dismisses its earlier decision, without a material change of circumstances". You seem to imply that split of base into base and ghc-internal is a material change, could you elaborate how is it related?

mpickering commented 5 months ago

(This issue is also discussed here: https://gitlab.haskell.org/ghc/ghc/-/issues/21791#note_566618)

Thanks for the link. Yes I agree that dismissing earlier decisions would have set a bad precedent.

@Bodigrim I think the material change in circumstances is that ghc-internal and base are now different packages. The goal here is now for ghc-internal to contain all wired in things and base to be a completely normal package. At the moment this warning is the most serious blocker to making base a normal package. The base package has to remain wired-in so that the compiler can identify which module is Data.List. (This issue is tracked by https://gitlab.haskell.org/ghc/ghc/-/issues/24904)

Given the first phase of the proposal has lasted 5 years it doesn't seem that anyone is motivated to carry the proposal to completion. It does seem a bit unnecessary to include a warning in the compiler about "future changes to the module" when there's no timeline or volunteer to implement these changes. Do we just carry on this warning forever?

adamgundry commented 5 months ago

-Wcompat-unqualified-imports requires that imports of Data.List are "either qualified or have an explicit import list". But having an explicit import list isn't enough to ensure compatibility with the proposed Glorious Future in which Data.List is monomorphic but Prelude is not, is it? You can still have

import Data.List (any)

foo = any p [x,y,z]

which will not issue a warning under -Wcompat-unqualified-imports, but will break once Data.List exports a different any from the one in Prelude.

There also seems to be a lack of enthusiasm for carrying out the change (e.g. the CLC guidelines include "Ask proponents of the change to prepare patches for all Stackage packages." as a precondition for carrying it out, which is a rather high bar).

On that basis, I think the most reasonable course of action would be for GHC to drop support for -Wcompat-unqualified-imports as it currently stands.

Bodigrim commented 5 months ago

-Wcompat-unqualified-imports requires that imports of Data.List are "either qualified or have an explicit import list". But having an explicit import list isn't enough to ensure compatibility with the proposed Glorious Future in which Data.List is monomorphic but Prelude is not, is it?

Correct and https://github.com/haskell/core-libraries-committee/blob/main/guides/monomorphic-data-list.md#when recognises this deficiency.


Here is my non-binding opinion on the matter:

The intention of CLC when defining the policy was to have a warning on by default (well, at least with -Wall) before breaking things. The precise name or nature of the warning are not that important. If GHC HQ / GHC SC decide that the existing flag is ill-designed or unsuitable under changed circumstances, it just means that the monomorphisation would have to wait until a new flag is designed, introduced and enabled by default, potentially indefinitely long.

Bodigrim commented 5 months ago

@tomjaguarpaw @hasufell @parsonsmatt @velveteer @mixphix @angerman any more opinions?

parsonsmatt commented 5 months ago

From what I can tell, we're at the point now where we have an accepted policy and plan for monomorphizing Data.List, and we're at the right time point to actually do this, since the relevant releases have happened (right?).

I'd be hesitant to scrap the warning without an official CLC decision to cancel the plan. We are currently making relatively long term plans in order to aid stability, and it would be bad if long-term plans were vulnerable to cancellation.

Bodigrim commented 5 months ago

we're at the right time point to actually do this, since the relevant releases have happened (right?).

What do you mean?

parsonsmatt commented 5 months ago

ie the warning was added in GHC X.(y-1) and we're at GHC X.Y right now? I'm not sure about the timeline. If we are still waiting for the appropriate release, do we have a mechanism in place to review the plan for it and attempt to accomplish it?

Bodigrim commented 5 months ago

The warning was never added to -Wall, so we are at GHC X.Y-2 at best.

angerman commented 5 months ago

I tend to agree with @adamgundry view on this. It might be best to just scrap the flag altogether. And reboot the whole discussion once someone steps up who actually drive this. Maybe we also need to consider some form of timeouts for changes that end up not happening (due to lack of interest, energy, ...). If a proposed change does not happen for say 4 or 6 releases (2-3 years), scrap the change until someone comes up to drive and implement it fully?

I'll admit this is a tricky situation, especially in light of it blocking other cleanups.

mpickering commented 2 months ago

GHC MR which removes this warning: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/13200

mpickering commented 1 month ago

The warning will be removed in 9.12. Thanks everyone.