Skip to content

Raise upper bound on hashable#399

Merged
chshersh merged 3 commits intokowainik:mainfrom
JackKelly-Bellroy:hashable-1.4
Apr 4, 2022
Merged

Raise upper bound on hashable#399
chshersh merged 3 commits intokowainik:mainfrom
JackKelly-Bellroy:hashable-1.4

Conversation

@JackKelly-Bellroy
Copy link
Copy Markdown
Contributor

Resolves #394

Generates a couple of warnings with hashable-1.4, because Eq is now a superclass of Hashable:

src/Relude/Nub.hs:115:1: warning: [-Wredundant-constraints]
    • Redundant constraint: Eq a
    • In the type signature for:
           hashNub :: forall a. (Eq a, Hashable a) => [a] -> [a]
    |
115 | hashNub :: forall a . (Eq a, Hashable a) => [a] -> [a]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

src/Relude/Nub.hs:142:1: warning: [-Wredundant-constraints]
    • Redundant constraint: Eq a
    • In the type signature for:
           unstableNub :: forall a. (Eq a, Hashable a) => [a] -> [a]
    |
142 | unstableNub :: (Eq a, Hashable a) => [a] -> [a]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I can fix the type signatures with CPP if you like.

Checklist:

HLint

  • I've changed the exposed interface (add new reexports, remove reexports, rename reexported things, etc.).
    • I've updated hlint.dhall accordingly to my changes (add new rules for the new imports, remove old ones, when they are outdated, etc.).
    • I've generated the new .hlint.yaml file (see this instructions).

General

  • I've updated the CHANGELOG with the short description of my latest changes.
  • All new and existing tests pass.
  • I keep the code style used in the files I've changed (see style-guide for more details).
  • I've used the stylish-haskell file.
  • My change requires the documentation updates.
    • I've updated the documentation accordingly.
  • I've added the [ci skip] text to the docs-only related commit's name.

@vrom911
Copy link
Copy Markdown
Member

vrom911 commented Mar 29, 2022

Hi @JackKelly-Bellroy !
Thanks a lot for your contributions 🙏🏼

Regarding warning, I would say yes, I think we should add the CPP pragma so we will keep our eye on this one.
Would you like to do that as part of this PR?

@JackKelly-Bellroy
Copy link
Copy Markdown
Contributor Author

CPP is done. There are several ways to do this: write constraint over multiple lines, CPP away the Eq a,; CPP over the type signature; CPP over the whole function definition. I chose to CPP the type signature because it seemed the least likely to confuse code formatters and syntax highlighters. Let me know if this needs to change.

Copy link
Copy Markdown
Contributor

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

Looks nice 🙂

Unfortunately, CPP is unavoidable. Probably we can remove it in some future after a while

@chshersh chshersh merged commit fcb3225 into kowainik:main Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support hashable-1.4?

3 participants