From 55f3a503d72d89d7c57a0b10093dd4bdb0488c42 Mon Sep 17 00:00:00 2001
From: "simonpj@microsoft.com" <unknown>
Date: Thu, 2 Nov 2006 12:04:41 +0000
Subject: [PATCH] Improve handling of unused imports (test is mod75)

---
 compiler/basicTypes/RdrName.lhs |    6 +++++-
 compiler/rename/RnEnv.lhs       |   36 +++++++++++++++++++-----------------
 compiler/rename/RnNames.lhs     |   24 ++++++++++++++++--------
 3 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/compiler/basicTypes/RdrName.lhs b/compiler/basicTypes/RdrName.lhs
index 3766b9f..f1ab0af8 100644
--- a/compiler/basicTypes/RdrName.lhs
+++ b/compiler/basicTypes/RdrName.lhs
@@ -35,7 +35,7 @@ module RdrName (
 	Provenance(..), pprNameProvenance,
 	Parent(..), 
 	ImportSpec(..), ImpDeclSpec(..), ImpItemSpec(..), 
-	importSpecLoc, importSpecModule
+	importSpecLoc, importSpecModule, isExplicitItem
   ) where 
 
 #include "HsVersions.h"
@@ -540,6 +540,10 @@ importSpecLoc (ImpSpec _    item)   = is_iloc item
 importSpecModule :: ImportSpec -> ModuleName
 importSpecModule is = is_mod (is_decl is)
 
+isExplicitItem :: ImpItemSpec -> Bool
+isExplicitItem ImpAll 			     = False
+isExplicitItem (ImpSome {is_explicit = exp}) = exp
+
 -- Note [Comparing provenance]
 -- Comparison of provenance is just used for grouping 
 -- error messages (in RnEnv.warnUnusedBinds)
diff --git a/compiler/rename/RnEnv.lhs b/compiler/rename/RnEnv.lhs
index 16c1b0b..32806f0 100644
--- a/compiler/rename/RnEnv.lhs
+++ b/compiler/rename/RnEnv.lhs
@@ -775,12 +775,12 @@ warnUnusedMatches    names = ifOptM Opt_WarnUnusedMatches (warnUnusedLocals name
 -------------------------
 --	Helpers
 warnUnusedGREs gres 
- = warnUnusedBinds [(n,Just p) | GRE {gre_name = n, gre_prov = p} <- gres]
+ = warnUnusedBinds [(n,p) | GRE {gre_name = n, gre_prov = p} <- gres]
 
 warnUnusedLocals names
- = warnUnusedBinds [(n,Nothing) | n<-names]
+ = warnUnusedBinds [(n,LocalDef) | n<-names]
 
-warnUnusedBinds :: [(Name,Maybe Provenance)] -> RnM ()
+warnUnusedBinds :: [(Name,Provenance)] -> RnM ()
 warnUnusedBinds names  = mappM_ warnUnusedName (filter reportable names)
  where reportable (name,_) 
 	| isWiredInName name = False	-- Don't report unused wired-in names
@@ -790,23 +790,25 @@ warnUnusedBinds names  = mappM_ warnUnusedName (filter reportable names)
 
 -------------------------
 
-warnUnusedName :: (Name, Maybe Provenance) -> RnM ()
-warnUnusedName (name, prov)
-  = addWarnAt loc $
+warnUnusedName :: (Name, Provenance) -> RnM ()
+warnUnusedName (name, LocalDef)
+  = addUnusedWarning name (srcLocSpan (nameSrcLoc name)) 
+		     (ptext SLIT("Defined but not used"))
+
+warnUnusedName (name, Imported is)
+  = mapM_ warn is
+  where
+    warn spec = addUnusedWarning name span msg
+	where
+	   span = importSpecLoc spec
+	   pp_mod = quotes (ppr (importSpecModule spec))
+	   msg = ptext SLIT("Imported from") <+> pp_mod <+> ptext SLIT("but not used")
+
+addUnusedWarning name span msg
+  = addWarnAt span $
     sep [msg <> colon, 
 	 nest 2 $ pprNonVarNameSpace (occNameSpace (nameOccName name))
 			<+> quotes (ppr name)]
-	-- TODO should be a proper span
-  where
-    (loc,msg) = case prov of
-		  Just (Imported is)
-		        -> (importSpecLoc imp_spec, imp_from (importSpecModule imp_spec))
-		        where
-			  imp_spec = head is
-		  other -> (srcLocSpan (nameSrcLoc name), unused_msg)
-
-    unused_msg   = text "Defined but not used"
-    imp_from mod = text "Imported from" <+> quotes (ppr mod) <+> text "but not used"
 \end{code}
 
 \begin{code}
diff --git a/compiler/rename/RnNames.lhs b/compiler/rename/RnNames.lhs
index 06cddd2..ae5ec95 100644
--- a/compiler/rename/RnNames.lhs
+++ b/compiler/rename/RnNames.lhs
@@ -1013,14 +1013,15 @@ reportUnusedNames export_decls gbl_env
     is_unused_local :: GlobalRdrElt -> Bool
     is_unused_local gre = isLocalGRE gre && isExternalName (gre_name gre)
     
-    unused_imports :: [GlobalRdrElt]
-    unused_imports = filter unused_imp defined_but_not_used
-    unused_imp (GRE {gre_prov = Imported imp_specs}) 
-	= not (all (module_unused . importSpecModule) imp_specs)
-	  && or [exp | ImpSpec { is_item = ImpSome { is_explicit = exp } } <- imp_specs]
-		-- Don't complain about unused imports if we've already said the
-		-- entire import is unused
-    unused_imp other = False
+    unused_imports :: [GlobalRdrElt]	
+    unused_imports = mapCatMaybes unused_imp defined_but_not_used
+    unused_imp :: GlobalRdrElt -> Maybe GlobalRdrElt	-- Result has trimmed Imported provenances
+    unused_imp gre@(GRE {gre_prov = LocalDef}) = Nothing
+    unused_imp gre@(GRE {gre_prov = Imported imp_specs}) 
+	| null trimmed_specs = Nothing
+	| otherwise	     = Just (gre {gre_prov = Imported trimmed_specs})
+	where
+	  trimmed_specs = filter report_if_unused imp_specs
     
     -- To figure out the minimal set of imports, start with the things
     -- that are in scope (i.e. in gbl_env).  Then just combine them
@@ -1096,6 +1097,7 @@ reportUnusedNames export_decls gbl_env
     --
     -- BUG WARNING: does not deal correctly with multiple imports of the same module
     --	 	    becuase direct_import_mods has only one entry per module
+    unused_imp_mods :: [(ModuleName, SrcSpan)]
     unused_imp_mods = [(mod_name,loc) | (mod,no_imp,loc) <- direct_import_mods,
 		       let mod_name = moduleName mod,
     		       not (mod_name `elemFM` minimal_imports1),
@@ -1108,6 +1110,12 @@ reportUnusedNames export_decls gbl_env
     module_unused :: ModuleName -> Bool
     module_unused mod = any (((==) mod) . fst) unused_imp_mods
 
+    report_if_unused :: ImportSpec -> Bool
+	-- Do we want to report this as an unused import?  
+    report_if_unused (ImpSpec {is_decl = d, is_item = i})
+	= not (module_unused (is_mod d)) -- Not if we've already said entire import is unused
+	  && isExplicitItem i		 -- Only if the import was explicit
+			
 ---------------------
 warnDuplicateImports :: [GlobalRdrElt] -> RnM ()
 -- Given the GREs for names that are used, figure out which imports 
-- 
1.7.10.4