Skip to content

Remove redundant constraints#11814

Open
leana8959 wants to merge 1 commit into
haskell:masterfrom
leana8959:remove-redundant-constraints
Open

Remove redundant constraints#11814
leana8959 wants to merge 1 commit into
haskell:masterfrom
leana8959:remove-redundant-constraints

Conversation

@leana8959
Copy link
Copy Markdown
Collaborator

@leana8959 leana8959 commented May 12, 2026 •
edited
Loading

There are quite some constraints that are not needed, so we remove them.
I added the flag to each of the packages that got this warning, except test packages.

TODO (for me) :

  • document why it's ok to remove field grammar constraints

Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

@leana8959 leana8959 force-pushed the remove-redundant-constraints branch 2 times, most recently from 3ee0590 to 30ae160 Compare May 12, 2026 14:23
@leana8959 leana8959 marked this pull request as ready for review May 12, 2026 14:25
ulysses4ever
ulysses4ever approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

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

A little scary how big it is. It'd be great if all of it was trivial (like replacing Semigroup m, Monoid m with Monoid m). The one thing I'm truly unsure of is FieldGrammar: it'd be good to document why it's okay to remove those.

Comment thread cabal-install/src/Distribution/Client/SetupWrapper.hs
Comment thread Cabal/Cabal.cabal Outdated
Comment thread Cabal-syntax/src/Distribution/PackageDescription/FieldGrammar.hs
@philderbeast
Copy link
Copy Markdown
Collaborator

philderbeast commented May 12, 2026

On the master branch, for building Cabal's packages, sampling important *.cabal files, we support "base >= 4.13" as minimum bound, so ghc-8.8.1 and later.

  • The Functor-Applicative-Monad changes landed in ghc-7.10
  • The Semigroup-Monoid Inversion landed in ghc-8.4.

From base-4.8.0.0 changelog entry, date Mar 2015 (ghc-7.10.1):

Make Applicative a superclass of Monad

From base-4.11.0.0 changelog entry, dated 8 March 2018 (ghc-8.4.1):

Make Semigroup a superclass of Monoid; export Semigroup((<>)) from Prelude; remove Monoid reexport from Data.Semigroup (#14191).

So we're safe to remove -Wredundant-constraints that would not have been redundant earlier on.

@Bodigrim
Copy link
Copy Markdown
Collaborator

Bodigrim commented May 12, 2026

I'd not bother touching HasVerbosity, HasCallStack and WithCallStack. Even if at the moment the functions does not use them, they might very easily start, and it would be a busy work to add it back.

The rest seems fine and safe to remove.

@leana8959
Copy link
Copy Markdown
Collaborator Author

leana8959 commented May 12, 2026

@Bodigrim That's a reasonable concern. I can put -Wno-redundant-constraints in each of the file where HasCallStack and alike are used (will do that tomorrow though).

@leana8959 leana8959 force-pushed the remove-redundant-constraints branch from 30ae160 to f029341 Compare May 13, 2026 09:39
@leana8959
Remove redundant constraints
ae60693 
Add -Wredundant-constraints ghc flag to ghc-optionss.config.

HasVerbosity, HasCallStack and WithCallStack constraints are not
touched. -Wno-redundant-constraints is added for these modules

FieldGrammar allows instances to specify a constraint that its methods
should follow. ParsecFieldGrammar is an instance of FieldGrammar
(when using the grammar as a parser) that c is instantiated as Parsec;
PrettyFieldGrammar is also an instance of FieldGrammar (when using
the grammar as a printer) that c is instantiated as Pretty.

With that in mind:

    For ParsecFieldGrammar, Parsec (List FSep Token String) holds.
    For PrettyFieldGrammar, Pretty (List Fsep Token String) holds.

I removed this and all tests still pass, so this constraint is not
needed (i.e. we actually don't need to parse or print a List FSep Token
String).
@leana8959 leana8959 force-pushed the remove-redundant-constraints branch from f029341 to ae60693 Compare May 13, 2026 12:16
@ulysses4ever
Copy link
Copy Markdown
Collaborator

ulysses4ever commented May 13, 2026

CI is green. The only thing left is a merge label.

@leana8959 leana8959 added the merge me Tell Mergify Bot to merge label May 13, 2026
@mergify mergify Bot added the ready and waiting Mergify is waiting out the cooldown period label May 13, 2026
@ulysses4ever ulysses4ever mentioned this pull request May 14, 2026
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@philderbeast philderbeast philderbeast approved these changes

@ulysses4ever ulysses4ever ulysses4ever approved these changes

Assignees

No one assigned

Labels

merge me Tell Mergify Bot to merge ready and waiting Mergify is waiting out the cooldown period

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@leana8959 @philderbeast @Bodigrim @ulysses4ever