Merge pull request #13718 from dotty-staging/java-varargs-erasure-3

Handle Java varargs `T...` where `T` is a class parameter

Author: smarter

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

@@ -376,18 +376,18 @@ class Definitions {
    *       <T> void meth8(T... args) {}
    *
    *       // B.scala
-   *       meth7(1) // OK
-   *       meth8(1) // OK
+   *       meth7(1) // OK (creates a reference array)
+   *       meth8(1) // OK (creates a primitive array and copies it into a reference array at Erasure)
    *       val ai = Array[Int](1)
-   *       meth7(ai: _*) // OK (will copy the array)
-   *       meth8(ai: _*) // OK (will copy the array)
+   *       meth7(ai: _*) // OK (will copy the array at Erasure)
+   *       meth8(ai: _*) // OK (will copy the array at Erasure)
    *
    *     Java repeated arguments are erased to arrays, so it would be safe to treat
    *     them in the same way: add an `& Object` to the parameter type to disallow
    *     passing primitives, but that would be very inconvenient as it is common to
    *     want to pass a primitive to an Object repeated argument (e.g.
    *     `String.format("foo: %d", 1)`). So instead we type them _without_ adding the
-   *     `& Object` and let `ElimRepeated` take care of doing any necessary adaptation
+   *     `& Object` and let `ElimRepeated` and `Erasure` take care of doing any necessary adaptation
    *     (note that adapting a primitive array to a reference array requires
    *     copying the whole array, so this transformation only preserves semantics
    *     if the callee does not try to mutate the varargs array which is a reasonable

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

@@ -299,7 +299,8 @@ object Types {
       loop(this)
     }
 
-    def isFromJavaObject(using Context): Boolean = typeSymbol eq defn.FromJavaObjectSymbol
+    def isFromJavaObject(using Context): Boolean =
+      isRef(defn.ObjectClass) && (typeSymbol eq defn.FromJavaObjectSymbol)
 
     def containsFromJavaObject(using Context): Boolean = this match
       case tp: OrType => tp.tp1.containsFromJavaObject || tp.tp2.containsFromJavaObject

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

@@ -114,22 +114,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
         if lastIdx >= 0 then
           val last = paramTypes(lastIdx)
           if last.isRepeatedParam then
-            // We need to be careful when handling Java repeated parameters
-            // of the form `Object...` or `T...` where `T` is unbounded:
-            // in both cases, `Object` will have been translated to `FromJavaObject`
-            // to allow passing primitives as repeated arguments, but we can't
-            // pass a primitive array as argument to such a method since the
-            // parameter will be erased to `Object[]`. To handle this correctly we
-            // drop usage of `FromJavaObject` as an element type here, the
-            // tree transformer of this phase is then responsible for handling
-            // mismatches by emitting the correct adaptation (cf `adaptToArray`).
-            // See also the documentation of `FromJavaObjectSymbol`.
-            val last1 =
-              if isJava && last.elemType.isFromJavaObject then
-                defn.ArrayOf(TypeBounds.upper(defn.ObjectType))
-              else
-                last.translateFromRepeated(toArray = isJava)
-            paramTypes.updated(lastIdx, last1)
+            paramTypes.updated(lastIdx, last.translateFromRepeated(toArray = isJava))
           else paramTypes
         else paramTypes
       tp.derivedLambdaType(paramNames, paramTypes1, resultType1)
@@ -144,8 +129,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
         val isJavaDefined = tree.fun.symbol.is(JavaDefined)
         val tpe = arg.expr.tpe
         if isJavaDefined then
-          val pt = tree.fun.tpe.widen.firstParamTypes.last
-          adaptToArray(arg.expr, pt.elemType.bounds.hi)
+          adaptToArray(arg.expr)
         else if tpe.derivesFrom(defn.ArrayClass) then
           arrayToSeq(arg.expr)
         else
@@ -154,58 +138,25 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
     }
     cpy.Apply(tree)(tree.fun, args)
 
-  /** Convert sequence argument to Java array */
-  private def seqToArray(tree: Tree)(using Context): Tree = tree match
+  private def adaptToArray(tree: Tree)(implicit ctx: Context): Tree = tree match
     case SeqLiteral(elems, elemtpt) =>
-      JavaSeqLiteral(elems, elemtpt)
+      JavaSeqLiteral(elems, elemtpt).withSpan(tree.span)
     case _ =>
-      val elemType = tree.tpe.elemType
-      var elemClass = erasure(elemType).classSymbol
-      if defn.NotRuntimeClasses.contains(elemClass) then
-        elemClass = defn.ObjectClass
-      end if
-      ref(defn.DottyArraysModule)
-        .select(nme.seqToArray)
-        .appliedToType(elemType)
-        .appliedTo(tree, clsOf(elemClass.typeRef))
-
-  /** Adapt a Seq or Array tree to be a subtype of `Array[_ <: $elemPt]`.
-   *
-   *  @pre `elemPt` must either be a super type of the argument element type or `Object`.
-   *        The special handling of `Object` is required to deal with the translation
-   *        of generic Java varargs in `elimRepeated`.
-   */
-  private def adaptToArray(tree: Tree, elemPt: Type)(implicit ctx: Context): Tree =
-    val elemTp = tree.tpe.elemType
-    val elemTpMatches = elemTp <:< elemPt
-    val treeIsArray = tree.tpe.derivesFrom(defn.ArrayClass)
-    if elemTpMatches && treeIsArray then
-      tree // No adaptation necessary
-    else tree match
-      case SeqLiteral(elems, elemtpt) =>
-        // By the precondition, we only have mismatches if elemPt is Object, in
-        // that case we use `FromJavaObject` as the element type to allow the
-        // sequence literal to typecheck no matter the types of the elements,
-        // Erasure will take care of any necessary boxing (see documentation
-        // of `FromJavaObjectSymbol` for more information).
-        val adaptedElemTpt = if elemTpMatches then elemtpt else TypeTree(defn.FromJavaObjectType)
-        JavaSeqLiteral(elems, adaptedElemTpt).withSpan(tree.span)
-      case _ =>
-        if treeIsArray then
-          // Convert an Array[T] to an Array[Object]
-          ref(defn.ScalaRuntime_toObjectArray)
-            .appliedTo(tree)
-        else if elemTpMatches then
-          // Convert a Seq[T] to an Array[$elemPt]
-          ref(defn.DottyArraysModule)
-            .select(nme.seqToArray)
-            .appliedToType(elemPt)
-            .appliedTo(tree, clsOf(elemPt))
+      val elemTp = tree.tpe.elemType
+      val adapted =
+        if tree.tpe.derivesFrom(defn.ArrayClass) then
+          tree
         else
-          // Convert a Seq[T] to an Array[Object]
-          ref(defn.ScalaRuntime_toArray)
-            .appliedToType(elemTp)
-            .appliedTo(tree)
+          ref(defn.DottyArraysModule)
+          .select(nme.seqToArray)
+          .appliedToType(elemTp)
+          .appliedTo(tree, clsOf(elemTp))
+      // This seemingly redundant type ascription is needed because the result
+      // type of `adapted` might be erased to `Object`, but we need to keep
+      // the precise result type at erasure for `Erasure.Boxing.cast` to adapt
+      // a primitive array into a reference array if needed.
+      // Test case in tests/run/t1360.scala.
+      Typed(adapted, TypeTree(defn.ArrayOf(elemTp)))
 
   /** Convert an Array into a scala.Seq */
   private def arrayToSeq(tree: Tree)(using Context): Tree =

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

@@ -325,6 +325,13 @@ object Erasure {
       assert(!pt.isInstanceOf[SingletonType], pt)
       if (pt isRef defn.UnitClass) unbox(tree, pt)
       else (tree.tpe.widen, pt) match {
+        // Convert primitive arrays into reference arrays, this path is only
+        // needed to handle repeated arguments, see
+        // `Definitions#FromJavaObjectSymbol` and `ElimRepeated#adaptToArray`.
+        case (JavaArrayType(treeElem), JavaArrayType(ptElem))
+        if treeElem.widen.isPrimitiveValueType && !ptElem.isPrimitiveValueType =>
+          cast(ref(defn.ScalaRuntime_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)) =>

tests/run/java-varargs-3/A.java

@@ -0,0 +1,14 @@
+class A<S> {
+  public void gen(S... args) {
+  }
+
+  public <T extends S> void gen2(S... args) {
+  }
+}
+class B<S extends java.io.Serializable> {
+  public void gen(S... args) {
+  }
+
+  public <T extends S> void gen2(S... args) {
+  }
+}

tests/run/java-varargs-3/Test.scala

@@ -0,0 +1,19 @@
+object Test {
+  def main(args: Array[String]): Unit = {
+    val ai = new A[Int]
+    ai.gen(1)
+    ai.gen2(1)
+    ai.gen(Array(1)*)
+    ai.gen2(Array(1)*)
+    ai.gen(Seq(1)*)
+    ai.gen2(Seq(1)*)
+
+    val b = new B[String]
+    b.gen("")
+    b.gen2("")
+    b.gen(Array("")*)
+    b.gen2(Array("")*)
+    b.gen(Seq("")*)
+    b.gen2(Seq("")*)
+  }
+}