Ghcide: Add support for record-dot-preprocessor

[pkapustin]

It would be very nice to support record dot syntax enabled by record-dot-preprocessor (either preprocessor or plugin).

[cocreature]

I think we might already support it as a plugin, see #192.

[pkapustin]

Is any configuration necessary?

I have a simple stack project with these lines in the source file:

{-# OPTIONS_GHC -fplugin=RecordDotPreprocessor #-}
{-# LANGUAGE DuplicateRecordFields, TypeApplications, FlexibleContexts, DataKinds, MultiParamTypeClasses, TypeSynonymInstances, FlexibleInstances #-}

...

data Company = Company {name :: String, owner :: Person}
data Person = Person {name :: String, age :: Int}

display :: Company -> String
display c = c.name ++ " is run by " ++ c.owner.name

nameAfterOwner :: Company -> Company
nameAfterOwner c = c{name = c.owner.name ++ "'s Company"}

...

The code compiles just fine, but ghcide complains:

Step 6/6: Type checking the files
File:     ./src/Lib.hs
Range:    10:14-10:18
Source:   typecheck
Severity: DsError
Message: 
  src/Lib.hs:10:15: error:
  Ambiguous occurrence ‘name’
  It could refer to either the field ‘name’,
  defined at src/Lib.hs:7:23
  or the field ‘name’, defined at src/Lib.hs:6:25

I am running ghcide 0.0.5 and GHC 8.6.5

[cocreature]

Not sure, I think our test case is a typechecker plugin. I wouldn’t be surprised if source plugins don’t work right now but it was worth a try at least :)

[ndmitchell]

See #282 which gets the preprocessor working - I specifically tested with record-dot-preprocessor.

I have rarely had success getting the plugin working on Windows in ghci, so have no idea if that works or not.

[pkapustin]

Thanks, it works with the preprocessor now!
I think the plugin still does not work for some reason. I was trying to debug this earlier today and printed out the list of loaded plugins in DynFlags after the initializePlugins call. RecordDotPreprocessor was in the list, but type checking still returned errors like if it was not loaded.

[ndmitchell]

I've tagged this with good first issue since I think a little bit of debugging might illuminate the issue. Could be as simple as failing to call some init method.

[neongreen]

@ndmitchell I'm interested in working on this. Do you think the problem is in ghcide (and is not necessarily specific to the RDP plugin), or in the plugin itself?

[ndmitchell]

@neongreen Pretty sure (~90%) the problem is in ghcide itself - but writing a minimal plugin (or taking a minimal plugin from somewhere else) would be a great first step to proving that.

[neongreen]

Hmmmm. But hasn't ghcide already been established to work with plugins, given https://github.com/haskell/ghcide/pull/192?

I.e. if it does work with plugins, then we're not looking for a minimal plugin, but we want to take a minimal plugin and progressively add RDP features to it until ghcide starts breaking. Right?

[ndmitchell]

True, so I guess there's more to it than that. The GHC Plugin https://hackage.haskell.org/package/ghc-8.10.1/docs/Plugins.html#t:Plugin allows plugins to run at the parsedResultAction (which RDP does) or tcPlugin (which the one in #192 does), so that's probably the culprit. It might well be that we need initPlugins in the parser, and the type checker, as the above patch only added them in the latter.

[srid]

I'm looking into this.

It might well be that we need initPlugins in the parser, and the type checker, as the above patch only added them in the latter.

The dynflags thing (which initPlugins updates) gets created and used in several places. It doesn't look like patching it using initializePlugins session in those places, at least in parseModule and processor functions, fixes it. I'll take a look at how the ghc plugin works, and do some more debugging.

[ndmitchell]

Normally plugins are specified in pragmas at the top of the file. You may need to make sure you call the init plugins after those pragmas have been applied to the file.

[srid]

Okay, this has been tricky so far, so I could use some tips.

AFAIU, the OPTIONS pragma that specifies the plugin gets loaded by the parseDynamicFilePragma function, which is used here:

https://github.com/haskell/ghcide/blob/59e8bb91a2b55f53e7ff15d263be88ccf017d117/src/Development/IDE/Core/Preprocessor.hs#L140-L148

I verified that the dflags in line 147 does contain the plugin module RecordDotPreprocessor. However, calling initializePlugins env dflags right after ('env' is passed higher up from the preprocessor function) -- which returns a new dflags that gets returned from this function -- has no effect.

Here, I simply assume that initializePlugins will do its job -- as in, it registers the plugin to be run at the appropriate phase, right? I seem to have run out of things to investigate here ... but surely I must be missing something ....

[ndmitchell]

One thing to try would be putting a non-existent plugin module, e.g. RecordDotPreprocessorNot in the options and see if you get an error. If you do, then something tried loading the plugin. If you don't, then its being ignored.

Maybe also check the cachedPlugins field of DynFlags, as that should gain the flags.

[srid]

Non-existent plugin does give an error as expeced. As cachedPlugins contains the plugin, upon doing initializePlugins. So I'm pretty sure the plugin gets loaded; well at least, at the end of preprocessor function. But somehow at the "Source: typecheck" stage it fails as if the record dot was never processed:

[ndmitchell]

One issue is that often GHC uses DynFlags that don't correspond to the current DynFlags, but to earlier DynFlags. I think that ModuleGuts (or similar) has an internal DynFlags. Possible that those are being consulted?

We have also copied some of the parsing functions. If you search the GHC code base, perhaps you can find where the parser plugins are applied. Is that something we are calling? Or is it an explicit call we should be adding on top? e.g. GHC might have a function reallyDoAParse which calls the plugin and then actualParse, and we might only be calling actualParse.

Thinking about it, the second of those seems more likely.

[srid]

Okay, progress!

Basically ghcide's parseFileContents should be taught (as it doesn't uses GHC's hscParse' -- which applies the plugins -- to do the parsing) to apply the plugin via withPlugins, passing it the plugins in parsedResultAction - mimicking what hscParse' does.

Doing that I can get a test module using the record preprocessor syntax to typecheck with ghcide.

[ndmitchell]

Nice work! That, plus a test case, seems all that's required to get a patch in.