Skip to content

Remove extra call to newHscEnvEqWithImportPaths #3676

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.

Sign up for GitHub

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 6 commits into from
Jul 2, 2023
Merged

Remove extra call to newHscEnvEqWithImportPaths #3676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bdb1906
Remove extra call to newHscEnvEqWithImportPaths
nlander Jun 27, 2023
b9a8454
Revert "Remove extra call to newHscEnvEqWithImportPaths"
nlander Jun 27, 2023
f570d5f
Add updateHscEnvEq helper function
nlander Jun 27, 2023
222b050
Add comment to updateHscEnvEq call
nlander Jun 30, 2023
20ebcc1
Update HscEnvEq equality comment
nlander Jun 30, 2023
c342b71
Merge branch 'master' into remove-extra-hscenveq
michaelpj Jul 2, 2023
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
7 changes: 6 additions & 1 deletion ghcide/src/Development/IDE/Core/Rules.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,12 @@ ghcSessionDepsDefinition fullModSummary GhcSessionDepsConfig{..} env file = do
#endif
session' <- liftIO $ mergeEnvs hsc moduleNode inLoadOrder depSessions

Just <$> liftIO (newHscEnvEqWithImportPaths (envImportPaths env) session' [])
-- Here we avoid a call to to `newHscEnvEqWithImportPaths`, which creates a new
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelpj Does this sufficiently explain what's going on here? Is there anything we should add?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say ask yourself:

  1. If I was reading this for the first time, would I know what is going on and why?
  2. Has this captured the knowledge that I gained in order to make the fix?

-- ExportsMap when it is called. We only need to create the ExportsMap once per
-- session, while `ghcSessionDepsDefinition` will be called for each file we need
-- to compile. `updateHscEnvEq` will refresh the HscEnv (session') and also
-- generate a new Unique.
Just <$> liftIO (updateHscEnvEq env session')

-- | Load a iface from disk, or generate it if there isn't one or it is out of date
-- This rule also ensures that the `.hie` and `.o` (if needed) files are written out.
Expand Down
9 changes: 8 additions & 1 deletion ghcide/src/Development/IDE/Types/HscEnvEq.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module Development.IDE.Types.HscEnvEq
hscEnvWithImportPaths,
newHscEnvEqPreserveImportPaths,
newHscEnvEqWithImportPaths,
updateHscEnvEq,
envImportPaths,
envPackageExports,
envVisibleModuleNames,
Expand Down Expand Up @@ -32,7 +33,8 @@ import System.Directory (makeAbsolute)
import System.FilePath

-- | An 'HscEnv' with equality. Two values are considered equal
-- if they are created with the same call to 'newHscEnvEq'.
-- if they are created with the same call to 'newHscEnvEq' or
-- 'updateHscEnvEq'.
data HscEnvEq = HscEnvEq
{ envUnique :: !Unique
, hscEnv :: !HscEnv
Expand All @@ -51,6 +53,11 @@ data HscEnvEq = HscEnvEq
-- If Nothing, 'listVisibleModuleNames' panic
}

updateHscEnvEq :: HscEnvEq -> HscEnv -> IO HscEnvEq
updateHscEnvEq oldHscEnvEq newHscEnv = do
let update newUnique = oldHscEnvEq { envUnique = newUnique, hscEnv = newHscEnv }
update <$> Unique.newUnique

-- | Wrap an 'HscEnv' into an 'HscEnvEq'.
newHscEnvEq :: FilePath -> HscEnv -> [(UnitId, DynFlags)] -> IO HscEnvEq
newHscEnvEq cradlePath hscEnv0 deps = do
Expand Down