SI-8786 fix generic signature for @varargs forwarder methods

When generating a varargs forwarder for

    def foo[T](a: T*)

the parameter type of the forwarder needs to be Array[Object]. If we
gnerate Array[T] in UnCurry, that would be erased to plain Object, and
the method would not be a valid varargs.

Unfortunately, setting the parameter type to Array[Object] lead to
an invalid generic signature - the generic signature should reflect the
real signature.

This change adds an attachment to the parameter symbol in the varargs
forwarder method and special-cases signature generation.

Also cleanes up the code to produce the varargs forwarder. For example,
type parameter and parameter symbols in the forwarder's method type were
not clones, but the same symbols from the original method were re-used.

10 files changed, 237 insertions, 92 deletions

diff --git a/README.md b/README.md
index 6ebb453176..ed42eadaaa 100644
--- a/README.md
+++ b/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.
diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala
index db8e203c1c..6678b565d5 100644
--- a/src/compiler/scala/tools/nsc/transform/Erasure.scala
+++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -343,7 +343,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
@@ -1227,4 +1238,5 @@ abstract class Erasure extends AddInterfaces
   }
 
   private class TypeRefAttachment(val tpe: TypeRef)
+  class TypeParamVarargsAttachment(val typeParamRef: Type)
 }
diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
index 374e8430d8..3047b8f89a 100644
--- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala
+++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
@@ -744,72 +744,88 @@ abstract class UnCurry extends InfoTransform
       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 = currentClass.newMethod(dd.name.toTermName, dd.pos, VARARGS | SYNTHETIC | flatdd.symbol.flags)
 
-      val theTyper   = typer.atOwner(dd, currentClass)
-      val flatparams = flatdd.symbol.paramss.head
       val isRepeated = enteringUncurry(dd.symbol.info.paramss.flatten.map(sym => definitions.isRepeatedParamType(sym.tpe)))
 
-      // 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)
-      }
+      val oldPs = flatdd.symbol.paramss.head
+
+      // see comment in method toArrayType below
+      val arrayTypesMappedToObject = mutable.Map.empty[Symbol, Type]
 
-      // 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 forwTpe = {
+        val (oldTps, tps) = dd.symbol.tpe match {
+          case PolyType(oldTps, _) =>
+            val newTps = oldTps.map(_.cloneSymbol(forwSym))
+            (oldTps, newTps)
+
+          case _ => (Nil, Nil)
         }
-        val seqargs = map2(locals, forwParams) {
-          case (null, argsym) => Ident(argsym)
-          case (l, _)         => l
+
+        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 `arrayTypesMappedToObject` map.
+            val originalArg = arg.substSym(oldTps, tps)
+            arrayTypesMappedToObject(newParam) = originalArg
+            // Store the type parameter that was replaced by Object to emit the correct generic signature
+            newParam.updateAttachment(new erasure.TypeParamVarargsAttachment(originalArg))
+            ObjectTpe
+          } else
+            arg
+          arrayType(elem)
         }
-        val end = if (forwsym.isConstructor) List(UNIT) else Nil
 
-        DefDef(forwsym, BLOCK(Apply(gen.mkAttributedRef(flatdd.symbol), seqargs) :: end : _*))
+        val ps = map2(oldPs, isRepeated)((oldParam, isRep) => {
+          val newParam = oldParam.cloneSymbol(forwSym)
+          val tp = if (isRep) toArrayType(oldParam.tpe, newParam) else oldParam.tpe
+          newParam.setInfo(tp)
+        })
+
+        val resTp = dd.symbol.tpe_*.finalResultType.substSym(oldPs, ps)
+        val mt = MethodType(ps, resTp)
+        val r = if (tps.isEmpty) mt else PolyType(tps, mt)
+        r.substSym(oldTps, tps)
+      }
+
+      forwSym.setInfo(forwTpe)
+      val newPs = forwTpe.params
+
+      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)
+            arrayTypesMappedToObject.get(param) match {
+              case Some(tp) => gen.mkCast(wrap, seqType(tp))
+              case _ => wrap
+            }
+          }
+        })
+
+        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 {
+      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.")
         case None =>
           // enter symbol into scope
-          currentClass.info.decls enter forwsym
-          addNewMember(forwtree)
+          currentClass.info.decls enter forwSym
+          addNewMember(forwTree)
       }
 
       flatdd
diff --git a/test/files/jvm/t8786-sig.scala b/test/files/jvm/t8786-sig.scala
new file mode 100644
index 0000000000..0745b650e6
--- /dev/null
+++ b/test/files/jvm/t8786-sig.scala
@@ -0,0 +1,116 @@
+class A[U] {
+  @annotation.varargs def m1[T]          (a: T*): T = a.head
+  @annotation.varargs def m2[T <: AnyRef](a: T*): T = a.head
+  @annotation.varargs def m3[T <: AnyVal](a: T*): T = a.head
+  @annotation.varargs def m4[T <: Int]   (a: T*): T = a.head
+  @annotation.varargs def m5[T <: String](a: T*): T = a.head
+  @annotation.varargs def m6             (a: String*): String = a.head
+  @annotation.varargs def m7             (a: Int*): Int = a.head
+  @annotation.varargs def m8             (a: U*): U = a.head
+
+  def n1[T]          (a: Array[T]): T = a(0)
+  def n2[T <: AnyRef](a: Array[T]): T = a(0)
+  def n3[T <: AnyVal](a: Array[T]): T = a(0)
+  def n4[T <: Int]   (a: Array[T]): T = a(0)
+  def n5[T <: String](a: Array[T]): T = a(0)
+  def n6             (a: Array[String]): String = a(0)
+  def n7             (a: Array[Int]): Int = a(0)
+  def n8             (a: Array[U]): U = a(0)
+}
+
+object Test extends App {
+  val a = classOf[A[_]]
+
+  def sig   (method: String, tp: Class[_]) = a.getDeclaredMethod(method, tp).toString
+  def genSig(method: String, tp: Class[_]) = a.getDeclaredMethod(method, tp).toGenericString
+  def bound (method: String, tp: Class[_]) = {
+    val m = a.getDeclaredMethod(method, tp)
+    m.getGenericParameterTypes.apply(0) match {
+      case _: Class[_] => ""
+      case gat: java.lang.reflect.GenericArrayType =>
+        val compTp = gat.getGenericComponentType.asInstanceOf[java.lang.reflect.TypeVariable[_]]
+        compTp.getBounds.apply(0).toString
+    }
+  }
+
+  def check(a: String, b: String) = {
+    assert(a == b, s"found: $a\nexpected: $b")
+  }
+
+  val sq = classOf[Seq[_]]
+  val ob = classOf[Object]
+  val ao = classOf[Array[Object]]
+  val as = classOf[Array[String]]
+  val ai = classOf[Array[Int]]
+
+  check(sig("m1", sq)   , "public java.lang.Object A.m1(scala.collection.Seq)")
+  check(sig("m2", sq)   , "public java.lang.Object A.m2(scala.collection.Seq)")
+  check(sig("m3", sq)   , "public java.lang.Object A.m3(scala.collection.Seq)")
+  check(sig("m4", sq)   , "public int A.m4(scala.collection.Seq)")
+  check(sig("m5", sq)   , "public java.lang.String A.m5(scala.collection.Seq)")
+  check(sig("m6", sq)   , "public java.lang.String A.m6(scala.collection.Seq)")
+  check(sig("m7", sq)   , "public int A.m7(scala.collection.Seq)")
+  check(sig("m8", sq)   , "public java.lang.Object A.m8(scala.collection.Seq)")
+
+  check(genSig("m1", sq), "public <T> T A.m1(scala.collection.Seq<T>)")
+  check(genSig("m2", sq), "public <T> T A.m2(scala.collection.Seq<T>)")
+  check(genSig("m3", sq), "public <T> T A.m3(scala.collection.Seq<T>)")
+  // TODO: the signature for is wrong for T <: Int, SI-9846. The signature should be
+  // `public int A.m4(scala.collection.Seq<java.lang.Object>)`. This is testing the status quo.
+  check(genSig("m4", sq), "public <T> T A.m4(scala.collection.Seq<T>)")
+  check(genSig("m5", sq), "public <T> T A.m5(scala.collection.Seq<T>)")
+  check(genSig("m6", sq), "public java.lang.String A.m6(scala.collection.Seq<java.lang.String>)")
+  check(genSig("m7", sq), "public int A.m7(scala.collection.Seq<java.lang.Object>)")
+  check(genSig("m8", sq), "public U A.m8(scala.collection.Seq<U>)")
+
+
+  // varargs forwarder
+
+  check(sig("m1", ao)   , "public java.lang.Object A.m1(java.lang.Object[])")
+  check(sig("m2", ao)   , "public java.lang.Object A.m2(java.lang.Object[])")
+  check(sig("m3", ao)   , "public java.lang.Object A.m3(java.lang.Object[])")
+  check(sig("m4", ao)   , "public int A.m4(java.lang.Object[])")
+  check(sig("m5", as)   , "public java.lang.String A.m5(java.lang.String[])")
+  check(sig("m6", as)   , "public java.lang.String A.m6(java.lang.String[])")
+  check(sig("m7", ai)   , "public int A.m7(int[])")
+  check(sig("m8", ao)   , "public java.lang.Object A.m8(java.lang.Object[])")
+
+  check(genSig("m1", ao), "public <T> T A.m1(T...)")
+  check(genSig("m2", ao), "public <T> T A.m2(T...)")
+  check(genSig("m3", ao), "public <T> T A.m3(T...)")
+  // testing status quo: signature is wrong for T <: Int, SI-9846
+  check(genSig("m4", ao), "public <T> T A.m4(T...)")
+  check(genSig("m5", as), "public <T> T A.m5(T...)")
+  check(genSig("m6", as), "public java.lang.String A.m6(java.lang.String...)")
+  check(genSig("m7", ai), "public int A.m7(int...)")
+  check(genSig("m8", ao), "public U A.m8(U...)")
+
+  check(bound("m1", ao) , "class java.lang.Object")
+  check(bound("m2", ao) , "class java.lang.Object")
+  check(bound("m3", ao) , "class java.lang.Object")
+  check(bound("m4", ao) , "class java.lang.Object")
+  check(bound("m5", as) , "class java.lang.String")
+  check(bound("m6", as) , "")
+  check(bound("m7", ai) , "")
+  check(bound("m8", ao) , "class java.lang.Object")
+
+
+  check(sig("n1", ob)   , "public java.lang.Object A.n1(java.lang.Object)")
+  check(sig("n2", ao)   , "public java.lang.Object A.n2(java.lang.Object[])")
+  check(sig("n3", ob)   , "public java.lang.Object A.n3(java.lang.Object)")
+  check(sig("n4", ob)   , "public int A.n4(java.lang.Object)")
+  check(sig("n5", as)   , "public java.lang.String A.n5(java.lang.String[])")
+  check(sig("n6", as)   , "public java.lang.String A.n6(java.lang.String[])")
+  check(sig("n7", ai)   , "public int A.n7(int[])")
+  check(sig("n8", ob)   , "public java.lang.Object A.n8(java.lang.Object)")
+
+  check(genSig("n1", ob), "public <T> T A.n1(java.lang.Object)")
+  check(genSig("n2", ao), "public <T> T A.n2(T[])")
+  check(genSig("n3", ob), "public <T> T A.n3(java.lang.Object)")
+  // testing status quo: signature is wrong for T <: Int, SI-9846
+  check(genSig("n4", ob), "public <T> T A.n4(java.lang.Object)")
+  check(genSig("n5", as), "public <T> T A.n5(T[])")
+  check(genSig("n6", as), "public java.lang.String A.n6(java.lang.String[])")
+  check(genSig("n7", ai), "public int A.n7(int[])")
+  check(genSig("n8", ob), "public U A.n8(java.lang.Object)")
+}
diff --git a/test/files/jvm/t8786/A_1.scala b/test/files/jvm/t8786/A_1.scala
new file mode 100644
index 0000000000..13c0ad191d
--- /dev/null
+++ b/test/files/jvm/t8786/A_1.scala
@@ -0,0 +1,3 @@
+class A {
+  @annotation.varargs def foo[T](a: Int, b: T*): T = b.head
+}
diff --git a/test/files/jvm/t8786/B_2.java b/test/files/jvm/t8786/B_2.java
new file mode 100644
index 0000000000..dc155a290f
--- /dev/null
+++ b/test/files/jvm/t8786/B_2.java
@@ -0,0 +1,22 @@
+public class B_2 {
+  private static int res = 0;
+
+  public static void m(char a[]) { res += 10; }
+  public static void m(String a) { res += 100; }
+  public static void m(Object a) { res += 1000; }
+
+  public static <T> T foo(int a, T... b) { return b[0]; }
+
+  public static <T> T bar(T b[]) { return b[0]; }
+
+  public static void main(String[] args) {
+    m(foo(15, "a", "b", "c"));
+    if (res != 100)
+      throw new Error("bad: "+ res);
+
+    A a = new A();
+    m(a.foo(16, "a", "b", "c"));
+    if (res != 200)
+      throw new Error("bad: " + res);
+  }
+}
diff --git a/test/files/jvm/t8786/Test_2.scala b/test/files/jvm/t8786/Test_2.scala
new file mode 100644
index 0000000000..76ccb4c3ed
--- /dev/null
+++ b/test/files/jvm/t8786/Test_2.scala
@@ -0,0 +1,3 @@
+object Test extends App {
+  B_2.main(null)
+}
diff --git a/test/files/jvm/varargs/JavaClass.java b/test/files/jvm/varargs/JavaClass.java
index 6928ee5adc..0cc3587c5e 100644
--- a/test/files/jvm/varargs/JavaClass.java
+++ b/test/files/jvm/varargs/JavaClass.java
@@ -1,16 +1,12 @@
-
-
-
 public class JavaClass {
-    public static <T> void varargz(int i, T... v) {
-    }
-
-    public static void callSomeAnnotations() {
-	VaClass va = new VaClass();
-	va.vs(4, "", "", "");
-	va.vi(1, 2, 3, 4);
-        varargz(5, 1.0, 2.0, 3.0);
-	va.vt(16, "", "", "");
-	System.out.println(va.vt1(16, "a", "b", "c"));
-    }
-}
\ No newline at end of file
+  public static <T> void varargz(int i, T... v) { }
+
+  public static void callSomeAnnotations() {
+      VaClass va = new VaClass();
+      va.vs(4, "", "", "");
+      va.vi(1, 2, 3, 4);
+      varargz(5, 1.0, 2.0, 3.0);
+      va.vt(16, "", "", "");
+      System.out.println(va.vt1(16, "a", "b", "c"));
+  }
+}
diff --git a/test/files/jvm/varargs/VaClass.scala b/test/files/jvm/varargs/VaClass.scala
index d83e63ace1..ee8c288a16 100644
--- a/test/files/jvm/varargs/VaClass.scala
+++ b/test/files/jvm/varargs/VaClass.scala
@@ -1,15 +1,8 @@
-
-
 import annotation.varargs
 
-
-
 class VaClass {
-
   @varargs def vs(a: Int, b: String*) = println(a + b.length)
   @varargs def vi(a: Int, b: Int*) = println(a + b.sum)
   @varargs def vt[T](a: Int, b: T*) = println(a + b.length)
-
-  // TODO remove type bound after fixing SI-8786, see also https://github.com/scala/scala/pull/3961
-  @varargs def vt1[T <: String](a: Int, b: T*): T = b.head
+  @varargs def vt1[T](a: Int, b: T*): T = b.head
 }
diff --git a/test/files/jvm/varargs/varargs.scala b/test/files/jvm/varargs/varargs.scala
index 6d2e707bdf..b09818f46f 100644
--- a/test/files/jvm/varargs/varargs.scala
+++ b/test/files/jvm/varargs/varargs.scala
@@ -1,21 +1,5 @@
-
-
-
-
-
-
 object Test {
   def main(args: Array[String]) {
     JavaClass.callSomeAnnotations
   }
 }
-
-
-
-
-
-
-
-
-
-