Skip to content

Fix #9911: Remove Override flag from varargs forwarder when there is no forwarder to override #9927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
val hasAnnotation = hasVarargsAnnotation(sym)
val hasRepeatedParam = hasRepeatedParams(sym)
if hasRepeatedParam then
if isJavaVarargsOverride || hasAnnotation || parentHasAnnotation(sym) then
val parentHasAnnotation = parentHasVarargsAnnotation(sym)
if isJavaVarargsOverride || hasAnnotation || parentHasAnnotation then
// java varargs are more restrictive than scala's
// see https://github.com/scala/bug/issues/11714
val validJava = isValidJavaVarArgs(sym.info)
Expand All @@ -54,7 +55,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
|""".stripMargin,
sym.sourcePos)
else
addVarArgsForwarder(sym, isJavaVarargsOverride, hasAnnotation)
addVarArgsForwarder(sym, isJavaVarargsOverride, hasAnnotation, parentHasAnnotation)
else if hasAnnotation then
report.error("A method without repeated parameters cannot be annotated with @varargs", sym.sourcePos)
end
Expand All @@ -79,7 +80,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>

private def hasVarargsAnnotation(sym: Symbol)(using Context) = sym.hasAnnotation(defn.VarargsAnnot)

private def parentHasAnnotation(sym: Symbol)(using Context) = sym.allOverriddenSymbols.exists(hasVarargsAnnotation)
private def parentHasVarargsAnnotation(sym: Symbol)(using Context) = sym.allOverriddenSymbols.exists(hasVarargsAnnotation)

private def isVarargsMethod(sym: Symbol)(using Context) =
hasVarargsAnnotation(sym) ||
Expand Down Expand Up @@ -259,14 +260,16 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
*
* @param original the original method symbol
* @param isBridge true if we are generating a "bridge" (synthetic override forwarder)
* @param hasAnnotation true if the method is annotated with `@varargs`
* @param parentHasAnnotation true if the method overrides a method that is annotated with `@varargs`
*
* 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 the original method.
*/
private def addVarArgsForwarder(original: Symbol, isBridge: Boolean, hasAnnotation: Boolean)(using Context): Unit =
private def addVarArgsForwarder(original: Symbol, isBridge: Boolean, hasAnnotation: Boolean, parentHasAnnotation: Boolean)(using Context): Unit =
val owner = original.owner
if !owner.isClass then
report.error("inner methods cannot be annotated with @varargs", original.sourcePos)
Expand All @@ -281,7 +284,10 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
// The java-compatible forwarder symbol
val forwarder =
original.copy(
flags = if isBridge then flags | Artifact else flags,
flags =
if isBridge then flags | Artifact
else if hasAnnotation && !parentHasAnnotation then flags &~ Override
else flags,
info = toJavaVarArgs(original.info)
).asTerm

Expand Down
24 changes: 24 additions & 0 deletions tests/pos/varargs-annot-override.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import scala.annotation.varargs

abstract class NoAnnot {
// java varargs forwarder: no
def f(args: String*): Unit
}
class B1 extends NoAnnot {
// java varargs forwarder: no
override def f(args: String*) = ()
}
class B2 extends NoAnnot {
// java varargs forwarder: yes, but it doesn't override anything
@varargs
override def f(args: String*) = ()
}
class C1 extends B2 {
// java varargs forwarder: yes, overrides parent forwarder
override def f(args: String*) = ()
}
class C2 extends B2 {
// java varargs forwarder: yes, overrides parent forwarder
@varargs
override def f(args: String*) = ()
}