ndmitchell
updates for compatibility with ghc-9.10
upgrade from the ghc-9.8 to the ghc-9.10 api. fixes https://github.com/ndmitchell/hlint/issues/1593
@alanz @zliu41 apply-refact updates (ghc-exactprint > 1.8.0.0) will be needed
I will bump the version number on my https://github.com/alanz/ghc-exactprint/tree/ghc-9.10 branch (tomorrow) if that will help. If I have a way of installing the new cabal for github CI, I can make a release
I will bump the version number
Done
thanks alan, that is indeed a help (although of course, we will need a release in due course). @zliu41 with this cabal.project file
allow-newer: all
source-repository-package
type: git
location: https://github.com/alanz/ghc-exactprint.git
tag: f334b0c2da5ca9193297e77a9d1fc620f859ffde
packages: apply-refact.cabal
cabal install all produces the errors in this pastebin https://pastebin.com/HyvFvR8R indicating a few fixes to apply-refact source itself will be needed in addition to this.
cabal install all produces the errors in this pastebin https://pastebin.com/HyvFvR8R indicating a few fixes to apply-refact source itself will be needed in addition to this.
@shayne-fletcher I've fixed these errors but there are many more. I'll continue fixing them over the next week. The working branch is https://github.com/mpickering/apply-refact/tree/ghc-9.10
Note: you need to use a Cabal-3.12-based pre-release of cabal-install to be able to compile ghc-exactprint.
See https://discourse.haskell.org/t/ann-cabal-3-12-0-0-released/9504#how-to-get-the-cabal-install-pre-release-3
cabal install all produces the errors in this pastebin https://pastebin.com/HyvFvR8R indicating a few fixes to apply-refact source itself will be needed in addition to this.
@shayne-fletcher I've fixed these errors but there are many more. I'll continue fixing them over the next week. The working branch is https://github.com/mpickering/apply-refact/tree/ghc-9.10
i hope this saves you some time @zliu41:
on the ghc-9.10.1-fixes branch on my fork https://github.com/shayne-fletcher/apply-refact.git, after making this change, of the 449 tests, the failure count is reduced from 249 to 79.
@zliu41 given the magnitude of the GHC 9.10.1 upgrade, it might make sense to strip the CPP, and adjust the bounds so the next version only works with GHC 9.10.1 (as originally happened with apply-refact, IIRC).
And my notes on the changes are at https://gist.github.com/alanz/e127e7561ddf1cfeb07fbdee9a966794
given the magnitude of the GHC 9.10.1 upgrade, it might make sense to strip the CPP
@alanz Good point! Let me see how bad it is. And thanks for the notes!
Heads up all, I am in the process of making the transform API pure, should be making a new ghc-exactprint release soon. See https://github.com/alanz/ghc-exactprint/commit/5e7e8abcbaa05075570a1703e95009b19c468d79
Update: I've made it build but there are many test failures. Will continue investigating. I don't have an ETA at the moment, but I should have some time to work on it during ZuriHac (if I haven't finished it by then).
I should have some time to work on it during ZuriHac
I can possibly lend a hand. But tied up from now until then
Update: I've made it build but there are many test failures. Will continue investigating. I don't have an ETA at the moment, but I should have some time to work on it during ZuriHac (if I haven't finished it by then).
as i mentioned above @zliu41 , removing the call to makeDeltaAst at line 739 of Internal.hs Right ast -> pure $ Right (makeDeltaAst ast)(on your branch) halves the failure count to 135 of 449 tests.
@shayne-fletcher Sorry I completely missed your message above :sweat_smile: Thanks for helping out!
Worked on it for about 2 hours today with the help of @alanz, and reduced the number of test failures from 175 to 172 😅 Will continue later.. looks like a lot more setEntryDPs needed.
Sorry for the lack of follow up - I've been on PTO since after ZuriHac. Will try to get back to this as soon as I can.
The problem I have: before GHC 9.10, EpAnn has both RealSrcSpan and delta (the latter is from AnchorOperation). Both are useful for apply-refact: the RealSrcSpan is used to locate the subexpression to be replaced, and the delta is convenient for carrying out the replacement - the old and new subexpressions often have different lengths, so using delta obviates the need to adjust a lot of RealSrcSpans.
However, in 9.10, there's only either RealSrcSpan or delta (from EpaLocation', depending on whether I makeDeltaAst or not), which appears to make the job a lot harder.
@alanz any suggestions? cc @jhrcek
A concrete example:
--input
yes = if x then (f y) else z
--expected
yes = if x then f y else z
--actual, GHC 9.10, extra space before `else`
yes = if x then f y else z
Prior to 9.10, all apply-refact needs to do is replacing (f y) with f y. It just works, presumably because the deltas are available from MovedAnchor, and else has EpaDelta (SameLine 1), so ghc-exactprint is able to do the right thing.
In 9.10, however, the fact that the deltas aren't available may necessitate updating the EpaSpan of else to make it work, but that would be infeasible to do in general.
By the way, I don't know why there's only one extra space in the output before else; I'd expect to see two, because the EpaSpan of else is unchanged, which means the else in the output should be in the same position as the input. So there seems to be something I'm missing. @alanz Do you know why that is? And how about a version of makeDeltaAst that keeps SrcSpans?
either RealSrcSpan or delta (from EpaLocation'
I have realised that this is a serious mistake.
The intended flow is you work on the original, and just have to use deltas where things change. But this does not work, when there are comments sprinkled around
This is what has me stuck in the retrie update.
I suspect we need to go back to a version with a span it it always for reference, and optional delta.
I suspect the best way forward is more long term
- I properly document what we have, and why
- We mutually agree what needs to change
- We do it
But that is likely to leave GHC 9.10.x out in the cold. Or at the least 9.10.1
See https://gitlab.haskell.org/ghc/ghc/-/merge_requests/12993, a first cut at restoring it. I think it will be pretty non-invasive
See https://gitlab.haskell.org/ghc/ghc/-/merge_requests/12993, a first cut at restoring it. I think it will be pretty non-invasive
Thanks, looks good. Hopefully this gets into 9.10.x; if not, I can make another attempt in apply-refact to make things work by processing the original modu and makeDeltaAst modu simultaneously. That would be hacky and ugly, and tricky to get right but it could potentially work in most cases.
That would be hacky and ugly, and tricky to get right
Exactly my experience for retrie. I think declaring 9.10.1 a bust and fixing it for 9.10.2+ is the only practical way forward
What's the update with Retrie? What should we be doing here?
I broke ghc-exactprint for GHC 9.10, so retrie cannot be updated. The fix will be in GHC 9.12, but given hlint builds effectively against GHC master you can try the version at https://github.com/alanz/ghc-exactprint/tree/ghc-head which compiles and passes all tests, using head.hackage, and an update to the 'extra' package.