SI-5652 Mangle names of potentially public lambda lifted methods.

This can happen if they are accessed from an inner class. If a
subclass is happens to lift a public method to the same name,
a VerifyError ensues.

The enclosed tests:
 - demonstrate the absense of the VerifyError
 - show the names generated for the lifted methods (which are
   unchanged if not called from an inner class, or if lifted
   into a trait implementation class.)
 - ensure that the callers are rewritten to call the correct
   method when multiple with the same name are lifted.

It's not ideal that this phase needs a priori knowledge of the
later phases to perform this mangling. A better fix would defer
this until the point when the methods are publicised, and leave
the unmangled private method in place and install an public,
mangled forwarder.

9 files changed, 76 insertions, 12 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
index 718e58b855..fad41ae98d 100644
--- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
+++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
@@ -10,7 +10,7 @@ import symtab._
 import Flags._
 import util.TreeSet
 import scala.collection.{ mutable, immutable }
-import scala.collection.mutable.LinkedHashMap
+import scala.collection.mutable.{ LinkedHashMap, LinkedHashSet }
 
 abstract class LambdaLift extends InfoTransform {
   import global._
@@ -50,6 +50,9 @@ abstract class LambdaLift extends InfoTransform {
     /** A hashtable storing calls between functions */
     private val called = new LinkedHashMap[Symbol, SymSet]
 
+    /** Symbols that are called from an inner class. */
+    private val calledFromInner = new LinkedHashSet[Symbol]
+
     /** The set of symbols that need to be renamed. */
     private val renamable = newSymSet
 
@@ -64,9 +67,6 @@ abstract class LambdaLift extends InfoTransform {
     /** Buffers for lifted out classes and methods */
     private val liftedDefs = new LinkedHashMap[Symbol, List[Tree]]
 
-    /** True if we are transforming under a ReferenceToBoxed node */
-    private var isBoxedRef = false
-
     private type SymSet = TreeSet[Symbol]
 
     private def newSymSet = new TreeSet[Symbol](_ isLess _)
@@ -138,6 +138,7 @@ abstract class LambdaLift extends InfoTransform {
     private def markCalled(sym: Symbol, owner: Symbol) {
       debuglog("mark called: " + sym + " of " + sym.owner + " is called by " + owner)
       symSet(called, owner) addEntry sym
+      if (sym.enclClass != owner.enclClass) calledFromInner addEntry sym
     }
 
     /** The traverse function */
@@ -214,15 +215,23 @@ abstract class LambdaLift extends InfoTransform {
 
       def renameSym(sym: Symbol) {
         val originalName = sym.name
-        val base = sym.name + nme.NAME_JOIN_STRING + (
-          if (sym.isAnonymousFunction && sym.owner.isMethod)
-            sym.owner.name + nme.NAME_JOIN_STRING
-          else ""
-        )
-        sym setName (
-          if (sym.name.isTypeName) unit.freshTypeName(base)
-          else unit.freshTermName(base)
+        def freshen(prefix: String): Name =
+          if (originalName.isTypeName) unit.freshTypeName(prefix)
+          else unit.freshTermName(prefix)
+
+        val newName: Name = (
+          if (sym.isAnonymousFunction && sym.owner.isMethod) {
+            freshen(sym.name + nme.NAME_JOIN_STRING + sym.owner.name + nme.NAME_JOIN_STRING)
+          } else {
+            // SI-5652 If the lifted symbol is accessed from an inner class, it will be made public. (where?)
+            //         Generating a a unique name, mangled with the enclosing class name, avoids a VerifyError
+            //         in the case that a sub-class happens to lifts out a method with the *same* name.
+            val name = freshen(sym.name + nme.NAME_JOIN_STRING)
+            if (originalName.isTermName && !sym.enclClass.isImplClass && calledFromInner(sym)) nme.expandedName(name, sym.enclClass)
+            else name
+          }
         )
+        sym setName newName
         debuglog("renaming in %s: %s => %s".format(sym.owner.fullLocationString, originalName, sym.name))
       }
 
diff --git a/test/files/run/t5652.check b/test/files/run/t5652.check
new file mode 100644
index 0000000000..11438ef217
--- /dev/null
+++ b/test/files/run/t5652.check
@@ -0,0 +1,8 @@
+public static final int T1$class.g$1(T1)
+public static int T1$class.f0(T1)
+public static void T1$class.$init$(T1)
+public final int A1.A1$$g$2()
+public int A1.f1()
+public final int A2.A2$$g$1()
+public int A2.f0()
+public int A2.f2()
diff --git a/test/files/run/t5652/t5652_1.scala b/test/files/run/t5652/t5652_1.scala
new file mode 100644
index 0000000000..5343f260a2
--- /dev/null
+++ b/test/files/run/t5652/t5652_1.scala
@@ -0,0 +1,6 @@
+trait T1 {
+  def f0 = { def g = 1 ; class A { def a = g } ; g ; new A().a }
+}
+class A1 {
+  def f1 = { def g = 1 ; class A { def a = g } ; g ; new A().a }
+}
diff --git a/test/files/run/t5652/t5652_2.scala b/test/files/run/t5652/t5652_2.scala
new file mode 100644
index 0000000000..765d16f8f5
--- /dev/null
+++ b/test/files/run/t5652/t5652_2.scala
@@ -0,0 +1,9 @@
+class A2 extends A1 with T1{
+	def f2 = { def g = 5 ; class A { def a = g }; g ; new A().a }
+}
+
+object Test extends A2 {
+	def main(args: Array[String]) {
+    println(Seq(Class.forName(classOf[T1].getName + "$class"), classOf[A1], classOf[A2]).flatMap(_.getDeclaredMethods.map(_.toString).sorted).mkString("\n"))
+  }
+}
diff --git a/test/files/run/t5652b.check b/test/files/run/t5652b.check
new file mode 100644
index 0000000000..ca9d0a74f0
--- /dev/null
+++ b/test/files/run/t5652b.check
@@ -0,0 +1,4 @@
+private final int A1.g$1()
+public int A1.f1()
+private final int A2.g$1()
+public int A2.f2()
diff --git a/test/files/run/t5652b/t5652b_1.scala b/test/files/run/t5652b/t5652b_1.scala
new file mode 100644
index 0000000000..72ba5dcd82
--- /dev/null
+++ b/test/files/run/t5652b/t5652b_1.scala
@@ -0,0 +1,3 @@
+class A1 {
+  def f1 = { def g = 5 ; class A { def a = 0 } ; new A; g }
+}
diff --git a/test/files/run/t5652b/t5652b_2.scala b/test/files/run/t5652b/t5652b_2.scala
new file mode 100644
index 0000000000..113736a24b
--- /dev/null
+++ b/test/files/run/t5652b/t5652b_2.scala
@@ -0,0 +1,9 @@
+class A2 extends A1 {
+	def f2 = { def g = 5 ; class A { def a = 0 } ; new A; g }
+}
+
+object Test extends A2 {
+  def main(args: Array[String]) {
+    println(Seq(classOf[A1], classOf[A2]).flatMap(_.getDeclaredMethods.map(_.toString).sorted).mkString("\n"))
+  }
+}
diff --git a/test/files/run/t5652c.check b/test/files/run/t5652c.check
new file mode 100644
index 0000000000..3b889e066d
--- /dev/null
+++ b/test/files/run/t5652c.check
@@ -0,0 +1,6 @@
+public final int A1.A1$$g$1()
+public final int A1.A1$$g$2()
+public int A1.f1()
+public int A1.f2()
+1
+2
diff --git a/test/files/run/t5652c/t5652c.scala b/test/files/run/t5652c/t5652c.scala
new file mode 100644
index 0000000000..c977483280
--- /dev/null
+++ b/test/files/run/t5652c/t5652c.scala
@@ -0,0 +1,10 @@
+class A1 {
+  def f1 = { def g = 1 ; class A { def a = g } ; new A().a }
+  def f2 = { def g = 2 ; class A { def a = g } ; new A().a }
+}
+
+object Test extends App {
+  println(classOf[A1].getDeclaredMethods.map(_.toString).sorted.mkString("\n"))
+  println(new A1().f1)
+  println(new A1().f2)
+}