-
-
Notifications
You must be signed in to change notification settings - Fork 369
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
Remove extra call to newHscEnvEqWithImportPaths #3676
Conversation
We also update the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Worth a comment explaining what's going on? |
|
put it in the code! |
@@ -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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
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:
- If I was reading this for the first time, would I know what is going on and why?
- Has this captured the knowledge that I gained in order to make the fix?
ff6d39e
to
222b050
Compare
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Submitting this at the request of @wz1000
Instead of calling
newHscEnvEqWithImportPaths
again inghcSessionDepsDefinition
, we only generate a newUnique
for theHscEnvEq
and update theenvUnique
field.