Change treatment of SAMs in overloading resolution

[odersky]

Fixes #11938

Overloading resolution had two stages to determine when an alternative is applicable"

1. Match with subtyping only
2. Match with implicit conversions and SAM conversions

If some alternatives are eligible under (1), only those alternatives are considered.
If no alternatives are eligible under (1), alternatives are searched using (2).

This behavior is different from Scala 2, which seems to use SAM conversions more aggressively.
I am not exactly sure what Scala 2 does. One obvious change would be to allow SAM conversions under (1).
But I am reluctant to do that, since a SAM conversion can easily be introduced by accident.
For instance, we might have two overloaded variants like this:

   def foo(x: C) = ...
   def foo(x: A => B) = ...
   foo((x: A) => x.toB)

Now, if C happens to be a SAM type that has an abstract method from A to B, this would be ambiguous.
Generally, it feels like a SAM conversion could too easily be considered by accident here.

On the other hand, if C was marked with @FunctionalInterface it would explicitly expect function
arguments, so then we should treat it as morally equivalent to a function type in Scala.

This is what this commit does. It refines the priority for searching applicable methods as follows:

1. Match with subtyping and with SAM conversions to functional interfaces
2. Match with implicit conversions and SAM conversions to other types

[odersky]

@lrytz @retronym @adriaanm Can you shed some light what the Scala 2 algorithm is here? Does it also treat @FunctonalInterfaces specially?

[lrytz]

Scala 2 doesn't treat FunctionalInterface specially.

I think this is handled in overloading resolution and type compatibility, which were changed in scala/scala#4971.

[smarter]

I would be wary of special-casing @FunctionalInterface, it means that if one of your dependency adds or remove this annotation from one of their type, the overloads you defined in your own code may suddenly start behaving differently. It also does not match how either Scala 2 or Java do things (if we wanted to match what Java does, we would have to treat FunctionN like any other SAM type, and consider SAM types in the first stage of overloading resolution too: https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.12.2.1)

[odersky]

OK, I have tried to make SAMconversion part of (1). There is one test that fails: tasty-overload-secondargs:

val x1 = X.andThen(1){case x if (x%2 == 0) => x}
| ^^^^^^^^^
| Ambiguous overload. The overloaded alternatives of method andThen in object X with types
| [A, B](a: A)(f: PartialFunction[A, B]): Option[B]
| [A, B](a: A)(f: A => B): B
| both match arguments ((1 : Int))

Now the function type and the partial function type are seen as ambiguous. What do we make of this?

[odersky]

New algorithm: Always consider a function type A as conforming to a corresponding SAM type B, unless
B is a subtype of A.

The unless... part is so that PartialFunction[A, B] is seen as strictly better than A => B., which is relevant to
make run-macros/tasty-overload-secondargs pass. Without the restruction A => B is as good as PartialFunction[A, B] because of SAM conversion and PartialFunction[A, B] is as good as A => B because it is a subtype. Same for every other
SAM type that inherits from a function type.

This is an alternate fix for #11938 that does not treat FunctionalInterfaces specifically.

[odersky]

Turns out the behavior for #11899 is now also the same as Scala 2.13.

For overloading purposes, we only look at the types, so it does not matter whether an argument is a lambda or a named function value.

[smarter]

For overloading purposes, we only look at the types, so it does not matter whether an argument is a lambda or a named function value.

To me, that sounds like a bug since we're dismissing applicable overloads and keeping non-applicable ones. Is the justification for this that it would complicate overloading resolution too much to take tree shape and not just type into account?

[AugustNagro]

Wow, very impressive.

I checked out fix-11938 and so far everything I've tried works great. The Comparator problem I brought up at the end of #11899 works as expected now too.

[odersky]

To me, that sounds like a bug since we're dismissing applicable overloads and keeping non-applicable ones. Is the justification for this that it would complicate overloading resolution too much to take tree shape and not just type into account?

That's one concern. Compatibility with Scala 2 is another. There's also a sort-of justification that the user probably meant the function instead of the fully generic type. So a type error is better than a silent fallback in that case.

[odersky]

Third attempt: Model SAM conversions precisely

Applicability testing as used in overloading resolution now considers
a SAM conversion only if the argument is a lambda (possibly an eta-converted
one). This matches exactly what normal adaptation does.

It turns #11899 into a pos test (now part of i11938.scala). That is Scala 3
will fallback to a generic alternative where Scala 2 failed with a type error.

[odersky]

It looks like whatever we do makes code explode. Overloading resolution is such a highly constrained space.

[odersky]

I have now tried about half a dozen schemes and nothing worked in all cases. The only one that did work was the original special treatment of functional interfaces. So I am afraid it's that or no fix.

[smarter]

If I were to choose I think I would go with the second attempt (the one described in #11945 (comment)) since it seems to match Scala 2 and Java best, and the situations where it would emit an error seem fairly rare.

[odersky]

All the other attempts I tried broke at least zio and scalacheck.

[smarter]

Ah, in that case I would prefer staying with the status quo unless and until we find something better.

[AugustNagro]

@smarter The problem with the status quo is that very basic Java interoperability is broken. Just using Comparator to sort a List doesn't compile in Scala 3, despite working fine in Scala 2.

case class Pair(s: String, i: Int)

val l = new java.util.ArrayList[Pair]()
l.add(Pair("b", 1)); l.add(Pair("a", 5)); l.add(Pair("b", 0))

l.sort(java.util.Comparator.comparing(p => p.s))

I imagine community-build libraries are Scala-centric and do not really exercise Java compat. But it's still a very important thing.

I don't claim to know the best option, but I don't think it's the status quo.

[smarter]

@AugustNagro I just tried your example and it works on master.

[AugustNagro]

@smarter That's great. I can confirm it's working now in RC2 (fails in RC1).

Obviously I would still like the 2nd overload selected here:

import java.util.function.Function

class Future[T](val initial: T) {
  def map[V](v: V): Unit = println(v)
  def map[U](fn: Function[T, U]): Unit = println(fn(initial))
}

val f = new Future(42)
val fn = (i: Int) => i.toString
f.map(fn)

Scala 2 already does, but gets hung up with a type error. I know you're ambivalent about @FunctionalInterface, but I don't see the problem with special casing it. If a library author removes the annotation, presumably the interface is no longer functional and shouldn't be treated that way.

In practice I doubt that anyone who adds this optional marker annotation ever removes it while keeping the interface a SAM.

As a compromise we could restrict @FunctionalInterface to JDK sources, since those certainly will never change. But I don't see any problem with the general approach.

[smarter]

In practice I doubt that anyone who adds this optional marker annotation ever removes it while keeping the interface a SAM.

It's not only removing this annotation but also adding it that will change overloading resolution.

I don't see the problem with special casing it.

We're presumably adding this special case to make our overload resolution closer to Scala 2 and Java, but since it doesn't match how either of them actually function, this will only work in some cases and not others, so we're complicating a fundamental aspect of the language for unclear benefits (we have no idea when it'll help and when it doesn't, since all of this is based on a single example).

As a compromise we could restrict @FunctionalInterface to JDK sources,

And what happens if we find out this isn't good enough for some popular Java API? We can't just keep adding more and more special cases to get every new example we come across to work. The only way out I can see here is to strive to match what https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.12.2 says when possible, but that will never happen if we keep inventing our own special cases instead.

[AugustNagro]

I see the sense in that. Would following the JLS break the community build?

…

On Sat, Apr 3, 2021, 5:49 PM Guillaume Martres ***@***.***> wrote: In practice I doubt that anyone who adds this optional marker annotation ever removes it while keeping the interface a SAM. It's not only removing this annotation but also adding it that will change overloading resolution. I don't see the problem with special casing it. We're presumably adding this special case to make our overload resolution closer to Scala 2 and Java, but since it doesn't match how either of them actually function, this will only work in some cases and not others, so we're complicating a fundamental aspect of the language for unclear benefits (we have no idea when it'll help and when it doesn't, since all of this is based on a single example). As a compromise we could restrict @FunctionalInterface <https://github.com/FunctionalInterface> to JDK sources, And what happens if we find out this isn't good enough for some popular Java API? We can't just keep adding more and more special cases to get every new example we come across to work. The only way out I can see here is to strive to match what https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.12.2 says when possible, but that will never happen if we keep inventing our own special cases instead. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11945 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQOKNWOVXGHD6UM7K57QALTG6ZTDANCNFSM42BS63TQ> .

[smarter]

Attempts 2 and 3 above did break it and are closer to the JLS, but it might be worthwhile to investigate what exactly broke and why.

[odersky]

Ah, in that case I would prefer staying with the status quo unless and until we find something better.

I don't think that's an option. There are very simple examples (such as run/i11938.scala) where Scala 2 and 3 behave differently in overloading resolution. That means we can silently change runtime semantics, and that in very common cases. I would argue this needs to be fixed ASAP.

The JLS is such a mess with respect to overloading that I believe we should not strive to copy it. At least I will not try to make an effort. Is there are writeup how SAMs are handled in Scala 2?

I believe we should take a stand here: SAM conversion is a kind of conversion and therefore should be handled together with implicit conversion. If we change that, we get ambiguous overloads in many situations because A might be a subtype of B and B might be SAM-convertible to A. We have to prioritize one over the other.

To fix the problem with overloading between Java functional interfaces and Any we special treat functional interfaces to be morally the same as functions, as far as overloading resolution is concerned. That would introduce ambiguity only if we had a SAM subclass of a Scala function class that was also marked as a functional interface. Hopefully that does not happen often in the wild. The whole thing is admittedly a hack. But I see nothing that would work better.

[odersky]

#11990 was my best shot for avoiding FunctionalInterface. But it also fails for zio.

[odersky]

Tentatively adding this to 3.0.0 milestone for backporting. Change in behavior from Scala 2 is a serious deviation. We might want to fix this sooner rather than later.

[SethTisue]

Is there are writeup how SAMs are handled in Scala 2?

scala/scala#4971 includes spec changes.

Perhaps scala/scala#5307 is relevant here as well?

[smarter]

Superceded by #12097