Skip to content

ResolveSuper: add missing methods overrides for disambiguation #5662

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

Merged
merged 2 commits into from
Dec 29, 2018

Conversation

smarter
Copy link
Member

@smarter smarter commented Dec 26, 2018

In mixin-overrides.scala, when we compute the overrides for QInt in
ResolveSuper, we need to add an override for Baz#op because it matches
the concrete method Bar#op (otherwise we'll get a "Conflicting default
methods" error at runtime). Before this commit, this did not happen
because Baz#op was not considered to match Bar#op, because one refers to
Baz.this.A and the other to Bar.this.A.

We fix this by calling matchingDenotation with a targetType
corresponding to the current class (QInt here) instead of the
trait where the denotation is defined.

This also allows us to remove the PrimitiveForwarders mini-phase which was
apparently doing a subset of this work and is now redundant.

In mixin-overrides.scala, when we compute the overrides for QInt in
ResolveSuper, we need to add an override for Baz#op because it matches
the concrete method Bar#op (otherwise we'll get a "Conflicting default
methods" error at runtime). Before this commit, this did not happen
because Baz#op was not considered to match Bar#op, because one refers to
Baz.this.A and the other to Bar.this.A.

We fix this by calling `matchingDenotation` with a `targetType`
corresponding to the current class (`QInt` here) instead of the
trait where the denotation is defined.

This also allows us to remove the PrimitiveForwarders mini-phase which was
apparently doing a subset of this work and is now redundant.
@odersky odersky merged commit 4eee106 into scala:master Dec 29, 2018
@allanrenucci allanrenucci deleted the fix-mixins branch December 29, 2018 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants