kadena-io / pact

The Pact Smart Contract Language
https://pact-language.readthedocs.io/en/stable/
BSD 3-Clause "New" or "Revised" License
580 stars 100 forks source link

Fix GHC 9.2.8 builds due to Prelude.liftA2 #1325

Closed enobayram closed 7 months ago

enobayram commented 8 months ago

liftA2 has been reexported from Prelude only since base-4.18 (GHC 9.6.X), so references to liftA2 without the accompanying Control.Applicative import causes build failures in earlier GHC versions. Simply adding import Control.Applicative (liftA2) to this repo fixes the issue for earlier GHC versions, but that causes an "unused import" warning in GHC 9.6, which becomes a build failure due to the -Werror flag.

This PR allows us to build pact with GHC 9.2.8 as well as GHC 9.6.3 without any issues, by importing Control.Applicative qualified and referring to liftA2 with the Control.Applicative. prefix, avoiding the warning in GHC 9.6.3 and fixing the missing reference in 9.2.8.

I'm not sure how important that is for the pact project, but chainweb-data currently can't be upgraded to GHC 9.6.X due its transitive dependencies (see https://github.com/kadena-io/chainweb-data/pull/172) and it also can't stay with an earlier pact version, since pact made a sudden jump from 8.10 to 9.6 skipping all the intermediate GHC releases, so it may, in general, be desirable to allow it to build with earlier versions.

Note that the fully qualified Control.Applicative.liftA2 reference is ugly, but I'm not sure what would've been a less ugly solution... Using CPP would be even uglier, or I could replace liftA2 with ... <$> ... <*> ..., but that could in (far-fetched) theory affect the performance, so opted for a fix that's obviously equivalent.

Feel free to close this PR if GHC 9.2.8 is considered irrelevant for pact.


I'm skipping the PR checklist since it's hard to imagine how this change can have any effect on the program behavior.

_Skipped PR checklist_ PR checklist: * [ ] Test coverage for the proposed changes * [ ] PR description contains example output from repl interaction or a snippet from unit test output * [ ] Documentation has been updated if new natives or FV properties have been added. To generate new documentation, issue `cabal run tests`. If they pass locally, docs are generated. * [ ] Any changes that could be relevant to users [have been recorded in the changelog](https://github.com/kadena-io/pact/blob/master/CHANGELOG.md) * [ ] In case of changes to the Pact trace output (`pact -t`), make sure [pact-lsp](https://github.com/kadena-io/pact-lsp) is in sync. Additionally, please justify why you should or should not do the following: * [ ] Confirm replay/back compat * [ ] Benchmark regressions * [ ] (For Kadena engineers) Run integration-tests against a Chainweb built with this version of Pact
enobayram commented 7 months ago

@0xd34df00d I've replaced liftA2 with <$> & <*>, anything else left before we can merge this?

emilypi commented 7 months ago

LGTM @enobayram - can @jwiegley or @jmcardon please :+1: this so we can get it in?