Skip to content

session-loader: Don't loop forever when we don't find a file in any multi component #4096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 2, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 26 additions & 10 deletions ghcide/session-loader/Development/IDE/Session.hs
Original file line number Diff line number Diff line change
Expand Up @@ -585,9 +585,21 @@
let new_cache = newComponentCache recorder optExtensions hieYaml _cfp hscEnv
all_target_details <- new_cache old_deps new_deps

let all_targets = concatMap fst all_target_details

let this_flags_map = HM.fromList (concatMap toFlagsMap all_targets)
this_dep_info <- getDependencyInfo $ maybeToList hieYaml
let (all_targets, this_flags_map, this_options)
= case HM.lookup _cfp flags_map' of
Just this -> (all_targets', flags_map', this)
Nothing -> (this_target_details : all_targets', HM.insert _cfp this_flags flags_map', this_flags)
where all_targets' = concat all_target_details
flags_map' = HM.fromList (concatMap toFlagsMap all_targets')
this_target_details = TargetDetails (TargetFile _cfp) this_error_env this_dep_info [_cfp]
this_flags = (this_error_env, this_dep_info)
this_error_env = ([this_error], Nothing)
this_error = ideErrorWithSource (Just "cradle") (Just DiagnosticSeverity_Error) _cfp
$ T.unlines
[ "No cradle target found. Is this file listed in the targets of your cradle?"
, "If you are using a .cabal file, please ensure that this module is listed in either the exposed-modules or other-modules section"
]

void $ modifyVar' fileToFlags $
Map.insert hieYaml this_flags_map
Expand Down Expand Up @@ -615,7 +627,7 @@
let !exportsMap' = createExportsMap $ mapMaybe (fmap hirModIface) modIfaces
liftIO $ atomically $ modifyTVar' (exportsMap shakeExtras) (exportsMap' <>)

return $ second Map.keys $ this_flags_map HM.! _cfp
return $ second Map.keys this_options

let consultCradle :: Maybe FilePath -> FilePath -> IO (IdeResult HscEnvEq, [FilePath])
consultCradle hieYaml cfp = do
Expand Down Expand Up @@ -655,7 +667,7 @@
InstallationMismatch{..} ->
return (([renderPackageSetupException cfp GhcVersionMismatch{..}], Nothing),[])
InstallationChecked _compileTime _ghcLibCheck -> do
atomicModifyIORef' cradle_files (\xs -> (cfp:xs,()))

Check warning on line 670 in ghcide/session-loader/Development/IDE/Session.hs

View workflow job for this annotation

GitHub Actions / Hlint check run

Warning in loadSessionWithOptions in module Development.IDE.Session: Use atomicModifyIORef'_ ▫︎ Found: "atomicModifyIORef' cradle_files (\\ xs -> (cfp : xs, ()))" ▫︎ Perhaps: "atomicModifyIORef'_ cradle_files ((:) cfp)"
session (hieYaml, toNormalizedFilePath' cfp, opts, libDir)
-- Failure case, either a cradle error or the none cradle
Left err -> do
Expand Down Expand Up @@ -810,7 +822,7 @@
-> HscEnv -- ^ An empty HscEnv
-> [ComponentInfo] -- ^ New components to be loaded
-> [ComponentInfo] -- ^ old, already existing components
-> IO [ ([TargetDetails], (IdeResult HscEnvEq, DependencyInfo))]
-> IO [ [TargetDetails] ]
newComponentCache recorder exts cradlePath _cfp hsc_env old_cis new_cis = do
let cis = Map.unionWith unionCIs (mkMap new_cis) (mkMap old_cis)
-- When we have multiple components with the same uid,
Expand Down Expand Up @@ -882,14 +894,13 @@
henv <- createHscEnvEq thisEnv (zip uids dfs)
let targetEnv = (if isBad ci then multi_errs else [], Just henv)
targetDepends = componentDependencyInfo ci
res = ( targetEnv, targetDepends)
logWith recorder Debug $ LogNewComponentCache res
logWith recorder Debug $ LogNewComponentCache (targetEnv, targetDepends)
evaluate $ liftRnf rwhnf $ componentTargets ci

let mk t = fromTargetId (importPaths df) exts (targetId t) targetEnv targetDepends
ctargets <- concatMapM mk (componentTargets ci)

return (L.nubOrdOn targetTarget ctargets, res)
return (L.nubOrdOn targetTarget ctargets)

{- Note [Avoiding bad interface files]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -1081,15 +1092,20 @@
-- A special target for the file which caused this wonderful
-- component to be created. In case the cradle doesn't list all the targets for
-- the component, in which case things will be horribly broken anyway.
-- Otherwise, we will immediately attempt to reload this module which
-- causes an infinite loop and high CPU usage.
--
-- When we have a single component that is caused to be loaded due to a
-- file, we assume the file is part of that component. This is useful
-- for bare GHC sessions, such as many of the ones used in the testsuite
--
-- We don't do this when we have multiple components, because each
-- component better list all targets or there will be anarchy.
-- It is difficult to know which component to add our file to in
-- that case.
-- Multi unit arguments are likely to come from cabal, which
-- does list all targets.
--
-- If we don't end up with a target for the current file in the end, then
-- we will report it as an error for that file
abs_fp <- liftIO $ makeAbsolute (fromNormalizedFilePath cfp)
let special_target = Compat.mkSimpleTarget df abs_fp
pure $ (df, special_target : targets) :| []
Expand Down
Loading