SI-1459 two bridges for impl of java generic vararg method

A Scala method that implements a generic, Java-defined
varargs method, needs two bridges:
  - to convert the collections for the repeated parameters (VBRIDGE)
  - to bridge the generics gap (BRIDGE)

Refchecks emits the varargs "bridges", and erasure takes care
of the other gap. Because a VBRIDGE was also an ARTIFACT,
it was wrongly considered inert with respect to erasure,
because `OverridingPairs` by default excluded artifacts.

Removed the artifact flag from those VBRIDGES, so that they
qualify for a real bridge. It would also work to include
VBRIDGE methods that are artifacts in BridgesCursor.

7 files changed, 43 insertions, 19 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala
index 7a06c0cf2c..6b987f0089 100644
--- a/src/compiler/scala/tools/nsc/transform/Erasure.scala
+++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -411,7 +411,15 @@ abstract class Erasure extends AddInterfaces
   override def newTyper(context: Context) = new Eraser(context)
 
   class ComputeBridges(unit: CompilationUnit, root: Symbol) {
-    assert(phase == currentRun.erasurePhase, phase)
+
+    class BridgesCursor(root: Symbol) extends overridingPairs.Cursor(root) {
+      override def parents              = List(root.info.firstParent)
+      // Varargs bridges may need generic bridges due to the non-repeated part of the signature of the involved methods.
+      // The vararg bridge is generated during refchecks (probably to simplify override checking),
+      // but then the resulting varargs "bridge" method may itself need an actual erasure bridge.
+      // TODO: like javac, generate just one bridge method that wraps Seq <-> varargs and does erasure-induced casts
+      override def exclude(sym: Symbol) = !sym.isMethod || super.exclude(sym)
+    }
 
     var toBeRemoved  = immutable.Set[Symbol]()
     val site         = root.thisType
@@ -419,12 +427,7 @@ abstract class Erasure extends AddInterfaces
     val bridgeTarget = mutable.HashMap[Symbol, Symbol]()
     var bridges      = List[Tree]()
 
-    val opc = enteringExplicitOuter {
-      new overridingPairs.Cursor(root) {
-        override def parents              = List(root.info.firstParent)
-        override def exclude(sym: Symbol) = !sym.isMethod || super.exclude(sym)
-      }
-    }
+    val opc = enteringExplicitOuter { new BridgesCursor(root) }
 
     def compute(): (List[Tree], immutable.Set[Symbol]) = {
       while (opc.hasNext) {
@@ -811,6 +814,16 @@ abstract class Erasure extends AddInterfaces
       }
     }
 
+    private class DoubleDefsCursor(root: Symbol) extends Cursor(root) {
+      // specialized members have no type history before 'specialize', causing double def errors for curried defs
+      override def exclude(sym: Symbol): Boolean = (
+           sym.isType
+        || super.exclude(sym)
+        || !sym.hasTypeAt(currentRun.refchecksPhase.id)
+      )
+      override def matches(lo: Symbol, high: Symbol) = !high.isPrivate
+    }
+
     /** Emit an error if there is a double definition. This can happen if:
      *
      *  - A template defines two members with the same name and erased type.
@@ -821,21 +834,12 @@ abstract class Erasure extends AddInterfaces
      */
     private def checkNoDoubleDefs(root: Symbol) {
       checkNoDeclaredDoubleDefs(root)
-      object opc extends Cursor(root) {
-        // specialized members have no type history before 'specialize', causing double def errors for curried defs
-        override def exclude(sym: Symbol): Boolean = (
-             sym.isType
-          || super.exclude(sym)
-          || !sym.hasTypeAt(currentRun.refchecksPhase.id)
-        )
-        override def matches(lo: Symbol, high: Symbol) = !high.isPrivate
-      }
       def isErasureDoubleDef(pair: SymbolPair) = {
         import pair._
         log(s"Considering for erasure clash:\n$pair")
         !exitingRefchecks(lowType matches highType) && sameTypeAfterErasure(low, high)
       }
-      opc.iterator filter isErasureDoubleDef foreach doubleDefError
+      (new DoubleDefsCursor(root)).iterator filter isErasureDoubleDef foreach doubleDefError
     }
 
     /**  Add bridge definitions to a template. This means:
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index 3b2e07bdbd..0b44566108 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -193,7 +193,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
         def varargBridge(member: Symbol, bridgetpe: Type): Tree = {
           log(s"Generating varargs bridge for ${member.fullLocationString} of type $bridgetpe")
 
-          val newFlags = (member.flags | VBRIDGE | ARTIFACT) & ~PRIVATE
+          val newFlags = (member.flags | VBRIDGE) & ~PRIVATE
           val bridge   = member.cloneSymbolImpl(clazz, newFlags) setPos clazz.pos
           bridge.setInfo(bridgetpe.cloneInfo(bridge))
           clazz.info.decls enter bridge
diff --git a/src/reflect/scala/reflect/internal/Flags.scala b/src/reflect/scala/reflect/internal/Flags.scala
index 754b96a9dd..64273f005f 100644
--- a/src/reflect/scala/reflect/internal/Flags.scala
+++ b/src/reflect/scala/reflect/internal/Flags.scala
@@ -164,7 +164,7 @@ class Flags extends ModifierFlags {
 
   final val LOCKED        = 1L << 39      // temporary flag to catch cyclic dependencies
   final val SPECIALIZED   = 1L << 40      // symbol is a generated specialized member
-  final val VBRIDGE       = 1L << 42      // symbol is a varargs bridge
+  final val VBRIDGE       = 1L << 42      // symbol is a varargs bridge (but not a bridge at the bytecode level)
 
   final val VARARGS       = 1L << 43      // symbol is a Java-style varargs method
   final val TRIEDCOOKING  = 1L << 44      // `Cooking` has been tried on this symbol
diff --git a/test/files/run/t1459generic.check b/test/files/run/t1459generic.check
new file mode 100644
index 0000000000..367ac87ad1
--- /dev/null
+++ b/test/files/run/t1459generic.check
@@ -0,0 +1,2 @@
+ab
+ab
diff --git a/test/files/run/t1459generic/Impl.scala b/test/files/run/t1459generic/Impl.scala
new file mode 100644
index 0000000000..9234e70456
--- /dev/null
+++ b/test/files/run/t1459generic/Impl.scala
@@ -0,0 +1,4 @@
+class Impl extends VarargGeneric[String] {
+  def genericOne(x: String, arg: String): String = x + arg
+  def genericVar(x: String, args: String*): String = x + args.head
+}
diff --git a/test/files/run/t1459generic/Test.java b/test/files/run/t1459generic/Test.java
new file mode 100644
index 0000000000..a97158796b
--- /dev/null
+++ b/test/files/run/t1459generic/Test.java
@@ -0,0 +1,10 @@
+public class Test {
+  public static void main(String[] args) throws Exception {
+    VarargGeneric vg = new Impl();
+    System.out.println(vg.genericOne("a", "b"));
+    System.out.println(vg.genericVar("a", "b"));
+    // should not result in java.lang.AbstractMethodError: Impl.genericVar(Ljava/lang/Object;[Ljava/lang/String;)Ljava/lang/String;
+    // --> genericVar needs a varargs bridge (scala -> java varargs) and a standard generics bridge
+    // (for comparison, including genericOne, which needs only a generics bridge)
+  }
+}
diff --git a/test/files/run/t1459generic/VarargGeneric.java b/test/files/run/t1459generic/VarargGeneric.java
new file mode 100644
index 0000000000..c043e39b40
--- /dev/null
+++ b/test/files/run/t1459generic/VarargGeneric.java
@@ -0,0 +1,4 @@
+public interface VarargGeneric<T> {
+  String genericOne(T x, String args);
+  String genericVar(T x, String... args);
+}