Support Wingman on ghc 9.2

[santiweight]

Wingman does not currently compile with 9.2. I've investigated a little and I think I've figured out one of the main barriers for the migration.

In <9.x, the staticPlugins field was in DynFlags. Something like:

data DynFlags = {
  ...,
  staticPlugins :: [StaticPlugin]
  }

In 9.2+, plugins are now contained in the Plugins datatype:

data Plugins = Plugins
  { staticPlugins :: ![StaticPlugin]
  , loadedPlugins :: ![LoadedPlugin]
  , loadedPluginDeps :: !([Linkable], PkgsLoaded)
  }

which is now found in HscEnv:

data HscEnv
  = HscEnv {
        hsc_dflags :: DynFlags,
        , hsc_plugins :: !Plugins
  }

Afaict, this means that the PluginDescriptor type should really have a HscEnvModifications field, or alternatively also include a GhcPluginsModifications field.

I figure this is quite a hefty and widespread change, since you would have to update all tutorials and current HLS plugins; so maybe someone knows of a better way to modify the HscEnv?

Paging @isovector and @anka-213 since they might have good feedback

[santiweight]

Linking to #2179

[michaelpj]

Well, I think pluginModifyDynFlags was added exclusively for wingman and I don't see any other users, so I think you can do what you want with it. That said, it seems subtle, looking at the original discussion: #1814

[ProofOfKeags]

Please <3. How pervasive a change would it be? Is it doable by a first time contributor?

[santiweight]

I investigated some more yesterday (I would be a first time contributor; though I've poked around the internals before).

The biggest barrier has ended up being the GHC exactprint API changes. I was able to modify the plugins logic to compile, but then ran into a bunch of errors relating to the new exactprint AST in GHC. This is definitely surmountable, but it is hard for me to tell immediately what parts of the AST isovector is using, and for what. I was working through whack-a-moling the exactprint errors when I stopped. I'll be coming back to it soon...

For example, I can't tell which pass Wingman lives in. Much of the logic is polymorphic in its GHC pass but that no longer works (I think) because you can only annotate certain passes of the GHC with certain annotations. So such polymorphism might be less possible iiuc.

@ProofOfKeags if you want to work on this together let me know :)

[santiweight]

I hope I'm not misusing the issue thread! But here's some materials that could help the migration (I haven'y looked through them yet.:

• a 20 min yt video on the new annotations: https://www.youtube.com/watch?v=GkoQbJofm1A&t=469s
• a blog similar to the above: https://blog.shaynefletcher.org/2021/05/annotations-in-ghc.html
• the design doc/spec for the in-tree exactprint changes: https://gitlab.haskell.org/ghc/ghc/-/wikis/api-annotations

[isovector]

I'm happy to help someone on getting this up and running! I don't have too much time to put towards it right now, but happy to answer questions / do a pairing session.

I can't tell which pass Wingman lives in.

Code synthesis happens in GhcPs, while extraction of the context for auto/etc happens in GhcTc.

---

I suspect most of the value in Wingman is in case split, which we could feasibly pull out as a much smaller plugin --- minimizing the maintenance burden on the GHC AST.

[isovector]

The static plugin stuff is necessary for case..of completion (but not function body splitting) and for inline metaprograms (which AFAIK nobody uses). In the former, we want to enable -XAllowEmptyCase, and in the latter, -XTemplateHaskellSplices

[santiweight]

I think, as you say, the best way forward could well be to extract the simpler case splitting logic stuff and maintain the two aspects of Wingman in separate plugins. Wingman is already quite large - but you would know best about whether the splitting would reduce maintenance burden.

[ProofOfKeags]

The biggest barrier has ended up being the GHC exactprint API changes. I was able to modify the plugins logic to compile, but then ran into a bunch of errors relating to the new exactprint AST in GHC. This is definitely surmountable, but it is hard for me to tell immediately what parts of the AST isovector is using, and for what. I was working through whack-a-moling the exactprint errors when I stopped. I'll be coming back to it soon...

Is it possible to get a diff of what happened to the GHC AST between then and now?

...and for inline metaprograms (which AFAIK nobody uses)

I haven't figured out how to use them. Perhaps discoverability and explorability is the barrier there. Perhaps that can be a different plugin still though.

I think, as you say, the best way forward could well be to extract the simpler case splitting logic stuff and maintain the two aspects of Wingman in separate plugins. Wingman is already quite large - but you would know best about whether the splitting would reduce maintenance burden.

I wonder if anything else is usefully carvable.

Lastly, do you have a branch you're working off of @santiweight

[santiweight]

I don't have a lot of work: just the plugin infrastructure update and the start of the GHC AST update.

but for that I got stuck with a whole bunch of GhcPs<->GhcTc generalization in Wingman, which might be best to split out. As much as the generalization is nice, it certainly incurs some mental overhead. This is something I'd really want @isovector's opinion on first in a phone call etc.

I can push a branch later today: are HLS devs okay with random draft PRs or is a fork preferable?

Is it possible to get a diff of what happened to the GHC AST between then and now?

It's not really going to be super helpful; the exactprint changes are mostly a logical shift in the GHC AST. The changes are well described in some of those above links, but migrating to the new AST is not a simple "use this datatype instead". Instead, we'll have to figure out whether the generalization Wingman has assumed is still applicable to the new AST...

In other words: the diff is the entire AST's annotation infrastructure, and every annotation for GhcPs and GhcTc has changed. So the diff will simply be to reimplement everything for the new exactprint annotations.

[ProofOfKeags]

In other words: the diff is the entire AST's annotation infrastructure, and every annotation for GhcPs and GhcTc has changed. So the diff will simply be to reimplement everything for the new exactprint annotations.

I see. Perhaps I should dive into the code, hence me asking about whether or not you had a branch. But I can also fork it myself and take a look. Just figuered a consolidated effort would be more effective.

[santiweight]

Also, with the new exactprint infrastructure, it might be best to only support GHC 9.2.1+. I'm a little apprehensive about the potential spaghetti caused by supporting two exactprint infrastructures as a newer-to-ghcer. Perhaps @alanz or Shayne Fletcher (I don't know his user) could give us an estimate on the difficulties of developing a compatibility layer vs using CPP.