Allow GHC plugins to be called with an updated StringBuffer

[adinapoli]

While converting LiquidHaskell into a plugin, we tried to integrate it with ghcide. It has been a journey with some lumps and bumps, but eventually we succeeded (with proviso). The plugin itself requires the minimum version of GHC to be 8.10.1 (for technical reasons) and soon enough we discovered that plugins and 8.10.1 don't get along too well, mostly due to https://gitlab.haskell.org/ghc/ghc/-/issues/18070 (which was originally discovered as part of #556).

However, I've tried building a custom ghc version (via Hadrian, as Leon Schoorl mentioned in that GHC ticket) and indeed got ghcide to sort of work with the LH plugin. I say "sort of" because we were observing some strange behaviour where diagnostics would be emitted only when saving the corresponding file, and not in real time, as usual. This happened (in short) for two reasons:

• ghcide seems to not be calling any other plugin "hook" than the typecheckResultAction. It doesn't do it directly,
  but rather indirectly as part of the GHC API call to typecheckModule. This means that plugins have only "one single place"
  where they can intercept the normal ghcide workflow and do something else;

• Crucially speaking, when ghcide was calling the typecheckResultAction, it was doing it with a stale view of the file
  being checked. In other terms, plugins received an input ModSummary that virtually pointed to the on disk snapshot of
  a file, and not the in memory one that the user was editing (via its editor of choice).

However, a point could be made that is ghcide's responsibility to call any registered plugin with the same view of the world that ghcide also "sees", to ensure there is no mismatch between what users can do and that plugins can do.

This patch fixes it:

While this works, it's also my first foray into ghcide hacking, so I have no clue whether or not this patch could add a severe overhead to the typechecking rule. If I understand correctly getFileContents caches the information, so in theory if the file doesn't change too often, this call is relatively cheap. Having said that, there might be better ways to achieve the same result I simply didn't think of.

Other caveats

This fix relies, as most fixes in this space, on the delicate house of cards provided by the GHC API. It works for LH (that needs to re-parse the input module) because the parseModule function from the GHC API looks first at the ms_hspp_buf content, and only if that's Nothing, it re-parses a module. On top of that, we have no guarantees (in the types, that is) that somebody else won't override the ms_hspp_buf downstream, before calling GHC.typecheckModule.

[digitalasset-cla]

All committers have signed the CLA.

[pepeiborra]

Thanks for the PR and the detailed description! I think that your diagnosis is spot on, and I am interested in figuring out why the ms_hspp_buf field is incorrectly populated in the ModSummary passed to typecheckModule.

Did you try populating the field in getModSummaryFromBuffer? The signature will need to be extended in order to take the StringBuffer as an argument, but that should be all.

https://github.com/digital-asset/ghcide/blob/master/src/Development/IDE/Core/Compile.hs#L431

[adinapoli]

Hey @pepeiborra ! Thanks for getting back to me so quickly 😉

Did you try populating the field in getModSummaryFromBuffer? The signature will need to be extended in order to take the StringBuffer as an argument, but that should be all.

I haven't. In my rabbit hole exploration I came across another homemade ModSummary construction as part of getModSummaryFromImports, and I've tried replacing the ms_hspp_buf content there, but that didn't change a thing. When I found the other ModSummary you mention (as part of getModSummaryFromBuffer) I didn't see any StringBuffer ready to be used as said "screw that, I am going to fix this as close as possible to the typecheckModule call 😉

But you make a good point, that ms_hspp_buf feels like the morally correct place to edit. I will try and report back.

[adinapoli]

@pepeiborra Ah, success! You intuition was definitely correct. Properly setting the ms_hspp_buf inside getModSummaryFromBuffer did the trick, and didn't require the extra call to getFileContents. Nice! 😉

[cocreature]

Great work, thank you!