Commit 6353efc7 authored by David Eichmann's avatar David Eichmann 🏋 Committed by Ben Gamari

Fix unused-import warnings

This patch fixes a fairly long-standing bug (dating back to 2015) in
RdrName.bestImport, namely

   commit 9376249b
   Author: Simon Peyton Jones <simonpj@microsoft.com>
   Date:   Wed Oct 28 17:16:55 2015 +0000

   Fix unused-import stuff in a better way

In that patch got the sense of the comparison back to front, and
thereby failed to implement the unused-import rules described in
  Note [Choosing the best import declaration] in RdrName

This led to Trac #13064 and #15393

Fixing this bug revealed a bunch of unused imports in libraries;
the ones in the GHC repo are part of this commit.

The two important changes are

* Fix the bug in bestImport

* Modified the rules by adding (a) in
     Note [Choosing the best import declaration] in RdrName
  Reason: the previosu rules made Trac #5211 go bad again.  And
  the new rule (a) makes sense to me.

In unravalling this I also ended up doing a few other things

* Refactor RnNames.ImportDeclUsage to use a [GlobalRdrElt] for the
  things that are used, rather than [AvailInfo]. This is simpler
  and more direct.

* Rename greParentName to greParent_maybe, to follow GHC
  naming conventions

* Delete dead code RdrName.greUsedRdrName

Bumps a few submodules.

Reviewers: hvr, goldfire, bgamari, simonmar, jrtc27

Subscribers: rwbarton, carter

Differential Revision: https://phabricator.haskell.org/D5312
parent 8d008b71
......@@ -53,14 +53,14 @@ module RdrName (
-- * GlobalRdrElts
gresFromAvails, gresFromAvail, localGREsFromAvail, availFromGRE,
greUsedRdrName, greRdrNames, greSrcSpan, greQualModName,
greRdrNames, greSrcSpan, greQualModName,
gresToAvailInfo,
-- ** Global 'RdrName' mapping elements: 'GlobalRdrElt', 'Provenance', 'ImportSpec'
GlobalRdrElt(..), isLocalGRE, isRecFldGRE, greLabel,
unQualOK, qualSpecOK, unQualSpecOK,
pprNameProvenance,
Parent(..),
Parent(..), greParent_maybe,
ImportSpec(..), ImpDeclSpec(..), ImpItemSpec(..),
importSpecLoc, importSpecModule, isExplicitItem, bestImport,
......@@ -657,18 +657,6 @@ greQualModName gre@(GRE { gre_name = name, gre_lcl = lcl, gre_imp = iss })
| (is:_) <- iss = is_as (is_decl is)
| otherwise = pprPanic "greQualModName" (ppr gre)
greUsedRdrName :: GlobalRdrElt -> RdrName
-- For imported things, return a RdrName to add to the used-RdrName
-- set, which is used to generate unused-import-decl warnings.
-- Return a Qual RdrName if poss, so that identifies the most
-- specific ImportSpec. See Trac #10890 for some good examples.
greUsedRdrName gre@GRE{ gre_name = name, gre_lcl = lcl, gre_imp = iss }
| lcl, Just mod <- nameModule_maybe name = Qual (moduleName mod) occ
| not (null iss), is <- bestImport iss = Qual (is_as (is_decl is)) occ
| otherwise = pprTrace "greUsedRdrName" (ppr gre) (Unqual occ)
where
occ = greOccName gre
greRdrNames :: GlobalRdrElt -> [RdrName]
greRdrNames gre@GRE{ gre_lcl = lcl, gre_imp = iss }
= (if lcl then [unqual] else []) ++ concatMap do_spec (map is_decl iss)
......@@ -696,8 +684,8 @@ mkParent _ (Avail _) = NoParent
mkParent n (AvailTC m _ _) | n == m = NoParent
| otherwise = ParentIs m
greParentName :: GlobalRdrElt -> Maybe Name
greParentName gre = case gre_par gre of
greParent_maybe :: GlobalRdrElt -> Maybe Name
greParent_maybe gre = case gre_par gre of
NoParent -> Nothing
ParentIs n -> Just n
FldParent n _ -> Just n
......@@ -716,7 +704,7 @@ gresToAvailInfo gres
add :: NameEnv AvailInfo -> GlobalRdrElt -> NameEnv AvailInfo
add env gre = extendNameEnv_Acc comb availFromGRE env
(fromMaybe (gre_name gre)
(greParentName gre)) gre
(greParent_maybe gre)) gre
where
-- We want to insert the child `k` into a list of children but
......@@ -1192,10 +1180,7 @@ instance Ord ImpItemSpec where
bestImport :: [ImportSpec] -> ImportSpec
-- Given a non-empty bunch of ImportSpecs, return the one that
-- imported the item most specifically (e.g. by name), using
-- textually-first as a tie breaker. This is used when reporting
-- redundant imports
-- See Note [Choosing the best import declaration]
bestImport iss
= case sortBy best iss of
(is:_) -> is
......@@ -1203,17 +1188,76 @@ bestImport iss
where
best :: ImportSpec -> ImportSpec -> Ordering
-- Less means better
-- Unqualified always wins over qualified; then
-- import-all wins over import-some; then
-- earlier declaration wins over later
best (ImpSpec { is_item = item1, is_decl = d1 })
(ImpSpec { is_item = item2, is_decl = d2 })
= best_item item1 item2 `thenCmp` (is_dloc d1 `compare` is_dloc d2)
= (is_qual d1 `compare` is_qual d2) `thenCmp`
(best_item item1 item2) `thenCmp`
(is_dloc d1 `compare` is_dloc d2)
best_item :: ImpItemSpec -> ImpItemSpec -> Ordering
best_item ImpAll ImpAll = EQ
best_item ImpAll (ImpSome {}) = GT
best_item (ImpSome {}) ImpAll = LT
best_item ImpAll (ImpSome {}) = LT
best_item (ImpSome {}) ImpAll = GT
best_item (ImpSome { is_explicit = e1 })
(ImpSome { is_explicit = e2 }) = e2 `compare` e1
-- False < True, so if e1 is explicit and e2 is not, we get LT
(ImpSome { is_explicit = e2 }) = e1 `compare` e2
-- False < True, so if e1 is explicit and e2 is not, we get GT
{- Note [Choosing the best import declaration]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
When reporting unused import declarations we use the following rules.
(see [wiki:Commentary/Compiler/UnusedImports])
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, ...)).
The general idea is that for each /occurrence/ of an imported name, we will
attribute that use to one import-item. Once we have processed all the
occurrences, any import items with no uses attributed to them are unused,
and are warned about. More precisely:
1. For every RdrName in the program text, find its GlobalRdrElt.
2. Then, from the [ImportSpec] (gre_imp) of that GRE, choose one
the "chosen import-item", and mark it "used". This is done
by 'bestImport'
3. After processing all the RdrNames, bleat about any
import-items that are unused.
This is done in RnNames.warnUnusedImportDecls.
The function 'bestImport' returns the dominant import among the
ImportSpecs it is given, implementing Step 2. We say import-item A
dominates import-item B if we choose A over B. In general, we try to
choose the import that is most likely to render other imports
unnecessary. Here is the dominance relationship we choose:
a) import Foo dominates import qualified Foo.
b) import Foo dominates import Foo(x).
c) Otherwise choose the textually first one.
Rationale for (a). Consider
import qualified M -- Import #1
import M( x ) -- Import #2
foo = M.x + x
The unqualified 'x' can only come from import #2. The qualified 'M.x'
could come from either, but bestImport picks import #2, because it is
more likely to be useful in other imports, as indeed it is in this
case (see Trac #5211 for a concrete example).
But the rules are not perfect; consider
import qualified M -- Import #1
import M( x ) -- Import #2
foo = M.x + M.y
The M.x will use import #2, but M.y can only use import #1.
-}
unQualSpecOK :: ImportSpec -> Bool
-- ^ Is in scope unqualified?
......
......@@ -21,7 +21,6 @@ import SMRep
import DynFlags
import Util
import Data.Foldable (foldl')
import Data.Bits
{-|
......
......@@ -29,7 +29,6 @@ import qualified TrieMap as TM
import UniqFM
import Unique
import Control.Arrow (first, second)
import Data.List (foldl')
-- -----------------------------------------------------------------------------
-- Eliminate common blocks
......
......@@ -37,7 +37,7 @@ import qualified Data.Set as Set
import Control.Monad.Fix
import Data.Array as Array
import Data.Bits
import Data.List (nub, foldl')
import Data.List (nub)
{- Note [Stack Layout]
......
......@@ -19,7 +19,7 @@ import CmmUtils
import CmmInfo
import CmmLive
import CmmSwitch
import Data.List (sortBy, foldl')
import Data.List (sortBy)
import Maybes
import Control.Monad
import Outputable
......
......@@ -17,7 +17,7 @@ import GhcPrelude
import qualified Data.IntMap.Strict as M
import qualified Data.IntSet as S
import Data.List (foldl', foldl1')
import Data.List (foldl1')
class IsSet set where
type ElemOf set
......
......@@ -35,7 +35,6 @@ import Cmm
import CmmUtils
import CLabel
import qualified Module
import CostCentre
import DynFlags
import FastString
......
......@@ -32,7 +32,7 @@ import PprCore ( pprCoreBindings, pprRules )
import OccurAnal( occurAnalyseExpr, occurAnalysePgm )
import Literal ( Literal(LitString) )
import Id
import Var ( varType, isNonCoVarId )
import Var ( isNonCoVarId )
import VarSet
import VarEnv
import DataCon
......
......@@ -60,7 +60,7 @@ import Name ( NamedThing(..), nameSrcSpan )
import SrcLoc ( SrcSpan(..), realSrcLocSpan, mkRealSrcLoc )
import Data.Bits
import MonadUtils ( mapAccumLM )
import Data.List ( mapAccumL, foldl' )
import Data.List ( mapAccumL )
import Control.Monad
import CostCentre ( CostCentre, ccFromThisModule )
import qualified Data.Set as S
......
......@@ -21,8 +21,6 @@ import Var
import Type (Type, typeSize)
import Id (isJoinId)
import Data.List (foldl')
data CoreStats = CS { cs_tm :: !Int -- Terms
, cs_ty :: !Int -- Types
, cs_co :: !Int -- Coercions
......
......@@ -53,7 +53,7 @@ import Name
import VarSet
import Rules
import VarEnv
import Var( EvVar, varType )
import Var( EvVar )
import Outputable
import Module
import SrcLoc
......
......@@ -41,7 +41,6 @@ import Util
-- Standard libraries
import Data.Array.Unboxed
import Foreign.Ptr
import GHC.IO ( IO(..) )
import GHC.Exts
{-
......
......@@ -57,16 +57,15 @@ import TysWiredIn
import DynFlags
import Outputable as Ppr
import GHC.Char
import GHC.Exts
import GHC.Exts.Heap
import GHC.IO ( IO(..) )
import SMRep ( roundUpTo )
import Control.Monad
import Data.Array.Base
import Data.Maybe
import Data.List
#if defined(INTEGER_GMP)
import GHC.Exts
import Data.Array.Base
import GHC.Integer.GMP.Internals
#endif
import qualified Data.Sequence as Seq
......
......@@ -22,7 +22,6 @@ import RdrName
import qualified Name
import Module
import RdrHsSyn
import qualified OccName
import OccName
import SrcLoc
import Type
......
......@@ -44,7 +44,6 @@ import DynFlags
import Data.Data hiding ( Fixity )
import Data.List hiding ( foldr )
import Data.Ord
import Data.Foldable ( Foldable(..) )
{-
************************************************************************
......
......@@ -18,7 +18,6 @@ module HsExtension where
import GhcPrelude
import GHC.Exts (Constraint)
import Data.Data hiding ( Fixity )
import PlaceHolder
import Name
......
......@@ -281,6 +281,9 @@ ieWrappedName (IEName (L _ n)) = n
ieWrappedName (IEPattern (L _ n)) = n
ieWrappedName (IEType (L _ n)) = n
lieWrappedName :: LIEWrappedName name -> name
lieWrappedName (L _ n) = ieWrappedName n
ieLWrappedName :: LIEWrappedName name -> Located name
ieLWrappedName (L l n) = L l (ieWrappedName n)
......
......@@ -90,7 +90,6 @@ import FastString
import Maybes( isJust )
import Data.Data hiding ( Fixity, Prefix, Infix )
import Data.List ( foldl' )
import Data.Maybe ( fromMaybe )
{-
......
......@@ -38,7 +38,6 @@ import Util
import Control.Monad.Trans.Class
import Control.Monad.Trans.Writer
import Data.Semigroup ( Semigroup )
import qualified Data.Semigroup as Semigroup
import Data.List ( nub )
import Data.Maybe ( catMaybes )
......
......@@ -34,7 +34,6 @@ module Ar
import GhcPrelude
import Data.Semigroup (Semigroup)
import Data.List (mapAccumL, isPrefixOf)
import Data.Monoid ((<>))
import Data.Binary.Get
......
......@@ -248,7 +248,9 @@ import qualified EnumSet
import GHC.Foreign (withCString, peekCString)
import qualified GHC.LanguageExtensions as LangExt
#if defined(GHCI)
import Foreign (Ptr) -- needed for 2nd stage
#endif
-- Note [Updating flag description in the User's Guide]
-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
......
......@@ -25,7 +25,6 @@ module DynamicLoading (
) where
import GhcPrelude
import HscTypes ( HscEnv )
import DynFlags
#if defined(GHCI)
......@@ -63,6 +62,7 @@ import GHC.Exts ( unsafeCoerce# )
#else
import HscTypes ( HscEnv )
import Module ( ModuleName, moduleNameString )
import Panic
......@@ -76,12 +76,13 @@ import Control.Monad ( unless )
-- actual compilation starts. Idempotent operation. Should be re-called if
-- pluginModNames or pluginModNameOpts changes.
initializePlugins :: HscEnv -> DynFlags -> IO DynFlags
initializePlugins hsc_env df
#if !defined(GHCI)
initializePlugins _ df
= do let pluginMods = pluginModNames df
unless (null pluginMods) (pluginError pluginMods)
return df
#else
initializePlugins hsc_env df
| map lpModuleName (plugins df) == pluginModNames df -- plugins not changed
&& all (\p -> lpArguments p == argumentsForPlugin p (pluginModNameOpts df))
(plugins df) -- arguments not changed
......
......@@ -50,7 +50,6 @@ import System.Directory
import System.FilePath
import Control.Monad
import Data.Time
import Data.List ( foldl' )
type FileExt = String -- Filename extension
......
......@@ -291,7 +291,6 @@ import GhcPrelude hiding (init)
import ByteCodeTypes
import InteractiveEval
import InteractiveEvalTypes
import TcRnDriver ( runTcInteractive )
import GHCi
import GHCi.RemoteTypes
......@@ -359,7 +358,6 @@ import Data.Set (Set)
import qualified Data.Sequence as Seq
import System.Directory ( doesFileExist )
import Data.Maybe
import Data.List ( find )
import Data.Time
import Data.Typeable ( Typeable )
import Data.Word ( Word8 )
......
......@@ -85,7 +85,6 @@ module HscMain
import GhcPrelude
import Data.Data hiding (Fixity, TyCon)
import DynFlags (addPluginModuleName)
import Id
import GHCi ( addSptEntry )
import GHCi.RemoteTypes ( ForeignHValue )
......
......@@ -17,7 +17,6 @@ import SrcLoc
import Util
import Data.Char
import Data.Foldable (foldl')
-- | Source Statistics
ppSourceStats :: Bool -> Located (HsModule GhcPs) -> SDoc
......
......@@ -95,7 +95,6 @@ import Data.List as List
import Data.Map (Map)
import Data.Set (Set)
import Data.Monoid (First(..))
import Data.Semigroup ( Semigroup )
import qualified Data.Semigroup as Semigroup
import qualified Data.Map as Map
import qualified Data.Map.Strict as MapStrict
......
......@@ -27,8 +27,6 @@ import Unique
import UniqFM
import UniqSet
import Data.Foldable (foldl')
-- | For a jump instruction at the end of a block, generate fixup code so its
-- vregs are in the correct regs for its destination.
--
......
......@@ -13,7 +13,6 @@ import Platform
import Data.Word
import Data.Bits
import Data.Foldable (foldl')
-- The PowerPC has 32 integer and 32 floating point registers.
-- This is 32bit PowerPC, so Word64 is inefficient - two Word32s are much
......
......@@ -15,7 +15,6 @@ import Platform
import Data.Word
import Data.Bits
import Data.Foldable (foldl')
--------------------------------------------------------------------------------
......
......@@ -13,7 +13,6 @@ import Platform
import Data.Word
import Data.Bits
import Data.Foldable (foldl')
newtype FreeRegs = FreeRegs Word32
deriving Show
......
......@@ -11,7 +11,6 @@ import Reg
import Panic
import Platform
import Data.Foldable (foldl')
import Data.Word
import Data.Bits
......
......@@ -106,7 +106,6 @@ import FastString
import Maybes
import Util
import ApiAnnotation
import HsExtension ( noExt )
import Data.List
import DynFlags ( WarningFlag(..) )
......
......@@ -77,7 +77,6 @@ import ListSetOps ( minusList )
import qualified GHC.LanguageExtensions as LangExt
import RnUnbound
import RnUtils
import Data.Maybe (isJust)
import qualified Data.Semigroup as Semi
import Data.Either ( partitionEithers )
import Data.List (find)
......@@ -638,21 +637,21 @@ lookupSubBndrOcc_helper must_have_parent warn_if_deprec parent rdr_name
NoParent -> Nothing
picked_gres :: [GlobalRdrElt] -> DisambigInfo
-- For Unqual, find GREs that are in scope qualified or unqualified
-- For Qual, find GREs that are in scope with that qualification
picked_gres gres
| isUnqual rdr_name
= mconcat (map right_parent gres)
| otherwise
= mconcat (map right_parent (pickGREs rdr_name gres))
right_parent :: GlobalRdrElt -> DisambigInfo
right_parent p
| Just cur_parent <- getParent p
= if parent == cur_parent
then DisambiguatedOccurrence p
else NoOccurrence
| otherwise
= UniqueOccurrence p
= case getParent p of
Just cur_parent
| parent == cur_parent -> DisambiguatedOccurrence p
| otherwise -> NoOccurrence
Nothing -> UniqueOccurrence p
-- This domain specific datatype is used to record why we decided it was
......
......@@ -5,7 +5,6 @@ import NameSet ( FreeVars )
import TcRnTypes
import SrcLoc ( Located )
import Outputable ( Outputable )
import HsExtension ( GhcPs, GhcRn )
rnLExpr :: LHsExpr GhcPs
-> RnM (LHsExpr GhcRn, FreeVars)
......
......@@ -63,13 +63,11 @@ import qualified GHC.LanguageExtensions as LangExt
import Control.Monad
import Data.Either ( partitionEithers, isRight, rights )
-- import qualified Data.Foldable as Foldable
import Data.Map ( Map )
import qualified Data.Map as Map
import Data.Ord ( comparing )
import Data.List ( partition, (\\), find, sortBy )
import qualified Data.Set as S
-- import qualified Data.Set as Set
import System.FilePath ((</>))
import System.IO
......@@ -1094,7 +1092,7 @@ gresFromIE decl_spec (L loc ie, avail)
= gresFromAvail prov_fn avail
where
is_explicit = case ie of
IEThingAll _ (L _ name) -> \n -> n == ieWrappedName name
IEThingAll _ name -> \n -> n == lieWrappedName name
_ -> \_ -> True
prov_fn name
= Just (ImpSpec { is_decl = decl_spec, is_item = item_spec })
......@@ -1217,44 +1215,11 @@ reportUnusedNames _export_decls gbl_env
is_unused_local :: GlobalRdrElt -> Bool
is_unused_local gre = isLocalGRE gre && isExternalName (gre_name gre)
{-
*********************************************************
{- *********************************************************************
* *
\subsection{Unused imports}
Missing signatures
* *
*********************************************************
This code finds which import declarations are unused. The
specification and implementation notes are here:
http://ghc.haskell.org/trac/ghc/wiki/Commentary/Compiler/UnusedImports
-}
type ImportDeclUsage
= ( LImportDecl GhcRn -- The import declaration
, [AvailInfo] -- What *is* used (normalised)
, [Name] ) -- What is imported but *not* used
warnUnusedImportDecls :: TcGblEnv -> RnM ()
warnUnusedImportDecls gbl_env
= do { uses <- readMutVar (tcg_used_gres gbl_env)
; let user_imports = filterOut (ideclImplicit . unLoc) (tcg_rn_imports gbl_env)
-- This whole function deals only with *user* imports
-- both for warning about unnecessary ones, and for
-- deciding the minimal ones
rdr_env = tcg_rdr_env gbl_env
fld_env = mkFieldEnv rdr_env
; let usage :: [ImportDeclUsage]
usage = findImportUsage user_imports uses
; traceRn "warnUnusedImportDecls" $
(vcat [ text "Uses:" <+> ppr uses
, text "Import usage" <+> ppr usage])
; whenWOptM Opt_WarnUnusedImports $
mapM_ (warnUnusedImport Opt_WarnUnusedImports fld_env) usage
; whenGOptM Opt_D_dump_minimal_imports $
printMinimalImports usage }
********************************************************************* -}
-- | Warn the user about top level binders that lack type signatures.
-- Called /after/ type inference, so that we can report the
......@@ -1312,29 +1277,50 @@ warnMissingSignatures gbl_env
; add_sig_warns }
{-
Note [The ImportMap]
~~~~~~~~~~~~~~~~~~~~
The ImportMap is a short-lived intermediate data structure records, for
each import declaration, what stuff brought into scope by that
declaration is actually used in the module.
*********************************************************
* *
\subsection{Unused imports}
* *
*********************************************************
The SrcLoc is the location of the END of a particular 'import'
declaration. Why *END*? Because we don't want to get confused
by the implicit Prelude import. Consider (Trac #7476) the module
import Foo( foo )
main = print foo
There is an implicit 'import Prelude(print)', and it gets a SrcSpan