Skip to content

Fix vararg overloading #9732

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

Merged
merged 1 commit into from
Sep 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 38 additions & 33 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`.
Expand Down
6 changes: 6 additions & 0 deletions tests/neg/overload_repeated.scala
Original file line number Diff line number Diff line change
@@ -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
}
23 changes: 23 additions & 0 deletions tests/run/overload_repeated/A_1.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
30 changes: 30 additions & 0 deletions tests/run/overload_repeated/B_2.scala
Original file line number Diff line number Diff line change
@@ -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
}
}
6 changes: 3 additions & 3 deletions tests/run/t8197.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}