diff --git a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala index 9911544bef65..a885518e66b6 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala @@ -4,7 +4,7 @@ package transform import core._ import StdNames.nme import Types._ -import dotty.tools.dotc.transform.MegaPhase._ +import transform.MegaPhase._ import ast.Trees._ import Flags._ import Contexts._ @@ -12,7 +12,6 @@ import Symbols._ import Constants._ import Decorators._ import Denotations._, SymDenotations._ -import dotty.tools.dotc.ast.tpd import TypeErasure.erasure import DenotTransformers._ @@ -34,12 +33,43 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => def transformInfo(tp: Type, sym: Symbol)(using Context): Type = elimRepeated(tp) + /** Create forwarder symbols for the methods that are annotated + * with `@varargs` or that override java varargs. + * + * The definitions (DefDef) for these symbols are created by transformDefDef. + */ override def transform(ref: SingleDenotation)(using Context): SingleDenotation = + def transformVarArgs(sym: Symbol, isJavaVarargsOverride: Boolean): Unit = + val hasAnnotation = hasVarargsAnnotation(sym) + val hasRepeatedParam = hasRepeatedParams(sym) + if hasRepeatedParam then + if isJavaVarargsOverride || hasAnnotation || parentHasAnnotation(sym) then + // java varargs are more restrictive than scala's + // see https://github.com/scala/bug/issues/11714 + val validJava = isValidJavaVarArgs(sym.info) + if !validJava then + report.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) + else + addVarArgsForwarder(sym, isJavaVarargsOverride, hasAnnotation) + else if hasAnnotation + report.error("A method without repeated parameters cannot be annotated with @varargs", sym.sourcePos) + end + 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 forwarder added by `addVarArgsForwarder` will. - ref1.copySymDenotation(initFlags = ref1.flags &~ Override) + case ref1: SymDenotation if ref1.is(Method, butNot = JavaDefined) => + val sym = ref1.symbol + val isJavaVarargsOverride = (ref1 ne ref) && overridesJava(sym) + transformVarArgs(sym, isJavaVarargsOverride) + if isJavaVarargsOverride then + // This method won't override the corresponding Java method at the end of this phase, + // only the forwarder added by `addVarArgsForwarder` will. + ref1.copySymDenotation(initFlags = ref1.flags &~ Override) + else + ref1 case ref1 => ref1 @@ -51,6 +81,11 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => private def parentHasAnnotation(sym: Symbol)(using Context) = sym.allOverriddenSymbols.exists(hasVarargsAnnotation) + private def isVarargsMethod(sym: Symbol)(using Context) = + hasVarargsAnnotation(sym) || + hasRepeatedParams(sym) && + (sym.allOverriddenSymbols.exists(s => s.is(JavaDefined) || hasVarargsAnnotation(s))) + /** Eliminate repeated parameters from method types. */ private def elimRepeated(tp: Type)(using Context): Type = tp.stripTypeVar match case tp @ MethodTpe(paramNames, paramTypes, resultType) => @@ -162,39 +197,36 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => /** Convert an Array into a scala.Seq */ private def arrayToSeq(tree: Tree)(using Context): Tree = - tpd.wrapArray(tree, tree.tpe.elemType) + wrapArray(tree, tree.tpe.elemType) - /** If method overrides a Java varargs method or is annotated with @varargs, add a varargs bridge. - * Also transform trees inside method annotation. - */ + /** Generate the method definitions for the varargs forwarders created in transform */ override def transformDefDef(tree: DefDef)(using Context): Tree = + // If transform reported an error, don't go further + if ctx.reporter.hasErrors then + return 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 - report.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 + val isVarArgs = atPhase(thisPhase)(isVarargsMethod(sym)) + if isVarArgs then + // Get the symbol generated in transform + val forwarderType = atPhase(thisPhase)(toJavaVarArgs(sym.info)) + val forwarderSym = currentClass.info.decl(sym.name).alternatives + .find(_.info.matches(forwarderType)) + .get + .symbol.asTerm + // Generate the method + val forwarderDef = polyDefDef(forwarderSym, 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(sym.termRef) + .appliedToTypes(trefs) + .appliedToArgss(init) + .appliedToArgs(last :+ wrapArray(vararg, elemtp)) + }) + Thicket(tree, forwarderDef) else - if hasAnnotation then - report.error("A method without repeated parameters cannot be annotated with @varargs", sym.sourcePos) tree /** Is there a repeated parameter in some parameter list? */ @@ -216,61 +248,52 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => 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) + /** Add the symbol of a Java varargs forwarder to the scope. + * It retains all the flags of the original method. * - * @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`. + * @param original the original method symbol + * @param isBridge true if we are generating a "bridge" (synthetic override forwarder) * - * 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]` + * A forwarder is necessary because the following holds: + * - the varargs in `original` will change from `RepeatedParam[T]` to `Seq[T]` after this phase + * - _but_ the callers of the method 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`. + * forwards it to the original method. */ - private def addVarArgsForwarder(ddef: DefDef, isBridge: Boolean)(using Context): Tree = - val original = ddef.symbol + private def addVarArgsForwarder(original: Symbol, isBridge: Boolean, hasAnnotation: Boolean)(using Context): Unit = + val owner = original.owner + if !owner.isClass then + report.error("inner methods cannot be annotated with @varargs", original.sourcePos) + return + + val classInfo = owner.info + + // For simplicity we always set the varargs flag, + // although it's not strictly necessary for overrides. + val flags = original.flags | JavaVarargs // 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 + val forwarder = original.copy( flags = if isBridge then flags | Artifact else flags, - info = toJavaVarArgs(ddef.symbol.info) + info = toJavaVarArgs(original.info) ).asTerm - } - // Find a method that would conflict with the forwarder if the latter existed. + // Find methods 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)) + val conflicts = + classInfo.member(original.name).altsWith { s => + s.matches(forwarder) && !(isBridge && s.is(JavaDefined)) } - } - - conflict match - case Some(conflict) => - report.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) + conflicts match + case conflict :: _ => + val src = + if hasAnnotation then "@varargs" + else if isBridge then "overriding a java varargs method" + else "@varargs (on overriden method)" + report.error(s"$src produces a forwarder method that conflicts with ${conflict.showDcl}", original.sourcePos) + case Nil => + forwarder.enteredAfter(thisPhase) /** Convert type from Scala to Java varargs method */ private def toJavaVarArgs(tp: Type)(using Context): Type = tp match diff --git a/tests/disabled/java-interop/failing/t1459/AbstractBase.java b/tests/disabled/java-interop/failing/t1459/AbstractBase.java deleted file mode 100755 index 492419416c45..000000000000 --- a/tests/disabled/java-interop/failing/t1459/AbstractBase.java +++ /dev/null @@ -1,5 +0,0 @@ -package base; - -public abstract class AbstractBase { - public abstract void doStuff(String... params); // !!! was Object.. -} \ No newline at end of file diff --git a/tests/disabled/java-interop/failing/t1459/App.scala b/tests/disabled/java-interop/failing/t1459/App.scala deleted file mode 100644 index 36e5022e94d8..000000000000 --- a/tests/disabled/java-interop/failing/t1459/App.scala +++ /dev/null @@ -1,18 +0,0 @@ -package foo -import base._ - -object App extends scala.App { - class Concrete extends AbstractBase { - override def doStuff(params:java.lang.String*): Unit = println("doStuff invoked") - } - - val impl = new Concrete - - //succeeds - impl.doStuff(null) - - val caller = new Caller - - // fails with AbstractMethodError - caller.callDoStuff(impl) -} diff --git a/tests/disabled/java-interop/failing/t2569/Child.scala b/tests/disabled/java-interop/failing/t2569/Child.scala deleted file mode 100644 index 64f4dc172f9f..000000000000 --- a/tests/disabled/java-interop/failing/t2569/Child.scala +++ /dev/null @@ -1,9 +0,0 @@ -package varargs - - class Child extends Parent { - - override def concatenate(strings: String*): String = - strings map("\"" + _ + "\"") mkString("(", ", ", ")") - - } - diff --git a/tests/disabled/java-interop/failing/t2569/Parent.java b/tests/disabled/java-interop/failing/t2569/Parent.java deleted file mode 100644 index 89421becbdd7..000000000000 --- a/tests/disabled/java-interop/failing/t2569/Parent.java +++ /dev/null @@ -1,13 +0,0 @@ -package varargs; - - public class Parent { - - public String concatenate(String... strings) { - StringBuilder builder = new StringBuilder(); - for (String s : strings) { - builder.append(s); - } - return builder.toString(); - } - - } diff --git a/tests/disabled/java-interop/failing/varargs-bridge/A.java b/tests/disabled/java-interop/failing/varargs-bridge/A.java deleted file mode 100644 index 3cd92864ba50..000000000000 --- a/tests/disabled/java-interop/failing/varargs-bridge/A.java +++ /dev/null @@ -1,8 +0,0 @@ -package test; -public class A { - - int foo(int... x) { - return x.length; - } - -} \ No newline at end of file diff --git a/tests/disabled/java-interop/failing/varargs-bridge/B.scala b/tests/disabled/java-interop/failing/varargs-bridge/B.scala deleted file mode 100644 index efd81b0bf89a..000000000000 --- a/tests/disabled/java-interop/failing/varargs-bridge/B.scala +++ /dev/null @@ -1,7 +0,0 @@ -package test -class B extends A { - override def foo(x: Int*): Int = x.length + 1 -} -object B extends App { - println(new B().foo(1, 2, 3)) -} diff --git a/tests/neg/varargs-annot.scala b/tests/neg/varargs-annot.scala index 085c211e4081..490d2ab93695 100644 --- a/tests/neg/varargs-annot.scala +++ b/tests/neg/varargs-annot.scala @@ -11,6 +11,15 @@ object Test { @varargs def v1(a: Int, b: String*) = a + b.length // error } + trait C { + @varargs def v(i: Int*) = () + } + + class D extends C { + override def v(i: Int*) = () // error + def v(i: Array[Int]) = () // 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 @@ -25,4 +34,10 @@ object Test { @varargs def v6: Int = 1 // error @varargs def v7(i: Int*)() = i.sum // error + def f() = + @varargs def inner(s: String*) = () // error + inner("wrong") + } + +@varargs def topLevel(s: String*) = () // ok \ No newline at end of file diff --git a/tests/pos/varargs-separate/Abstract_1.scala b/tests/pos/varargs-separate/Abstract_1.scala new file mode 100644 index 000000000000..d53497ce01ee --- /dev/null +++ b/tests/pos/varargs-separate/Abstract_1.scala @@ -0,0 +1,10 @@ +import scala.annotation.varargs + +abstract class Abs { + @varargs def counter(s: String*) = () +} + +trait T { + @varargs def counter(s: String*): Unit +} + diff --git a/tests/pos/varargs-separate/Impl_2.scala b/tests/pos/varargs-separate/Impl_2.scala new file mode 100644 index 000000000000..7e2f426b7bf6 --- /dev/null +++ b/tests/pos/varargs-separate/Impl_2.scala @@ -0,0 +1,14 @@ +import scala.annotation.varargs + +class Impl extends Abs { + override def counter(s: String*): Unit = () +} + +trait B extends T { + override def counter(s: String*): Unit = () +} + +class C extends B { + override def counter(s: String*) = () +} + diff --git a/tests/run/varargs-extend-java-2/AbstractBase.java b/tests/run/varargs-extend-java-2/AbstractBase.java new file mode 100755 index 000000000000..b1fa53077ac6 --- /dev/null +++ b/tests/run/varargs-extend-java-2/AbstractBase.java @@ -0,0 +1,5 @@ +package base; + +public abstract class AbstractBase { + public abstract void doStuff(Object... params); +} diff --git a/tests/disabled/java-interop/failing/t1459/Caller.java b/tests/run/varargs-extend-java-2/Caller.java similarity index 58% rename from tests/disabled/java-interop/failing/t1459/Caller.java rename to tests/run/varargs-extend-java-2/Caller.java index 4ae51d8c5736..5c6f2ffed7c7 100755 --- a/tests/disabled/java-interop/failing/t1459/Caller.java +++ b/tests/run/varargs-extend-java-2/Caller.java @@ -2,6 +2,7 @@ public class Caller { public void callDoStuff(AbstractBase impl) { - impl.doStuff("abc"); // was new Object()); + impl.doStuff("abc"); + impl.doStuff(new Object()); } -} \ No newline at end of file +} diff --git a/tests/run/varargs-extend-java-2/Test.scala b/tests/run/varargs-extend-java-2/Test.scala new file mode 100644 index 000000000000..d704fc9881b9 --- /dev/null +++ b/tests/run/varargs-extend-java-2/Test.scala @@ -0,0 +1,13 @@ +import base._ + +object Test extends App { + class Concrete extends AbstractBase { + override def doStuff(params: AnyRef*): Unit = println("doStuff invoked") + } + + val impl = Concrete() + impl.doStuff(null) + + val caller = Caller() + caller.callDoStuff(impl) +} diff --git a/tests/run/vararg-extend-java/Vararg.java b/tests/run/varargs-extend-java/Vararg.java similarity index 100% rename from tests/run/vararg-extend-java/Vararg.java rename to tests/run/varargs-extend-java/Vararg.java diff --git a/tests/run/vararg-extend-java/VarargAbstract.java b/tests/run/varargs-extend-java/VarargAbstract.java similarity index 100% rename from tests/run/vararg-extend-java/VarargAbstract.java rename to tests/run/varargs-extend-java/VarargAbstract.java diff --git a/tests/run/vararg-extend-java/test.scala b/tests/run/varargs-extend-java/test.scala similarity index 100% rename from tests/run/vararg-extend-java/test.scala rename to tests/run/varargs-extend-java/test.scala