wiki:Commentary/Compiler/UnusedImports

Version 1 (modified by simonpj, 5 years ago) (diff)

--

Unused imports

GHC has a series of bugs related to the "report unused imports" flags, including #1148, #2267, #1074, #2436. This page describes a new design.

The idea is that for each use of an imported name, we will attribute that use to one or more import decls. Then, any import decls with no uses attributed to them are unused, and are warned about.

The current story

Currently (GHC 6.10) we report three different things:

  • warnUnusedModules: import M, where nothing is used from M
  • warnUnusedImports: import M(f), where f is unused, and M doesn't fall under warnUnusedModules
  • warnDuplicateImports: import M + import M(f), even when f is used complain about duplicate import of f

Examples

The hard bit is to specify what the warning should do. Consider these examples, where Foo exports x and y, and FooPlus re-exports all of Foo, plus z:

  module X0 where            	   module X1 where	
    import Foo	             	     import Foo		
    import Foo( x )          	     import Foo( x )	
    bar = x	             	     bar = x+y		

  module X2 where            	   module X3 where	
    import Foo( x, y )	     	     import Foo( x, y )	
    import Foo( x )	     	     import Foo( x )	
    bar = x		     	     bar = x + y         
 
  module X4 where            	   module X5 where	      
    import Foo( x, y ) 	     	     import Foo( x, y ) as Bar 
    import Foo( x, y )	     	     import Foo( x, y )	      
    bar = x + y		     	     bar = x + Bar.y           
 
  module X6 where                  module X7 whjre	
    import Foo( x, y ) as Bar	     import FooPlus(x,y)	
    import Foo( x, y ) 		     import FooPlus(y,z)	
    bar = Foo.x + Bar.y		     import FooPlus(z,x)	
				     bar = (x,y,z)       

  module X8
    import Control.Monad
    import Control.Monad.State
    import Control.Monad.Reader
	-- NB : Control.Monad.State re-exports all of Control.Monad
	--      so the first decl is actually redundant

Which import is redudant, in each case?

Also: we might warn if you import the same module more than once, and the imports can be combined (ie they have the same 'qualified' and 'as' attributes)

  module Y1 where
    import Foo(x)
    import Foo(y)
    bar = (x,y)

Here both are used, but we might want to suggest combining them.

Specfication

We can at least agree on this:

  • If the warning suggests that an import can be omitted, and you omit it, the program should still compile.
  • It's not worth trying to be too subtle. The 90% case is very simple.

Here is an attempt at a specification:

  1. Say that an import-item is either an entire import-all decl (eg import Foo), or a particular item in an import list (eg import Foo( ..., x, ...)).
  2. For every RdrName in the program text, choose one of the import-items that brought it into scope, the "chosen import-item", and mark it "used".
  3. Now bleat about any import-items that are unused. For a decl import Foo(x,y), if both the x and y items are unused, it'd be better to bleant about the entire decl rather than the individual items.

In step 2, the lookup mechanism on RdrNames already takes account of whether the RdrName was qualified, and which imports have the right qualification etc.

The import-item choosing step 2 implies that there is a total order on import-items. We say import-item A dominates import-item B if we chooose A over B. Here is one possible dominance relationship:

  • import Foo dominates import Foo(x). (You could also argue that the reverse should hold.)
  • Otherwise choose the textually first one.

Note that this algorithm chooses exactly one import-item in step 2. It would also be sound to choose more than one if there was a tie, but then completely-duplicate imports might not be reported.

Note that if we have an import item import Foo (Bar(bar)), then it's marked as used if either Bar or bar are used. We could have yet finer resolution and report even unused sub-items.


Implementation

We want to collect the set of all RdrNames that are mentioned in the program. We must collect RdrNames not Names:

   import Foo( x ) as Bar
   import Foo( x )
   q = (Foo.x, Bar.x)

Here both imports are required, but you can only tell that by seeing the RdrNames?, not by knowing that the name 'x' is used.

I think that all lookups go through a single function,

RnEnv.lookupGreRn_maybe

So in RnEnv.lookupGreRn_maybe, if (gre_prov gre) is (Imported _) hen put rdr_name in a new

tcg_used_imports :: TcRef (Set RdrName)

in TcGblEnv. All the tcg_used_imports are in scope; if not, we report an error and do not add it to tct_used_imports.

Other notes

  • Any particular (in-scope) used RdrName is bought into scope by one or more RdrName.ImportSpec's. You can find these ImportSpecs in the GRE returned by the lookup.
  • The unit of "unused import" reporting is one of these ImportSpecs.
  • Suppose that 'rn' is a used, imported RdrName, and 'iss' is the [ImportSpecs] that brought it into scope. Then, to a first approximation all the iss are counted 'used'.
  • We can compare ImportSpecs for equality by their SrcSpans

Also perhpas:

  • In TcRnDriver.tcRnImports, save import_decls in a new tcg_rn_rdr_imports :: Maybe [LImportDecl RdrName] in TcGblEnv
  • In RnNames.reportUnusedNames, call a function that actually does the hard work. The ugly part is when we have an import of Foo(..). Then we need to dig through the environments, similar to what RnNames.exports_from_avail does, so that we can expand the (..).