Skip to content
New issue

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

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

Already on GitHub? # to your account

Interpolate any type vars from comparing against SelectionProto #16348

Merged
merged 1 commit into from
Dec 13, 2022

Conversation

dwijnand
Copy link
Member

If the method being selected is polymorphic, then it is added, along
with fresh type vars, to the constraint. Normally this is fine, because
when the selection is then applied, the type vars are interpolated
(during tree simplification). But if the method being selected is a
macro that's being jointly compiled, then the compilation may be
suspended, which leaves a trailing type lambda in the constraint.

I tried addressing this in a few different ways. Initially with an
exemption in TreeChecker, but that seemed too remote and too liberal.
Later I tried addressing it specifically in inlining (in inlineIfNeeded)
but that also seemed too remote and extensive. In the end I thought
handling this around the testSubType call seemed most suitable.
Interpolating on all the new type vars in general is too eager, so I
ended up guarding it only for SelectionProto, sadly.

@dwijnand dwijnand marked this pull request as ready for review November 16, 2022 16:14
@dwijnand dwijnand requested a review from smarter November 16, 2022 16:15
@dwijnand dwijnand force-pushed the leaky-testSubType-constraint-lambdas branch from 1734cc1 to a4ed79b Compare November 23, 2022 15:14
@dwijnand
Copy link
Member Author

dwijnand commented Dec 1, 2022

@smarter Any thoughts here? I'm not sure what approach to take, or whether anyone cares about the issue.

@smarter
Copy link
Member

smarter commented Dec 12, 2022

I'm not sure either, would replacing the super.typedXX(...) calls by super.typed(...) calls to include the simplify step work?

@dwijnand
Copy link
Member Author

dwijnand commented Dec 12, 2022

No, I fairly sure not, because that would loop around between InlineTyper.typedApply and super.typed..

I think the architectural way to do this might be to shuffle the logic that InlineTyper is doing into its adapt post-processing, which is post-simplify - but that seemed like a bigger change that I would want to double-check first.

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

Let's go with this for now then.

@Kordyjan Kordyjan added this to the 3.3.0-RC1 milestone Dec 12, 2022
@dwijnand dwijnand assigned dwijnand and unassigned smarter Dec 12, 2022
@dwijnand dwijnand force-pushed the leaky-testSubType-constraint-lambdas branch from a4ed79b to 251b7c8 Compare December 12, 2022 20:50
@dwijnand dwijnand force-pushed the leaky-testSubType-constraint-lambdas branch from 251b7c8 to 3b795ee Compare December 13, 2022 14:31
@dwijnand dwijnand enabled auto-merge December 13, 2022 14:32
@dwijnand dwijnand merged commit b21a457 into scala:main Dec 13, 2022
@dwijnand dwijnand deleted the leaky-testSubType-constraint-lambdas branch December 14, 2022 10:50
@Kordyjan Kordyjan modified the milestones: 3.3.0-RC1, 3.3.0 Aug 1, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-Ycheck returns non-empty constraint assertion error from an inline method calling a macro
3 participants