Various changes needed for ghc-worker integration#321
Conversation
|
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
| 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.
Saizan
force-pushed
the
public-dap
branch
2 times, most recently
from
a36168f to
f550b56
Compare
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.
so that paths from user interaction are disambiguated closer to parsing. Also looking up a module by path now goes through getAllLoadedModulesWithPaths.
2 participants
No description provided.