[project @ 2001-03-14 23:19:42 by simonpj]
authorsimonpj <unknown>
Wed, 14 Mar 2001 23:19:42 +0000 (23:19 +0000)
committersimonpj <unknown>
Wed, 14 Mar 2001 23:19:42 +0000 (23:19 +0000)
-------------------------------
Fix the dreaded export list bug
-------------------------------

With unfailing regularity I manage to get the following wrong:

module A(f) where
  f = ...

module B(f) where
  import A(f)

We must ensure that if A.f changes its type (etc) then B.hi
gets changed, so that people who import B will get recompiled.

There's a large comment with RnIfaces.mkImportInfo, and some
reorganisation in Rename, with a few mainly cosmetic consequences
in RnEnv.

[Simon: I think this will fix the 'OccurAnal not recompiled' problem.]

ghc/compiler/rename/Rename.lhs
ghc/compiler/rename/RnEnv.lhs
ghc/compiler/rename/RnIfaces.lhs

index 4269aad..055e3de 100644 (file)
@@ -34,7 +34,7 @@ import RnEnv          ( availsToNameSet, mkIfaceGlobalRdrEnv,
                          emptyAvailEnv, unitAvailEnv, availEnvElts, 
                          plusAvailEnv, groupAvails, warnUnusedImports, 
                          warnUnusedLocalBinds, warnUnusedModules, 
-                         lookupSrcName, addImplicitFVs,
+                         lookupSrcName, getImplicitStmtFVs, getImplicitModuleFVs, rnSyntaxNames,
                          newGlobalName, unQualInScope,, ubiquitousNames
                        )
 import Module           ( Module, ModuleName, WhereFrom(..),
@@ -45,7 +45,7 @@ import Name           ( Name, nameIsLocalOrFrom, nameModule )
 import NameEnv
 import NameSet
 import RdrName         ( foldRdrEnv, isQual )
-import PrelNames       ( SyntaxMap, pRELUDE_Name )
+import PrelNames       ( SyntaxMap, vanillaSyntaxMap, pRELUDE_Name )
 import ErrUtils                ( dumpIfSet, dumpIfSet_dyn, showPass, 
                          printErrorsAndWarnings, errorsFound )
 import Bag             ( bagToList )
@@ -133,15 +133,16 @@ renameStmt dflags hit hst pcs scope_module this_module local_env stmt
         doDump [] stmt [] `thenRn_` returnRn (print_unqual, Nothing)
     else
 
-    let filtered_fvs = fvs `delListFromNameSet` rdrEnvElts local_env in
-
        -- Add implicit free vars, and close decls
-    addImplicitFVs rdr_env Nothing filtered_fvs
-                               `thenRn` \ (slurp_fvs, syntax_map) ->
-    slurpImpDecls slurp_fvs    `thenRn` \ decls ->
+    getImplicitStmtFVs                                 `thenRn` \ implicit_fvs ->
+    let
+       filtered_fvs = fvs `delListFromNameSet` rdrEnvElts local_env 
+       source_fvs   = implicit_fvs `plusFV` filtered_fvs
+    in
+    slurpImpDecls source_fvs                   `thenRn` \ decls ->
 
     doDump binders stmt decls  `thenRn_`
-    returnRn (print_unqual, Just (binders, (syntax_map, stmt, decls)))
+    returnRn (print_unqual, Just (binders, (vanillaSyntaxMap, stmt, decls)))
 
   where
      doc = text "context for compiling expression"
@@ -237,15 +238,30 @@ rename this_module contents@(HsModule _ _ exports imports local_decls mod_deprec
     else
 
        -- SLURP IN ALL THE NEEDED DECLARATIONS
-    addImplicitFVs gbl_env (Just (mod_name, rn_local_decls)) 
-                  source_fvs                                                   `thenRn` \ (slurp_fvs, sugar_map) -> 
-    traceRn (text "Source FVs:" <+> fsep (map ppr (nameSetToList slurp_fvs)))  `thenRn_`
-    slurpImpDecls slurp_fvs            `thenRn` \ rn_imp_decls ->
+       -- Find out what re-bindable names to use for desugaring
+    getImplicitModuleFVs mod_name rn_local_decls       `thenRn` \ implicit_fvs ->
+    rnSyntaxNames gbl_env source_fvs                   `thenRn` \ (source_fvs1, sugar_map) ->
+    let
+       export_fvs  = availsToNameSet export_avails
+       source_fvs2 = source_fvs1 `plusFV` export_fvs
+               -- The export_fvs make the exported names look just as if they
+               -- occurred in the source program.  For the reasoning, see the
+               -- comments with RnIfaces.mkImportInfo
+               -- It also helps reportUnusedNames, which of course must not complain
+               -- that 'f' isn't mentioned if it is mentioned in the export list
+
+       source_fvs3 = implicit_fvs `plusFV` source_fvs2
+               -- It's important to do the "plus" this way round, so that
+               -- when compiling the prelude, locally-defined (), Bool, etc
+               -- override the implicit ones. 
 
+    in
+    traceRn (text "Source FVs:" <+> fsep (map ppr (nameSetToList source_fvs3)))        `thenRn_`
+    slurpImpDecls source_fvs3                  `thenRn` \ rn_imp_decls ->
     rnDump rn_imp_decls rn_local_decls         `thenRn_` 
 
        -- GENERATE THE VERSION/USAGE INFO
-    mkImportInfo mod_name imports                      `thenRn` \ my_usages ->
+    mkImportInfo mod_name imports              `thenRn` \ my_usages ->
 
        -- BUILD THE MODULE INTERFACE
     let
@@ -280,7 +296,9 @@ rename this_module contents@(HsModule _ _ exports imports local_decls mod_deprec
        -- REPORT UNUSED NAMES, AND DEBUG DUMP 
     reportUnusedNames mod_iface print_unqualified 
                      imports global_avail_env
-                     source_fvs export_avails rn_imp_decls     `thenRn_`
+                     source_fvs2 rn_imp_decls          `thenRn_`
+               -- NB: source_fvs2: include exports (else we get bogus 
+               --     warnings of unused things) but not implicit FVs.
 
     returnRn (print_unqualified, Just (is_exported, mod_iface, (sugar_map, final_decls)))
   where
@@ -582,11 +600,10 @@ reportUnusedNames :: ModIface -> PrintUnqualified
                  -> [RdrNameImportDecl] 
                  -> AvailEnv
                  -> NameSet            -- Used in this module
-                 -> Avails             -- Exported by this module
                  -> [RenamedHsDecl] 
                  -> RnMG ()
 reportUnusedNames my_mod_iface unqual imports avail_env 
-                 source_fvs export_avails imported_decls
+                 used_names imported_decls
   = warnUnusedModules unused_imp_mods                          `thenRn_`
     warnUnusedLocalBinds bad_locals                            `thenRn_`
     warnUnusedImports bad_imp_names                            `thenRn_`
@@ -595,11 +612,6 @@ reportUnusedNames my_mod_iface unqual imports avail_env
     this_mod   = mi_module my_mod_iface
     gbl_env    = mi_globals my_mod_iface
     
-       -- The export_fvs make the exported names look just as if they
-       -- occurred in the source program.  
-    export_fvs = availsToNameSet export_avails
-    used_names = source_fvs `plusFV` export_fvs
-
     -- Now, a use of C implies a use of T,
     -- if C was brought into scope by T(..) or T(C)
     really_used_names = used_names `unionNameSets`
index c8090f9..6ff21b0 100644 (file)
@@ -12,7 +12,6 @@ import {-# SOURCE #-} RnHiFiles
 
 import HscTypes                ( ModIface(..) )
 import HsSyn
-import RnHsSyn         ( RenamedHsDecl )
 import RdrHsSyn                ( RdrNameIE )
 import RdrName         ( RdrName, rdrNameModule, rdrNameOcc, isQual, isUnqual, isOrig,
                          mkRdrUnqual, mkRdrQual, qualifyRdrName, lookupRdrEnv, foldRdrEnv
@@ -352,41 +351,20 @@ lookupSysBinder rdr_name
 %*                                                     *
 %*********************************************************
 
-@addImplicitFVs@ forces the renamer to slurp in some things which aren't
+@getXImplicitFVs@ forces the renamer to slurp in some things which aren't
 mentioned explicitly, but which might be needed by the type checker.
 
 \begin{code}
-addImplicitFVs :: GlobalRdrEnv
-              -> Maybe (ModuleName, [RenamedHsDecl])   -- Nothing when compling an expression
-              -> FreeVars                              -- Free in the source
-              -> RnMG (FreeVars, SyntaxMap)            -- Augmented source free vars
-
-addImplicitFVs gbl_env maybe_mod source_fvs
-  =    -- Find out what re-bindable names to use for desugaring
-     rnSyntaxNames gbl_env source_fvs          `thenRn` \ (source_fvs1, sugar_map) ->
-
-       -- Find implicit FVs thade
-    extra_implicits maybe_mod          `thenRn` \ extra_fvs ->
-    
-    let
-       implicit_fvs = ubiquitousNames `plusFV` extra_fvs
-       slurp_fvs    = implicit_fvs `plusFV` source_fvs1
-               -- It's important to do the "plus" this way round, so that
-               -- when compiling the prelude, locally-defined (), Bool, etc
-               -- override the implicit ones. 
-    in
-    returnRn (slurp_fvs, sugar_map)
-
-  where
-    extra_implicits Nothing            -- Compiling a statement
-      = returnRn (mkFVs [printName, bindIOName, returnIOName, failIOName])
+getImplicitStmtFVs     -- Compiling a statement
+  = returnRn (mkFVs [printName, bindIOName, returnIOName, failIOName]
+             `plusFV` ubiquitousNames)
                -- These are all needed implicitly when compiling a statement
                -- See TcModule.tc_stmts
 
-    extra_implicits (Just (mod_name, decls))   -- Compiling a module
-      = lookupOrigNames deriv_occs             `thenRn` \ deriving_names ->
-       returnRn (deriving_names `plusFV` implicit_main)
-      where
+getImplicitModuleFVs mod_name decls    -- Compiling a module
+  = lookupOrigNames deriv_occs         `thenRn` \ deriving_names ->
+    returnRn (deriving_names `plusFV` implicit_main `plusFV` ubiquitousNames)
+  where
        -- Add occurrences for IO or PrimIO
        implicit_main |  mod_name == mAIN_Name
                      || mod_name == pREL_MAIN_Name = unitFV ioTyConName
index 5eb4e30..93f4732 100644 (file)
@@ -68,18 +68,64 @@ import Util         ( sortLt )
 %*                                                     *
 %*********************************************************
 
-mkImportInof figures out what the ``usage information'' for this
+mkImportInfo figures out what the ``usage information'' for this
 moudule is; that is, what it must record in its interface file as the
 things it uses.  
 
 We produce a line for every module B below the module, A, currently being
 compiled:
        import B <n> ;
-to record the fact that A does import B indireclty.  This is used to decide
+to record the fact that A does import B indirectly.  This is used to decide
 to look to look for B.hi rather than B.hi-boot when compiling a module that
 imports A.  This line says that A imports B, but uses nothing in it.
 So we'll get an early bale-out when compiling A if B's version changes.
 
+The usage information records:
+
+\begin{itemize}
+\item  (a) anything reachable from its body code
+\item  (b) any module exported with a @module Foo@
+\item   (c) anything reachable from an exported item
+\end{itemize}
+
+Why (b)?  Because if @Foo@ changes then this module's export list
+will change, so we must recompile this module at least as far as
+making a new interface file --- but in practice that means complete
+recompilation.
+
+Why (c)?  Consider this:
+\begin{verbatim}
+       module A( f, g ) where  |       module B( f ) where
+         import B( f )         |         f = h 3
+         g = ...               |         h = ...
+\end{verbatim}
+
+Here, @B.f@ isn't used in A.  Should we nevertheless record @B.f@ in
+@A@'s usages?  Our idea is that we aren't going to touch A.hi if it is
+*identical* to what it was before.  If anything about @B.f@ changes
+than anyone who imports @A@ should be recompiled in case they use
+@B.f@ (they'll get an early exit if they don't).  So, if anything
+about @B.f@ changes we'd better make sure that something in A.hi
+changes, and the convenient way to do that is to record the version
+number @B.f@ in A.hi in the usage list.  If B.f changes that'll force a
+complete recompiation of A, which is overkill but it's the only way to 
+write a new, slightly different, A.hi.
+
+But the example is tricker.  Even if @B.f@ doesn't change at all,
+@B.h@ may do so, and this change may not be reflected in @f@'s version
+number.  But with -O, a module that imports A must be recompiled if
+@B.h@ changes!  So A must record a dependency on @B.h@.  So we treat
+the occurrence of @B.f@ in the export list *just as if* it were in the
+code of A, and thereby haul in all the stuff reachable from it.
+
+       *** Conclusion: if A mentions B.f in its export list,
+           behave just as if A mentioned B.f in its source code,
+           and slurp in B.f and all its transitive closure ***
+
+[NB: If B was compiled with -O, but A isn't, we should really *still*
+haul in all the unfoldings for B, in case the module that imports A *is*
+compiled with -O.  I think this is the case.]
+
 \begin{code}
 mkImportInfo :: ModuleName                     -- Name of this module
             -> [ImportDecl n]                  -- The import decls
@@ -132,9 +178,14 @@ mkImportInfo this_mod imports
 
        import_info0 = foldModuleEnv mk_imp_info  []           pit
        import_info1 = foldModuleEnv mk_imp_info  import_info0 hit
-       import_info  = [ (mod_name, orphans, is_boot, NothingAtAll) 
-                      | (mod_name, (orphans, is_boot)) <- fmToList (iImpModInfo ifaces) ] ++ 
-                      import_info1
+       import_info  = not_even_opened_imports ++ import_info1
+
+               -- Recall that iImpModInfo describes modules that have been mentioned
+               -- in the import lists of interfaces we have opened, but which we have
+               -- not even opened when compiling this module
+       not_even_opened_imports = [ (mod_name, orphans, is_boot, NothingAtAll) 
+                                 | (mod_name, (orphans, is_boot)) <- fmToList (iImpModInfo ifaces) ]
+
        
        mk_imp_info :: ModIface -> [ImportVersion Name] -> [ImportVersion Name]
        mk_imp_info iface so_far
@@ -149,6 +200,8 @@ mkImportInfo this_mod imports
          | import_all_mod                              -- Case (a) and (b); the import-all part
          = if is_home_pkg_mod then
                go_for_it (Specifically mod_vers (Just export_vers) [] rules_vers)
+               -- Since the module isn't in the mv_map, presumably we
+               -- didn't actually import anything at all from it
            else
                go_for_it (Everything mod_vers)