Skip to content

Strengthen overloading resolution to deal with extension methods #6116

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 7 commits into from
Mar 21, 2019

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Mar 19, 2019

If resolving with the first parameter list yields a draw, and there are
further argument lists following the first one, take these into account
as well in order to arrive at a single best alternative.

This is particularly useful for extension methods, where we might
well have several overloaded extension methods with the same first
parameter list.

odersky added 7 commits March 19, 2019 12:09
This does not yet fully solve the problem of overloaded extensions,
since the identified overloaded alternatives will still be ambiguous
if the first parameter list is the same.

But at least we get a notice what extension method was tried.
If resolving with the first parameter list yields a draw, and there are
further argument lists following the first one, take these into account
as well in order to arrive at a single best alternative.

This is particularly useful for extension methods, where we might
well have several overloaded extension methods with the same first
parameter list.
Use NoPrefix since that way the info of the TermRef is
guaranteed to be the info of the Symbol.

More tests for overloading behavior of extension methods
FunProto and PolyProto now print without exposing internal names.
Extension methods cannot override normal methods and vice versa.
Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

LGTM

tp.memberBasedOnFlags(name, required = ExtensionMethod) match {
case mbr: SingleDenotation => qualifies(mbr)
case mbr => mbr.hasAltWith(qualifies(_))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of the dispatch here? Semantically it seems the same as:

tp.memberBasedOnFlags(name, required = ExtensionMethod).hasAltWith(qualifies(_))

Is it for performance? It seems we save several exists check this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's for performance. We optimize for the common case where the member is not overloaded.

@odersky odersky merged commit 8755bdf into scala:master Mar 21, 2019
@allanrenucci allanrenucci deleted the fix-ext-overload branch March 21, 2019 08:05
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