Skip to content

GHC 9.4#885

Merged
mrkkrp merged 1 commit intomasterfrom
amesgen/ghc-9.4
Nov 9, 2022
Merged

GHC 9.4#885
mrkkrp merged 1 commit intomasterfrom
amesgen/ghc-9.4

Conversation

@amesgen
Copy link
Copy Markdown
Contributor

@amesgen amesgen commented May 12, 2022 •
edited
Loading

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
@tfausak tfausak mentioned this pull request Aug 17, 2022
Closed
@ysangkok
Copy link
Copy Markdown

ysangkok commented Aug 22, 2022 •
edited
Loading

@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
Copy Markdown
Contributor 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.

amesgen
amesgen commented Aug 23, 2022
View reviewed changes
expected-failures/brittany.txt
+ }
+

AST of input and AST of formatted code differ.
Copy link
Copy Markdown
Contributor Author

@amesgen amesgen Aug 23, 2022

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
@July541 July541 mentioned this pull request Sep 17, 2022
Closed
34 tasks
@9999years 9999years mentioned this pull request Oct 21, 2022
Closed
mrkkrp
mrkkrp reviewed Nov 2, 2022
View reviewed changes
Copy link
Copy Markdown
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?

@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
Copy Markdown

9999years commented Nov 9, 2022

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
Copy Markdown

georgefst commented Nov 9, 2022

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
Copy Markdown

9999years commented Nov 9, 2022

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
@9999years 9999years mentioned this pull request Nov 10, 2022
Closed
@amesgen amesgen mentioned this pull request Apr 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrkkrp mrkkrp mrkkrp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@amesgen @ysangkok @9999years @georgefst @mrkkrp