Merge pull request #9271 from TheElectronWill/varargs-annotation

Author: smarter

.gitmodules

@@ -67,7 +67,7 @@
 	url = https://github.com/dotty-staging/upickle.git
 [submodule "community-build/community-projects/sconfig"]
 	path = community-build/community-projects/sconfig
-	url = https://github.com/ekrich/sconfig.git
+	url = https://github.com/dotty-staging/sconfig.git
 [submodule "community-build/community-projects/zio"]
 	path = community-build/community-projects/zio
 	url = https://github.com/dotty-staging/zio.git

community-build/community-projects/sconfig

@@ -829,6 +829,7 @@ class Definitions {
   @tu lazy val FunctionalInterfaceAnnot: ClassSymbol = ctx.requiredClass("java.lang.FunctionalInterface")
   @tu lazy val InfixAnnot: ClassSymbol = ctx.requiredClass("scala.annotation.infix")
   @tu lazy val AlphaAnnot: ClassSymbol = ctx.requiredClass("scala.annotation.alpha")
+  @tu lazy val VarargsAnnot: ClassSymbol = ctx.requiredClass("scala.annotation.varargs")
 
   // A list of annotations that are commonly used to indicate that a field/method argument or return
   // type is not null. These annotations are used by the nullification logic in JavaNullInterop to

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

@@ -28,138 +28,221 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
 
   override def phaseName: String = ElimRepeated.name
 
-  override def changesMembers: Boolean = true // the phase adds vararg bridges
+  override def changesMembers: Boolean = true // the phase adds vararg forwarders
 
   def transformInfo(tp: Type, sym: Symbol)(using Context): Type =
     elimRepeated(tp)
 
   override def transform(ref: SingleDenotation)(using Context): SingleDenotation =
-    super.transform(ref) match {
+    super.transform(ref) match
       case ref1: SymDenotation if (ref1 ne ref) && overridesJava(ref1.symbol) =>
         // This method won't override the corresponding Java method at the end of this phase,
-        // only the bridge added by `addVarArgsBridge` will.
+        // only the forwarder added by `addVarArgsForwarder` will.
         ref1.copySymDenotation(initFlags = ref1.flags &~ Override)
       case ref1 =>
         ref1
-    }
 
   override def mayChange(sym: Symbol)(using Context): Boolean = sym.is(Method)
 
   private def overridesJava(sym: Symbol)(using Context) = sym.allOverriddenSymbols.exists(_.is(JavaDefined))
 
-  private def elimRepeated(tp: Type)(using Context): Type = tp.stripTypeVar match {
+  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
     case tp @ MethodTpe(paramNames, paramTypes, resultType) =>
       val resultType1 = elimRepeated(resultType)
       val paramTypes1 =
-        if (paramTypes.nonEmpty && paramTypes.last.isRepeatedParam) {
+        if paramTypes.nonEmpty && paramTypes.last.isRepeatedParam then
           val last = paramTypes.last.translateFromRepeated(toArray = tp.isJavaMethod)
           paramTypes.init :+ last
-        }
         else paramTypes
       tp.derivedLambdaType(paramNames, paramTypes1, resultType1)
     case tp: PolyType =>
       tp.derivedLambdaType(tp.paramNames, tp.paramInfos, elimRepeated(tp.resultType))
     case tp =>
       tp
-  }
 
   def transformTypeOfTree(tree: Tree)(using Context): Tree =
     tree.withType(elimRepeated(tree.tpe))
 
+  override def transformTypeApply(tree: TypeApply)(using Context): Tree =
+    transformTypeOfTree(tree)
+
   override def transformIdent(tree: Ident)(using Context): Tree =
     transformTypeOfTree(tree)
 
   override def transformSelect(tree: Select)(using Context): Tree =
     transformTypeOfTree(tree)
 
-  override def transformApply(tree: Apply)(using Context): Tree = {
+  override def transformApply(tree: Apply)(using Context): Tree =
     val args = tree.args.mapConserve {
       case arg: Typed if isWildcardStarArg(arg) =>
         val isJavaDefined = tree.fun.symbol.is(JavaDefined)
         val tpe = arg.expr.tpe
-        if (isJavaDefined && tpe.derivesFrom(defn.SeqClass))
+        if isJavaDefined && tpe.derivesFrom(defn.SeqClass) then
           seqToArray(arg.expr)
-        else if (!isJavaDefined && tpe.derivesFrom(defn.ArrayClass))
+        else if !isJavaDefined && tpe.derivesFrom(defn.ArrayClass)
           arrayToSeq(arg.expr)
         else
           arg.expr
       case arg => arg
     }
     transformTypeOfTree(cpy.Apply(tree)(tree.fun, args))
-  }
 
   /** Convert sequence argument to Java array */
-  private def seqToArray(tree: Tree)(using Context): Tree = tree match {
+  private def seqToArray(tree: Tree)(using Context): Tree = tree match
     case SeqLiteral(elems, elemtpt) =>
       JavaSeqLiteral(elems, elemtpt)
     case _ =>
       val elemType = tree.tpe.elemType
       var elemClass = erasure(elemType).classSymbol
-      if (defn.NotRuntimeClasses.contains(elemClass)) elemClass = defn.ObjectClass
+      if defn.NotRuntimeClasses.contains(elemClass) then
+        elemClass = defn.ObjectClass
+      end if
       ref(defn.DottyArraysModule)
         .select(nme.seqToArray)
         .appliedToType(elemType)
         .appliedTo(tree, clsOf(elemClass.typeRef))
-  }
 
   /** Convert Java array argument to Scala Seq */
   private def arrayToSeq(tree: Tree)(using Context): Tree =
     tpd.wrapArray(tree, tree.tpe.elemType)
 
-  override def transformTypeApply(tree: TypeApply)(using Context): Tree =
-    transformTypeOfTree(tree)
-
-  /** If method overrides a Java varargs method, add a varargs bridge.
-   *  Also transform trees inside method annotation
+  /** If method overrides a Java varargs method or is annotated with @varargs, add a varargs bridge.
+   *  Also transform trees inside method annotation.
    */
   override def transformDefDef(tree: DefDef)(using Context): Tree =
-    atPhase(thisPhase) {
-      if (tree.symbol.info.isVarArgsMethod && overridesJava(tree.symbol))
-        addVarArgsBridge(tree)
+    val sym = tree.symbol
+    val hasAnnotation = hasVarargsAnnotation(sym)
+
+    // atPhase(thisPhase) is used where necessary to see the repeated
+    // parameters before their elimination
+    val hasRepeatedParam = atPhase(thisPhase)(hasRepeatedParams(sym))
+    if hasRepeatedParam then
+      val isOverride = atPhase(thisPhase)(overridesJava(sym))
+      if isOverride || hasAnnotation || parentHasAnnotation(sym) then
+        // java varargs are more restrictive than scala's
+        // see https://github.com/scala/bug/issues/11714
+        val validJava = atPhase(thisPhase)(isValidJavaVarArgs(sym.info))
+        if !validJava then
+          ctx.error("""To generate java-compatible varargs:
+                    |  - there must be a single repeated parameter
+                    |  - it must be the last argument in the last parameter list
+                    |""".stripMargin,
+            sym.sourcePos)
+          tree
+        else
+          // non-overrides cannot be synthetic otherwise javac refuses to call them
+          addVarArgsForwarder(tree, isBridge = isOverride)
       else
         tree
-    }
+    else
+      if hasAnnotation then
+        ctx.error("A method without repeated parameters cannot be annotated with @varargs", sym.sourcePos)
+      tree
+
+  /** Is there a repeated parameter in some parameter list? */
+  private def hasRepeatedParams(sym: Symbol)(using Context): Boolean =
+    sym.info.paramInfoss.flatten.exists(_.isRepeatedParam)
 
-  /** Add a Java varargs bridge
-   *  @param  ddef  the original method definition which is assumed to override
-   *                a Java varargs method JM up to this phase.
-   *  @return  a thicket consisting of `ddef` and a varargs bridge method
-   *           which overrides the Java varargs method JM from this phase on
-   *           and forwards to `ddef`.
+  /** Is this the type of a method that has a repeated parameter type as
+   *  its last parameter in the last parameter list?
+   */
+  private def isValidJavaVarArgs(tp: Type)(using Context): Boolean = tp match
+    case mt: MethodType =>
+      val initp :+ lastp = mt.paramInfoss
+      initp.forall(_.forall(!_.isRepeatedParam)) &&
+      lastp.nonEmpty &&
+      lastp.init.forall(!_.isRepeatedParam) &&
+      lastp.last.isRepeatedParam
+    case pt: PolyType =>
+      isValidJavaVarArgs(pt.resultType)
+    case _ =>
+      throw new Exception("Match error in @varargs checks. This should not happen, please open an issue " + tp)
+
+
+  /** Add a Java varargs forwarder
+   *  @param ddef     the original method definition
+   *  @param isBridge true if we are generating a "bridge" (synthetic override forwarder)
+   *
+   *  @return a thicket consisting of `ddef` and an additional method
+   *          that forwards java varargs to `ddef`. It retains all the
+   *          flags of `ddef` except `Private`.
    *
-   *  A bridge is necessary because the following hold
+   *  A forwarder is necessary because the following hold:
    *    - the varargs in `ddef` will change from `RepeatedParam[T]` to `Seq[T]` after this phase
-   *    - _but_ the callers of `ddef` expect its varargs to be changed to `Array[? <: T]`, since it overrides
-   *      a Java varargs
-   *  The solution is to add a "bridge" method that converts its argument from `Array[? <: T]` to `Seq[T]` and
+   *    - _but_ the callers of `ddef` expect its varargs to be changed to `Array[? <: T]`
+   *  The solution is to add a method that converts its argument from `Array[? <: T]` to `Seq[T]` and
    *  forwards it to `ddef`.
    */
-  private def addVarArgsBridge(ddef: DefDef)(using Context): Tree = {
-    val original = ddef.symbol.asTerm
-    val bridge = original.copy(
-      flags = ddef.symbol.flags &~ Private | Artifact,
-      info = toJavaVarArgs(ddef.symbol.info)).enteredAfter(thisPhase).asTerm
-    val bridgeDef = polyDefDef(bridge, trefs => vrefss => {
-      val (vrefs :+ varArgRef) :: vrefss1 = vrefss
-      // Can't call `.argTypes` here because the underlying array type is of the
-      // form `Array[? <: SomeType]`, so we need `.argInfos` to get the `TypeBounds`.
-      val elemtp = varArgRef.tpe.widen.argInfos.head
-      ref(original.termRef)
-        .appliedToTypes(trefs)
-        .appliedToArgs(vrefs :+ tpd.wrapArray(varArgRef, elemtp))
-        .appliedToArgss(vrefss1)
-    })
-
-    Thicket(ddef, bridgeDef)
-  }
+  private def addVarArgsForwarder(ddef: DefDef, isBridge: Boolean)(using ctx: Context): Tree =
+    val original = ddef.symbol
+
+    // The java-compatible forwarder symbol
+    val sym = atPhase(thisPhase) {
+      // Capture the flags before they get modified by #transform.
+      // For simplicity we always set the varargs flag,
+      // although it's not strictly necessary for overrides.
+      val flags = original.flags | JavaVarargs
+      original.copy(
+        flags = if isBridge then flags | Artifact else flags,
+        info = toJavaVarArgs(ddef.symbol.info)
+      ).asTerm
+    }
+
+    // Find a method that would conflict with the forwarder if the latter existed.
+    // This needs to be done at thisPhase so that parent @varargs don't conflict.
+    val conflict = atPhase(thisPhase) {
+      currentClass.info.member(sym.name).alternatives.find { s =>
+        s.matches(sym) &&
+        !(isBridge && s.asSymDenotation.is(JavaDefined))
+      }
+    }
+
+    conflict match
+      case Some(conflict) =>
+        ctx.error(s"@varargs produces a forwarder method that conflicts with ${conflict.showDcl}", original.sourcePos)
+        ddef
+      case None =>
+        val bridgeDef = polyDefDef(sym.enteredAfter(thisPhase), trefs => vrefss => {
+          val init :+ (last :+ vararg) = vrefss
+          // Can't call `.argTypes` here because the underlying array type is of the
+          // form `Array[? <: SomeType]`, so we need `.argInfos` to get the `TypeBounds`.
+          val elemtp = vararg.tpe.widen.argInfos.head
+          ref(original.termRef)
+            .appliedToTypes(trefs)
+            .appliedToArgss(init)
+            .appliedToArgs(last :+ tpd.wrapArray(vararg, elemtp))
+          })
+        Thicket(ddef, bridgeDef)
 
   /** Convert type from Scala to Java varargs method */
-  private def toJavaVarArgs(tp: Type)(using Context): Type = tp match {
+  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)
-  }
+      tp.resultType match
+        case m: MethodType => // multiple param lists
+          tp.derivedLambdaType(tp.paramNames, tp.paramInfos, toJavaVarArgs(m))
+        case _ =>
+          val init :+ last = tp.paramInfos
+          val vararg = varargArrayType(last)
+          tp.derivedLambdaType(tp.paramNames, init :+ vararg, tp.resultType)
+
+  /** Translate a repeated type T* to an `Array[? <: Upper]`
+   *  such that it is compatible with java varargs.
+   *
+   *  When necessary 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) // Array[? <: T]
+    val element = array.elemType.hiBound // T
+
+    if element <:< defn.AnyRefType || element.typeSymbol.isPrimitiveValueClass then array
+    else defn.ArrayOf(TypeBounds.upper(AndType(element, defn.AnyRefType))) // Array[? <: T & AnyRef]
 }

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

@@ -186,10 +186,11 @@ object GenericSignatures {
         case defn.ArrayOf(elemtp) =>
           if (isUnboundedGeneric(elemtp))
             jsig(defn.ObjectType)
-          else {
+          else
             builder.append(ClassfileConstants.ARRAY_TAG)
-            jsig(elemtp)
-          }
+            elemtp match
+              case TypeBounds(lo, hi) => jsig(hi.widenDealias)
+              case _ => jsig(elemtp)
 
         case RefOrAppliedType(sym, pre, args) =>
           if (sym == defn.PairClass && tp.tupleArity > Definitions.MaxTupleArity)
@@ -313,8 +314,8 @@ object GenericSignatures {
     *  - If the list contains one or more occurrences of scala.Array with
     *    type parameters El1, El2, ... then the dominator is scala.Array with
     *    type parameter of intersectionDominator(List(El1, El2, ...)).           <--- @PP: not yet in spec.
-    *  - Otherwise, the list is reduced to a subsequence containing only types
-    *    which are not subtypes of other listed types (the span.)
+    *  - Otherwise, the list is reduced to a subsequence containing only the
+    *    types that are not supertypes of other listed types (the span.)
     *  - If the span is empty, the dominator is Object.
     *  - If the span contains a class Tc which is not a trait and which is
     *    not Object, the dominator is Tc.                                        <--- @PP: "which is not Object" not in spec.

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

@@ -26,3 +26,5 @@ i7868.scala
 enum-java
 zero-arity-case-class.scala
 tuple-ops.scala
+i7212
+varargs-abstract

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

@@ -0,0 +1,28 @@
+import annotation.varargs
+
+// Failing varargs annotation
+object Test {
+
+  trait A {
+    def v1(a: Int, b: Array[String]) = a
+  }
+
+  trait B extends A {
+    @varargs def v1(a: Int, b: String*) = a + b.length // error
+  }
+
+  @varargs def nov(a: Int) = 0 // error: A method without repeated parameters cannot be annotated with @varargs
+  @varargs def v(a: Int, b: String*) = a + b.length // ok
+  def v(a: Int, b: String) = a // ok
+
+  @varargs def v2(a: Int, b: String*) = 0 // error
+  def v2(a: Int, b: Array[String]) = 0
+
+  @varargs def v3(a: String*)(b: Int) = b + a.length // error
+  @varargs def v4(a: String)(b: Int) = b + a.length // error
+  @varargs def v5(a: String)(b: Int*) = a + b.sum // ok
+
+  @varargs def v6: Int = 1 // error
+  @varargs def v7(i: Int*)() = i.sum // error
+
+}

tests/neg/varargs-annot.scala

@@ -0,0 +1,18 @@
+import scala.annotation.varargs
+
+object VarArgs {
+   @varargs
+   def two(a: Int)(b: String*): Nothing = ???
+
+   @varargs
+   def twoPrimitive(a: Int)(b: Int*): Nothing = ???
+
+   @varargs
+   def three(a3: Int)(b3: String)(c3: String*): Nothing = ???
+
+   @varargs
+   def threePrimitive(a3: Int)(b3: String)(c3: Int*): Nothing = ???
+
+   @varargs
+   def emptyOk()()(xs: String*): Nothing = ???
+}