Incorrect "unnecessary import" warning

[NeilMitchell]

With the code:

{-# OPTIONS_GHC -Wall -fno-warn-missing-methods #-}
module Test where
import Data.Typeable
import Data.Data
data Test = Test
instance Typeable Test where

I get the warning:

    Warning: Module `Data.Typeable' is imported, but nothing from it is used
               except perhaps instances visible in `Data.Typeable'
             To suppress this warning, use: import Data.Typeable()

It seems like it's decided to pick up Typeable from Data.Data, but it's also available from Data.Typeable which would be a better choice. I'm not sure how this should be solved, but the current behaviour seems incorrect. Perhaps if two modules are imported, both of which export the same thing, they should both be allowed.

[simonpj]

As it happens, the HEAD says

bash-3.2$ ~/builds/HEAD/inplace/bin/ghc-stage2 -c T3419.hs

T3419.hs:4:0:
    Warning: The import of `Data.Data' is redundant
               except perhaps to import instances from `Data.Data'
             To import instances alone, use: import Data.Data()

but we are making an arbitrary choice here; either import can be omitted without making the program fail to compile. Still, I don't think it's right to say nothing here; after all, only one of those imports is needed. Why do you think that "the current behaviour is incorrect"?.

See Commentary/Compiler/UnusedImports for what we've now done in the HEAD. Feel free to suggest specifications other than the one given there. For example, you could say that the "home module" of an entity dominates, so in this case pick Data.Typeable over Data.Data. That might be why your nose twitched?

Simon

[NeilMitchell]

It seems wrong to make an arbitrary choice. In my example Typeable should be imported from Data.Typeable usually, because that is the "home module" - but I certainly wouldn't want GHC to know about home modules. I think the only sensible answer for a warning is to identify the set of modules from which only one is required, and list them all - anything else is non-deterministic.

Of course, the alternative would be to not give a warning at all. Consider:

import Data.Typeable
import Data.Data
instance Data Foo where
instance Typeable Foo where

In this case technically the Typeable import is unnecessary, but I don't think it's something that should be warned on, as arguably it's better style.

[igloo]

We recently rewrote the code for this warning in the HEAD, and found a couple of places where there were non-obvious choices.

One was the case you're describing, where there are re-exports. The two choices were

• consider both imports used; assuming, as you say, that the user has mentally allocated different imported entites to different imports, so the user believes that they are both used even though one of them is technically redundant. I had another example of this, with Control.Monad.State, Control.Monad.Reader and Control.Monad.
• consider only the first import used; this is what is currently implemented. The first import decl to provide each of Data and Typeable is Data.Typeable, so nothing marks Data.Data as used, so it is warned about.

The other was to do with restricted vs complete imports. In this module:

module Foo where
import Data.List (sort)
import Data.List
f = sort

which import should we warn about?

Both of these are just a question of what we want; the code changes required are minimal.

[igloo]

Too late to change this for 6.12, so I'd suggest that if anyone is not happy with the new behaviour that it would be best brought up on the users mailing list once 6.12 has been released.