Skip to content

Commit 107cb5d

Browse files
Generate varargs bridge with the right flags on @varargs annotation
1 parent aa64078 commit 107cb5d

File tree

5 files changed

+60
-31
lines changed

5 files changed

+60
-31
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,7 @@ class Definitions {
829829
@tu lazy val FunctionalInterfaceAnnot: ClassSymbol = ctx.requiredClass("java.lang.FunctionalInterface")
830830
@tu lazy val InfixAnnot: ClassSymbol = ctx.requiredClass("scala.annotation.infix")
831831
@tu lazy val AlphaAnnot: ClassSymbol = ctx.requiredClass("scala.annotation.alpha")
832+
@tu lazy val VarargsAnnot: ClassSymbol = ctx.requiredClass("scala.annotation.varargs")
832833

833834
// A list of annotations that are commonly used to indicate that a field/method argument or return
834835
// type is not null. These annotations are used by the nullification logic in JavaNullInterop to

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

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -34,34 +34,34 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
3434
elimRepeated(tp)
3535

3636
override def transform(ref: SingleDenotation)(using Context): SingleDenotation =
37-
super.transform(ref) match {
37+
super.transform(ref) match
3838
case ref1: SymDenotation if (ref1 ne ref) && overridesJava(ref1.symbol) =>
3939
// This method won't override the corresponding Java method at the end of this phase,
4040
// only the bridge added by `addVarArgsBridge` will.
4141
ref1.copySymDenotation(initFlags = ref1.flags &~ Override)
4242
case ref1 =>
4343
ref1
44-
}
4544

4645
override def mayChange(sym: Symbol)(using Context): Boolean = sym.is(Method)
4746

4847
private def overridesJava(sym: Symbol)(using Context) = sym.allOverriddenSymbols.exists(_.is(JavaDefined))
4948

50-
private def elimRepeated(tp: Type)(using Context): Type = tp.stripTypeVar match {
49+
private def hasVargsAnnotation(sym: Symbol)(using ctx: Context) = sym.hasAnnotation(defn.VarargsAnnot)
50+
51+
/** Eliminate repeated parameters from method types. */
52+
private def elimRepeated(tp: Type)(using Context): Type = tp.stripTypeVar match
5153
case tp @ MethodTpe(paramNames, paramTypes, resultType) =>
5254
val resultType1 = elimRepeated(resultType)
5355
val paramTypes1 =
54-
if (paramTypes.nonEmpty && paramTypes.last.isRepeatedParam) {
56+
if paramTypes.nonEmpty && paramTypes.last.isRepeatedParam then
5557
val last = paramTypes.last.translateFromRepeated(toArray = tp.isJavaMethod)
5658
paramTypes.init :+ last
57-
}
5859
else paramTypes
5960
tp.derivedLambdaType(paramNames, paramTypes1, resultType1)
6061
case tp: PolyType =>
6162
tp.derivedLambdaType(tp.paramNames, tp.paramInfos, elimRepeated(tp.resultType))
6263
case tp =>
6364
tp
64-
}
6565

6666
def transformTypeOfTree(tree: Tree)(using Context): Tree =
6767
tree.withType(elimRepeated(tree.tpe))
@@ -72,35 +72,35 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
7272
override def transformSelect(tree: Select)(using Context): Tree =
7373
transformTypeOfTree(tree)
7474

75-
override def transformApply(tree: Apply)(using Context): Tree = {
75+
override def transformApply(tree: Apply)(using Context): Tree =
7676
val args = tree.args.mapConserve {
7777
case arg: Typed if isWildcardStarArg(arg) =>
7878
val isJavaDefined = tree.fun.symbol.is(JavaDefined)
7979
val tpe = arg.expr.tpe
80-
if (isJavaDefined && tpe.derivesFrom(defn.SeqClass))
80+
if isJavaDefined && tpe.derivesFrom(defn.SeqClass) then
8181
seqToArray(arg.expr)
82-
else if (!isJavaDefined && tpe.derivesFrom(defn.ArrayClass))
82+
else if !isJavaDefined && tpe.derivesFrom(defn.ArrayClass)
8383
arrayToSeq(arg.expr)
8484
else
8585
arg.expr
8686
case arg => arg
8787
}
8888
transformTypeOfTree(cpy.Apply(tree)(tree.fun, args))
89-
}
9089

9190
/** Convert sequence argument to Java array */
92-
private def seqToArray(tree: Tree)(using Context): Tree = tree match {
91+
private def seqToArray(tree: Tree)(using Context): Tree = tree match
9392
case SeqLiteral(elems, elemtpt) =>
9493
JavaSeqLiteral(elems, elemtpt)
9594
case _ =>
9695
val elemType = tree.tpe.elemType
9796
var elemClass = erasure(elemType).classSymbol
98-
if (defn.NotRuntimeClasses.contains(elemClass)) elemClass = defn.ObjectClass
97+
if defn.NotRuntimeClasses.contains(elemClass) then
98+
elemClass = defn.ObjectClass
99+
end if
99100
ref(defn.DottyArraysModule)
100101
.select(nme.seqToArray)
101102
.appliedToType(elemType)
102103
.appliedTo(tree, clsOf(elemClass.typeRef))
103-
}
104104

105105
/** Convert Java array argument to Scala Seq */
106106
private def arrayToSeq(tree: Tree)(using Context): Tree =
@@ -109,36 +109,44 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
109109
override def transformTypeApply(tree: TypeApply)(using Context): Tree =
110110
transformTypeOfTree(tree)
111111

112-
/** If method overrides a Java varargs method, add a varargs bridge.
113-
* Also transform trees inside method annotation
112+
/** If method overrides a Java varargs method or is annotated with @varargs, add a varargs bridge.
113+
* Also transform trees inside method annotation.
114114
*/
115115
override def transformDefDef(tree: DefDef)(using Context): Tree =
116116
atPhase(thisPhase) {
117-
if (tree.symbol.info.isVarArgsMethod && overridesJava(tree.symbol))
118-
addVarArgsBridge(tree)
117+
val isOverride = overridesJava(tree.symbol)
118+
val hasAnnotation = hasVargsAnnotation(tree.symbol)
119+
if tree.symbol.info.isVarArgsMethod && (isOverride || hasAnnotation) then
120+
// non-overrides need the varargs bytecode flag and cannot be synthetic
121+
// otherwise javac refuses to call them.
122+
addVarArgsBridge(tree, isOverride)
119123
else
120124
tree
121125
}
122126

123127
/** Add a Java varargs bridge
124-
* @param ddef the original method definition which is assumed to override
125-
* a Java varargs method JM up to this phase.
126-
* @return a thicket consisting of `ddef` and a varargs bridge method
127-
* which overrides the Java varargs method JM from this phase on
128-
* and forwards to `ddef`.
128+
* @param ddef the original method definition
129+
* @param addFlag the flag to add to the method symbol
130+
131+
* @return a thicket consisting of `ddef` and a varargs bridge method
132+
* which forwards java varargs to `ddef`. It retains all the
133+
* flags of `ddef` except `Private`.
129134
*
130-
* A bridge is necessary because the following hold
135+
* A bridge is necessary because the following hold:
131136
* - the varargs in `ddef` will change from `RepeatedParam[T]` to `Seq[T]` after this phase
132-
* - _but_ the callers of `ddef` expect its varargs to be changed to `Array[? <: T]`, since it overrides
133-
* a Java varargs
137+
* - _but_ the callers of `ddef` expect its varargs to be changed to `Array[? <: T]`
134138
* The solution is to add a "bridge" method that converts its argument from `Array[? <: T]` to `Seq[T]` and
135139
* forwards it to `ddef`.
136140
*/
137-
private def addVarArgsBridge(ddef: DefDef)(using Context): Tree = {
141+
private def addVarArgsBridge(ddef: DefDef, synthetic: Boolean)(using Context): Tree =
138142
val original = ddef.symbol.asTerm
143+
// For simplicity we always set the varargs flag
144+
val flags = ddef.symbol.flags | JavaVarargs &~ Private
139145
val bridge = original.copy(
140-
flags = ddef.symbol.flags &~ Private | Artifact,
141-
info = toJavaVarArgs(ddef.symbol.info)).enteredAfter(thisPhase).asTerm
146+
flags = if synthetic then flags | Artifact else flags,
147+
info = toJavaVarArgs(ddef.symbol.info)
148+
).enteredAfter(thisPhase).asTerm
149+
142150
val bridgeDef = polyDefDef(bridge, trefs => vrefss => {
143151
val (vrefs :+ varArgRef) :: vrefss1 = vrefss
144152
// Can't call `.argTypes` here because the underlying array type is of the
@@ -151,15 +159,14 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
151159
})
152160

153161
Thicket(ddef, bridgeDef)
154-
}
155162

156163
/** Convert type from Scala to Java varargs method */
157-
private def toJavaVarArgs(tp: Type)(using Context): Type = tp match {
164+
private def toJavaVarArgs(tp: Type)(using Context): Type = tp match
158165
case tp: PolyType =>
159166
tp.derivedLambdaType(tp.paramNames, tp.paramInfos, toJavaVarArgs(tp.resultType))
160167
case tp: MethodType =>
161168
val inits :+ last = tp.paramInfos
162169
val last1 = last.translateFromRepeated(toArray = true)
163170
tp.derivedLambdaType(tp.paramNames, inits :+ last1, tp.resultType)
164-
}
171+
165172
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,4 @@ i7868.scala
2626
enum-java
2727
zero-arity-case-class.scala
2828
tuple-ops.scala
29+
i7212

tests/run/i7212/CompatVargs.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import scala.annotation._
2+
3+
class CompatVargs {
4+
@varargs
5+
def vargs(args: String*): Unit = println(args)
6+
7+
def vargsFromScala(): Unit =
8+
vargs("single")
9+
vargs("a", "b")
10+
vargs(Seq("a", "b"): _*)
11+
}

tests/run/i7212/Test.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
public class Test {
2+
public static void main(String[] args) {
3+
CompatVargs c = new CompatVargs();
4+
c.vargs("single");
5+
c.vargs("a", "b");
6+
c.vargs(new String[]{"a", "b"});
7+
c.vargsFromScala();
8+
}
9+
}

0 commit comments

Comments
 (0)