Skip to content

Drop Support for GHC 8.10 #7037

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 11 commits into from
Apr 29, 2025
Merged

Drop Support for GHC 8.10 #7037

merged 11 commits into from
Apr 29, 2025

Conversation

zeme-wana
Copy link
Contributor

This PR removes support for GHC 8.10.

@zeme-wana zeme-wana marked this pull request as draft April 14, 2025 16:14
@zeme-wana zeme-wana added the No Changelog Required Add this to skip the Changelog Check label Apr 14, 2025
@@ -71,7 +71,7 @@ The golden files contain the counted CPU and memory budget of the scripts.

### Versioning of `marlowe-internal`

Note that the off-chain code is evolving. However the on-chain code is very stable and is compatible with GHC 8.10.7. For best benchmarking results, eventually we may have to update some of these files by hand if the on chain code is updated. (We don't want to depend on the Marlowe repository because this will have the problem of circular dependency.)
Note that the off-chain code is evolving. However the on-chain code is very stable and is compatible with !!! GHC 8.10.7. For best benchmarking results, eventually we may have to update some of these files by hand if the on chain code is updated. (We don't want to depend on the Marlowe repository because this will have the problem of circular dependency.)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this note still relevant?

Copy link
Contributor

Choose a reason for hiding this comment

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

CC @zliu41

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it is relevant

@@ -166,7 +166,7 @@ GHC Core. Not doing so results in 20+% larger Core for builtins.

However in a few specific cases we use @NOINLINE@ instead to get tighter Core. @NOINLINE@ is the
same as @OPAQUE@ except the latter _actually_ prevents GHC from inlining the definition unlike the
former (we use the former because we currently have to support GHC-8.10).
former (we use the former because we currently have to support !!! GHC-8.10).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we replace all NOINLINE with OPAQUE?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

@zeme-wana zeme-wana marked this pull request as ready for review April 14, 2025 19:36
Copy link
Contributor

@kwxm kwxm left a comment

Choose a reason for hiding this comment

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

This looks OK to me, although I'm not very familiar with some of the stuff that mentions 8.10 (like the Marlowe benchmarks), so we should wait for review from other people. There might be other things that we can improve with 8.10 gone, but it's hard to be sure if there aren't any comments saying so.

Copy link
Contributor

@effectfully effectfully left a comment

Choose a reason for hiding this comment

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

Impressive how you managed to track down all this very diverse and often implicit references to GHC-8.10.

I just pushed a few commits to the branch reversing some of the stuff we did earlier to support GHC-8.10. Hope I didn't break anything, but it's possible, so please bear with me in that case.

@@ -71,7 +71,7 @@ The golden files contain the counted CPU and memory budget of the scripts.

### Versioning of `marlowe-internal`

Note that the off-chain code is evolving. However the on-chain code is very stable and is compatible with GHC 8.10.7. For best benchmarking results, eventually we may have to update some of these files by hand if the on chain code is updated. (We don't want to depend on the Marlowe repository because this will have the problem of circular dependency.)
Note that the off-chain code is evolving. However the on-chain code is very stable and is compatible with !!! GHC 8.10.7. For best benchmarking results, eventually we may have to update some of these files by hand if the on chain code is updated. (We don't want to depend on the Marlowe repository because this will have the problem of circular dependency.)
Copy link
Contributor

Choose a reason for hiding this comment

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

CC @zliu41

@kwxm kwxm temporarily deployed to github-pages April 29, 2025 14:24 — with GitHub Actions Inactive
@kwxm kwxm temporarily deployed to github-pages April 29, 2025 15:42 — with GitHub Actions Inactive
Copy link
Contributor

@kwxm kwxm Apr 29, 2025

Choose a reason for hiding this comment

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

This has changed because of the replacement of some variable names with underscores here.

@kwxm kwxm merged commit 5de7bbe into master Apr 29, 2025
64 checks passed
@kwxm kwxm deleted the drop-ghc810 branch April 29, 2025 18:56
@kwxm kwxm mentioned this pull request Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Changelog Required Add this to skip the Changelog Check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants