Skip to content

Fix handling of Java varargs #8718

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 4 commits into from
May 25, 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
11 changes: 4 additions & 7 deletions compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -483,13 +483,10 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
case NullTag => emit(asm.Opcodes.ACONST_NULL)

case ClazzTag =>
val toPush: BType = {
toTypeKind(const.typeValue) match {
case kind: PrimitiveBType => boxedClassOfPrimitive(kind)
case kind => kind
}
}
mnode.visitLdcInsn(toPush.toASMType)
val tp = toTypeKind(const.typeValue)
// classOf[Int] is transformed to Integer.TYPE by ClassOf
assert(!tp.isPrimitive, s"expected class type in classOf[T], found primitive type $tp")
mnode.visitLdcInsn(tp.toASMType)

case EnumTag =>
val sym = const.symbolValue
Expand Down
22 changes: 22 additions & 0 deletions compiler/src/dotty/tools/dotc/core/TypeApplications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,28 @@ class TypeApplications(val self: Type) extends AnyVal {
def translateToRepeated(from: ClassSymbol)(using Context): Type =
translateParameterized(from, defn.RepeatedParamClass)

/** Translate `T` by `T & Object` in the situations where an `Array[T]`
* coming from Java would need to be interpreted as an `Array[T & Object]`
* to be erased correctly.
*
* This is necessary because a fully generic Java array erases to an array of Object,
* whereas a fully generic Java array erases to Object to allow primitive arrays
* as subtypeS.
*
* Note: According to
* <http://cr.openjdk.java.net/~briangoetz/valhalla/sov/02-object-model.html>,
* in the future the JVM will consider that:
*
* int[] <: Integer[] <: Object[]
*
* So hopefully our grand-children will not have to deal with this non-sense!
*/
def translateJavaArrayElementType(using Context): Type =
if self.typeSymbol.isAbstractOrParamType && !self.derivesFrom(defn.ObjectClass) then
AndType(self, defn.ObjectType)
else
self

/** If this is an encoding of a (partially) applied type, return its arguments,
* otherwise return Nil.
* Existential types in arguments are returned as TypeBounds instances.
Expand Down
12 changes: 2 additions & 10 deletions compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -395,16 +395,8 @@ class ClassfileParser(
tpe
case ARRAY_TAG =>
while ('0' <= sig(index) && sig(index) <= '9') index += 1
var elemtp = sig2type(tparams, skiptvs)
// make unbounded Array[T] where T is a type variable into Array[T with Object]
// (this is necessary because such arrays have a representation which is incompatible
// with arrays of primitive types.
// NOTE that the comparison to Object only works for abstract types bounded by classes that are strict subclasses of Object
// if the bound is exactly Object, it will have been converted to Any, and the comparison will fail
// see also RestrictJavaArraysMap (when compiling java sources directly)
if (elemtp.typeSymbol.isAbstractOrParamType && !(elemtp.derivesFrom(defn.ObjectClass)))
elemtp = AndType(elemtp, defn.ObjectType)
defn.ArrayOf(elemtp)
val elemtp = sig2type(tparams, skiptvs)
defn.ArrayOf(elemtp.translateJavaArrayElementType)
case '(' =>
// we need a method symbol. given in line 486 by calling getType(methodSym, ..)
val paramtypes = new ListBuffer[Type]()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,22 +67,9 @@ object Scala2Unpickler {
case tp: MethodType =>
val lastArg = tp.paramInfos.last
assert(lastArg isRef defn.ArrayClass)
val elemtp0 :: Nil = lastArg.baseType(defn.ArrayClass).argInfos
val elemtp = elemtp0 match {
case AndType(t1, t2) => // drop intersection with Object for abstract types and parameters in varargs. Erasure can handle them.
if t2.isAnyRef then
t1 match {
case t1: TypeParamRef => t1
case t1: TypeRef if t1.symbol.isAbstractOrParamType => t1
case _ => elemtp0
}
else elemtp0
case _ =>
elemtp0
}
tp.derivedLambdaType(
tp.paramNames,
tp.paramInfos.init :+ defn.RepeatedParamType.appliedTo(elemtp),
tp.paramInfos.init :+ lastArg.translateParameterized(defn.ArrayClass, defn.RepeatedParamClass),
tp.resultType)
case tp: PolyType =>
tp.derivedLambdaType(tp.paramNames, tp.paramInfos, arrayToRepeated(tp.resultType))
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
ref(defn.DottyArraysModule)
.select(nme.seqToArray)
.appliedToType(elemType)
.appliedTo(tree, Literal(Constant(elemClass.typeRef)))
.appliedTo(tree, clsOf(elemClass.typeRef))
}

/** Convert Java array argument to Scala Seq */
Expand Down
5 changes: 0 additions & 5 deletions compiler/src/dotty/tools/dotc/transform/Erasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,6 @@ object Erasure {
assert(!pt.isInstanceOf[SingletonType], pt)
if (pt isRef defn.UnitClass) unbox(tree, pt)
else (tree.tpe.widen, pt) match {
case (JavaArrayType(treeElem), JavaArrayType(ptElem))
if treeElem.widen.isPrimitiveValueType && !ptElem.isPrimitiveValueType =>
// See SI-2386 for one example of when this might be necessary.
cast(ref(defn.runtimeMethodRef(nme.toObjectArray)).appliedTo(tree), pt)

// When casting between two EVTs, we need to check which one underlies the other to determine
// whether u2evt or evt2u should be used.
case (tp1 @ ErasedValueType(tycon1, underlying1), tp2 @ ErasedValueType(tycon2, underlying2)) =>
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/TreeChecker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ class TreeChecker extends Phase with SymTransformer {
|Original tree : ${tree.show}
|After checking: ${tree1.show}
|Why different :
""".stripMargin + core.TypeComparer.explained((tp1 <:< tp2)(_))
""".stripMargin + core.TypeComparer.explained(tp1 <:< tp2)
if (tree.hasType) // it might not be typed because Typer sometimes constructs new untyped trees and resubmits them to typedUnadapted
assert(isSubType(tree1.tpe, tree.typeOpt), divergenceMsg(tree1.tpe, tree.typeOpt))
tree1
Expand Down
9 changes: 9 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1641,6 +1641,15 @@ class Typer extends Namer
else if (tpt1.symbol == defn.orType)
checkedArgs = checkedArgs.mapconserve(arg =>
checkSimpleKinded(checkNoWildcard(arg)))
else if (ctx.compilationUnit.isJava)
if (tpt1.symbol eq defn.ArrayClass) || (tpt1.symbol eq defn.RepeatedParamClass) then
checkedArgs match {
case List(arg) =>
val elemtp = arg.tpe.translateJavaArrayElementType
if (elemtp ne arg.tpe)
checkedArgs = List(TypeTree(elemtp).withSpan(arg.span))
case _ =>
}
assignType(cpy.AppliedTypeTree(tree)(tpt1, checkedArgs), tpt1, checkedArgs)
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/explicit-nulls/run/generic-java-array-src/Test.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ object Test {
// then on the Scala side we'll need to pass a nullable array.
// i.e. with explicit nulls the previously-implicit cast becomes an explicit
// type annotation.
val x = new Array[Int|Null](1)
val x = new Array[Integer|Null](1)
x(0) = 10
println(JA.get(x))

Expand Down
9 changes: 9 additions & 0 deletions tests/neg/i533/JA.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class JA {
public static <T> T get(T[] arr) {
return arr[0];
}

public static <T> T getVarargs(T ...arr) {
return arr[0];
}
}
8 changes: 8 additions & 0 deletions tests/neg/i533/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
object Test {
def main(args: Array[String]): Unit = {
val x = new Array[Int](1)
x(0) = 10
println(JA.get(x)) // error
println(JA.getVarargs(x: _*)) // error
}
}
2 changes: 1 addition & 1 deletion tests/pos/arrays2.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ one warning found

// #2461
object arrays3 {
def apply[X](xs : X*) : java.util.List[X] = java.util.Arrays.asList(xs: _*)
def apply[X <: AnyRef](xs : X*) : java.util.List[X] = java.util.Arrays.asList(xs: _*)
}
6 changes: 5 additions & 1 deletion tests/run/i533/JA.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@ class JA {
public static <T> T get(T[] arr) {
return arr[0];
}
}

public static <T> T getVarargs(T ...arr) {
return arr[0];
}
}
3 changes: 2 additions & 1 deletion tests/run/i533/Test.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
object Test {
def main(args: Array[String]): Unit = {
val x = new Array[Int](1)
val x = new Array[Integer](1)
x(0) = 10
println(JA.get(x))
println(JA.getVarargs(x: _*))
}
}
7 changes: 7 additions & 0 deletions tests/run/java-varargs/A_1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class A_1 {
public static void foo(int... args) {
}

public static <T> void gen(T... args) {
}
}
8 changes: 8 additions & 0 deletions tests/run/java-varargs/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
object Test {
def main(args: Array[String]): Unit = {
A_1.foo(Array(1): _*)
A_1.foo(Seq(1): _*)
A_1.gen(Array(1): _*)
A_1.gen(Seq(1): _*)
}
}