Skip to content

Commit 0d2760d

Browse files
lrytzlrytz
committed
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.
1 parent 6612ba0 commit 0d2760d

File tree

10 files changed

+237
-92
lines changed

10 files changed

+237
-92
lines changed

‎README.md

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

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,18 @@ abstract class Erasure extends AddInterfaces
343343

344344
case MethodType(params, restpe) =>
345345
val buf = new StringBuffer("(")
346-
params foreach (p => buf append jsig(p.tpe))
346+
params foreach (p => {
347+
val tp = p.attachments.get[TypeParamVarargsAttachment] match {
348+
case Some(att) =>
349+
// For @varargs forwarders, a T* parameter has type Array[Object] in the forwarder
350+
// instead of Array[T], as the latter would erase to Object (instead of Array[Object]).
351+
// To make the generic signature correct ("[T", not "[Object"), an attachment on the
352+
// parameter symbol stores the type T that was replaced by Object.
353+
buf.append("["); att.typeParamRef
354+
case _ => p.tpe
355+
}
356+
buf append jsig(tp)
357+
})
347358
buf append ")"
348359
buf append (if (restpe.typeSymbol == UnitClass || sym0.isConstructor) VOID_TAG.toString else jsig(restpe))
349360
buf.toString
@@ -1227,4 +1238,5 @@ abstract class Erasure extends AddInterfaces
12271238
}
12281239

12291240
private class TypeRefAttachment(val tpe: TypeRef)
1241+
class TypeParamVarargsAttachment(val typeParamRef: Type)
12301242
}

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

Lines changed: 67 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -744,72 +744,88 @@ abstract class UnCurry extends InfoTransform
744744
if (!dd.symbol.hasAnnotation(VarargsClass) || !enteringUncurry(mexists(dd.symbol.paramss)(sym => definitions.isRepeatedParamType(sym.tpe))))
745745
return flatdd
746746

747-
def toArrayType(tp: Type): Type = {
748-
val arg = elementType(SeqClass, tp)
749-
// to prevent generation of an `Object` parameter from `Array[T]` parameter later
750-
// as this would crash the Java compiler which expects an `Object[]` array for varargs
751-
// e.g. def foo[T](a: Int, b: T*)
752-
// becomes def foo[T](a: Int, b: Array[Object])
753-
// instead of def foo[T](a: Int, b: Array[T]) ===> def foo[T](a: Int, b: Object)
754-
arrayType(
755-
if (arg.typeSymbol.isTypeParameterOrSkolem) ObjectTpe
756-
else arg
757-
)
758-
}
747+
val forwSym = currentClass.newMethod(dd.name.toTermName, dd.pos, VARARGS | SYNTHETIC | flatdd.symbol.flags)
759748

760-
val theTyper = typer.atOwner(dd, currentClass)
761-
val flatparams = flatdd.symbol.paramss.head
762749
val isRepeated = enteringUncurry(dd.symbol.info.paramss.flatten.map(sym => definitions.isRepeatedParamType(sym.tpe)))
763750

764-
// create the type
765-
val forwformals = map2(flatparams, isRepeated) {
766-
case (p, true) => toArrayType(p.tpe)
767-
case (p, false)=> p.tpe
768-
}
769-
val forwresult = dd.symbol.tpe_*.finalResultType
770-
val forwformsyms = map2(forwformals, flatparams)((tp, oldparam) =>
771-
currentClass.newValueParameter(oldparam.name.toTermName, oldparam.pos).setInfo(tp)
772-
)
773-
def mono = MethodType(forwformsyms, forwresult)
774-
val forwtype = dd.symbol.tpe match {
775-
case MethodType(_, _) => mono
776-
case PolyType(tps, _) => PolyType(tps, mono)
777-
}
751+
val oldPs = flatdd.symbol.paramss.head
752+
753+
// see comment in method toArrayType below
754+
val arrayTypesMappedToObject = mutable.Map.empty[Symbol, Type]
778755

779-
// create the symbol
780-
val forwsym = currentClass.newMethod(dd.name.toTermName, dd.pos, VARARGS | SYNTHETIC | flatdd.symbol.flags) setInfo forwtype
781-
def forwParams = forwsym.info.paramss.flatten
782-
783-
// create the tree
784-
val forwtree = theTyper.typedPos(dd.pos) {
785-
val locals = map3(forwParams, flatparams, isRepeated) {
786-
case (_, fp, false) => null
787-
case (argsym, fp, true) =>
788-
Block(Nil,
789-
gen.mkCast(
790-
gen.mkWrapArray(Ident(argsym), elementType(ArrayClass, argsym.tpe)),
791-
seqType(elementType(SeqClass, fp.tpe))
792-
)
793-
)
756+
val forwTpe = {
757+
val (oldTps, tps) = dd.symbol.tpe match {
758+
case PolyType(oldTps, _) =>
759+
val newTps = oldTps.map(_.cloneSymbol(forwSym))
760+
(oldTps, newTps)
761+
762+
case _ => (Nil, Nil)
794763
}
795-
val seqargs = map2(locals, forwParams) {
796-
case (null, argsym) => Ident(argsym)
797-
case (l, _) => l
764+
765+
def toArrayType(tp: Type, newParam: Symbol): Type = {
766+
val arg = elementType(SeqClass, tp)
767+
val elem = if (arg.typeSymbol.isTypeParameterOrSkolem && !(arg <:< AnyRefTpe)) {
768+
// To prevent generation of an `Object` parameter from `Array[T]` parameter later
769+
// as this would crash the Java compiler which expects an `Object[]` array for varargs
770+
// e.g. def foo[T](a: Int, b: T*)
771+
// becomes def foo[T](a: Int, b: Array[Object])
772+
// instead of def foo[T](a: Int, b: Array[T]) ===> def foo[T](a: Int, b: Object)
773+
//
774+
// In order for the forwarder method to type check we need to insert a cast:
775+
// def foo'[T'](a: Int, b: Array[Object]) = foo[T'](a, wrapRefArray(b).asInstanceOf[Seq[T']])
776+
// The target element type for that cast (T') is stored in the `arrayTypesMappedToObject` map.
777+
val originalArg = arg.substSym(oldTps, tps)
778+
arrayTypesMappedToObject(newParam) = originalArg
779+
// Store the type parameter that was replaced by Object to emit the correct generic signature
780+
newParam.updateAttachment(new erasure.TypeParamVarargsAttachment(originalArg))
781+
ObjectTpe
782+
} else
783+
arg
784+
arrayType(elem)
798785
}
799-
val end = if (forwsym.isConstructor) List(UNIT) else Nil
800786

801-
DefDef(forwsym, BLOCK(Apply(gen.mkAttributedRef(flatdd.symbol), seqargs) :: end : _*))
787+
val ps = map2(oldPs, isRepeated)((oldParam, isRep) => {
788+
val newParam = oldParam.cloneSymbol(forwSym)
789+
val tp = if (isRep) toArrayType(oldParam.tpe, newParam) else oldParam.tpe
790+
newParam.setInfo(tp)
791+
})
792+
793+
val resTp = dd.symbol.tpe_*.finalResultType.substSym(oldPs, ps)
794+
val mt = MethodType(ps, resTp)
795+
val r = if (tps.isEmpty) mt else PolyType(tps, mt)
796+
r.substSym(oldTps, tps)
797+
}
798+
799+
forwSym.setInfo(forwTpe)
800+
val newPs = forwTpe.params
801+
802+
val theTyper = typer.atOwner(dd, currentClass)
803+
val forwTree = theTyper.typedPos(dd.pos) {
804+
val seqArgs = map3(newPs, oldPs, isRepeated)((param, oldParam, isRep) => {
805+
if (!isRep) Ident(param)
806+
else {
807+
val parTp = elementType(ArrayClass, param.tpe)
808+
val wrap = gen.mkWrapArray(Ident(param), parTp)
809+
arrayTypesMappedToObject.get(param) match {
810+
case Some(tp) => gen.mkCast(wrap, seqType(tp))
811+
case _ => wrap
812+
}
813+
}
814+
})
815+
816+
val forwCall = Apply(gen.mkAttributedRef(flatdd.symbol), seqArgs)
817+
DefDef(forwSym, if (forwSym.isConstructor) Block(List(forwCall), UNIT) else forwCall)
802818
}
803819

804820
// check if the method with that name and those arguments already exists in the template
805-
currentClass.info.member(forwsym.name).alternatives.find(s => s != forwsym && s.tpe.matches(forwsym.tpe)) match {
821+
currentClass.info.member(forwSym.name).alternatives.find(s => s != forwSym && s.tpe.matches(forwSym.tpe)) match {
806822
case Some(s) => reporter.error(dd.symbol.pos,
807823
"A method with a varargs annotation produces a forwarder method with the same signature "
808824
+ s.tpe + " as an existing method.")
809825
case None =>
810826
// enter symbol into scope
811-
currentClass.info.decls enter forwsym
812-
addNewMember(forwtree)
827+
currentClass.info.decls enter forwSym
828+
addNewMember(forwTree)
813829
}
814830

815831
flatdd

‎test/files/jvm/t8786-sig.scala

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
class A[U] {
2+
@annotation.varargs def m1[T] (a: T*): T = a.head
3+
@annotation.varargs def m2[T <: AnyRef](a: T*): T = a.head
4+
@annotation.varargs def m3[T <: AnyVal](a: T*): T = a.head
5+
@annotation.varargs def m4[T <: Int] (a: T*): T = a.head
6+
@annotation.varargs def m5[T <: String](a: T*): T = a.head
7+
@annotation.varargs def m6 (a: String*): String = a.head
8+
@annotation.varargs def m7 (a: Int*): Int = a.head
9+
@annotation.varargs def m8 (a: U*): U = a.head
10+
11+
def n1[T] (a: Array[T]): T = a(0)
12+
def n2[T <: AnyRef](a: Array[T]): T = a(0)
13+
def n3[T <: AnyVal](a: Array[T]): T = a(0)
14+
def n4[T <: Int] (a: Array[T]): T = a(0)
15+
def n5[T <: String](a: Array[T]): T = a(0)
16+
def n6 (a: Array[String]): String = a(0)
17+
def n7 (a: Array[Int]): Int = a(0)
18+
def n8 (a: Array[U]): U = a(0)
19+
}
20+
21+
object Test extends App {
22+
val a = classOf[A[_]]
23+
24+
def sig (method: String, tp: Class[_]) = a.getDeclaredMethod(method, tp).toString
25+
def genSig(method: String, tp: Class[_]) = a.getDeclaredMethod(method, tp).toGenericString
26+
def bound (method: String, tp: Class[_]) = {
27+
val m = a.getDeclaredMethod(method, tp)
28+
m.getGenericParameterTypes.apply(0) match {
29+
case _: Class[_] => ""
30+
case gat: java.lang.reflect.GenericArrayType =>
31+
val compTp = gat.getGenericComponentType.asInstanceOf[java.lang.reflect.TypeVariable[_]]
32+
compTp.getBounds.apply(0).toString
33+
}
34+
}
35+
36+
def check(a: String, b: String) = {
37+
assert(a == b, s"found: $a\nexpected: $b")
38+
}
39+
40+
val sq = classOf[Seq[_]]
41+
val ob = classOf[Object]
42+
val ao = classOf[Array[Object]]
43+
val as = classOf[Array[String]]
44+
val ai = classOf[Array[Int]]
45+
46+
check(sig("m1", sq) , "public java.lang.Object A.m1(scala.collection.Seq)")
47+
check(sig("m2", sq) , "public java.lang.Object A.m2(scala.collection.Seq)")
48+
check(sig("m3", sq) , "public java.lang.Object A.m3(scala.collection.Seq)")
49+
check(sig("m4", sq) , "public int A.m4(scala.collection.Seq)")
50+
check(sig("m5", sq) , "public java.lang.String A.m5(scala.collection.Seq)")
51+
check(sig("m6", sq) , "public java.lang.String A.m6(scala.collection.Seq)")
52+
check(sig("m7", sq) , "public int A.m7(scala.collection.Seq)")
53+
check(sig("m8", sq) , "public java.lang.Object A.m8(scala.collection.Seq)")
54+
55+
check(genSig("m1", sq), "public <T> T A.m1(scala.collection.Seq<T>)")
56+
check(genSig("m2", sq), "public <T> T A.m2(scala.collection.Seq<T>)")
57+
check(genSig("m3", sq), "public <T> T A.m3(scala.collection.Seq<T>)")
58+
// TODO: the signature for is wrong for T <: Int, SI-9846. The signature should be
59+
// `public int A.m4(scala.collection.Seq<java.lang.Object>)`. This is testing the status quo.
60+
check(genSig("m4", sq), "public <T> T A.m4(scala.collection.Seq<T>)")
61+
check(genSig("m5", sq), "public <T> T A.m5(scala.collection.Seq<T>)")
62+
check(genSig("m6", sq), "public java.lang.String A.m6(scala.collection.Seq<java.lang.String>)")
63+
check(genSig("m7", sq), "public int A.m7(scala.collection.Seq<java.lang.Object>)")
64+
check(genSig("m8", sq), "public U A.m8(scala.collection.Seq<U>)")
65+
66+
67+
// varargs forwarder
68+
69+
check(sig("m1", ao) , "public java.lang.Object A.m1(java.lang.Object[])")
70+
check(sig("m2", ao) , "public java.lang.Object A.m2(java.lang.Object[])")
71+
check(sig("m3", ao) , "public java.lang.Object A.m3(java.lang.Object[])")
72+
check(sig("m4", ao) , "public int A.m4(java.lang.Object[])")
73+
check(sig("m5", as) , "public java.lang.String A.m5(java.lang.String[])")
74+
check(sig("m6", as) , "public java.lang.String A.m6(java.lang.String[])")
75+
check(sig("m7", ai) , "public int A.m7(int[])")
76+
check(sig("m8", ao) , "public java.lang.Object A.m8(java.lang.Object[])")
77+
78+
check(genSig("m1", ao), "public <T> T A.m1(T...)")
79+
check(genSig("m2", ao), "public <T> T A.m2(T...)")
80+
check(genSig("m3", ao), "public <T> T A.m3(T...)")
81+
// testing status quo: signature is wrong for T <: Int, SI-9846
82+
check(genSig("m4", ao), "public <T> T A.m4(T...)")
83+
check(genSig("m5", as), "public <T> T A.m5(T...)")
84+
check(genSig("m6", as), "public java.lang.String A.m6(java.lang.String...)")
85+
check(genSig("m7", ai), "public int A.m7(int...)")
86+
check(genSig("m8", ao), "public U A.m8(U...)")
87+
88+
check(bound("m1", ao) , "class java.lang.Object")
89+
check(bound("m2", ao) , "class java.lang.Object")
90+
check(bound("m3", ao) , "class java.lang.Object")
91+
check(bound("m4", ao) , "class java.lang.Object")
92+
check(bound("m5", as) , "class java.lang.String")
93+
check(bound("m6", as) , "")
94+
check(bound("m7", ai) , "")
95+
check(bound("m8", ao) , "class java.lang.Object")
96+
97+
98+
check(sig("n1", ob) , "public java.lang.Object A.n1(java.lang.Object)")
99+
check(sig("n2", ao) , "public java.lang.Object A.n2(java.lang.Object[])")
100+
check(sig("n3", ob) , "public java.lang.Object A.n3(java.lang.Object)")
101+
check(sig("n4", ob) , "public int A.n4(java.lang.Object)")
102+
check(sig("n5", as) , "public java.lang.String A.n5(java.lang.String[])")
103+
check(sig("n6", as) , "public java.lang.String A.n6(java.lang.String[])")
104+
check(sig("n7", ai) , "public int A.n7(int[])")
105+
check(sig("n8", ob) , "public java.lang.Object A.n8(java.lang.Object)")
106+
107+
check(genSig("n1", ob), "public <T> T A.n1(java.lang.Object)")
108+
check(genSig("n2", ao), "public <T> T A.n2(T[])")
109+
check(genSig("n3", ob), "public <T> T A.n3(java.lang.Object)")
110+
// testing status quo: signature is wrong for T <: Int, SI-9846
111+
check(genSig("n4", ob), "public <T> T A.n4(java.lang.Object)")
112+
check(genSig("n5", as), "public <T> T A.n5(T[])")
113+
check(genSig("n6", as), "public java.lang.String A.n6(java.lang.String[])")
114+
check(genSig("n7", ai), "public int A.n7(int[])")
115+
check(genSig("n8", ob), "public U A.n8(java.lang.Object)")
116+
}

‎test/files/jvm/t8786/A_1.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
class A {
2+
@annotation.varargs def foo[T](a: Int, b: T*): T = b.head
3+
}

‎test/files/jvm/t8786/B_2.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
public class B_2 {
2+
private static int res = 0;
3+
4+
public static void m(char a[]) { res += 10; }
5+
public static void m(String a) { res += 100; }
6+
public static void m(Object a) { res += 1000; }
7+
8+
public static <T> T foo(int a, T... b) { return b[0]; }
9+
10+
public static <T> T bar(T b[]) { return b[0]; }
11+
12+
public static void main(String[] args) {
13+
m(foo(15, "a", "b", "c"));
14+
if (res != 100)
15+
throw new Error("bad: "+ res);
16+
17+
A a = new A();
18+
m(a.foo(16, "a", "b", "c"));
19+
if (res != 200)
20+
throw new Error("bad: " + res);
21+
}
22+
}

‎test/files/jvm/t8786/Test_2.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
object Test extends App {
2+
B_2.main(null)
3+
}

‎test/files/jvm/varargs/JavaClass.java

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,12 @@
1-
2-
3-
41
public class JavaClass {
5-
public static <T> void varargz(int i, T... v) {
6-
}
7-
8-
public static void callSomeAnnotations() {
9-
VaClass va = new VaClass();
10-
va.vs(4, "", "", "");
11-
va.vi(1, 2, 3, 4);
12-
varargz(5, 1.0, 2.0, 3.0);
13-
va.vt(16, "", "", "");
14-
System.out.println(va.vt1(16, "a", "b", "c"));
15-
}
16-
}
2+
public static <T> void varargz(int i, T... v) { }
3+
4+
public static void callSomeAnnotations() {
5+
VaClass va = new VaClass();
6+
va.vs(4, "", "", "");
7+
va.vi(1, 2, 3, 4);
8+
varargz(5, 1.0, 2.0, 3.0);
9+
va.vt(16, "", "", "");
10+
System.out.println(va.vt1(16, "a", "b", "c"));
11+
}
12+
}

‎test/files/jvm/varargs/VaClass.scala

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,8 @@
1-
2-
31
import annotation.varargs
42

5-
6-
73
class VaClass {
8-
94
@varargs def vs(a: Int, b: String*) = println(a + b.length)
105
@varargs def vi(a: Int, b: Int*) = println(a + b.sum)
116
@varargs def vt[T](a: Int, b: T*) = println(a + b.length)
12-
13-
// TODO remove type bound after fixing SI-8786, see also https://github.com/scala/scala/pull/3961
14-
@varargs def vt1[T <: String](a: Int, b: T*): T = b.head
7+
@varargs def vt1[T](a: Int, b: T*): T = b.head
158
}

‎test/files/jvm/varargs/varargs.scala

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,5 @@
1-
2-
3-
4-
5-
6-
71
object Test {
82
def main(args: Array[String]) {
93
JavaClass.callSomeAnnotations
104
}
115
}
12-
13-
14-
15-
16-
17-
18-
19-
20-
21-

0 commit comments

Comments
 (0)