Re: Migration guide for multiple home units

PUBLIC
Thanks, based on this I got things going. I might have to come back to you if it turns out something's still broken in my original full application.
-----Original Message-----
From: Matthew Pickering
Gergo, I rewrote your example program for the new API but didn't quite manage to finish today. I will finish it off and post it tomorrow.
Matt
On Mon, Jun 20, 2022 at 6:16 AM Erdi, Gergo
wrote: PUBLIC
I managed to get this working, but I would like some feedback from Matt or others on whether this is the intended way of doing this.
1. The `extendMG` change can be brute-forced just to keep things going, by looking up dependency `ModSummary`s by `ModName` (we don't have full `Module`s at this point yet!):
registerModSummary :: (GhcMonad m) => ModSummary -> m () registerModSummary ms = modifySession \env -> let mg = hsc_mod_graph env -- TODO: of course with a bit more housekeeping we can do better than this... edges = [ NodeKey_Module $ msKey ms' | dep <- deps, ms' <- mgModSummaries mg, ms_mod_name ms' == dep ] mg' = extendMG mg edges ms in env{ hsc_mod_graph = mg' } where deps = map (unLoc . snd) $ ms_textual_imps ms
2. `registerModule` can be kept mostly as-is, since it only uses `modifyUnitState` to change the active unit:
registerModule :: (GhcMonad m) => ModDetails -> ModIface -> m () registerModule details iface = modifySession $ extendHpt . addModule where hmi = HomeModInfo iface details Nothing
mod = mi_module iface modOrig = ModOrigin (Just True) [] [] True
addModule = modifyUnitState $ \us -> us { moduleNameProvidersMap = M.insert (moduleName mod) (M.singleton mod modOrig) $ moduleNameProvidersMap us }
extendHpt = hscUpdateHUG $ addHomeModInfoToHug hmi
modifyUnitState :: (UnitState -> UnitState) -> HscEnv -> HscEnv modifyUnitState f env = env { hsc_unit_env = let ue = hsc_unit_env env in let units = ue_units ue units' = f units in ue_setUnits units' ue }
3. The tricky part is getting `addUnit` right. For this, based on the implementation of `initUnits`, I came up with the following:
modifyUnitEnv :: (UnitEnv -> UnitEnv) -> HscEnv -> HscEnv modifyUnitEnv f env = env { hsc_unit_env = let ue = hsc_unit_env env in f ue }
addUnit :: DynFlags -> UnitId -> HscEnv -> HscEnv addUnit dflags unitId = modifyUnitEnv $ \ue -> let dbs = ue_unit_dbs ue unit_state = ue_units ue home_unit = ue_homeUnit ue in flip updateHug ue $ unitEnv_insert unitId $ HomeUnitEnv { homeUnitEnv_units = unit_state , homeUnitEnv_unit_dbs = dbs , homeUnitEnv_dflags = dflags , homeUnitEnv_hpt = emptyHomePackageTable , homeUnitEnv_home_unit = home_unit }
setCurrentUnit :: UnitId -> HscEnv -> HscEnv setCurrentUnit unitId = modifyUnitEnv $ ue_setActiveUnit unitId
So my questions about this:
1. How does setting the home unit make sense? By doing this, I am effectively setting the home unit to `main` for all units, since that's the initial `ue_homeUnit` of the initial unit environment. Or does it not matter because after `addUnit`, I call `setCurrentUnit` anyway? I've found that I can't use the unit I am just adding as its own home unit, because that then leads to module name resolution problems in `main`: every imported module from `main` is searched for in `main` instead of its correct unit.
2. Speaking of `main`, why is it that when adding units, I have to skip `mainUnitId`, otherwise module resolution breaks again?
3. Unlike the previous version, I am no longer creating and putting `UnitInfo`s anywhere. Where is this going to bite me? Where (if anywhere) should I put `UnitInfo`s with the new setup?
Thanks, Gergo
-----Original Message----- From: ÉRDI Gergő
Sent: Sunday, June 19, 2022 12:32 PM To: Erdi, Gergo Cc: GHC Devs ; Montelatici, Raphael Laurent Subject: [External] Re: Migration guide for multiple home units On Thu, 16 Jun 2022, Erdi, Gergo via ghc-devs wrote:
Is there a migration guide for GHC API clients for the new “multiple home units” feature?
OK so in concrete terms, please see my attached program which is a heavily cut-down, standalone version of my real program. On commit fd42ab5fa1df847a6b595dfe4b63d9c7eecbf400^ (i.e. 3219610e3ba6cb6a5cd1f4e32e2b4befea5bd384) it compiles and works as expected. On commit fd42ab5fa1df847a6b595dfe4b63d9c7eecbf400 onwards, two problems pop up:
1. `extendMG` has changed and now requires manually specifying outgoing dependency edges. I thought the whole point of `summariseFile` was to collect this information? The reason I need to `extendMG` at that point is to get intra-unit orphan instances working.
2. `modifyUnitState` and its two uses (`addUnit` and `registerModule`) need to be updated to the new API. I think it makes sense that these need changing, since they touch exactly on the issue of which units are being compiled right now. However, I don't know how to update these. Also, I guess `setHomeUnit` should change the `CurrentUnit` instead of the `HomeUnit` now?
Thanks, Gergo
This email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please delete all copies and notify the sender immediately. You may wish to refer to the incorporation details of Standard Chartered PLC, Standard Chartered Bank and their subsidiaries at https: //www.sc.com/en/our-locations
Where you have a Financial Markets relationship with Standard Chartered PLC, Standard Chartered Bank and their subsidiaries (the "Group"), information on the regulatory standards we adhere to and how it may affect you can be found in our Regulatory Compliance Statement at https: //www.sc.com/rcs/ and Regulatory Compliance Disclosures at http: //www.sc.com/rcs/fm
Insofar as this communication is not sent by the Global Research team and contains any market commentary, the market commentary has been prepared by the sales and/or trading desk of Standard Chartered Bank or its affiliate. It is not and does not constitute research material, independent research, recommendation or financial advice. Any market commentary is for information purpose only and shall not be relied on for any other purpose and is subject to the relevant disclaimers available at https: //www.sc.com/en/regulatory-disclosures/#market-disclaimer.
Insofar as this communication is sent by the Global Research team and contains any research materials prepared by members of the team, the research material is for information purpose only and shall not be relied on for any other purpose, and is subject to the relevant disclaimers available at https: //research.sc.com/research/api/application/static/terms-and-conditions.
Insofar as this e-mail contains the term sheet for a proposed transaction, by responding affirmatively to this e-mail, you agree that you have understood the terms and conditions in the attached term sheet and evaluated the merits and risks of the transaction. We may at times also request you to sign the term sheet to acknowledge the same.
Please visit https: //www.sc.com/en/regulatory-disclosures/dodd-frank/ for important information with respect to derivative products.
This email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please delete all copies and notify the sender immediately. You may wish to refer to the incorporation details of Standard Chartered PLC, Standard Chartered Bank and their subsidiaries at https: //www.sc.com/en/our-locations Where you have a Financial Markets relationship with Standard Chartered PLC, Standard Chartered Bank and their subsidiaries (the "Group"), information on the regulatory standards we adhere to and how it may affect you can be found in our Regulatory Compliance Statement at https: //www.sc.com/rcs/ and Regulatory Compliance Disclosures at http: //www.sc.com/rcs/fm Insofar as this communication is not sent by the Global Research team and contains any market commentary, the market commentary has been prepared by the sales and/or trading desk of Standard Chartered Bank or its affiliate. It is not and does not constitute research material, independent research, recommendation or financial advice. Any market commentary is for information purpose only and shall not be relied on for any other purpose and is subject to the relevant disclaimers available at https: //www.sc.com/en/regulatory-disclosures/#market-disclaimer. Insofar as this communication is sent by the Global Research team and contains any research materials prepared by members of the team, the research material is for information purpose only and shall not be relied on for any other purpose, and is subject to the relevant disclaimers available at https: //research.sc.com/research/api/application/static/terms-and-conditions. Insofar as this e-mail contains the term sheet for a proposed transaction, by responding affirmatively to this e-mail, you agree that you have understood the terms and conditions in the attached term sheet and evaluated the merits and risks of the transaction. We may at times also request you to sign the term sheet to acknowledge the same. Please visit https: //www.sc.com/en/regulatory-disclosures/dodd-frank/ for important information with respect to derivative products.

PUBLIC
In case anyone finds this interesting, I ended up splitting the unit initialization into two parts: one where all units are registered, without any package dependencies, and one where the package dependencies of a single unit are registered. This allowed us to start with just knowing which units are in play, use summariseFile to discover module dependencies, and then use that to compute package dependencies. It's basically just a refactoring of Matt's code from his email up the chain.
setUnits :: [(UnitId, DynFlags -> DynFlags)] -> HscEnv -> IO HscEnv
setUnits units hsc_env = do
home_unit_graph <- createHomeUnitGraph hsc_env units'
setHomeUnitGraph hsc_env dummy_home_unit home_unit_graph
where
dflags0 = hsc_dflags hsc_env
units'@((dummy_home_unit, _):_) =
[ (unit_id, unitFlags dflags0{ homeUnitId_ = unit_id, packageFlags = [] })
| (unit_id, unitFlags) <- units
]
setUnitDeps :: UnitId -> [UnitId] -> HscEnv -> IO HscEnv
setUnitDeps unit_id deps hsc_env = do
home_unit_env <- fillHomeUnitEnv hsc_env home_units new_dflags
let home_unit_graph = unitEnv_insert unit_id home_unit_env home_unit_graph0
setHomeUnitGraph hsc_env unit_id home_unit_graph
where
old_dflags = ue_unitFlags unit_id $ hsc_unit_env hsc_env
new_dflags = old_dflags
{ packageFlags = [ ExposePackage "" (UnitIdArg (RealUnit $ Definite uid)) (ModRenaming True []) | uid <- deps]
}
home_unit_graph0 = ue_home_unit_graph . hsc_unit_env $ hsc_env
home_units = unitEnv_keys home_unit_graph0
setHomeUnitGraph :: HscEnv -> UnitId -> HomeUnitGraph -> IO HscEnv
setHomeUnitGraph hsc_env unit_id home_unit_graph = do
unit_env <- assertUnitEnvInvariant <$> initUnitEnv unit_id home_unit_graph (ghcNameVersion dflags0) (targetPlatform dflags0)
return $ hsc_env { hsc_unit_env = unit_env }
where
dflags0 = hsc_dflags hsc_env
createHomeUnitGraph :: HscEnv -> [(UnitId, DynFlags)] -> IO HomeUnitGraph
createHomeUnitGraph hsc_env units =
unitEnv_new <$> traverse (fillHomeUnitEnv hsc_env home_units) (Map.fromList units)
where
home_units = Set.fromList $ map fst units
fillHomeUnitEnv :: HscEnv -> Set.Set UnitId -> DynFlags -> IO HomeUnitEnv
fillHomeUnitEnv hsc_env home_units dflags = do
(dbs, unit_state, home_unit, _) <- initUnits logger dflags Nothing home_units
pure $ HomeUnitEnv
{ homeUnitEnv_units = unit_state
, homeUnitEnv_unit_dbs = Just dbs
, homeUnitEnv_dflags = dflags
, homeUnitEnv_hpt = emptyHomePackageTable
, homeUnitEnv_home_unit = Just home_unit
}
where
logger = hsc_logger hsc_env
-----Original Message-----
From: Erdi, Gergo
Sent: Tuesday, June 21, 2022 5:42 PM
To: Matthew Pickering
participants (1)
-
Erdi, Gergo