Opened 11 years ago

Closed 8 years ago

#1074 closed bug (fixed)

-fwarn-unused-imports complains about wrong import

Reported by: guest Owned by: igloo
Priority: normal Milestone: 6.12.1
Component: Compiler Version: 6.6
Keywords: Cc: jyasskin@…, ganesh.sittampalam@…, gwern0@…, johan.tibell@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

If a symbol is accessible through multiple imports, ghc seems to complain about the first one, even if that's the one it's actually accessed through.

$ cat Test.hs
module Test where

import qualified Control.Monad (ap)
import qualified Control.Monad.Reader

foo :: IO ()
foo = return id `Control.Monad.ap` return ()

$ ghc --version
The Glorious Glasgow Haskell Compilation System, version 6.6

$ ghc -c Test.hs -Werror -fwarn-unused-imports

Test.hs:3:0:
    Warning: Module `Control.Monad' is imported, but nothing from it is used,
               except perhaps instances visible in `Control.Monad'
             To suppress this warning, use: import Control.Monad()

$

Attachments (1)

Test.hs (1.0 KB) - added by duncan 9 years ago.
test case

Download all attachments as: .zip

Change History (23)

comment:1 Changed 11 years ago by guest

Cc: jyasskin@… added

comment:2 Changed 11 years ago by simonpj

Milestone: _|_
severity: normalminor

Good bug report. Alas, it's not easy to fix. Here's why.

To report unused imports, GHC gathers up all the entities that are referred to (Names, in GHC's terminology). But that doesn't say *how* they are referred to, so it does not distinguish 'ap' from 'Control.Monad.ap'.

In this case, the import of Control.Monad is indeed required, because the reference was a qualified one.

So really this exposes an architectural shortcoming in the way GHC reports unused imports. Of course it's fixable, but it'd be non-trivial, so don't hold your breath.

Simon

comment:3 Changed 10 years ago by ghc@…

I assume that this is the same bug, that I encountered, too. It is possibly the same like http://hackage.haskell.org/trac/ghc/ticket/1148 I remember that such wrong warnings occured when switching from GHC 6.2 to GHC 6.4. Here I have setup a self-contained example:

http://users.informatik.uni-halle.de/~thielema/Haskell/ImportWarning/

I insert the example here:

module A where

type T = Int
module B ( module A ) where

import A
module C where

{-
This import is necessary but is reported as unnecessary.
"necessary" means:
If you remove that import statement, then the module cannot be compiled.
Of course, we could use the identifiers from B,
since B just re-exports A.
But formally I consider it a bug to declare the import of A as unnecessary.
-}
import qualified A

{-
This import is unnecessary but no warning is emitted.
If this import is removed, everything compiles fine without warnings.
-}
import B


f :: String -> A.T
f = read

comment:4 Changed 10 years ago by Isaac Dupree

I think this is the same bug, that I ran into:

module Foo where
import qualified Prelude as A (take)
import qualified Prelude as B

Warning: Imported from `Prelude' but not used: `B.take'

The module it refers to is the last applicable one in the import list - even if there would never be a warning about importing a particular identifier when it is not explicitly named in a relevant import list, and even if B.somethingElse is used. Re-ordering the imports "fixes" the warning; also, adding ...as B hiding (take) "fixes" it. Maybe this bit is fixable, assuming ghc remembers the import lines...

Also I noticed that replacing take with Int results in no warning at all - is this intentional? (note that, apparently GHC *never* gives unused-*module*-import warnings for Prelude, if considering this)

P.S. if {{{"necessary" means: If you remove that import statement, then the module cannot be compiled.}}} then GHC is designed (whether or not I agree with that design) to warn about those, when the only uses of that import are by function definitions that are never used or exported.

comment:5 Changed 10 years ago by simonpj

Thanks for further examples. The more examples we have, the more likely we are to get it right when we eventually fix this one! (It's trickier than one would think.)

But as my comment above says, to fix it properly requires some real work, and since it's (by definition) not a show-stopper, it's not top of the priority list. (Unless you all say it is!)

Simon

comment:6 Changed 10 years ago by simonpj

When fixing this, be sure to fix #1384 at the same time.

Simon

comment:7 Changed 10 years ago by simonmar

Architecture: powerpcMultiple
Operating System: MacOS XMultiple

comment:8 Changed 10 years ago by igloo

Here's the behaviour I'd like:

For each use of a variable foo, mark imports

import M (..., foo, ...)

as "used" if any exist; otherwise mark any imports

import M

which export foo as "used". Mark all imports of the form

import M ()

as used.

Then warn about any imports not marked as "used".

comment:9 Changed 10 years ago by igloo

See also #2267.

comment:10 Changed 9 years ago by simonmar

Architecture: MultipleUnknown/Multiple

comment:11 Changed 9 years ago by simonmar

Operating System: MultipleUnknown/Multiple

comment:12 Changed 9 years ago by igloo

Milestone: _|_6.10.2
Priority: normalhigh

This is biting more and more people, as warnings are increasingly enabled by users. Also, it's a real pain to workaround. Let's just rewrite the warning code and fix it.

See also #2681, #1148, #2267.

(nb. my algorithm above needs extending to work with qualified imports too)

comment:13 Changed 9 years ago by NeilMitchell

Cc: neil.mitchell.2@… added

comment:14 Changed 9 years ago by igloo

Owner: set to igloo

comment:15 Changed 9 years ago by guest

Cc: gwern0@… added

comment:16 Changed 9 years ago by tibbe

Cc: johan.tibell@… added

comment:17 Changed 9 years ago by ganesh

Cc: ganesh.sittampalam@… added; neil.mitchell.2@… removed

comment:18 Changed 9 years ago by igloo

Milestone: 6.10.26.10 branch

Changed 9 years ago by duncan

Attachment: Test.hs added

test case

comment:19 Changed 9 years ago by duncan

I've added a test case (based on the tar package). The expected outcome is no warnings with -Wall. The current outcome is:

Test.hs:3:0:
    Warning: Module `System.FilePath' is imported, but nothing from it is used,
               except perhaps instances visible in `System.FilePath'
             To suppress this warning, use: import System.FilePath()

The specific instance of the problem in this test case is that we import:

import qualified System.FilePath         as FilePath.Native
import qualified System.FilePath.Posix   as FilePath.Posix
import qualified System.FilePath.Windows as FilePath.Windows

and use things qualified from all three of them. The System.FilePath module is a re-export of one of the other two modules (but which one depends on the platform).

comment:20 Changed 9 years ago by igloo

Priority: highnormal

comment:21 Changed 9 years ago by igloo

Milestone: 6.10 branch6.12.1

comment:22 Changed 8 years ago by igloo

Resolution: fixed
Status: newclosed

Fixed by

Mon Jul  6 12:25:03 BST 2009  simonpj@microsoft.com
  * Major patch to fix reporting of unused imports
Note: See TracTickets for help on using tickets.