SI-6260 Avoid double-def error with lambdas over value classes

Post-erasure of value classs in method signatures to the underlying
type wreaks havoc when the erased signature overlaps with the
generic signature from an overriden method. There just isn't room
for both. But we *really* need both; callers to the interface method
will be passing boxed values that the bridge needs to unbox and
pass to the specific method that accepts unboxed values.

This most commonly turns up with value classes that erase to
Object that are used as the parameter or the return type of
an anonymous function.

This was thought to have been intractable, unless we chose
a different name for the unboxed, specific method in the
subclass. But that sounds like a big task that would require
call-site rewriting, ala specialization.

But there is an important special case in which we don't need
to rewrite call sites. If the class defining the method is
anonymous, there is actually no need for the unboxed method;
it will *only* ever be called via the generic method.

I came to this realisation when looking at how Java 8 lambdas
are handled. I was expecting bridge methods, but found none.
The lambda body is placed directly in a method exactly matching
the generic signature.

This commit detects the clash between bridge and target,
and recovers for anonymous classes by mangling the name
of the target method's symbol. This is used as the bytecode
name. The generic bridge forward to that, as before, with
the requisite box/unbox operations.

17 files changed, 94 insertions, 41 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
index 933a2f70a1..b28a4a507d 100644
--- a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
+++ b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
@@ -282,18 +282,9 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
         if (sym == NoSymbol) sym.toString
         else s"$sym: ${sym.tpe} in ${sym.owner}"
 
-      def clashError(bm: Symbol) = {
-        unit.error(
-          applyMethodDef.symbol.pos,
-            sm"""bridge generated for member ${fulldef(applyMethodDef.symbol)}
-                |which overrides ${fulldef(getMember(abstractFunctionErasedType.typeSymbol, nme.apply))}
-                |clashes with definition of the member itself;
-                |both have erased type ${exitingPostErasure(bm.tpe)}""")
-        }
-
         bridgeMethod foreach (bm =>
           if (bm.symbol.tpe =:= applyMethodDef.symbol.tpe)
-            clashError(bm.symbol)
+            erasure.expandNameOfMethodThatClashesBridge(applyMethodDef.symbol)
         )
 
         val body = members ++ List(constr, applyMethodDef) ++ bridgeMethod
diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala
index ccfddab94a..e2ffafb850 100644
--- a/src/compiler/scala/tools/nsc/transform/Erasure.scala
+++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -403,19 +403,19 @@ abstract class Erasure extends AddInterfaces
      *  @param  other    The overidden symbol for which the bridge was generated
      *  @param  bridge   The bridge
      */
-    def checkBridgeOverrides(member: Symbol, other: Symbol, bridge: Symbol): Boolean = {
+    def checkBridgeOverrides(member: Symbol, other: Symbol, bridge: Symbol): Seq[(Position, String)] = {
       def fulldef(sym: Symbol) =
         if (sym == NoSymbol) sym.toString
         else s"$sym: ${sym.tpe} in ${sym.owner}"
       var noclash = true
+      val clashErrors = mutable.Buffer[(Position, String)]()
       def clashError(what: String) = {
-        noclash = false
-        unit.error(
-          if (member.owner == root) member.pos else root.pos,
-          sm"""bridge generated for member ${fulldef(member)}
-              |which overrides ${fulldef(other)}
-              |clashes with definition of $what;
-              |both have erased type ${exitingPostErasure(bridge.tpe)}""")
+        val pos = if (member.owner == root) member.pos else root.pos
+        val msg = sm"""bridge generated for member ${fulldef(member)}
+                      |which overrides ${fulldef(other)}
+                      |clashes with definition of $what;
+                      |both have erased type ${exitingPostErasure(bridge.tpe)}"""
+        clashErrors += Tuple2(pos, msg)
       }
       for (bc <- root.baseClasses) {
         if (settings.debug)
@@ -440,7 +440,7 @@ abstract class Erasure extends AddInterfaces
           }
         }
       }
-      noclash
+      clashErrors
     }
 
     /** TODO - work through this logic with a fine-toothed comb, incorporating
@@ -478,8 +478,18 @@ abstract class Erasure extends AddInterfaces
       bridge setInfo (otpe cloneInfo bridge)
       bridgeTarget(bridge) = member
 
-      if (!(member.tpe exists (_.typeSymbol.isDerivedValueClass)) ||
-          checkBridgeOverrides(member, other, bridge)) {
+      def sigContainsValueClass = (member.tpe exists (_.typeSymbol.isDerivedValueClass))
+
+      val shouldAdd = (
+            !sigContainsValueClass
+        ||  (checkBridgeOverrides(member, other, bridge) match {
+              case Nil => true
+              case es if member.owner.isAnonymousClass => expandNameOfMethodThatClashesBridge(member); true
+              case es => for ((pos, msg) <- es) unit.error(pos, msg); false
+            })
+      )
+
+      if (shouldAdd) {
         exitingErasure(root.info.decls enter bridge)
         if (other.owner == root) {
           exitingErasure(root.info.decls.unlink(other))
@@ -1127,5 +1137,10 @@ abstract class Erasure extends AddInterfaces
     }
   }
 
+  final def expandNameOfMethodThatClashesBridge(sym: Symbol) {
+    log(s"Expanding name of ${sym.debugLocationString} as it clashes with bridge. Renaming deemed safe because the owner is anonymous.")
+    sym.expandName(sym.owner)
+  }
+
   private class TypeRefAttachment(val tpe: TypeRef)
 }
diff --git a/test/files/neg/t6260-named.check b/test/files/neg/t6260-named.check
new file mode 100644
index 0000000000..ed6ab5e76f
--- /dev/null
+++ b/test/files/neg/t6260-named.check
@@ -0,0 +1,13 @@
+t6260-named.scala:12: error: bridge generated for member method apply: (a: C[Any])C[Any] in object O
+which overrides method apply: (v1: T1)R in trait Function1
+clashes with definition of the member itself;
+both have erased type (v1: Object)Object
+    def apply(a: C[Any]) = a
+        ^
+t6260-named.scala:14: error: bridge generated for member method apply: (a: C[Any])C[Any] in class X
+which overrides method apply: (a: A)A in trait T
+clashes with definition of the member itself;
+both have erased type (a: Object)Object
+  class X extends T[C[Any]] { def apply(a: C[Any]) = a }
+                                  ^
+two errors found
diff --git a/test/files/neg/t6260-named.scala b/test/files/neg/t6260-named.scala
new file mode 100644
index 0000000000..7ce13476eb
--- /dev/null
+++ b/test/files/neg/t6260-named.scala
@@ -0,0 +1,15 @@
+class C[A](private val a: Any) extends AnyVal
+trait T[A] {
+  def apply(a: A): A
+}
+
+object Test {
+  (x: C[Any]) => {println(s"f($x)"); x} // okay
+  new T[C[Any]] { def apply(a: C[Any]) = a } // okay
+
+  // we can't rename the specific apply methid to avoid the clash
+  object O extends Function1[C[Any], C[Any]] {
+    def apply(a: C[Any]) = a
+  }
+  class X extends T[C[Any]] { def apply(a: C[Any]) = a }
+}
diff --git a/test/files/neg/t6260.check b/test/files/neg/t6260.check
deleted file mode 100644
index 60c4add143..0000000000
--- a/test/files/neg/t6260.check
+++ /dev/null
@@ -1,13 +0,0 @@
-t6260.scala:3: error: bridge generated for member method apply: (bx: Box[X])Box[Y] in <$anon: Box[X] => Box[Y]>
-which overrides method apply: (v1: T1)R in trait Function1
-clashes with definition of the member itself;
-both have erased type (v1: Object)Object
-    ((bx: Box[X]) => new Box(f(bx.x)))(this)
-                  ^
-t6260.scala:8: error: bridge generated for member method apply: (bx: Box[X])Box[Y] in <$anon: Box[X] => Box[Y]>
-which overrides method apply: (v1: T1)R in trait Function1
-clashes with definition of the member itself;
-both have erased type (v1: Object)Object
-    ((bx: Box[X]) => new Box(f(bx.x)))(self)
-                  ^
-two errors found
diff --git a/test/files/neg/t6260b.check b/test/files/neg/t6260b.check
deleted file mode 100644
index 3a7e8947aa..0000000000
--- a/test/files/neg/t6260b.check
+++ /dev/null
@@ -1,7 +0,0 @@
-t6260b.scala:3: error: bridge generated for member method apply: ()X in <$anon: () => X>
-which overrides method apply: ()R in trait Function0
-clashes with definition of the member itself;
-both have erased type ()Object
-class Y { def f = new X("") or new X("") }
-                               ^
-one error found
diff --git a/test/files/neg/delambdafy_t6260_method.check b/test/files/pos/delambdafy_t6260_method.check
index f5cd6947d1..f5cd6947d1 100644
--- a/test/files/neg/delambdafy_t6260_method.check
+++ b/test/files/pos/delambdafy_t6260_method.check
diff --git a/test/files/neg/delambdafy_t6260_method.flags b/test/files/pos/delambdafy_t6260_method.flags
index 48b438ddf8..48b438ddf8 100644
--- a/test/files/neg/delambdafy_t6260_method.flags
+++ b/test/files/pos/delambdafy_t6260_method.flags
diff --git a/test/files/neg/delambdafy_t6260_method.scala b/test/files/pos/delambdafy_t6260_method.scala
index 93b5448227..93b5448227 100644
--- a/test/files/neg/delambdafy_t6260_method.scala
+++ b/test/files/pos/delambdafy_t6260_method.scala
diff --git a/test/files/neg/t6260.flags b/test/files/pos/t6260.flags
index 2349d8294d..2349d8294d 100644
--- a/test/files/neg/t6260.flags
+++ b/test/files/pos/t6260.flags
diff --git a/test/files/neg/t6260.scala b/test/files/pos/t6260.scala
index 93b5448227..93b5448227 100644
--- a/test/files/neg/t6260.scala
+++ b/test/files/pos/t6260.scala
diff --git a/test/files/neg/t6260b.scala b/test/files/pos/t6260b.scala
index 73e2e58f73..73e2e58f73 100644
--- a/test/files/neg/t6260b.scala
+++ b/test/files/pos/t6260b.scala
diff --git a/test/files/run/t6260-delambdafy.check b/test/files/run/t6260-delambdafy.check
new file mode 100644
index 0000000000..b2a7bed988
--- /dev/null
+++ b/test/files/run/t6260-delambdafy.check
@@ -0,0 +1,4 @@
+f(C@2e)
+
+Test$lambda$1$$apply
+apply
diff --git a/test/files/run/t6260-delambdafy.flags b/test/files/run/t6260-delambdafy.flags
new file mode 100644
index 0000000000..48b438ddf8
--- /dev/null
+++ b/test/files/run/t6260-delambdafy.flags
@@ -0,0 +1 @@
+-Ydelambdafy:method
diff --git a/test/files/run/t6260-delambdafy.scala b/test/files/run/t6260-delambdafy.scala
new file mode 100644
index 0000000000..056b1edd4e
--- /dev/null
+++ b/test/files/run/t6260-delambdafy.scala
@@ -0,0 +1,12 @@
+class C[A](private val a: Any) extends AnyVal
+
+object Test {
+  val f = (x: C[Any]) => {println(s"f($x)"); x}
+  def main(args: Array[String]) {
+     f(new C("."))
+     val methods = f.getClass.getDeclaredMethods.map(_.getName).sorted
+     println("")
+     println(methods.mkString("\n"))
+  }
+}
+
diff --git a/test/files/run/t6260c.check b/test/files/run/t6260c.check
new file mode 100644
index 0000000000..1a57f2d741
--- /dev/null
+++ b/test/files/run/t6260c.check
@@ -0,0 +1,5 @@
+f(C@2e)
+
+Test$$anonfun$$apply
+apply
+g(C@2e)
diff --git a/test/files/run/t6260c.scala b/test/files/run/t6260c.scala
new file mode 100644
index 0000000000..845dc157b7
--- /dev/null
+++ b/test/files/run/t6260c.scala
@@ -0,0 +1,17 @@
+class C[A](private val a: Any) extends AnyVal
+
+object Test {
+  val f = (x: C[Any]) => {println(s"f($x)"); x}
+  trait T[A] {
+    def apply(a: A): A
+  }
+  val g = new T[C[Any]] { def apply(a: C[Any]) = { println(s"g($a)"); a } }
+  def main(args: Array[String]) {
+     f(new C("."))
+     val methods = f.getClass.getDeclaredMethods.map(_.getName).sorted
+     println("")
+     println(methods.mkString("\n"))
+     g.apply(new C("."))
+  }
+}
+