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

Strengthen overloading resolution to deal with extension methods #6116

Merged
merged 7 commits into from Mar 21, 2019

Conversation

@odersky
Copy link
Contributor

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 some commits Mar 19, 2019

Allow for overloaded extensions in hasExtensionMethod
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.
Strengthen overloading resolution to deal with extension methods
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.
Fix advanced candidate setup
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
More user friendly printing of prototypes
FunProto and PolyProto now print without exposing internal names.
Implement overriding checks for extension methods
Extension methods cannot override normal methods and vice versa.

@odersky odersky requested a review from liufengyun Mar 20, 2019

@liufengyun
Copy link
Contributor

left a comment

LGTM

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

This comment has been minimized.

Copy link
@liufengyun

liufengyun Mar 20, 2019

Contributor

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.

This comment has been minimized.

Copy link
@odersky

odersky Mar 20, 2019

Author Contributor

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

@odersky odersky merged commit 8755bdf into lampepfl:master Mar 21, 2019

2 checks passed

CLA User signed CLA
Details
continuous-integration/drone/pr the build was successful
Details

@allanrenucci allanrenucci deleted the dotty-staging:fix-ext-overload branch Mar 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.