[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 <ezyang@…>): In [changeset:"640fe14255706ab9c6a1fa101d9b05dfabdc6556/ghc"]: {{{ #!CommitTicketReference repository="ghc" revision="640fe14255706ab9c6a1fa101d9b05dfabdc6556" Remove unnecessary loadInterface for TH quoted name. Summary: The load was introduced a32d3e4da0aceb624c958f02cad7327e17ac94db to fix a bug where deprecations assumed that the name in question had already had their interface loaded. The new deprecation code no longer makes this assumption and just loads the interface, so this eager load is not necessary. Verified that TH_reifyType2 continues to work. Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu> Test Plan: validate Reviewers: simonpj, austin Subscribers: bgamari, thomie Differential Revision: https://phabricator.haskell.org/D891 GHC Trac Issues: #10419 }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/10419#comment:4> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
participants (1)
-
GHC