Skip to content
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

Actually force usages #3713

Merged
merged 3 commits into from
Jul 25, 2023
Merged

Actually force usages #3713

merged 3 commits into from
Jul 25, 2023

Conversation

wz1000
Copy link
Collaborator

@wz1000 wz1000 commented Jul 12, 2023

In 00f4e61 we attempted to share the filepaths in GHC's usage field.
Unfortunately the mi_usages field is lazy and so these turned out to not actually be shared because
the thunk responsible for sharing was never evaluated.

Add the right strictness annotations to ensure these are actually shared

@michaelpj
Copy link
Collaborator

Seems to not build on all GHCs

@wz1000 wz1000 force-pushed the wip/usage-leak branch 3 times, most recently from 2c81b4f to 9ade701 Compare July 18, 2023 10:41
@wz1000 wz1000 added the merge me Label to trigger pull request merge label Jul 25, 2023
@michaelpj
Copy link
Collaborator

Still doesn't build.

Unfortunately the `mi_usages` is evaluated by `checkOldIface` before we have a chance to do the sharing,
resulting in them not actually being shared on the first load.

Solution is to share the usages before `checkOldIface` has a chance to inspect them, breaking the sharing.
@mergify mergify bot merged commit c9519af into master Jul 25, 2023
43 of 45 checks passed
@fendor fendor mentioned this pull request Aug 8, 2023
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants