Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use custom warning category for Data.List.NonEmpty.unzip #258

Closed
Bodigrim opened this issue Feb 21, 2024 · 12 comments
Closed

Use custom warning category for Data.List.NonEmpty.unzip #258

Bodigrim opened this issue Feb 21, 2024 · 12 comments
Labels
approved Approved by CLC vote base-4.20 Implemented in base-4.20 (GHC 9.10)

Comments

@Bodigrim
Copy link
Collaborator

Bodigrim commented Feb 21, 2024

In #86 (comment) we decided to attach to Data.List.NonEmpty.unzip a warning {-# DEPRECATED unzip "This function will be made monomorphic in GHC 9.14, consider switching to Data.Functor.unzip" #-}.

Since then, following GHC#541, GHC gained an ability to annotate warnings with custom categories to allow users disable them in a fine-grained manner. Because of this material change, I suggest we amend #86 by adding a category:

-{-# DEPRECATED unzip "This function will be made monomorphic in GHC 9.14, consider switching to Data.Functor.unzip" #-}
+{-# WARNING in "x-data-list-nonempty-unzip" unzip "This function will be made monomorphic in GHC 9.14, consider switching to Data.Functor.unzip" #-}

This would allow users, who already use Data.List.NonEmpty.unzip with monomorphic NonEmpty arguments, just disable the warning with {-# OPTIONS_GHC -Wno-data-list-nonempty-unzip #-} instead of changing their code to use Data.Functor.unzip (which arguably pushes them into a wrong direction of intentionally polymorphic function).

I'm not sure what's the best category name here. I provisionally put x-data-list-nonempty-unzip, but open for other options.

GHC 9.10 will be the first GHC release shipped with #86, so I'm keen to reach a conclusion on this very soon, before a release branch is forked.

Cf. https://gitlab.haskell.org/ghc/ghc/-/issues/24316

The MR is available at https://gitlab.haskell.org/ghc/ghc/-/merge_requests/12157.

@parsonsmatt
Copy link

Sounds great! Let's do it.

Maybe we should even WARN when a WARNING doesn't have a custom name 🤔

@hasufell
Copy link
Member

Do we want short lived warning categories? The proposed one seems very specific.

I don't have a better idea though, so whatever works.

@tomjaguarpaw
Copy link
Member

tomjaguarpaw commented Feb 22, 2024

I'm not sure I see the point in making this change so users can suppress a warning in merely two releases (9.10 and 9.12). If a user has already migrated away then it's irrelevant to them. If a user has already suppressed this deprecation it's going to cause them churn. If they haven't, do they really care?

EDIT: I still prefer my suggestion at #86 (comment) which is to introduce a specifically monomorphic version which users can switch to.

@Bodigrim
Copy link
Collaborator Author

I'm not sure I see the point in making this change so users can suppress a warning in merely two releases (9.10 and 9.12). If a user has already migrated away then it's irrelevant to them. If a user has already suppressed this deprecation it's going to cause them churn. If they haven't, do they really care?

While discussion in the proposal suggests the warning to be introduced in 9.8, the MR was delayed for various reasons and landed after GHC 9.8 fork date. So GHC 9.10 will be the first version featuring it. Does it explain?

@tomjaguarpaw
Copy link
Member

Ah yes, I see

GHC 9.10 will be the first GHC release shipped with #86

That makes more sense. The deprecation has not yet appeared: https://hackage.haskell.org/package/base-4.19.1.0/docs/Data-List-NonEmpty.html#v:unzip.

So the proposal is to not use DEPRECATED at all but instead use a custom warning. That sounds fine.

@Bodigrim
Copy link
Collaborator Author

If there are no more comments / opinions, I'll trigger a vote soon.

@Bodigrim
Copy link
Collaborator Author

Bodigrim commented Mar 1, 2024

Dear CLC members, let's vote on the proposal to amend #86 by adding a category:

-{-# DEPRECATED unzip "This function will be made monomorphic in GHC 9.14, consider switching to Data.Functor.unzip" #-}
+{-# WARNING in "x-data-list-nonempty-unzip" unzip "This function will be made monomorphic in GHC 9.14, consider switching to Data.Functor.unzip" #-}

The MR is available at https://gitlab.haskell.org/ghc/ghc/-/merge_requests/12157.

@tomjaguarpaw @mixphix @hasufell @velveteer @parsonsmatt @angerman


+1 from me, unsurprisingly.

@tomjaguarpaw
Copy link
Member

+1

2 similar comments
@velveteer
Copy link
Contributor

+1

@hasufell
Copy link
Member

hasufell commented Mar 2, 2024

+1

@Bodigrim
Copy link
Collaborator Author

Bodigrim commented Mar 2, 2024

Thanks all, that's enough votes to approve.
(Sorry for cutting it short, I'm keen to get the MR merged and backported before GHC 9.10 alpha 1)

@hsenag
Copy link
Member

hsenag commented Jun 2, 2024

Just to note that I have some code that is already using it monomorphically and had to add two suppressions to keep it warning-free across a range of GHCs. That was a bit frustrating particularly as it took me a while to work out that was the right solution. My first instinct was to switch to Data.Functor.unzip as suggested, but that isn't available in older GHCs and anyway calling the monomorphic variant is more sensible here.

{-# OPTIONS_GHC -Wno-unrecognised-warning-flags #-}
{-# OPTIONS_GHC -Wno-x-data-list-nonempty-unzip #-}

@Bodigrim Bodigrim added the base-4.20 Implemented in base-4.20 (GHC 9.10) label Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved by CLC vote base-4.20 Implemented in base-4.20 (GHC 9.10)
Projects
None yet
Development

No branches or pull requests

6 participants