Hannes Siebenhandl pushed to branch wip/fendor/drop-preloadclosure-from-unitstate at Glasgow Haskell Compiler / GHC Commits: 3d360285 by fendor at 2026-05-27T13:52:02+02:00 Drop `preloadClosure` from `UnitState` It is always hard-coded to the same value. Backpack Unit instantiation isn't using it any more. Allows us to simplify the API and get rid of `improveUnit`. - - - - - 6 changed files: - + changelog.d/T27308 - compiler/GHC/Driver/Backpack.hs - compiler/GHC/Iface/Load.hs - compiler/GHC/Iface/Recomp.hs - compiler/GHC/Unit.hs - compiler/GHC/Unit/State.hs Changes: ===================================== changelog.d/T27308 ===================================== @@ -0,0 +1,10 @@ +section: compiler +synopsis: Drop `preloadClosure` from `UnitState` +issues: #27308 +mrs: !16108 + +description: { + Drop `preloadClosure` from `UnitState` as it is always set to the empty set. + This allows to simplify the `UnitState` and related functions. +} + ===================================== compiler/GHC/Driver/Backpack.hs ===================================== @@ -241,7 +241,6 @@ withBkpSession cid insts deps session_type do_this = do -- Synthesize the flags , packageFlags = packageFlags dflags ++ map (\(uid0, rn) -> let uid = unwireUnit unit_state - $ improveUnit unit_state $ renameHoleUnit unit_state (listToUFM insts) uid0 in ExposePackage (showSDoc dflags @@ -321,8 +320,7 @@ buildUnit session cid insts lunit = do TcSession -> return () _ -> compileInclude (length deps0) (i, dep) - -- IMPROVE IT - let deps = map (improveUnit (hsc_units hsc_env)) deps0 + let deps = deps0 mb_old_eps <- case session of TcSession -> fmap Just getEpsGhc ===================================== compiler/GHC/Iface/Load.hs ===================================== @@ -914,13 +914,13 @@ findAndReadIface hsc_env doc_str mod wanted_mod hi_boot_file = do && not (isOneShot (ghcMode dflags)) then return (Failed (HomeModError mod loc)) else do - r <- read_file hooks logger name_cache unit_state dflags wanted_mod (ml_hi_file loc) + r <- read_file hooks logger name_cache dflags wanted_mod (ml_hi_file loc) case r of Failed err -> return (Failed $ BadIfaceFile err) Succeeded (iface,_fp) -> do - r2 <- load_dynamic_too_maybe hooks logger name_cache unit_state + r2 <- load_dynamic_too_maybe hooks logger name_cache (setDynamicNow dflags) wanted_mod iface loc case r2 of @@ -936,20 +936,20 @@ findAndReadIface hsc_env doc_str mod wanted_mod hi_boot_file = do err -- | Check if we need to try the dynamic interface for -dynamic-too -load_dynamic_too_maybe :: Hooks -> Logger -> NameCache -> UnitState -> DynFlags +load_dynamic_too_maybe :: Hooks -> Logger -> NameCache -> DynFlags -> Module -> ModIface -> ModLocation -> IO (MaybeErr MissingInterfaceError ()) -load_dynamic_too_maybe hooks logger name_cache unit_state dflags wanted_mod iface loc +load_dynamic_too_maybe hooks logger name_cache dflags wanted_mod iface loc -- Indefinite interfaces are ALWAYS non-dynamic. | not (moduleIsDefinite (mi_module iface)) = return (Succeeded ()) - | gopt Opt_BuildDynamicToo dflags = load_dynamic_too hooks logger name_cache unit_state dflags wanted_mod iface loc + | gopt Opt_BuildDynamicToo dflags = load_dynamic_too hooks logger name_cache dflags wanted_mod iface loc | otherwise = return (Succeeded ()) -load_dynamic_too :: Hooks -> Logger -> NameCache -> UnitState -> DynFlags +load_dynamic_too :: Hooks -> Logger -> NameCache -> DynFlags -> Module -> ModIface -> ModLocation -> IO (MaybeErr MissingInterfaceError ()) -load_dynamic_too hooks logger name_cache unit_state dflags wanted_mod iface loc = do - read_file hooks logger name_cache unit_state dflags wanted_mod (ml_dyn_hi_file loc) >>= \case +load_dynamic_too hooks logger name_cache dflags wanted_mod iface loc = do + read_file hooks logger name_cache dflags wanted_mod (ml_dyn_hi_file loc) >>= \case Succeeded (dynIface, _) | mi_mod_hash iface == mi_mod_hash dynIface -> return (Succeeded ()) @@ -963,10 +963,10 @@ load_dynamic_too hooks logger name_cache unit_state dflags wanted_mod iface loc -read_file :: Hooks -> Logger -> NameCache -> UnitState -> DynFlags +read_file :: Hooks -> Logger -> NameCache -> DynFlags -> Module -> FilePath -> IO (MaybeErr ReadInterfaceError (ModIface, FilePath)) -read_file hooks logger name_cache unit_state dflags wanted_mod file_path = do +read_file hooks logger name_cache dflags wanted_mod file_path = do -- Figure out what is recorded in mi_module. If this is -- a fully definite interface, it'll match exactly, but @@ -975,7 +975,7 @@ read_file hooks logger name_cache unit_state dflags wanted_mod file_path = do case getModuleInstantiation wanted_mod of (_, Nothing) -> wanted_mod (_, Just indef_mod) -> - instModuleToModule unit_state + instModuleToModule (uninstantiateInstantiatedModule indef_mod) read_result <- readIface hooks logger dflags name_cache wanted_mod' file_path case read_result of ===================================== compiler/GHC/Iface/Recomp.hs ===================================== @@ -620,7 +620,7 @@ checkMergedSignatures hsc_env mod_summary self_recomp = do new_merged = case lookupUniqMap (requirementContext unit_state) (ms_mod_name mod_summary) of Nothing -> [] - Just r -> sort $ map (instModuleToModule unit_state) r + Just r -> sort $ map instModuleToModule r if old_merged == new_merged then up_to_date logger (text "signatures to merge in unchanged" $$ ppr new_merged) else return $ needsRecompileBecause SigsMergeChanged ===================================== compiler/GHC/Unit.hs ===================================== @@ -314,32 +314,6 @@ field in the SDocContext to pretty-print. (i.e. GHC doesn't correctly call `pprWithUnitState` before pretty-printing a UnitId), that's what will be shown to the user so it's no big deal. - -Note [VirtUnit to RealUnit improvement] -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Over the course of instantiating VirtUnits on the fly while typechecking an -indefinite library, we may end up with a fully instantiated VirtUnit. I.e. -one that could be compiled and installed in the database. During -type-checking we generate a virtual UnitId for it, say "abc". - -Now the question is: do we have a matching installed unit in the database? -Suppose we have one with UnitId "xyz" (provided by Cabal so we don't know how -to generate it). The trouble is that if both units end up being used in the -same type-checking session, their names won't match (e.g. "abc:M.X" vs -"xyz:M.X"). - -As we want them to match we just replace the virtual unit with the installed -one: for some reason this is called "improvement". - -There is one last niggle: improvement based on the unit database means -that we might end up developing on a unit that is not transitively -depended upon by the units the user specified directly via command line -flags. This could lead to strange and difficult to understand bugs if those -instantiations are out of date. The solution is to only improve a -unit id if the new unit id is part of the 'preloadClosure'; i.e., the -closure of all the units which were explicitly specified. - Note [Representation of module/name variables] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In our ICFP'16, we use <A> to represent module holes, and {A.T} to represent ===================================== compiler/GHC/Unit/State.hs ===================================== @@ -7,7 +7,6 @@ module GHC.Unit.State ( -- * Reading the package config, and processing cmdline args UnitState(..), - PreloadUnitClosure, UnitDatabase (..), UnitErr (..), emptyUnitState, @@ -29,7 +28,6 @@ module GHC.Unit.State ( lookupPackageName, resolvePackageImport, - improveUnit, searchPackageId, listVisibleModuleNames, lookupModuleInAllUnits, @@ -89,7 +87,6 @@ import GHC.Unit.Home import GHC.Types.Unique.FM import GHC.Types.Unique.DFM -import GHC.Types.Unique.Set import GHC.Types.Unique.DSet import GHC.Types.Unique.Map import GHC.Types.Unique @@ -267,8 +264,6 @@ originEmpty :: ModuleOrigin -> Bool originEmpty (ModOrigin Nothing [] [] False) = True originEmpty _ = False -type PreloadUnitClosure = UniqSet UnitId - -- | 'UniqFM' map from 'Unit' to a 'UnitVisibility'. type VisibilityMap = UniqMap Unit UnitVisibility @@ -431,13 +426,6 @@ data UnitState = UnitState { -- may have the 'exposed' flag be 'False'.) unitInfoMap :: UnitInfoMap, - -- | The set of transitively reachable units according - -- to the explicitly provided command line arguments. - -- A fully instantiated VirtUnit may only be replaced by a RealUnit from - -- this set. - -- See Note [VirtUnit to RealUnit improvement] - preloadClosure :: PreloadUnitClosure, - -- | A mapping of 'PackageName' to 'UnitId'. If several units have the same -- package name (e.g. different instantiations), then we return one of them... -- This is used when users refer to packages in Backpack includes. @@ -490,7 +478,6 @@ data UnitState = UnitState { emptyUnitState :: UnitState emptyUnitState = UnitState { unitInfoMap = emptyUniqMap, - preloadClosure = emptyUniqSet, packageNameMap = emptyUFM, wireMap = emptyUniqMap, unwireMap = emptyUniqMap, @@ -516,7 +503,7 @@ type UnitInfoMap = UniqMap UnitId UnitInfo -- | Find the unit we know about with the given unit, if any lookupUnit :: UnitState -> Unit -> Maybe UnitInfo -lookupUnit pkgs = lookupUnit' (allowVirtualUnits pkgs) (unitInfoMap pkgs) (preloadClosure pkgs) +lookupUnit pkgs = lookupUnit' (allowVirtualUnits pkgs) (unitInfoMap pkgs) -- | A more specialized interface, which doesn't require a 'UnitState' (so it -- can be used while we're initializing 'DynFlags') @@ -524,16 +511,15 @@ lookupUnit pkgs = lookupUnit' (allowVirtualUnits pkgs) (unitInfoMap pkgs) (prelo -- Parameters: -- * a boolean specifying whether or not to look for on-the-fly renamed interfaces -- * a 'UnitInfoMap' --- * a 'PreloadUnitClosure' -lookupUnit' :: Bool -> UnitInfoMap -> PreloadUnitClosure -> Unit -> Maybe UnitInfo -lookupUnit' allowOnTheFlyInst pkg_map closure u = case u of +lookupUnit' :: Bool -> UnitInfoMap -> Unit -> Maybe UnitInfo +lookupUnit' allowOnTheFlyInst pkg_map u = case u of HoleUnit -> error "Hole unit" RealUnit i -> lookupUniqMap pkg_map (unDefinite i) VirtUnit i | allowOnTheFlyInst -> -- lookup UnitInfo of the indefinite unit to be instantiated and -- instantiate it on-the-fly - fmap (renameUnitInfo pkg_map closure (instUnitInsts i)) + fmap (renameUnitInfo pkg_map (instUnitInsts i)) (lookupUniqMap pkg_map (instUnitInstanceOf i)) | otherwise @@ -907,7 +893,6 @@ applyTrustFlag prec_map unusable pkgs flag = applyPackageFlag :: UnitPrecedenceMap -> UnitInfoMap - -> PreloadUnitClosure -> UnusableUnits -> Bool -- if False, if you expose a package, it implicitly hides -- any previously exposed packages with the same name @@ -916,10 +901,10 @@ applyPackageFlag -> PackageFlag -- flag to apply -> MaybeErr UnitErr VisibilityMap -- Now exposed -applyPackageFlag prec_map pkg_map closure unusable no_hide_others pkgs vm flag = +applyPackageFlag prec_map pkg_map unusable no_hide_others pkgs vm flag = case flag of ExposePackage _ arg (ModRenaming b rns) -> - case findPackages prec_map pkg_map closure arg pkgs unusable of + case findPackages prec_map pkg_map arg pkgs unusable of Left ps -> Failed (PackageFlagErr flag ps) Right (p:_) -> Succeeded vm' where @@ -983,7 +968,7 @@ applyPackageFlag prec_map pkg_map closure unusable no_hide_others pkgs vm flag = _ -> panic "applyPackageFlag" HidePackage str -> - case findPackages prec_map pkg_map closure (PackageArg str) pkgs unusable of + case findPackages prec_map pkg_map (PackageArg str) pkgs unusable of Left ps -> Failed (PackageFlagErr flag ps) Right ps -> Succeeded $ foldl' delFromUniqMap vm (map mkUnit ps) @@ -992,12 +977,11 @@ applyPackageFlag prec_map pkg_map closure unusable no_hide_others pkgs vm flag = -- if the 'UnitArg' has a renaming associated with it. findPackages :: UnitPrecedenceMap -> UnitInfoMap - -> PreloadUnitClosure -> PackageArg -> [UnitInfo] -> UnusableUnits -> Either [(UnitInfo, UnusableUnitReason)] [UnitInfo] -findPackages prec_map pkg_map closure arg pkgs unusable +findPackages prec_map pkg_map arg pkgs unusable = let ps = mapMaybe (finder arg) pkgs in if null ps then Left (mapMaybe (\(x,y) -> finder arg x >>= \x' -> return (x',y)) @@ -1015,7 +999,7 @@ findPackages prec_map pkg_map closure arg pkgs unusable -> Just p VirtUnit inst | instUnitInstanceOf inst == unitId p - -> Just (renameUnitInfo pkg_map closure (instUnitInsts inst) p) + -> Just (renameUnitInfo pkg_map (instUnitInsts inst) p) _ -> Nothing selectPackages :: UnitPrecedenceMap -> PackageArg -> [UnitInfo] @@ -1030,10 +1014,10 @@ selectPackages prec_map arg pkgs unusable else Right (sortByPreference prec_map ps, rest) -- | Rename a 'UnitInfo' according to some module instantiation. -renameUnitInfo :: UnitInfoMap -> PreloadUnitClosure -> [(ModuleName, Module)] -> UnitInfo -> UnitInfo -renameUnitInfo pkg_map closure insts conf = +renameUnitInfo :: UnitInfoMap -> [(ModuleName, Module)] -> UnitInfo -> UnitInfo +renameUnitInfo pkg_map insts conf = let hsubst = listToUFM insts - smod = renameHoleModule' pkg_map closure hsubst + smod = renameHoleModule' pkg_map hsubst new_insts = map (\(k,v) -> (k,smod v)) (unitInstantiations conf) in conf { unitInstantiations = new_insts, @@ -1631,7 +1615,7 @@ mkUnitState logger cfg = do -- user tries to enable an unusable package, we should let them know. -- vis_map2 <- mayThrowUnitErr - $ foldM (applyPackageFlag prec_map prelim_pkg_db emptyUniqSet unusable + $ foldM (applyPackageFlag prec_map prelim_pkg_db unusable (unitConfigHideAll cfg) pkgs1) vis_map1 other_flags @@ -1660,7 +1644,7 @@ mkUnitState logger cfg = do | otherwise = vis_map2 plugin_vis_map2 <- mayThrowUnitErr - $ foldM (applyPackageFlag prec_map prelim_pkg_db emptyUniqSet unusable + $ foldM (applyPackageFlag prec_map prelim_pkg_db unusable hide_plugin_pkgs pkgs1) plugin_vis_map1 (reverse (unitConfigFlagsPlugins cfg)) @@ -1712,7 +1696,7 @@ mkUnitState logger cfg = do $ closeUnitDeps pkg_db $ zip (map toUnitId preload3) (repeat Nothing) - let mod_map1 = mkModuleNameProvidersMap logger cfg pkg_db emptyUniqSet vis_map + let mod_map1 = mkModuleNameProvidersMap logger cfg pkg_db vis_map mod_map2 = mkUnusableModuleNameProvidersMap unusable mod_map = mod_map2 `plusUniqMap` mod_map1 @@ -1722,9 +1706,8 @@ mkUnitState logger cfg = do , explicitUnits = explicit_pkgs , homeUnitDepends = home_unit_deps , unitInfoMap = pkg_db - , preloadClosure = emptyUniqSet , moduleNameProvidersMap = mod_map - , pluginModuleNameProvidersMap = mkModuleNameProvidersMap logger cfg pkg_db emptyUniqSet plugin_vis_map + , pluginModuleNameProvidersMap = mkModuleNameProvidersMap logger cfg pkg_db plugin_vis_map , packageNameMap = pkgname_map , wireMap = wired_map , unwireMap = listToUniqMap [ (v,k) | (k,v) <- nonDetUniqMapToList wired_map ] @@ -1764,10 +1747,9 @@ mkModuleNameProvidersMap :: Logger -> UnitConfig -> UnitInfoMap - -> PreloadUnitClosure -> VisibilityMap -> ModuleNameProvidersMap -mkModuleNameProvidersMap logger cfg pkg_map closure vis_map = +mkModuleNameProvidersMap logger cfg pkg_map vis_map = -- What should we fold on? Both situations are awkward: -- -- * Folding on the visibility map means that we won't create @@ -1839,7 +1821,7 @@ mkModuleNameProvidersMap logger cfg pkg_map closure vis_map = hiddens = [(m, mkModMap pk m ModHidden) | m <- hidden_mods] pk = mkUnit pkg - unit_lookup uid = lookupUnit' (unitConfigAllowVirtual cfg) pkg_map closure uid + unit_lookup uid = lookupUnit' (unitConfigAllowVirtual cfg) pkg_map uid `orElse` pprPanic "unit_lookup" (ppr uid) exposed_mods = unitExposedModules pkg @@ -2190,28 +2172,6 @@ fsPackageName info = fs where PackageName fs = unitPackageName info - --- | Given a fully instantiated 'InstantiatedUnit', improve it into a --- 'RealUnit' if we can find it in the package database. -improveUnit :: UnitState -> Unit -> Unit -improveUnit state u = improveUnit' (unitInfoMap state) (preloadClosure state) u - --- | Given a fully instantiated 'InstantiatedUnit', improve it into a --- 'RealUnit' if we can find it in the package database. -improveUnit' :: UnitInfoMap -> PreloadUnitClosure -> Unit -> Unit -improveUnit' _ _ uid@(RealUnit _) = uid -- short circuit -improveUnit' pkg_map closure uid = - -- Do NOT lookup indefinite ones, they won't be useful! - case lookupUnit' False pkg_map closure uid of - Nothing -> uid - Just pkg -> - -- Do NOT improve if the indefinite unit id is not - -- part of the closure unique set. See - -- Note [VirtUnit to RealUnit improvement] - if unitId pkg `elementOfUniqSet` closure - then mkUnit pkg - else uid - -- | Check the database to see if we already have an installed unit that -- corresponds to the given 'InstantiatedUnit'. -- @@ -2219,15 +2179,15 @@ improveUnit' pkg_map closure uid = -- references a matching installed unit. -- -- See Note [VirtUnit to RealUnit improvement] -instUnitToUnit :: UnitState -> InstantiatedUnit -> Unit -instUnitToUnit state iuid = +instUnitToUnit :: InstantiatedUnit -> Unit +instUnitToUnit iuid = -- NB: suppose that we want to compare the instantiated -- unit p[H=impl:H] against p+abcd (where p+abcd -- happens to be the existing, installed version of -- p[H=impl:H]. If we *only* wrap in p[H=impl:H] -- VirtUnit, they won't compare equal; only -- after improvement will the equality hold. - improveUnit state $ VirtUnit iuid + VirtUnit iuid -- | Substitution on module variables, mapping module names to module @@ -2239,30 +2199,30 @@ type ShHoleSubst = ModuleNameEnv Module -- @p[A=\<A>]:B@ maps to @p[A=q():A]:B@ with @A=q():A@; -- similarly, @\<A>@ maps to @q():A@. renameHoleModule :: UnitState -> ShHoleSubst -> Module -> Module -renameHoleModule state = renameHoleModule' (unitInfoMap state) (preloadClosure state) +renameHoleModule state = renameHoleModule' (unitInfoMap state) -- | Substitutes holes in a 'Unit', suitable for renaming when -- an include occurs; see Note [Representation of module/name variables]. -- -- @p[A=\<A>]@ maps to @p[A=\<B>]@ with @A=\<B>@. renameHoleUnit :: UnitState -> ShHoleSubst -> Unit -> Unit -renameHoleUnit state = renameHoleUnit' (unitInfoMap state) (preloadClosure state) +renameHoleUnit state = renameHoleUnit' (unitInfoMap state) --- | Like 'renameHoleModule', but requires only 'ClosureUnitInfoMap' +-- | Like 'renameHoleModule', but requires only 'UnitInfoMap' -- so it can be used by "GHC.Unit.State". -renameHoleModule' :: UnitInfoMap -> PreloadUnitClosure -> ShHoleSubst -> Module -> Module -renameHoleModule' pkg_map closure env m +renameHoleModule' :: UnitInfoMap -> ShHoleSubst -> Module -> Module +renameHoleModule' pkg_map env m | not (isHoleModule m) = - let uid = renameHoleUnit' pkg_map closure env (moduleUnit m) + let uid = renameHoleUnit' pkg_map env (moduleUnit m) in mkModule uid (moduleName m) | Just m' <- lookupUFM env (moduleName m) = m' -- NB m = <Blah>, that's what's in scope. | otherwise = m --- | Like 'renameHoleUnit, but requires only 'ClosureUnitInfoMap' +-- | Like 'renameHoleUnit', but requires only 'UnitInfoMap' -- so it can be used by "GHC.Unit.State". -renameHoleUnit' :: UnitInfoMap -> PreloadUnitClosure -> ShHoleSubst -> Unit -> Unit -renameHoleUnit' pkg_map closure env uid = +renameHoleUnit' :: UnitInfoMap -> ShHoleSubst -> Unit -> Unit +renameHoleUnit' pkg_map env uid = case uid of (VirtUnit InstantiatedUnit{ instUnitInstanceOf = cid @@ -2271,19 +2231,18 @@ renameHoleUnit' pkg_map closure env uid = -> if isNullUFM (intersectUFM_C const (udfmToUfm (getUniqDSet fh)) env) then uid -- Functorially apply the substitution to the instantiation, - -- then check the 'ClosureUnitInfoMap' to see if there is + -- then check the 'UnitInfoMap' to see if there is -- a compiled version of this 'InstantiatedUnit' we can improve to. -- See Note [VirtUnit to RealUnit improvement] - else improveUnit' pkg_map closure $ - mkVirtUnit cid - (map (\(k,v) -> (k, renameHoleModule' pkg_map closure env v)) insts) + else mkVirtUnit cid + (map (\(k,v) -> (k, renameHoleModule' pkg_map env v)) insts) _ -> uid -- | Injects an 'InstantiatedModule' to 'Module' (see also -- 'instUnitToUnit'. -instModuleToModule :: UnitState -> InstantiatedModule -> Module -instModuleToModule pkgstate (Module iuid mod_name) = - mkModule (instUnitToUnit pkgstate iuid) mod_name +instModuleToModule :: InstantiatedModule -> Module +instModuleToModule (Module iuid mod_name) = + mkModule (instUnitToUnit iuid) mod_name -- | Print unit-ids with UnitInfo found in the given UnitState pprWithUnitState :: UnitState -> SDoc -> SDoc View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/3d36028538cb8c040f35dd439479fcc7715aea09 -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/3d36028538cb8c040f35dd439479fcc7715aea09 You're receiving this email because of your account on gitlab.haskell.org.