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

Different overloading rules in Scala 2 and 3 for SAMs #11938

Closed
odersky opened this issue Mar 29, 2021 · 2 comments · Fixed by #12097
Closed

Different overloading rules in Scala 2 and 3 for SAMs #11938

odersky opened this issue Mar 29, 2021 · 2 comments · Fixed by #12097

Comments

@odersky
Copy link
Contributor

odersky commented Mar 29, 2021

Related to #11899:

import java.util.function.Function

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

object Test {
  val f = new Future(42)
  val fn = (i: Int) => i.toString
  def main(args: Array[String]): Unit =
    f.map((i: Int) => i.toString)
}

If I compile and run this code with one version of map I get 42. But if I uncomment the first generic overload of map I get Test$$$Lambda$2/1705736037@12bc6874 in Scala 3. Scala 2 still prints 42.

The difference seems to be that SAM conversion is considered only in second rank for eligibility of overloading resolution. So Scala 3 picks the first overload, but Scala 2 picks the second. The problem in #11899 seems to be that Scala-2 thought it could do a SAM conversion when doing overloading resolution but ran into a type error later.

Interestingly, if the second overload of map for String is uncommented, but the first is left commented, both Scala 2 and 3 print 42. So Scala 3 does figure out it can do a SAM conversion if no other alternative is applicable.

@odersky
Copy link
Contributor Author

odersky commented Mar 29, 2021

Here's the comment that backs my analysis:

  /** This private version of `resolveOverloaded` does the bulk of the work of
   *  overloading resolution, but does neither result adaptation nor apply insertion.
   *  It might be called twice from the public `resolveOverloaded` method, once with
   *  implicits and SAM conversions enabled, and once without.
   */
  private def resolveOverloaded1(alts: List[TermRef], pt: Type)(using Context): List[TermRef] = ...

@odersky
Copy link
Contributor Author

odersky commented Mar 29, 2021

In fact Applications.scala shows several comments and examples referring to SAM types (just grep for SAM). It shows that there are difficult tradeoffs to make.

odersky added a commit to dotty-staging/dotty that referenced this issue Mar 30, 2021
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`.

This is an alternate fix for scala#11938 that does not treat FunctionalInterfaces specifically.
@odersky odersky assigned smarter and unassigned odersky and smarter Mar 30, 2021
odersky added a commit to dotty-staging/dotty that referenced this issue Apr 1, 2021
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`.

This is an alternate fix for scala#11938 that does not treat FunctionalInterfaces specifically.
smarter added a commit to dotty-staging/dotty that referenced this issue Apr 15, 2021
`resolveOverloaded` is called twice, first with implicit conversions
off, then on. Before this commit, turning off implicit conversions also
turned off SAM conversions, this behavior does not match Java or Scala 2
which means we could end up picking a different overload than they
do (cf scala#11938).

This commit enables SAM conversions in the first pass, _except_ for
conversions to PartialFunction as that would break a lot of existing
code and wouldn't match how either Java or Scala 2 pick overloads.

This is an alternative to scala#11945 (which special-cased
SAM types with an explicit `@FunctionalInterfaces` annotation) and scala#11990
(which special-cased SAM types that subtype a scala Function type).
Special-casing PartialFunction instead seems more defensible since it's
already special-cased in Scala 2 and is not considered a SAM by Java.

Fixes scala#11938.
smarter added a commit to dotty-staging/dotty that referenced this issue Apr 15, 2021
`resolveOverloaded` is called twice, first with implicit conversions
off, then on. Before this commit, turning off implicit conversions also
turned off SAM conversions, this behavior does not match Java or Scala 2
which means we could end up picking a different overload than they
do (cf scala#11938).

This commit enables SAM conversions in the first pass, _except_ for
conversions to PartialFunction as that would break a lot of existing
code and wouldn't match how either Java or Scala 2 pick overloads.

This is an alternative to scala#11945 (which special-cased
SAM types with an explicit `@FunctionalInterfaces` annotation) and scala#11990
(which special-cased SAM types that subtype a scala Function type).
Special-casing PartialFunction instead seems more defensible since it's
already special-cased in Scala 2 and is not considered a SAM type by
Java.

Fixes scala#11938.
smarter added a commit to dotty-staging/dotty that referenced this issue Apr 15, 2021
`resolveOverloaded` is called twice, first with implicit conversions
off, then on. Before this commit, turning off implicit conversions also
turned off SAM conversions, this behavior does not match Java or Scala 2
which means we could end up picking a different overload than they
do (cf scala#11938).

This commit enables SAM conversions in the first pass, _except_ for
conversions to PartialFunction as that would break a lot of existing
code and wouldn't match how either Java or Scala 2 pick overloads.

This is an alternative to scala#11945 (which special-cased
SAM types with an explicit `@FunctionalInterfaces` annotation) and scala#11990
(which special-cased SAM types that subtype a scala Function type).
Special-casing PartialFunction instead seems more defensible since it's
already special-cased in Scala 2 and is not considered a SAM type by
Java.

Fixes scala#11938.
smarter added a commit to dotty-staging/dotty that referenced this issue Apr 17, 2021
`resolveOverloaded` is called twice, first with implicit conversions
off, then on. Before this commit, turning off implicit conversions also
turned off SAM conversions, this behavior does not match Java or Scala 2
which means we could end up picking a different overload than they
do (cf scala#11938).

This commit enables SAM conversions in the first pass, _except_ for
conversions to PartialFunction as that would break a lot of existing
code and wouldn't match how either Java or Scala 2 pick overloads.

This is an alternative to scala#11945 (which special-cased
SAM types with an explicit `@FunctionalInterfaces` annotation) and scala#11990
(which special-cased SAM types that subtype a scala Function type).
Special-casing PartialFunction instead seems more defensible since it's
already special-cased in Scala 2 and is not considered a SAM type by
Java.

Fixes scala#11938.
michelou pushed a commit to michelou/dotty that referenced this issue Apr 18, 2021
`resolveOverloaded` is called twice, first with implicit conversions
off, then on. Before this commit, turning off implicit conversions also
turned off SAM conversions, this behavior does not match Java or Scala 2
which means we could end up picking a different overload than they
do (cf scala#11938).

This commit enables SAM conversions in the first pass, _except_ for
conversions to PartialFunction as that would break a lot of existing
code and wouldn't match how either Java or Scala 2 pick overloads.

This is an alternative to scala#11945 (which special-cased
SAM types with an explicit `@FunctionalInterfaces` annotation) and scala#11990
(which special-cased SAM types that subtype a scala Function type).
Special-casing PartialFunction instead seems more defensible since it's
already special-cased in Scala 2 and is not considered a SAM type by
Java.

Fixes scala#11938.
@Kordyjan Kordyjan added this to the 3.0.1 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment