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

Avoid Data.List.{head,tail} #160

Closed
wants to merge 3 commits into from
Closed

Avoid Data.List.{head,tail} #160

wants to merge 3 commits into from

Conversation

Bodigrim
Copy link
Contributor

@Bodigrim Bodigrim commented Dec 1, 2022

CLC has approved the proposal to add {-# WARNING #-} to Data.List.{head,tail} (haskell/core-libraries-committee#87). It means that usage of head and tail will emit compile-time warnings.

This patch eliminates the only usage of head in parsec.

@phadej
Copy link
Collaborator

phadej commented Dec 1, 2022

TBH, I'd rather disable the warning. Newer version is not clearer at all.

@Bodigrim
Copy link
Contributor Author

Bodigrim commented Dec 1, 2022

No problem, warning disabled.

@@ -1,6 +1,9 @@
{-# LANGUAGE DeriveDataTypeable #-}
{-# LANGUAGE Safe #-}

-- Disable {-# WARNING #-} for Data.List.head:
{-# OPTIONS_GHC -Wno-warnings-deprecations #-}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll hold merging this until ghc-proposals/ghc-proposals#541 is reviewed and hopefully implemented. Disabling all warnings is too blunt.

I feel Data.List.head/tail warning/deprecations was rushed.

@Bodigrim
Copy link
Contributor Author

I'm not a fan of disabling warnings myself. I've made another attempt, which not only avoids head but also hopefully has a benefit of being more clear than the original. How does it look now?

@Bodigrim
Copy link
Contributor Author

Bodigrim commented Jan 8, 2023

@phadej any chance to look at this please?

FWIW I do not plan to backport the {-# WARNING #-} into ghc-9.6 branch, which should give proponents of more granular warnings plenty of time to implement them.

@phadej
Copy link
Collaborator

phadej commented Jan 8, 2023

has a benefit of being more clear than the original. How does it look now?

I disagree it being clearer. I'll try myself.

Is there a hurry?

@Bodigrim
Copy link
Contributor Author

Bodigrim commented Jan 8, 2023

Is there a hurry?

I'd be very grateful if we make some progress here, parsec is the last stumbling block remaining, other boot packages are already compliant.

@phadej
Copy link
Collaborator

phadej commented Jan 8, 2023

other boot packages are already compliant.

Is there a requirement for them [boot packages] to be warning free? What set, -Wall? Why?

@Bodigrim
Copy link
Contributor Author

Bodigrim commented Jan 8, 2023

Yes.

@phadej
Copy link
Collaborator

phadej commented Jan 8, 2023

Why? That's stupid. There's nothing wrong in parsec. Why I need to change it. Yeah, sure head is bad, but it was fine here for 10 years, and you are not even going to remove it from Prelude in any foreseeable future. Are you?

@Bodigrim
Copy link
Contributor Author

Bodigrim commented Jan 8, 2023

It's not me setting rules here, just hadrian/build --flavour=validate barks at any boot library warning.

At the moment I have no plans to remove Prelude.head indeed.

@phadej
Copy link
Collaborator

phadej commented Jan 8, 2023

I complained to GHC folks: https://gitlab.haskell.org/ghc/ghc/-/issues/22729 I'm very frustrated by this.

I made the change in #164 which I'm more happy with structurally. (I don't find your version clearer that what is in master)

@phadej phadej closed this Jan 8, 2023
@Bodigrim
Copy link
Contributor Author

Bodigrim commented Jan 8, 2023

Thanks @phadej, I'm very happy with this resolution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants