smuggler icon indicating copy to clipboard operation
smuggler copied to clipboard

Reuse more logic from GHC regarding detecting of unused imports

Open chshersh opened this issue 6 years ago • 4 comments

See here:

  • https://github.com/kowainik/smuggler/pull/67#discussion_r322067960

chshersh avatar Sep 10 '19 04:09 chshersh

There is a function printMinimalImports that seems to be designed for this purpose (see, eg, https://gitlab.haskell.org/ghc/ghc/issues/15439) that produces a TcRnTypes.RnM (). It may be trivial to wire it up to smuggler, but I haven't figured out how. Pointers welcome.

jrp2014 avatar Mar 29 '20 18:03 jrp2014

@jrp2014 This particular issue is actually about a different thing. @vrom911 refactored the printMinimalImports function in GHC, so we could use getMinimalImports to implement #42.

This issue is about refactoring another GHC function. Currently our implementation of unused imports has some duplicate code with GHC. Our code:

https://github.com/kowainik/smuggler/blob/b2ddcdad06a270e0b956f2203f9bebcd72301c53/src/Smuggler/Plugin.hs#L68-L96

Most work is done by the compiler, we only pattern-match on imports to decide whether the import is redudant or not. Here is the same code in GHC:

  • https://gitlab.haskell.org/ghc/ghc/-/blob/5c3cadf5db0d7eb859ff2c278ab07585c7df17b5/compiler/rename/RnNames.hs#L1478-1502

So first we need to refactor the warnUnusedImport function in GHC to be able to reuse more logic from the compiler. This is also less trivial, because in smuggler we would like to implement the removal of unused hidings, which is not supported by GHC at the moment, but can be easily added in here, since we have all required information. But GHC refactoring should be performed with these extension capabilities in mind.

chshersh avatar Apr 01 '20 13:04 chshersh

Thanks. Yes. Sorry, I should have pointed t getMinimalImports. That seems to do the trick, although it retains stubs for instance only imports, which is OK, at least at this stage.

The point at which I am stuck is grafting the result back into the ast and printing out the result (if there was a change). If I've understood smuggler correctly, it reparses the source to get an ast, replaces the imports, and exactPrints the result back out. It's a pity that that extra work is necessary, but I assume that the results of the parsing phase of the compiler are not available directly to a typechecker plug-in. I haven't dug too deep into the compiler but any pointers would, as I say, be welcome.

In summary, can we get away without using exactprint? (At a later stage it would be good to move to using ghc-lib.)

As things stand, with GHC 8.8.3 smuggler doesn't pass its unit tests.

jrp2014 avatar Apr 01 '20 17:04 jrp2014

I've now had a bit more of a look at this.

Regrettably, the getMinimalImports function generates a GhcRn minimal import list, which means that it can't just be grafted back into the GhcPs ast and ann that ghc-exactprint provides.

I haven't completely figured out how the ast and anns work together. Unless I've missed something, smuggler seems to be able to delete unused imports by deleting their anns but leaving the original ast, leaving the exactPrint function to resolve any discrepancies.

I wondered whether the exactprint transform machinery might provide a way of manipulating the ast and anns together.

jrp2014 avatar Apr 10 '20 17:04 jrp2014