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

Improve overloading resolution if expected type is not FunProto #14733

Merged
merged 1 commit into from Mar 23, 2022

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Mar 21, 2022

If expected type is a selection proto or similar we picked all alternatives
that were compatible with it. But if the expected type was meant to be
used after extension method expansion, then no alternative would be classified
as compatible.

We now drop all method alternatives in this case. If only one alternative remains,
we pick that one.

Fixes #14729

If expected type is a selection proto or similar we picked all alternatives
that were compatible with it. But if the expected type was meant to be
used after extension method expansion, then no alternative would be classified
as compatible.

We now drop all method alternatives in this case. If only one alternative remains,
we pick that one.

Fixes scala#14279
// pick any alternatives that are not methods since these might be convertible
// to the expected type, or be used as extension method arguments.
val convertible = alts.filterNot(alt =>
normalize(alt, IgnoredProto(pt)).widenSingleton.isInstanceOf[MethodType])
Copy link
Member

Choose a reason for hiding this comment

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

Is using normalize really needed here or could we do:

Suggested change
normalize(alt, IgnoredProto(pt)).widenSingleton.isInstanceOf[MethodType])
alt.widenSingleton.isInstanceOf[MethodOrPoly])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's needed for skipping using clauses

Copy link
Member

Choose a reason for hiding this comment

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

I see. normalize will create fresh type variables for PolyTypes, they'll be gc'ed eventually but if we wanted to avoid creating them in the first place we could perhaps have a stripPolyAndImplicit method, or add a flag to normalize for situations where we don't keep around its result.

// pick any alternatives that are not methods since these might be convertible
// to the expected type, or be used as extension method arguments.
val convertible = alts.filterNot(alt =>
normalize(alt, IgnoredProto(pt)).widenSingleton.isInstanceOf[MethodType])
Copy link
Member

Choose a reason for hiding this comment

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

I see. normalize will create fresh type variables for PolyTypes, they'll be gc'ed eventually but if we wanted to avoid creating them in the first place we could perhaps have a stripPolyAndImplicit method, or add a flag to normalize for situations where we don't keep around its result.

@smarter smarter assigned odersky and unassigned smarter Mar 23, 2022
@odersky
Copy link
Contributor Author

odersky commented Mar 23, 2022

I see. normalize will create fresh type variables for PolyTypes, they'll be gc'ed eventually but if we wanted to avoid creating them in the first place we could perhaps have a stripPolyAndImplicit method, or add a flag to normalize for situations where we don't keep around its result.

I the inefficiency won't matter since this case is executed rarely. In fact not executing this case caused the error that was fixed. So that should be the only time so far we fall into this case.

@odersky odersky merged commit fa3893e into scala:main Mar 23, 2022
@odersky odersky deleted the fix-14729 branch March 23, 2022 16:49
@Kordyjan Kordyjan added this to the 3.1.3 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants