-
Notifications
You must be signed in to change notification settings - Fork 483
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
Conversation
plutus-benchmark/marlowe/README.md
Outdated
@@ -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.) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CC @zliu41
There was a problem hiding this comment.
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). |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
There was a problem hiding this 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.
There was a problem hiding this 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.
plutus-benchmark/marlowe/README.md
Outdated
@@ -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.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CC @zliu41
There was a problem hiding this comment.
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.
This PR removes support for GHC 8.10.