Opened 11 years ago

Closed 8 years ago

#1148 closed bug (fixed)

Bad warnings about unused imports that are in fact needed

Reported by: brianh Owned by:
Priority: lowest Milestone:
Component: Compiler Version: 6.8.2
Keywords: warning unused import Cc:
Operating System: Windows Architecture: x86
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

If I compile the following code with the -W flag I get a warning about the import of Data.Ix that shouldn't be generated, since this import is actually needed:

{-# OPTIONS_GHC -fglasgow-exts #-}
module ArrayBoundedU
   ( T
   , create
   , at
   ) where

import Data.Ix
import qualified Data.Array.Unboxed as Array
import Data.Array.Base (unsafeAt)

newtype T i e = T (Array.UArray i e)

create :: (Ix i, Bounded i, Array.IArray Array.UArray e) => [(i,e)] -> T i e
create ies = T (Array.array (minBound, maxBound) ies)

at :: (Ix i, Bounded i, Array.IArray Array.UArray e) => T i e -> i -> e
at (T a) i = unsafeAt a (index (minBound, maxBound) i)

with a sample Main

module Main where

import Data.Ix
import ArrayBoundedU

data I = One | Two | Three
   deriving (Eq, Ord, Ix, Bounded)

main = do
   let
      a = create [(One, 10::Int), (Two, 20), (Three, 30)]
   putStrLn (show (at a Two))

built using ghc6.6 with:

ghc --make Main.hs -W

I get the warning:

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

However if I do this then the code doesn't compile at all, since the import is needed.

This is just a very trivial little thing but it prevents me from being able to get my code to compile with no warnings (unless I switched the warning off altogether but then something else which really was unused would go undetected).

Change History (8)

comment:1 Changed 11 years ago by igloo

Milestone: 6.8

Confirmed (also in 6.6 branch and HEAD).

comment:2 Changed 11 years ago by simonpj

I took a look. The whole way that unused-imports are reported is wrong, really. We collect a set of "free variables", which are used Names, but that loses the info about whether the program text mentioned the thing qualified or unqualified. And that makes a difference to whether an import is unnecessary:

  import qualifed Foo( x, y )
  import Foo( x )
  p = x
  q = Foo.x

If (but only if) the binding for p is omitted, the second import is unnecessary. So we should really collect the set of used RdrNames.

This would not be too big a deal. In lookupOccRn, record the RdrName for non-local lookups in a mutable variable. Look at that set in reportUnusedNames.

But it'd take a couple of hours.

comment:3 Changed 10 years ago by igloo

Milestone: 6.8 branch_|_

comment:4 Changed 10 years ago by Eelis-

Version: 6.66.8.2

I ran into this bug, and isolated it to this:

import List
import Prelude
main = return () where b = List.null ""

GHC warns that nothing from List is used, but the code fails to compile without the import.

comment:5 Changed 10 years ago by simonpj

This is actually a slightly different issue to the one that started this ticket. Consider

module Foo(x) where
  import List(null)
  x = 3
  y1 = null
  y2 = y1
  y3 = y2

What would you like?

  • Report y3 unused, only. The trouble here is that when you delete y3 you now get a new warning that y2 is unused. And so on.
  • Report y3,y2,y1,null all unused? This gets them all in one one go, but if you don't respond to them all (e.g. you delete only y1), the module won't compile. And currently different flags affect unused (a) imports and (b) local bindings, so GHC might report null unused, but not y3,y2,y1.

It's not clear what the best design is. Currently GHC follows the latter, but I'm open to suggestions.

Simon

comment:6 Changed 10 years ago by simonmar

I like the current behaviour. GHC used to have the former behaviour, and there were complaints that we didn't detect unused cycles.

(I presume the original bug about reporting unused qualified names still stands, though).

comment:7 Changed 10 years ago by Eelis-

Hm, I'm confused. Maybe my testcase was bad, because it might've suggested that the warning was simply a consequence of b being unused. Here's a better testcase:

  import List
  import Prelude

  main = do
    l <- getLine
    print $ List.null l

Here there can be absolutely no doubt that List.null is used, yet still GHC warns that nothing from List is used. Furthermore, the warning goes away if the "import Prelude" line is removed or if the two import lines are reversed. It seems to me that this can only be considered a genuine bug, not a reasonable consequence of any particular design choice.

comment:8 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.