Merge pull request #9732 from dotty-staging/fix-java-overload

Fix vararg overloading

Author: odersky

compiler/src/dotty/tools/dotc/typer/Applications.scala

@@ -1452,9 +1452,11 @@ trait Applications extends Compatibility {
     /** Is alternative `alt1` with type `tp1` as specific as alternative
      *  `alt2` with type `tp2` ?
      *
-     *    1. A method `alt1` of type (p1: T1, ..., pn: Tn)U is as specific as `alt2`
-     *       if `alt2` is applicable to arguments (p1, ..., pn) of types T1,...,Tn
-     *       or if `alt1` is nullary.
+     *    1. A method `alt1` of type `(p1: T1, ..., pn: Tn)U` is as specific as `alt2`
+     *       if `alt1` is nullary or `alt2` is applicable to arguments (p1, ..., pn) of
+     *       types T1,...,Tn. If the last parameter `pn` has a vararg type T*, then
+     *       `alt1` must be applicable to arbitrary numbers of `T` parameters (which
+     *       implies that it must be a varargs method as well).
      *    2. A polymorphic member of type [a1 >: L1 <: U1, ..., an >: Ln <: Un]T is as
      *       specific as `alt2` of type `tp2` if T is as specific as `tp2` under the
      *       assumption that for i = 1,...,n each ai is an abstract type name bounded
@@ -1464,36 +1466,39 @@ trait Applications extends Compatibility {
      *       b. as specific as a member of any other type `tp2` if `tp1` is compatible
      *          with `tp2`.
      */
-    def isAsSpecific(alt1: TermRef, tp1: Type, alt2: TermRef, tp2: Type): Boolean = trace(i"isAsSpecific $tp1 $tp2", overload) { tp1 match {
-      case tp1: MethodType => // (1)
-        val formals1 =
-          if (tp1.isVarArgsMethod && tp2.isVarArgsMethod) tp1.paramInfos.map(_.repeatedToSingle)
-          else tp1.paramInfos
-        isApplicableMethodRef(alt2, formals1, WildcardType) ||
-        tp1.paramInfos.isEmpty && tp2.isInstanceOf[LambdaType]
-      case tp1: PolyType => // (2)
-        inContext(ctx.fresh.setExploreTyperState()) {
-          // Fully define the PolyType parameters so that the infos of the
-          // tparams created below never contain TypeRefs whose underling types
-          // contain uninstantiated TypeVars, this could lead to cycles in
-          // `isSubType` as a TypeVar might get constrained by a TypeRef it's
-          // part of.
-          val tp1Params = tp1.newLikeThis(tp1.paramNames, tp1.paramInfos, defn.AnyType)
-          fullyDefinedType(tp1Params, "type parameters of alternative", alt1.symbol.span)
-
-          val tparams = newTypeParams(alt1.symbol, tp1.paramNames, EmptyFlags, tp1.instantiateParamInfos(_))
-          isAsSpecific(alt1, tp1.instantiate(tparams.map(_.typeRef)), alt2, tp2)
-        }
-      case _ => // (3)
-        tp2 match {
-          case tp2: MethodType => true // (3a)
-          case tp2: PolyType if tp2.resultType.isInstanceOf[MethodType] => true // (3a)
-          case tp2: PolyType => // (3b)
-            explore(isAsSpecificValueType(tp1, constrained(tp2).resultType))
-          case _ => // (3b)
-            isAsSpecificValueType(tp1, tp2)
-        }
-    }}
+    def isAsSpecific(alt1: TermRef, tp1: Type, alt2: TermRef, tp2: Type): Boolean = trace(i"isAsSpecific $tp1 $tp2", overload) {
+      tp1 match
+        case tp1: MethodType => // (1)
+          tp1.paramInfos.isEmpty && tp2.isInstanceOf[LambdaType]
+          || {
+            if tp1.isVarArgsMethod
+              tp2.isVarArgsMethod
+              && isApplicableMethodRef(alt2, tp1.paramInfos.map(_.repeatedToSingle), WildcardType)
+            else
+              isApplicableMethodRef(alt2, tp1.paramInfos, WildcardType)
+          }
+        case tp1: PolyType => // (2)
+          inContext(ctx.fresh.setExploreTyperState()) {
+            // Fully define the PolyType parameters so that the infos of the
+            // tparams created below never contain TypeRefs whose underling types
+            // contain uninstantiated TypeVars, this could lead to cycles in
+            // `isSubType` as a TypeVar might get constrained by a TypeRef it's
+            // part of.
+            val tp1Params = tp1.newLikeThis(tp1.paramNames, tp1.paramInfos, defn.AnyType)
+            fullyDefinedType(tp1Params, "type parameters of alternative", alt1.symbol.span)
+
+            val tparams = newTypeParams(alt1.symbol, tp1.paramNames, EmptyFlags, tp1.instantiateParamInfos(_))
+            isAsSpecific(alt1, tp1.instantiate(tparams.map(_.typeRef)), alt2, tp2)
+          }
+        case _ => // (3)
+          tp2 match
+            case tp2: MethodType => true // (3a)
+            case tp2: PolyType if tp2.resultType.isInstanceOf[MethodType] => true // (3a)
+            case tp2: PolyType => // (3b)
+              explore(isAsSpecificValueType(tp1, constrained(tp2).resultType))
+            case _ => // 3b)
+              isAsSpecificValueType(tp1, tp2)
+    }
 
     /** Test whether value type `tp1` is as specific as value type `tp2`.
      *  Let's abbreviate this to `tp1 <:s tp2`.

tests/neg/overload_repeated.scala

@@ -0,0 +1,6 @@
+object Test {
+  def bar1(x: Any) = 1
+  def bar1(x: String*) = 2
+
+  assert(bar1("") == 1) // error: ambiguous in Scala 2 and Scala 3
+}

tests/run/overload_repeated/A_1.java

@@ -0,0 +1,23 @@
+class A_1 {
+  public static int foo1(Object x) { return 1; }
+  public static int foo1(String... x) { return 2; }
+
+  public static int foo2(Object x) { return 1; }
+  public static int foo2(Object... x) { return 2; }
+
+  public static <T> int foo3(T x) { return 1; }
+  public static <T> int foo3(T... x) { return 2; }
+
+  public static <T> int foo4(T x) { return 1; }
+  public static <T> int foo4(T x, T... y) { return 2; }
+
+  public static boolean check() {
+    // Java prefers non-varargs to varargs:
+    // https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.12.2
+    return
+        foo1("") == 1 &&
+        foo2("") == 1 &&
+        foo3("") == 1 &&
+        foo4("") == 1;
+  }
+}

tests/run/overload_repeated/B_2.scala

@@ -0,0 +1,30 @@
+object Test {
+  def bar1(x: Any) = 1
+  def bar1(x: String*) = 2
+
+  def bar2(x: Any) = 1
+  def bar2(x: Any*) = 2
+
+  def bar3[T](x: T): Int = 1
+  def bar3[T](x: T*): Int = 2
+
+  def bar4[T](x: T): Int = 1
+  def bar4[T](x: T, xs: T*): Int = 2
+
+  def main(args: Array[String]): Unit = {
+    // In Java, varargs are always less specific than non-varargs (see
+    // https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.12.2),
+    // this isn't true in Scala which leads to `foo1` being ambiguous.
+    assert(A_1.check())
+    // assert(A_1.foo1("") == 1) // Works in Java, ambiguous in Scala 2 and Dotty
+    assert(A_1.foo2("") == 1) // Same as in Java and Scala 2
+    assert(A_1.foo3("") == 1) // Same as in Java and Scala 2
+    assert(A_1.foo4("") == 1) // Same as in Java and Scala 2
+
+    // Same with Scala varargs:
+    // assert(bar1("") == 1) // Works in Java, ambiguous in Scala 2 and Dotty
+    assert(bar2("") == 1) // same in Scala 2
+    assert(bar3("") == 1) // same in Scala 2
+    assert(bar4("") == 1) // same in Scala 2
+  }
+}

tests/run/t8197.scala

@@ -9,8 +9,8 @@ class Foo(val x: A = null) {
 }
 
 object Test extends App {
-  // both constructors of `Foo` are applicable. Overloading resolution
-  // will eliminate the alternative that uses a default argument, therefore
-  // the vararg constructor is chosen.
+  // both constructors of `Foo` are applicable and neither is more specific
+  // than the other. As a fallback, overloading resolution will eliminate the
+  // alternative that uses a default argument, therefore the vararg constructor is chosen.
   assert((new Foo).x != null)
 }