-
Notifications
You must be signed in to change notification settings - Fork 205
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
Filter implicit parameter instances from data-dependencies #7320
Conversation
Occasionally GHC likes to float implicit parameters to the top-level making them look like any other typeclass instances. Implicit parameters are by design not unique so if we try to reconstruct them we get an error about a duplicate instance. I haven’t figured out when exactly GHC floats things to the top-level but I did verify that this testcase breaks without the fix. changelog_begin changelog_end
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.
The fix looks good!
-- how to reliably stop it from doing this so be careful when changing this. | ||
] | ||
withCurrentDirectory (tmpDir </> "dep") $ | ||
callProcessSilent damlc ["build", "-o", "dep.dar"] |
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 wonder if we can make a test that reliably generates an implicit parameter instance?
Or perhaps, add some a check to this test that an implicit parameter instance is in the DAR.
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.
Ah, I wonder if we can slip it in to the "Simple DALF" test or something. (It's not so simple...)
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’ve added an assertion that we have 2 implicit instances. Simple DALF would work as well I guess but this has the advantage that it tests what our users actually hit which seems useful. I still don’t understand how to generate it reliably. Even in this example you can remove the type signature, the inferred type stays the same but now it’s inlined 🤷
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.
Awesome, thank you!
changelog_begin changelog_end
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.
This scares me a bit. But thanks a lot for the quick fix.
Occasionally GHC likes to float implicit parameters to the top-level
making them look like any other typeclass instances. Implicit
parameters are by design not unique so if we try to reconstruct them
we get an error about a duplicate instance.
I haven’t figured out when exactly GHC floats things to the top-level
but I did verify that this testcase breaks without the fix.
changelog_begin
changelog_end
Pull Request Checklist
CHANGELOG_BEGIN
andCHANGELOG_END
tagsNOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with
/AzurePipelines run
totrigger the build.