From 579c01b41841bd361dd8a32db923cc47b9bde4b8 Mon Sep 17 00:00:00 2001
From: Guillaume Raffin <theelectronwill@gmail.com>
Date: Thu, 1 Oct 2020 15:22:46 +0200
Subject: [PATCH] Remove Override flag from varargs forwarders when there is no
 forwarder to override

---
 .../tools/dotc/transform/ElimRepeated.scala   | 16 +++++++++----
 tests/pos/varargs-annot-override.scala        | 24 +++++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)
 create mode 100644 tests/pos/varargs-annot-override.scala

diff --git a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala
index ecf2995cf30a..c02a434da7ef 100644
--- a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala
+++ b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala
@@ -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)
@@ -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
@@ -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) ||
@@ -259,6 +260,8 @@ 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
@@ -266,7 +269,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
    *  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)
@@ -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
 
diff --git a/tests/pos/varargs-annot-override.scala b/tests/pos/varargs-annot-override.scala
new file mode 100644
index 000000000000..8b42f093867a
--- /dev/null
+++ b/tests/pos/varargs-annot-override.scala
@@ -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*) = ()
+}