From d7296ca17db960b28846969d42da325eef294a71 Mon Sep 17 00:00:00 2001 From: simonpj Date: Wed, 14 Mar 2001 23:19:42 +0000 Subject: [PATCH] [project @ 2001-03-14 23:19:42 by simonpj] ------------------------------- 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 | 54 +++++++++++++++++++------------- ghc/compiler/rename/RnEnv.lhs | 38 +++++------------------ ghc/compiler/rename/RnIfaces.lhs | 63 +++++++++++++++++++++++++++++++++++--- 3 files changed, 99 insertions(+), 56 deletions(-) diff --git a/ghc/compiler/rename/Rename.lhs b/ghc/compiler/rename/Rename.lhs index 4269aad..055e3de 100644 --- a/ghc/compiler/rename/Rename.lhs +++ b/ghc/compiler/rename/Rename.lhs @@ -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` diff --git a/ghc/compiler/rename/RnEnv.lhs b/ghc/compiler/rename/RnEnv.lhs index c8090f9..6ff21b0 100644 --- a/ghc/compiler/rename/RnEnv.lhs +++ b/ghc/compiler/rename/RnEnv.lhs @@ -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 diff --git a/ghc/compiler/rename/RnIfaces.lhs b/ghc/compiler/rename/RnIfaces.lhs index 5eb4e30..93f4732 100644 --- a/ghc/compiler/rename/RnIfaces.lhs +++ b/ghc/compiler/rename/RnIfaces.lhs @@ -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 ; -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) -- 1.7.10.4