Merge pull request #8718 from dotty-staging/java-varargs

Fix handling of Java varargs

Author: odersky

compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala

@@ -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

compiler/src/dotty/tools/dotc/core/TypeApplications.scala

@@ -413,6 +413,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.

compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala

@@ -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]()

compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala

@@ -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))

compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala

@@ -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 */

compiler/src/dotty/tools/dotc/transform/Erasure.scala

@@ -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)) =>

compiler/src/dotty/tools/dotc/transform/TreeChecker.scala

@@ -328,7 +328,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

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

@@ -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)
     }
   }

tests/explicit-nulls/run/generic-java-array-src/Test.scala

@@ -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))
 

tests/neg/i533/JA.java

@@ -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];
+  }
+}

tests/neg/i533/Test.scala

@@ -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
+  }
+}

tests/pos/arrays2.scala

@@ -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: _*)
 }

tests/run/i533/JA.java

@@ -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];
+  }
+}

tests/run/i533/Test.scala

@@ -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: _*))
   }
 }

tests/run/java-varargs/A_1.java

@@ -0,0 +1,7 @@
+class A_1 {
+  public static void foo(int... args) {
+  }
+
+  public static <T> void gen(T... args) {
+  }
+}

tests/run/java-varargs/Test_2.scala

@@ -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): _*)
+  }
+}