Skip to content

Cleanup CPPs, remove support for GHC 9.4 #4567

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

Merged
merged 10 commits into from
Jun 4, 2025

Conversation

dschrempf
Copy link
Collaborator

Closes #4529.

Also,

  • update flake lock
  • some girl scout changes while trying to understand some code

@dschrempf dschrempf force-pushed the cleanup-cpps-purge-ghc94 branch from 1f5bef2 to 47cc528 Compare April 23, 2025 17:44
@fendor
Copy link
Collaborator

fendor commented Apr 24, 2025

Fyi, in the biweekly HLS meeting, we have discussed to not drop support for 9.4 until the next release 2.11.0.0 which will happen once GHC 9.10.2 is released.

Afterwards, we will drop 9.4 support.

@dschrempf
Copy link
Collaborator Author

Thanks for the information! I leave this PR in draft mode for now, but feel free to close it, and I can reopen in due time!

@michaelpj
Copy link
Collaborator

btw you might find this useful: https://www.michaelpj.com/blog/2023/11/20/cpp-partial-evaluation.html

@fendor
Copy link
Collaborator

fendor commented May 23, 2025

@dschrempf since we released HLS 2.11, we are ready to drop support for GHC 9.4 :)

@dschrempf dschrempf force-pushed the cleanup-cpps-purge-ghc94 branch from 47cc528 to 8a5b3b8 Compare May 25, 2025 10:53
@dschrempf dschrempf force-pushed the cleanup-cpps-purge-ghc94 branch 2 times, most recently from 026a91c to 3e4a801 Compare June 1, 2025 15:23
@dschrempf dschrempf changed the title Cleanup CPPs, begin deprecation of GHC 9.4 Cleanup CPPs, remove support for GHC 9.4 Jun 1, 2025
@dschrempf
Copy link
Collaborator Author

dschrempf commented Jun 1, 2025

@fendor I have removed #if MIN_VERSION_ghc(9,4,0) statements, but there are a lot of #if MIN_VERSION_ghc(9,5,0) statement left. Also, this is a bit of a nightmare because stylish-haskell fails to parse some of the amended files having many CPP statements. I had to rearrange two already.

Also, the Nix CI fails for MacOS for a reasons I don't understand. I have to investigate. I do not have more time now, maybe I get to it tomorrow. If you take over, please let me know!

Cheers!

EDIT: Ad Nix failure for MacOS. I am not sure if we should test this in CI.

@dschrempf
Copy link
Collaborator Author

dschrempf commented Jun 2, 2025

I tackled all MIN_VERSION_ghc(9,5,...) macros, as well as GLASGOW_HASKELL macros related to GHC 9.4. I also removed GHC 9.4 from the tests.

There is one question left in the tests. At a point we expect "ghc94.expected" (I didn't undertand this, maybe it was wrong in the first place?). I will highlight the code change with a comment.

Also there is a lot more macros a'la MIN_VERSION_ghc(9,6,...) which we also can remove by now, but I don't have any energy left ^^. This PR is HUGE already.

Depending on how the CI will go: I think we should merge this soon, conflicts will be really ugly.

@@ -44,7 +44,9 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macOS-latest]
# TODO: Fix compilation problems on macOS.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

NOTE: I remove macOS from the Nix CI.

@dschrempf
Copy link
Collaborator Author

I just realized this is PR 4567.

@dschrempf dschrempf force-pushed the cleanup-cpps-purge-ghc94 branch from e2a9871 to df9d591 Compare June 4, 2025 03:55
@dschrempf
Copy link
Collaborator Author

Some notes:

  • I think stylish-haskell and CPPs do not play well togehter. stylish-haskell seems to ignore conditional CPPs entirely and fails in many cases. How did you deal with this in the past? I had to copy some code into both branches of the conditionals for stylish-haskell to parse the files.

  • It is difficult to know which warnings lead to CI errors, and which warnings don't. Do you think it is possible to use the same warning/error configuration for local builds and the CI?

  • In general, I think somebody has to clean build HLS and fix warnings. I think some warnings are induced by changes between GHC and base libraries, but even those should be fixed with CPP statements, shouldn't we?

Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

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

Amazing work, I didn't anticipate this would be so much work! Thank you for taking care of this, it is much appreciated!

@fendor fendor marked this pull request as ready for review June 4, 2025 07:28
@fendor
Copy link
Collaborator

fendor commented Jun 4, 2025

I think stylish-haskell and CPPs do not play well togehter.

formatters don't work with CPP in general, we usually just ignore these kinds of errors in the pre-commit job, it is not blocking 🙊 Sorry for not telling you sooner, but I think you improved the overall code base considerably!

It is difficult to know which warnings lead to CI errors

Good point, I think ci runs with -fpedantic, but maybe we should borrow some lingo from javscript and have a flag such as -fci and document that in the user docs?

In general, I think somebody has to clean build HLS and fix warnings.

Making ghcide compile without warnings is a big amount of work with little payoff... Not sure anyone wants to tackle that.

@fendor fendor enabled auto-merge (squash) June 4, 2025 07:43
@fendor fendor merged commit 93a5f1b into haskell:master Jun 4, 2025
35 checks passed
@dschrempf dschrempf deleted the cleanup-cpps-purge-ghc94 branch June 4, 2025 11: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.

Cleanup CPP in GHC/Orphans.hs
3 participants