[project @ 2005-07-22 14:00:34 by simonpj]
authorsimonpj <unknown>
Fri, 22 Jul 2005 14:00:35 +0000 (14:00 +0000)
committersimonpj <unknown>
Fri, 22 Jul 2005 14:00:35 +0000 (14:00 +0000)
MERGE TO STABLE

Fix a long-standing bug in dependency tracking.
If you have

import M( x )

then you must recompile if M's export list changes, because it might
no longer export x.  Until now we have only done that if the import was

import M

I can't think why this bug has lasted so long.  Thanks to Ian Lynagh
for pointing it out.

ghc/compiler/iface/MkIface.lhs
ghc/compiler/main/HscTypes.lhs
ghc/compiler/rename/RnNames.lhs
ghc/compiler/typecheck/TcRnTypes.lhs

index 8757279..5be56bf 100644 (file)
@@ -185,7 +185,6 @@ import IfaceSyn             ( IfaceDecl(..), IfaceClassOp(..), IfaceConDecl(..),
 import LoadIface       ( readIface, loadInterface )
 import BasicTypes      ( Version, initialVersion, bumpVersion )
 import TcRnMonad
-import TcRnTypes       ( mkModDeps )
 import HscTypes                ( ModIface(..), ModDetails(..), 
                          ModGuts(..), IfaceExport,
                          HscEnv(..), hscEPS, Dependencies(..), FixItem(..), 
@@ -641,7 +640,7 @@ bump_unless False v = bumpVersion v
 \begin{code}
 mkUsageInfo :: HscEnv 
            -> HomeModules
-           -> ModuleEnv (Module, Maybe Bool, SrcSpan)
+           -> ModuleEnv (Module, Bool, SrcSpan)
            -> [(Module, IsBootInterface)]
            -> NameSet -> IO [Usage]
 mkUsageInfo hsc_env hmods dir_imp_mods dep_mods used_names
@@ -676,9 +675,9 @@ mk_usage_info pit hsc_env hmods dir_imp_mods dep_mods proto_used_names
                     mod = nameModule name
                     add_item occs _ = occ:occs
     
-    import_all mod = case lookupModuleEnv dir_imp_mods mod of
-                       Just (_,imp_all,_) -> isNothing imp_all
-                       Nothing            -> False
+    depend_on_exports mod = case lookupModuleEnv dir_imp_mods mod of
+                               Just (_,no_imp,_) -> not no_imp
+                               Nothing           -> True
     
     -- We want to create a Usage for a home module if 
     -- a) we used something from; has something in used_names
@@ -691,7 +690,7 @@ mk_usage_info pit hsc_env hmods dir_imp_mods dep_mods proto_used_names
       |  isNothing maybe_iface -- We can't depend on it if we didn't
       || not (isHomeModule hmods mod)  -- even open the interface!
       || (null used_occs
-         && not all_imported
+         && isNothing export_vers
          && not orphan_mod)
       = Nothing                        -- Record no usage info
     
@@ -712,9 +711,8 @@ mk_usage_info pit hsc_env hmods dir_imp_mods dep_mods proto_used_names
         version_env  = mi_ver_fn    iface
         mod_vers     = mi_mod_vers  iface
         rules_vers   = mi_rule_vers iface
-        all_imported = import_all mod 
-        export_vers | all_imported = Just (mi_exp_vers iface)
-                   | otherwise    = Nothing
+        export_vers | depend_on_exports mod = Just (mi_exp_vers iface)
+                   | otherwise             = Nothing
     
        -- The sort is to put them into canonical order
         used_occs = lookupModuleEnv ent_map mod `orElse` []
index 3f93389..20e84ab 100644 (file)
@@ -822,9 +822,16 @@ data Usage
        -- time round, but if someone has added a new rule you might need it this time
 
        -- The export list field is (Just v) if we depend on the export list:
-       --      i.e. we imported the module without saying exactly what we imported
-       -- We need to recompile if the module exports changes, because we might
-       -- now have a name clash in the importing module.
+       --      i.e. we imported the module directly, whether or not we
+       --           enumerated the things we imported, or just imported everything
+       -- We need to recompile if M's exports change, because 
+       -- if the import was    import M,       we might now have a name clash in the 
+       --                                      importing module.
+       -- if the import was    import M(x)     M might no longer export x
+       -- The only way we don't depend on the export list is if we have
+       --                      import M()
+       -- And of course, for modules that aren't imported directly we don't
+       -- depend on their export lists
 \end{code}
 
 
index 5b888b7..1f73462 100644 (file)
@@ -221,11 +221,10 @@ importsFromImportDecl this_mod
                 ASSERT2( not (pkg `elem` dep_pkgs deps), ppr pkg <+> ppr (dep_pkgs deps) )
                 ([], pkg : dep_pkgs deps)
 
+       -- True <=> import M ()
        import_all = case imp_details of
-                       Just (is_hiding, ls)     -- Imports are spec'd explicitly
-                         | not is_hiding -> Just (not (null ls))
-                       _ -> Nothing            -- Everything is imported, 
-                                               -- (or almost everything [hiding])
+                       Just (is_hiding, ls) -> not is_hiding && null ls        
+                       other                -> False
 
        -- unqual_avails is the Avails that are visible in *unqualified* form
        -- We need to know this so we know what to export when we see
@@ -848,7 +847,7 @@ reportUnusedNames export_decls gbl_env
    
     imports = tcg_imports gbl_env
 
-    direct_import_mods :: [(Module, Maybe Bool, SrcSpan)]
+    direct_import_mods :: [(Module, Bool, SrcSpan)]
        -- See the type of the imp_mods for this triple
     direct_import_mods = moduleEnvElts (imp_mods imports)
 
@@ -859,11 +858,11 @@ 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 = [(mod,loc) | (mod,imp,loc) <- direct_import_mods,
+    unused_imp_mods = [(mod,loc) | (mod,no_imp,loc) <- direct_import_mods,
                       not (mod `elemFM` minimal_imports1),
                       mod /= pRELUDE,
-                      imp /= Just False]
-       -- The Just False part is not to complain about
+                      not no_imp]
+       -- The not no_imp part is not to complain about
        -- import M (), which is an idiom for importing
        -- instance declarations
     
index c0cce28..e8b0b48 100644 (file)
@@ -487,22 +487,18 @@ data ImportAvails
                -- So the starting point is all things that are in scope as 'M.x',
                -- which is what this field tells us.
 
-       imp_mods :: ModuleEnv (Module, Maybe Bool, SrcSpan),
+       imp_mods :: ModuleEnv (Module, Bool, SrcSpan),
                -- Domain is all directly-imported modules
-               -- Maybe value answers the question "is the import restricted?"
-               --   Nothing    => unrestricted import (e.g., "import Foo")
-               --   Just True  => restricted import, at least one entity (e.g., "import Foo(x)")
-               --   Just False => fully restricted import (e.g., "import Foo ()")
-               --
-               --  A distinction is made between the first and the third in order
-               --  to more precisely emit warnings about unused imports.
+               -- Bool means:
+               --   True => import was "import Foo ()"
+               --   False  => import was some other form
                --
                -- We need the Module in the range because we can't get
                --      the keys of a ModuleEnv
                -- Used 
                --   (a) to help construct the usage information in 
-               --       the interface file; if we import everything we
-               --       need to recompile if the module version changes
+               --       the interface file; if we import somethign we
+               --       need to recompile if the export version changes
                --   (b) to specify what child modules to initialise
 
        imp_dep_mods :: ModuleEnv (Module, IsBootInterface),