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

Support ghc-9.12 #510

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Support ghc-9.12 #510

wants to merge 1 commit into from

Conversation

erikd
Copy link
Contributor

@erikd erikd commented Nov 8, 2024

Description

Should not be merged until SRP and most allow-newers are removed.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated
  • All visible changes are prepended to the latest section of a CHANGELOG.md for the affected packages.
    New section is never added with the code changes. (See RELEASING.md)
  • When applicable, versions are updated in .cabal and CHANGELOG.md files according to the
    versioning process.
  • The version bounds in .cabal files for all affected packages are updated.
    If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)
  • Self-reviewed the diff

@erikd erikd requested review from nfrisby and lehins as code owners November 8, 2024 02:33
@erikd erikd marked this pull request as draft November 8, 2024 02:33
--
-- -- Copied from the "NoThunks.Class"
-- class GShowTypeOf f where gShowTypeOf :: f x -> String
-- instance Datatype c => GShowTypeOf (D1 c f) where gShowTypeOf = datatypeName
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about this. GWNoThunks seems to have been removed from the nothunks library.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have asked @nfrisby in Slack about this.

Comment on lines +98 to +107
if impl(ghc >= 9.12)
ghc-options: -Wno-deriving-typeable
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is better to remove all places where Typeable is being derived, instead of turning off the warning

Copy link
Contributor Author

@erikd erikd Dec 13, 2024

Choose a reason for hiding this comment

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

I am assuming that all places that Typeable is derived, it is needed for compilers < 9.12.

Assuming that's the case, there are two options:

  • DIsabling the warning for ghc >- 9.12
  • Use CPP to hide the derivation of Typeable whne ghc >= 9.12

I thought the former was the least invasive solution.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am assuming that all places that Typeable is derived

FYI. All types by default in haskell starting with ghc-7.10 have Typeable, so it does not need to be derived anymore. That is exactly what the warning is about.

So, please do not disable warnings, they are actually trying to tell you something.

All you need to do is remove deriving Typeable. That's all.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here is some documentation that you can read about it: https://hackage.haskell.org/package/base-4.20.0.1/docs/Data-Typeable.html#description

@erikd erikd force-pushed the erikd/ghc-9.12 branch 4 times, most recently from 8abcd5a to 36fd7e6 Compare November 16, 2024 22:44
@erikd erikd force-pushed the erikd/ghc-9.12 branch 3 times, most recently from 77ee665 to a4c941c Compare December 13, 2024 06:23
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