Check parent methods for varargs annotation

Author: TheElectronWill

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

@@ -46,7 +46,9 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
 
   private def overridesJava(sym: Symbol)(using Context) = sym.allOverriddenSymbols.exists(_.is(JavaDefined))
 
-  private def hasVargsAnnotation(sym: Symbol)(using ctx: Context) = sym.hasAnnotation(defn.VarargsAnnot)
+  private def hasVarargsAnnotation(sym: Symbol)(using Context) = sym.hasAnnotation(defn.VarargsAnnot)
+
+  private def parentHasAnnotation(sym: Symbol)(using Context) = sym.allOverriddenSymbols.exists(hasVarargsAnnotation)
 
   /** Eliminate repeated parameters from method types. */
   private def elimRepeated(tp: Type)(using Context): Type = tp.stripTypeVar match
@@ -114,8 +116,9 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
    */
   override def transformDefDef(tree: DefDef)(using ctx: Context): Tree =
     ctx.atPhase(thisPhase) {
-      val isOverride = overridesJava(tree.symbol)
-      val hasAnnotation = hasVargsAnnotation(tree.symbol)
+      val sym = tree.symbol
+      val isOverride = overridesJava(sym)
+      val hasAnnotation = hasVarargsAnnotation(sym) || parentHasAnnotation(sym)
       if tree.symbol.info.isVarArgsMethod && (isOverride || hasAnnotation) then
         // non-overrides need the varargs bytecode flag and cannot be synthetic
         // otherwise javac refuses to call them.
@@ -125,7 +128,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
     }
 
   /** Add a Java varargs bridge
-   *  @param  ddef   the original method definition
+   *  @param ddef    the original method definition
    *  @param addFlag the flag to add to the method symbol
 
    *  @return a thicket consisting of `ddef` and a varargs bridge method
@@ -141,6 +144,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
   private def addVarArgsBridge(ddef: DefDef, synthetic: Boolean)(using Context): Tree =
     val original = ddef.symbol.asTerm
     // For simplicity we always set the varargs flag
+    // although it's not strictly necessary for overrides
     val flags = ddef.symbol.flags | JavaVarargs &~ Private
     val bridge = original.copy(
         flags = if synthetic then flags | Artifact else flags,
@@ -162,11 +166,26 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
 
   /** Convert type from Scala to Java varargs method */
   private def toJavaVarArgs(tp: Type)(using Context): Type = tp match
-    case tp: PolyType =>
-      tp.derivedLambdaType(tp.paramNames, tp.paramInfos, toJavaVarArgs(tp.resultType))
-    case tp: MethodType =>
-      val inits :+ last = tp.paramInfos
-      val last1 = last.translateFromRepeated(toArray = true)
-      tp.derivedLambdaType(tp.paramNames, inits :+ last1, tp.resultType)
+      case tp: PolyType =>
+        tp.derivedLambdaType(tp.paramNames, tp.paramInfos, toJavaVarArgs(tp.resultType))
+      case tp: MethodType =>
+        val inits :+ last = tp.paramInfos
+        val vararg = varargArrayType(last)
+        val params = inits :+ vararg
+        tp.derivedLambdaType(tp.paramNames, params, tp.resultType)
+
+  /** Translate a repeated type T* to an `Array[? <: Upper]`
+   *  such that it is compatible with java varargs.
+   *
+   *  If T is not a primitive type, we set `Upper = T & AnyRef`
+   *  to prevent the erasure of `Array[? <: Upper]` to Object,
+   *  which would break the varargs from Java.
+   */
+  private def varargArrayType(tp: Type)(using Context): Type =
+    val array = tp.translateFromRepeated(toArray = true)
+    val element = array.elemType.typeSymbol
+
+    if element.isPrimitiveValueClass then array
+    else defn.ArrayOf(TypeBounds.upper(AndType(element.typeRef, defn.AnyRefType)))
 
 }

compiler/test/dotc/run-test-pickling.blacklist

@@ -27,3 +27,4 @@ enum-java
 zero-arity-case-class.scala
 tuple-ops.scala
 i7212
+varargs-abstract

tests/run/varargs-abstract/Test.java

@@ -0,0 +1,19 @@
+import java.util.Comparator;
+
+public class Test {
+  public static void main(String[] args) {
+    ClassImplementsClass c = new ClassImplementsClass();
+
+    c.x("a", "b", "c");
+    c.y("a", "b", "c");
+    c.z("a", "b", "c");
+
+    VarargAbstractClass i = new ClassImplementsClass();
+
+    i.x("a", "b", "c");
+    i.y("a", "b", "c");
+    // i.z("a", "b", "c");
+    // ClassCastException at runtime because the generated
+    // signature of z doesn't mention the type parameter (it should)
+  }
+}

tests/run/varargs-abstract/test.scala

@@ -0,0 +1,18 @@
+import scala.annotation.varargs
+
+abstract class VarargAbstractClass[T] {
+  @varargs
+  def x(els: String*): Int
+
+  @varargs
+  def y(els: String*): Int
+
+  @varargs
+  def z(els: T*): Int
+}
+class ClassImplementsClass extends VarargAbstractClass[String] {
+
+  override def x(els: String*): Int = els.length
+  override def y(els: String*): Int = els.length
+  override def z(els: String*): Int = els.length
+}