Various changes needed for ghc-worker integration#321
Conversation
A debugRunner might not set any of the loaded modules as targets, so we can't rely on those to ensure they are included in the result by downsweep. Then we do a downsweep only for the in-memory modules and manually construct the union of the graphs.
|
Let me know once I should look. Cool! |
|
@alt-romes ready to review (prob. since before zurihac but forgot to ping) |
alt-romes
left a comment
alt-romes
left a comment
There was a problem hiding this comment.
Looks good to me, just a few comments
| loadHomeUnit | ||
|
|
||
| fixHomeUnitsDynFlagsForIIDecl | ||
| GHC.modifySessionM $ liftIO . addInteractiveGhcDebuggerUnit |
There was a problem hiding this comment.
does this end up getting called twice? Once here, once in initHomeUnitEnv? Is this idempotent and/or expected?
| getModuleByPath path = do | ||
| -- get all loaded modules this every time as the loaded modules may have changed | ||
| lms <- getAllLoadedModules | ||
| lms <- mapM (\ m -> fmap (m,) . liftIO . makeAbsolute . msHsFilePath $ m) =<< getAllLoadedModules |
There was a problem hiding this comment.
How often is getModuleByPath called? I reckon pretty often. Maybe getAllLoadedModules absolute paths should be somehow cache?
Feels like it would be slightly cleaner if getAllLoadedModules (INVARIANT: returns normalized Absolute paths) had a cache
There was a problem hiding this comment.
Not so often? Mostly when the dap-client queries or manipulates breakpoints.
Though now that I look at this again we should not makeAbsolute these paths, but rather use the workingDir from their units' DynFlags.
How would you invalidate a loadedModules :: [ModSummary] cache with patched paths though? Currently we only load modules in runDebugger I think, but I could see that changing.
2 participants
No description provided.