Merge pull request scala#5630 from adriaanm/rebase-5557

[backport] SI-10071 SI-8786 varargs methods

Author: adriaanm

README.md

@@ -134,7 +134,7 @@ codebase and re-compiles too many files, resulting in long build times (check
 [sbt#1104](https://github.com/sbt/sbt/issues/1104) for progress on that front). In the
 meantime you can:
   - Enable "ant mode" in which sbt only re-compiles source files that were modified.
-    Create a file `local.sbt` containing the line `(incOptions in ThisBuild) := (incOptions in ThisBuild).value.withNameHashing(false).withAntStyle(true)`.
+    Create a file `local.sbt` containing the line `antStyle := true`.
     Add an entry `local.sbt` to your `~/.gitignore`.
   - Use IntelliJ IDEA for incremental compiles (see [IDE Setup](#ide-setup) below) - its
     incremental compiler is a bit less conservative, but usually correct.

project/PartestUtil.scala

@@ -29,7 +29,7 @@ object PartestUtil {
     val knownUnaryOptions = List(
       "--pos", "--neg", "--run", "--jvm", "--res", "--ant", "--scalap", "--specialized",
       "--scalacheck", "--instrumented", "--presentation", "--failed", "--update-check",
-      "--show-diff", "--verbose", "--terse", "--debug", "--version", "--self-test", "--help")
+      "--show-diff", "--show-log", "--verbose", "--terse", "--debug", "--version", "--self-test", "--help")
     val srcPathOption = "--srcpath"
     val grepOption = "--grep"
 

src/compiler/scala/tools/nsc/transform/Erasure.scala

@@ -336,7 +336,18 @@ abstract class Erasure extends AddInterfaces
 
         case MethodType(params, restpe) =>
           val buf = new StringBuffer("(")
-          params foreach (p => buf append jsig(p.tpe))
+          params foreach (p => {
+            val tp = p.attachments.get[TypeParamVarargsAttachment] match {
+              case Some(att) =>
+                // For @varargs forwarders, a T* parameter has type Array[Object] in the forwarder
+                // instead of Array[T], as the latter would erase to Object (instead of Array[Object]).
+                // To make the generic signature correct ("[T", not "[Object"), an attachment on the
+                // parameter symbol stores the type T that was replaced by Object.
+                buf.append("["); att.typeParamRef
+              case _         => p.tpe
+            }
+            buf append jsig(tp)
+          })
           buf append ")"
           buf append (if (restpe.typeSymbol == UnitClass || sym0.isConstructor) VOID_TAG.toString else jsig(restpe))
           buf.toString

src/compiler/scala/tools/nsc/transform/UnCurry.scala

@@ -26,6 +26,8 @@ import scala.reflect.internal.util.ListOfNil
  *  - for every repeated Scala parameter `x: T*' --> x: Seq[T].
  *  - for every repeated Java parameter `x: T...' --> x: Array[T], except:
  *    if T is an unbounded abstract type, replace --> x: Array[Object]
+ *  - for every method defining repeated parameters annotated with @varargs, generate
+ *    a synthetic Java-style vararg method
  *  - for every argument list that corresponds to a repeated Scala parameter
  *       (a_1, ..., a_n) => (Seq(a_1, ..., a_n))
  *  - for every argument list that corresponds to a repeated Java parameter
@@ -43,6 +45,8 @@ import scala.reflect.internal.util.ListOfNil
  *          def liftedTry$1 = try { x_i } catch { .. }
  *          meth(x_1, .., liftedTry$1(), .. )
  *        }
+ *  - remove calls to elidable methods and replace their bodies with NOPs when elide-below
+ *    requires it
  */
 /*</export> */
 abstract class UnCurry extends InfoTransform
@@ -593,7 +597,13 @@ abstract class UnCurry extends InfoTransform
               case None    => newRhs
             }
           )
-          addJavaVarargsForwarders(dd, flatdd)
+          // Only class members can reasonably be called from Java due to name mangling.
+          // Additionally, the Uncurry info transformer only adds a forwarder symbol to class members,
+          // since the other symbols are not part of the ClassInfoType (see reflect.internal.transform.UnCurry)
+          if (dd.symbol.owner.isClass)
+            addJavaVarargsForwarders(dd, flatdd)
+          else
+            flatdd
 
         case tree: Try =>
           if (tree.catches exists (cd => !treeInfo.isCatchCase(cd)))
@@ -684,7 +694,7 @@ abstract class UnCurry extends InfoTransform
               // declared type and assign this to a synthetic val. Later, we'll patch
               // the method body to refer to this, rather than the parameter.
               val tempVal: ValDef = {
-                // SI-9442: using the "uncurry-erased" type (the one after the uncurry phase) can lead to incorrect 
+                // SI-9442: using the "uncurry-erased" type (the one after the uncurry phase) can lead to incorrect
                 // tree transformations. For example, compiling:
                 // ```
                 //   def foo(c: Ctx)(l: c.Tree): Unit = {
@@ -713,7 +723,7 @@ abstract class UnCurry extends InfoTransform
                 //    to redo this desugaring manually here
                 // 2. the type needs to be normalized, since `gen.mkCast` checks this (no HK here, just aliases have
                 //    to be expanded before handing the type to `gen.mkAttributedCast`, which calls `gen.mkCast`)
-                val info0 = 
+                val info0 =
                   enteringUncurry(p.symbol.info) match {
                     case DesugaredParameterType(desugaredTpe) =>
                       desugaredTpe
@@ -755,80 +765,59 @@ abstract class UnCurry extends InfoTransform
         if (!hasRepeated) reporter.error(dd.symbol.pos, "A method without repeated parameters cannot be annotated with the `varargs` annotation.")
       }
 
-    /* Called during post transform, after the method argument lists have been flattened.
-     * It looks for the method in the `repeatedParams` map, and generates a Java-style
+    /**
+     * Called during post transform, after the method argument lists have been flattened.
+     * It looks for the forwarder symbol in the symbol attachments and generates a Java-style
      * varargs forwarder.
+     *
+     * @note The Java-style varargs method symbol is generated in the Uncurry info transformer. If the
+     *       symbol can't be found this method reports a warning and carries on.
+     * @see  [[scala.reflect.internal.transform.UnCurry]]
      */
     private def addJavaVarargsForwarders(dd: DefDef, flatdd: DefDef): DefDef = {
       if (!dd.symbol.hasAnnotation(VarargsClass) || !enteringUncurry(mexists(dd.symbol.paramss)(sym => definitions.isRepeatedParamType(sym.tpe))))
         return flatdd
 
-      def toArrayType(tp: Type): Type = {
-        val arg = elementType(SeqClass, tp)
-        // to prevent generation of an `Object` parameter from `Array[T]` parameter later
-        // as this would crash the Java compiler which expects an `Object[]` array for varargs
-        //   e.g.        def foo[T](a: Int, b: T*)
-        //   becomes     def foo[T](a: Int, b: Array[Object])
-        //   instead of  def foo[T](a: Int, b: Array[T]) ===> def foo[T](a: Int, b: Object)
-        arrayType(
-          if (arg.typeSymbol.isTypeParameterOrSkolem) ObjectTpe
-          else arg
-        )
+      val forwSym: Symbol = {
+        currentClass.info // make sure the info is up to date, so the varargs forwarder symbol has been generated
+        flatdd.symbol.attachments.get[VarargsSymbolAttachment] match {
+          case Some(VarargsSymbolAttachment(sym)) => sym
+          case None =>
+            reporter.warning(dd.pos, s"Could not generate Java varargs forwarder for ${flatdd.symbol}. Please file a bug.")
+            return flatdd
+        }
       }
 
-      val theTyper   = typer.atOwner(dd, currentClass)
-      val flatparams = flatdd.symbol.paramss.head
+      val newPs = forwSym.tpe.params
       val isRepeated = enteringUncurry(dd.symbol.info.paramss.flatten.map(sym => definitions.isRepeatedParamType(sym.tpe)))
+      val oldPs = flatdd.symbol.paramss.head
 
-      // create the type
-      val forwformals = map2(flatparams, isRepeated) {
-        case (p, true) => toArrayType(p.tpe)
-        case (p, false)=> p.tpe
-      }
-      val forwresult = dd.symbol.tpe_*.finalResultType
-      val forwformsyms = map2(forwformals, flatparams)((tp, oldparam) =>
-        currentClass.newValueParameter(oldparam.name.toTermName, oldparam.pos).setInfo(tp)
-      )
-      def mono = MethodType(forwformsyms, forwresult)
-      val forwtype = dd.symbol.tpe match {
-        case MethodType(_, _) => mono
-        case PolyType(tps, _) => PolyType(tps, mono)
-      }
-
-      // create the symbol
-      val forwsym = currentClass.newMethod(dd.name.toTermName, dd.pos, VARARGS | SYNTHETIC | flatdd.symbol.flags) setInfo forwtype
-      def forwParams = forwsym.info.paramss.flatten
-
-      // create the tree
-      val forwtree = theTyper.typedPos(dd.pos) {
-        val locals = map3(forwParams, flatparams, isRepeated) {
-          case (_, fp, false)       => null
-          case (argsym, fp, true)   =>
-            Block(Nil,
-              gen.mkCast(
-                gen.mkWrapArray(Ident(argsym), elementType(ArrayClass, argsym.tpe)),
-                seqType(elementType(SeqClass, fp.tpe))
-              )
-            )
-        }
-        val seqargs = map2(locals, forwParams) {
-          case (null, argsym) => Ident(argsym)
-          case (l, _)         => l
-        }
-        val end = if (forwsym.isConstructor) List(UNIT) else Nil
+      val theTyper = typer.atOwner(dd, currentClass)
+      val forwTree = theTyper.typedPos(dd.pos) {
+        val seqArgs = map3(newPs, oldPs, isRepeated)((param, oldParam, isRep) => {
+          if (!isRep) Ident(param)
+          else {
+            val parTp = elementType(ArrayClass, param.tpe)
+            val wrap = gen.mkWrapArray(Ident(param), parTp)
+            param.attachments.get[TypeParamVarargsAttachment] match {
+              case Some(TypeParamVarargsAttachment(tp)) => gen.mkCast(wrap, seqType(tp))
+              case _ => wrap
+            }
+          }
+        })
 
-        DefDef(forwsym, BLOCK(Apply(gen.mkAttributedRef(flatdd.symbol), seqargs) :: end : _*))
+        val forwCall = Apply(gen.mkAttributedRef(flatdd.symbol), seqArgs)
+        DefDef(forwSym, if (forwSym.isConstructor) Block(List(forwCall), UNIT) else forwCall)
       }
 
       // check if the method with that name and those arguments already exists in the template
-      currentClass.info.member(forwsym.name).alternatives.find(s => s != forwsym && s.tpe.matches(forwsym.tpe)) match {
-        case Some(s) => reporter.error(dd.symbol.pos,
-                                   "A method with a varargs annotation produces a forwarder method with the same signature "
-                                   + s.tpe + " as an existing method.")
+      enteringUncurry(currentClass.info.member(forwSym.name).alternatives.find(s => s != forwSym && s.tpe.matches(forwSym.tpe))) match {
+        case Some(s) =>
+          reporter.error(dd.symbol.pos,
+            s"A method with a varargs annotation produces a forwarder method with the same signature ${s.tpe} as an existing method.")
         case None =>
           // enter symbol into scope
-          currentClass.info.decls enter forwsym
-          addNewMember(forwtree)
+          addNewMember(forwTree)
       }
 
       flatdd

src/reflect/scala/reflect/internal/StdAttachments.scala

@@ -58,4 +58,7 @@ trait StdAttachments {
     * error to indicate that the earlier observation was incomplete.
     */
   case object KnownDirectSubclassesCalled extends PlainAttachment
+
+  /** An attachment carrying information between uncurry and erasure */
+  case class TypeParamVarargsAttachment(val typeParamRef: Type)
 }

src/reflect/scala/reflect/internal/transform/UnCurry.scala

@@ -4,13 +4,20 @@ package internal
 package transform
 
 import Flags._
+import scala.collection.mutable
 
 trait UnCurry {
 
   val global: SymbolTable
   import global._
   import definitions._
 
+  /**
+   * The synthetic Java vararg method symbol corresponding to a Scala vararg method
+   * annotated with @varargs.
+   */
+  case class VarargsSymbolAttachment(varargMethod: Symbol)
+
   /** Note: changing tp.normalize to tp.dealias in this method leads to a single
    *  test failure: run/t5688.scala, where instead of the expected output
    *    Vector(ta, tb, tab)
@@ -65,18 +72,74 @@ trait UnCurry {
     def apply(tp0: Type): Type = {
       val tp = expandAlias(tp0)
       tp match {
-        case ClassInfoType(parents, decls, clazz) =>
+        case ClassInfoType(parents, decls, clazz) if !clazz.isJavaDefined =>
           val parents1 = parents mapConserve uncurry
-          if (parents1 eq parents) tp
-          else ClassInfoType(parents1, decls, clazz) // @MAT normalize in decls??
+          val varargOverloads = mutable.ListBuffer.empty[Symbol]
+
+          // Not using `hasAnnotation` here because of dreaded cyclic reference errors:
+          // it may happen that VarargsClass has not been initialized yet and we get here
+          // while processing one of its superclasses (such as java.lang.Object). Since we
+          // don't need the more precise `matches` semantics, we only check the symbol, which
+          // is anyway faster and safer
+          for (decl <- decls if decl.annotations.exists(_.symbol == VarargsClass)) {
+            if (mexists(decl.paramss)(sym => definitions.isRepeatedParamType(sym.tpe))) {
+              varargOverloads += varargForwarderSym(clazz, decl, exitingPhase(phase)(decl.info))
+            }
+          }
+          if ((parents1 eq parents) && varargOverloads.isEmpty) tp
+          else {
+            val newDecls = decls.cloneScope
+            varargOverloads.foreach(newDecls.enter)
+            ClassInfoType(parents1, newDecls, clazz)
+          } // @MAT normalize in decls??
+
         case PolyType(_, _) =>
           mapOver(tp)
+
         case _ =>
           tp
       }
     }
   }
 
+  private def varargForwarderSym(currentClass: Symbol, origSym: Symbol, newInfo: Type): Symbol = {
+    val forwSym = origSym.cloneSymbol(currentClass, VARARGS | SYNTHETIC | origSym.flags & ~DEFERRED, origSym.name.toTermName).withoutAnnotations
+
+    // we are using `origSym.info`, which contains the type *before* the transformation
+    // so we still see repeated parameter types (uncurry replaces them with Seq)
+    val isRepeated = origSym.info.paramss.flatten.map(sym => definitions.isRepeatedParamType(sym.tpe))
+    val oldPs = newInfo.paramss.head
+    def toArrayType(tp: Type, newParam: Symbol): Type = {
+      val arg = elementType(SeqClass, tp)
+      val elem = if (arg.typeSymbol.isTypeParameterOrSkolem && !(arg <:< AnyRefTpe)) {
+        // To prevent generation of an `Object` parameter from `Array[T]` parameter later
+        // as this would crash the Java compiler which expects an `Object[]` array for varargs
+        //   e.g.        def foo[T](a: Int, b: T*)
+        //   becomes     def foo[T](a: Int, b: Array[Object])
+        //   instead of  def foo[T](a: Int, b: Array[T]) ===> def foo[T](a: Int, b: Object)
+        //
+        // In order for the forwarder method to type check we need to insert a cast:
+        //   def foo'[T'](a: Int, b: Array[Object]) = foo[T'](a, wrapRefArray(b).asInstanceOf[Seq[T']])
+        // The target element type for that cast (T') is stored in the  TypeParamVarargsAttachment
+//        val originalArg = arg.substSym(oldTps, tps)
+        // Store the type parameter that was replaced by Object to emit the correct generic signature
+        newParam.updateAttachment(new TypeParamVarargsAttachment(arg))
+        ObjectTpe
+      } else
+        arg
+      arrayType(elem)
+    }
+
+    foreach2(forwSym.paramss.flatten, isRepeated)((p, isRep) =>
+      if (isRep) {
+        p.setInfo(toArrayType(p.info, p))
+      }
+    )
+
+    origSym.updateAttachment(VarargsSymbolAttachment(forwSym))
+    forwSym
+  }
+
   /** - return symbol's transformed type,
    *  - if symbol is a def parameter with transformed type T, return () => T
    *

src/reflect/scala/reflect/runtime/JavaUniverseForce.scala

@@ -42,6 +42,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.SyntheticUnitAttachment
     this.SubpatternsAttachment
     this.KnownDirectSubclassesCalled
+    this.TypeParamVarargsAttachment
     this.noPrint
     this.typeDebug
     this.Range
@@ -444,6 +445,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     definitions.ScalaValueClassesNoUnit
     definitions.ScalaValueClasses
 
+    uncurry.VarargsSymbolAttachment
     uncurry.DesugaredParameterType
     erasure.GenericArray
     erasure.scalaErasure