[GHC] #10419: Refactor LoadIface to distinguish getting a ModIface versus loading into EPT
#10419: Refactor LoadIface to distinguish getting a ModIface versus loading into EPT -------------------------------------+------------------------------------- Reporter: ezyang | Owner: ezyang Type: task | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.11 (Type checker) | Operating System: Unknown/Multiple Keywords: | Type of failure: None/Unknown Architecture: | Blocked By: Unknown/Multiple | Related Tickets: Test Case: | Blocking: | Differential Revisions: | -------------------------------------+------------------------------------- At the moment, making a call to `loadSysInterface` (or the related interfaces) has two effects: 1. You get a `ModIface` 2. The declarations/instances/etc of the interface are type-checked and loaded into the EPT In fact, users of these functions rarely want both of these; they either only want the `ModIface`, or only want to load up the declarations. Here is a survey I did, where `I` indicates a `ModIface` was wanted, `D` indicates we wanted to load the declarations, and `ID` means both were wanted; {{{ RnSplice loadInterfaceForName: D rnBracket: deprecation checking of identifiers in brackets () RnEnv loadInterfaceForName I warnIfDeprecated: deprecation checking of GlobalRdrElt (mi_warn_fun) I lookupFixityRn: fixity lookup (mi_fix_fun) loadSrcInterface_maybe ID lookupQualifiedNameGHCi: implicit import RnNames loadSrcInterface ID rnImportDecl: source import! $$$$$ I printMinimalImports: minimal import calculation (mi_exports) DsMonad loadInterface I loadModule: load and return exported entities (like a source import but for Module) special case for Data.Array.Parallel DynamicLoading loadPluginInterface D forceLoadModuleInterfaces: what. Linker loadInterface I getLinkDeps: poke the dependencies (mi_boot, mi_deps) loadUserInterface I random ass thing to check if it's a sigof XXX (mi_sig_of) MkIface loadInterface I? needInterface: utility checkModUsage: given the usage information from the old hi, determine if recompilation is required (hashes only) FamInst loadSysInterface D getFamInsts: load instance into EPS and return it TcDeriv loadInterfaceForName I getDataConFixityFun: looks at mi_fix_fn TcRnDriver loadSysInterface I tcRnSignature: signature renaming hack D loadUnqualIfaces: for accurate available instance reporting loadModuleInterfaces D tcRnImports: load orphans loadModuleInterface getModuleInterface: load a 'Module' (GHCi) D hscCheckSafe' (HscMain, mi_trust, mi_trust_pkg, mi_deps) ID getPackageModuleInfo (contains interface) loadSrcInterface D runTcInteractive: mi_deps/dep_orphs, pull in orphans from interactive context ic_imports TcSplice loadInterfaceForModule I reifyModule: mi_usages only }}} It's well worth noting that for most `Name`s, it is NOT NECESSARY, since when we pull on it with `loadDecl`, the interface will get loaded in. Consequently, I think some of the cases where we're loading interfaces are unnecessary, e.g. the case in `rnBracket`. The primary cases are dealing with orphan imports; there are multiple sites which redo this logic, and it should all be centralized in one place. So here is the concrete proposal: * Add a new function for taking a `ModIface` and loading it into the EPT. * Change the rest of the existing `loadXXXInterface` functions to NOT load declarations. We'll actually typecheck the interface file when we pull on the `Name` in question during type checking. * Introduce a new function to `LoadIface` for loading orphans (i.e. what to do when a source level import occurs). Have `lookupQualifiedNameGHCi`, `rnImportDecl`, `tcRnImports` and `runTcInteractive` use this function. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10419 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#10419: Refactor LoadIface to distinguish getting a ModIface versus loading into EPT -------------------------------------+------------------------------------- Reporter: ezyang | Owner: ezyang Type: task | Status: new Priority: normal | Milestone: Component: Compiler (Type | Version: 7.11 checker) | Keywords: Resolution: | Architecture: Operating System: Unknown/Multiple | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by simonpj): Hmm. Note that * If you want a declaration you have to load the `ModIface`, and we don't want to do that twice, so D implies I. * If you want the `ModIface` it's not much work to load all the declarations because we typecheck them lazily (via `forkM` in `TcIface`). So it's not clear to me that you are going to save much work. So: what's your goal here? Just efficiency? The proposal would mean that in the `ExternalPackageState` there might be members of the `eps_PIT` whose declarations have not been typechecked and loaded into the `eps_PTE`. Currently it's an invariant that they all are. That might be fine but it needs documenting. Here just for reference is the current defn of EPS: {{{ data ExternalPackageState = EPS { eps_is_boot :: !(ModuleNameEnv (ModuleName, IsBootInterface)), eps_PIT :: !PackageIfaceTable, eps_PTE :: !PackageTypeEnv, eps_inst_env :: !PackageInstEnv, eps_fam_inst_env :: !PackageFamInstEnv, eps_rule_base :: !PackageRuleBase, eps_vect_info :: !PackageVectInfo, eps_ann_env :: !PackageAnnEnv, eps_mod_fam_inst_env :: !(ModuleEnv FamInstEnv), eps_stats :: !EpsStats } }}} Thinking about the invariant leads to some questions: * Consider a module whose `ModIface` M we have loaded, into the `eps_PIT`. Now suppose we want the declaration for `M.foo`. Do we:[[BR]][[BR]] 1. Find `foo`'s `IfaceDecl` in M's `ModIface` and add that declaration alone to `eps_PTE`? But how do we find `foo`'s `IfaceDecl`, remembering that one `IfaceDecl` may bind many `Name`s.? You'll need to build a little index; but that is (in effect) what the `eps_PTE` already is! [[BR]][[BR]] 2. Typecheck all the declarations in M's `ModIface` and add all of them to the `eps_PTE`? If so, how do we record that we have done this so we don't repeat it? Maybe it's enough that future lookups will find `M.foo` in the PTE. * Currently we assume that if we've loaded an interface for a non-orphan module we've loaded its instances into `eps_inst_env`. But that won't be true any more. Or will it? * Similarly we'd have to think about `eps_ann_env`, `eps_mod_fam_inst_env` etc. So currently I'm unconvinced. Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10419#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#10419: Refactor LoadIface to distinguish getting a ModIface versus loading into EPT -------------------------------------+------------------------------------- Reporter: ezyang | Owner: ezyang Type: task | Status: closed Priority: normal | Milestone: Component: Compiler (Type | Version: 7.11 checker) | Keywords: Resolution: invalid | Architecture: Operating System: Unknown/Multiple | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Changes (by ezyang): * status: new => closed * resolution: => invalid Comment: My original motivation was clarity surrounding plugin interface loading, but actually I think there's a better way to solve this problem (#10420), so I guess let's let this die. Though, I think it may still be useful to add a new function to `LoadIface` expressly for orphan loading, to consolidate this logic. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10419#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#10419: Refactor LoadIface to distinguish getting a ModIface versus loading into EPT -------------------------------------+------------------------------------- Reporter: ezyang | Owner: ezyang Type: task | Status: closed Priority: normal | Milestone: Component: Compiler (Type | Version: 7.11 checker) | Keywords: Resolution: invalid | Architecture: Operating System: Unknown/Multiple | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by simonpj):
Though, I think it may still be useful to add a new function to `LoadIface` expressly for orphan loading, to consolidate this logic.
Maybe so. Which logic did you have in mind, and where is it duplicated? Can you suggest a patch? Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10419#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#10419: Refactor LoadIface to distinguish getting a ModIface versus loading into
EPT
-------------------------------------+-------------------------------------
Reporter: ezyang | Owner: ezyang
Type: task | Status: closed
Priority: normal | Milestone:
Component: Compiler (Type | Version: 7.11
checker) | Keywords:
Resolution: invalid | Architecture:
Operating System: Unknown/Multiple | Unknown/Multiple
Type of failure: None/Unknown | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Revisions:
-------------------------------------+-------------------------------------
Comment (by Edward Z. Yang
participants (1)
-
GHC