Handle polymorphic overloads with apply members #6756
Conversation
When we do overloading resolution, we first replace alternatives that could not possibly match the expected type by their `apply` members if they exist, however the existing logic failed to select `apply` members from the result of a polymorphic parameterless def. We fix this by simply replacing the PolyType by its result type without making up any type variable, the resulting behavior matches the existing behavior of Scala 2 on the added test file.
| case _ => | ||
| alt | ||
| } | ||
| qual.member(nme.apply).alternatives.map(TermRef(alt, nme.apply, _)) |
There was a problem hiding this comment.
I find this extremely dicey. alt.apply is not even a well formed term and its type refers an unbound param ref. So this does not look right to me. Scala 2's behavior is no excuse, since it has many situations where bound variables escape. So I do not think this is the right fix, and I am not sure we need a fix at all, since it seems impossible to describe the fix in terms that are simple and sound.
There was a problem hiding this comment.
alt.apply is not even a well formed term
But this is also true for the case where alt is a monomorphic method since it's not a stable prefix, and yet the existing code will happily make up a TermRef for it. If we wanted to be stricter we could use .narrow instead.
its type refers an unbound param ref.
True, since this is only used for overloading resolution, it seemed OK to me. If we want to avoid that, an alternative would be to make the resulting types polymorphic, e.g. if we start with:
def foo2[T]: T => StringThen we could pretend that it really has type:
[T](x: T): StringThis would mean that switching between having a polymorphic method with a parameter list and a polymorphic method that returns an object with the same parameter list would not affect which overload gets chosen, this is a nice property to have and it matches what happens with monomorphic methods. The only downside I see is that it doesn't match Scala 2 behavior, but I don't think that's a big deal here (and the current behavior of dotty master can also select a different overload than Scala 2, so we wouldn't be regressing).
There was a problem hiding this comment.
You'd certainly get into problems if the polytype is already in use in the current constraint. Yes, we can move the PolyType around, onto the apply method, but these are complicated acrobatics which make the code base harder to understand. Do we have a hard reason for doing this? Being compatible with scalac is not reason enough by itself, I think.
Instead of doing overload selection directly on the result of the
PolyType which contains unbounded paramrefs, we now first call
`wildApprox` to get rid of the paramrefs. The resulting behavior is
different from what Scala 2 does, but it makes polymorphic overloads
with apply members behave more like polymorphic overloads with a
parameter list, e.g given:
def a(x: Any): Any
def a[T <: Int]: T => T
a(1)
def b(x: Any): Any
def b[T <: Int](x: T): T
b(1)
in both cases the second overload will be selected.
|
Updated to use |
The recent Dotty upgrade includes scala/scala3#6756 which fixes overload selection for `bracket_`.
The recent Dotty upgrade includes scala/scala3#6756 which fixes overload selection for `bracket_`.
The recent Dotty upgrade includes scala/scala3#6756 which fixes overload selection for `bracket_`.
The recent Dotty upgrade includes scala/scala3#6756 which fixes overload selection for `bracket_`.
2 participants
When we do overloading resolution, we first replace alternatives that
could not possibly match the expected type by their
applymembers ifthey exist, however the existing logic failed to select
applymembersfrom the result of a polymorphic parameterless def. We fix this by
simply replacing the PolyType by its result type without making up any
type variable, the resulting behavior matches the existing behavior of
Scala 2 on the added test file.