Opened 2 years ago

Last modified 2 months ago

#8363 new bug

Order matters for unused import warnings when reexporting identifiers

Reported by: bergmark Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.7
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

Import order seem to change whether unused import warnings trigger when a module re-exports an identifier, with another module importing it and another module exporting the same identifier.

Reproduction:

module Foo ( (<$>) , g  ) where

import Control.Applicative

g :: Int
g = 1
module Main where

import Control.Applicative
import Foo

main :: IO ()
main = print =<< ((+2) <$> return g)
$ ghc -fwarn-unused-imports Main.hs
[1 of 2] Compiling Foo              ( Foo.hs, Foo.o )
[2 of 2] Compiling Main             ( Main.hs, Main.o )
Linking Main ...

If Main is not using g then this gives a warning that the import of Foo is redundant.

If we switch the order of the imports we do get the warning:

module Main where

import Foo
import Control.Applicative

main :: IO ()
main = print =<< ((+2) <$> return g)
> ghc -fwarn-unused-imports Main.hs
[1 of 2] Compiling Foo              ( Foo.hs, Foo.o )
[2 of 2] Compiling Main             ( Main.hs, Main.o )

Main.hs:4:1:
    Warning: The import of `Control.Applicative' is redundant
               except perhaps to import instances from `Control.Applicative'
             To import instances alone, use: import Control.Applicative()
Linking Main ...

I expected both versions of Main to produce the same warning. Tested on GHC 7.7.20130824 and 7.4.2.

Change History (4)

comment:1 Changed 2 years ago by simonpj

Yes, you're right. In general it's a hard problem to find the minimal set of imports that are required. (It's a kind of set-covering problem.) GHC uses a cheap and cheerful heuristic that usually does reasonably well.

The code is very localised, in RnNames.warnUnusedImportDecls. If anyone can do better, go for it!

Simon

comment:2 Changed 2 months ago by thomie

  • Keywords newcomer added

comment:3 Changed 2 months ago by rwbarton

Be aware though that it has become somewhat popular to exploit this bug(?) to avoid writing CPP to manage the changes in Prelude's exports in 7.10.

module Main where

import Control.Applicative -- needed in 7.8, and not a warning in 7.10
                           -- because it comes before the Prelude import,
                           -- though it actually is redundant in 7.10
import Prelude             -- has the effect of canceling the implicit
                           -- import of Prelude

...

comment:4 Changed 2 months ago by thomie

  • Keywords newcomer removed
Note: See TracTickets for help on using tickets.