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

GHC 9.4 #885

Merged
merged 1 commit into from
Nov 9, 2022
Merged

GHC 9.4 #885

merged 1 commit into from
Nov 9, 2022

Conversation

amesgen
Copy link
Member

@amesgen amesgen commented May 12, 2022

Soft blocked by input-output-hk/haskell.nix#1595 for GHC 9.4 on CI. Postponed, can happen later


Overview:

  • Switch to ghc-lib-parser-9.4.*. This includes:
    • Support for the new syntactic features in GHC 9.4 (changelog):
      • \cases via LambdaCase
      • OPAQUE pragmas
      • Unboxed sum type constructors like (# | #).
    • ghc-lib-parser-9.4.1 builds with GHC 8.10, but it is not explicitly supported. Hence, I switched from 8.10 to 9.2 as the default version. This required some changes in extract-hackage-info.
    • GHC AST changes:
      • More unreachable cases got TTG-ed away 🎉
      • For exact-printing purposes, many constructors now contain HsTokens, which we ignore.
  • Some changes to the nix setup: some previous haskell.nix workarounds are now unnecessary. Also, we now only set the GHC flags added in Minimize binary sizes #793 when actually building the executable, as otherwise the entire ghc library is rebuilt due to changed flags (which takes quite some time).
  • Ormolu Live:
    • I needed to patch ghc-lib-parser slightly for it to compile on GHCJS 8.10.7.
    • closure-compiler optimizations no longer preserves the semantics (all inputs suddenly fail to parse). Disabling them works, but increases the JS sizes by a lot. I haven't looked into this in detail, especially as we hopefully will be able to switch away from GHCJS to either the native WASM or JS backend in GHC 9.6.

@amesgen amesgen force-pushed the amesgen/ghc-9.4 branch 2 times, most recently from eb9139e to 57b0f6f Compare August 8, 2022 21:10
@ysangkok
Copy link

ysangkok commented Aug 22, 2022

@amesgen Hi again :) Can't you just remove th-lift-instances? It doesn't seem to be needed on modern GHC versions according to phadej/file-embed-lzma#11 . I tried building Ormolu without it but ghc-lib-parser is taking forever to build (seems unrelated)

EDIT: Oh, seems like it is needed

src/Ormolu/Fixity/Internal.hs:133:13: error:
    • No instance for (Lift (Map String FixityMap))
        arising from the first field of ‘HackageInfo’
          (type ‘Map String FixityMap’)
      Possible fix:
        use a standalone 'deriving instance' declaration,
          so you can specify the instance context yourself
    • When deriving the instance for (Lift HackageInfo)
    |
133 |   deriving (TH.Lift)
    |             ^^^^^^^

@amesgen
Copy link
Member Author

amesgen commented Aug 23, 2022

Hi @ysangkok! Yeah, we need the Lift instance for containers. We could also require containers >= 0.6.6 (which has those instances natively) and hence remove th-lift-instances, but GHCJS (which we use here) does not play nicely with reinstalling boot packages, so I didn't go with it ATM.

+ }
+

AST of input and AST of formatted code differ.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is due to us (knowningly, see #725) not supporting DatatypeContexts, but it would be trivial to add.

All other new expected-failures seem to be known comment-related idempotency bugs.

@amesgen amesgen marked this pull request as ready for review August 23, 2022 08:25
Copy link
Member

@mrkkrp mrkkrp left a comment

Choose a reason for hiding this comment

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

Great job, as usual! I think that the main prerequisite for merging it is still the availability of GHC 9.4.1 in haskell.nix. Do you know if there have been any updates regarding that?

.buildkite/pipeline.yml Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
cabal.project Show resolved Hide resolved
cabal.project Show resolved Hide resolved
ormolu.cabal Show resolved Hide resolved
src/Ormolu/Printer/Meat/Common.hs Show resolved Hide resolved
default.nix Show resolved Hide resolved
@amesgen amesgen force-pushed the amesgen/ghc-9.4 branch 3 times, most recently from 0d0c682 to 4863f5f Compare November 3, 2022 09:39
@9999years
Copy link

Is this still blocked on haskell.nix support? We've had GHC 9.4 in nixpkgs for a while now, and releasing GHC 9.4 support for Ormolu is blocking GHC 9.4 support for Fourmolu. In turn, this is blocking improved GHC 9.4 support in haskell-language-server. I'd really love to get this released on Hackage, even if haskell.nix can't build it, because everything else using GHC 9.4 can.

@georgefst
Copy link

Is this still blocked on haskell.nix support? We've had GHC 9.4 in nixpkgs for a while now, and releasing GHC 9.4 support for Ormolu is blocking GHC 9.4 support for Fourmolu. In turn, this is blocking improved GHC 9.4 support in haskell-language-server. I'd really love to get this released on Hackage, even if haskell.nix can't build it, because everything else using GHC 9.4 can.

This is slightly confusing: do you mean to suggest that HLS is blocked on Fourmolu, but somehow not on Ormolu directly? It seems to me that Fourmolu is mostly irrelevant to the issue at hand (although we are certainly looking forward to this PR being merged, and grateful as ever for the effort!).

@9999years
Copy link

do you mean to suggest that HLS is blocked on Fourmolu, but somehow not on Ormolu directly?

The Fourmolu maintainers don't want to incorporate+release this patch to add GHC 9.4 support until Ormolu does. HLS can't support Fourmolu or Ormolu on GHC 9.4 until they can be built on GHC 9.4.

Fourmolu is directly relevant for me (we use it to format our Haskell code at Mercury) but only related to this issue as an example of downstream work on GHC 9.4 that's blocked (directly or indirectly) on this PR being merged.

Either way, I would like to reiterate that I'm extremely grateful to @amesgen for putting this patch together & for all the maintainers and reviewers who are working to merge it.

If there's anything I can do to accelerate these efforts, please let me know.

@mrkkrp mrkkrp merged commit 54642a7 into master Nov 9, 2022
@mrkkrp mrkkrp deleted the amesgen/ghc-9.4 branch November 9, 2022 18:43
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.

5 participants