Fix scala-js#2382: Mangle $outer fields at link time.

As a deserialization hack, we mangle all Scala fields that are called `$$outer$f`, i.e., those that were `protected val $outer` in scalac. We mangle them with their type, which is always locally known (in `FieldDef`s and `Select`s, which are the only two IR nodes dealing with Scala fields). This almost fixes the name clashes. It is still possible to have a clash for classes that are nested in the same class, but for those, all the clashing `$outer` pointers must have the same value, and so the clash is not a problem in practice. For `$outer` pointers at different levels of nesting, which are the ones that were causing scala-js#2382, the type of the `$outer` pointers must be different, by construction. The mangling by type therefore always distinguishes them.
sjrd · May 25, 2016 · eb08405 · eb08405
1 parent bcb23cc
commit eb08405
Show file tree

Hide file tree

Showing 3 changed files with 188 additions and 4 deletions.
diff --git a/ir/src/main/scala/org/scalajs/core/ir/Serializers.scala b/ir/src/main/scala/org/scalajs/core/ir/Serializers.scala
@@ -589,6 +589,8 @@ object Serializers {
       Set("0.6.0", "0.6.3", "0.6.4", "0.6.5").contains(sourceVersion)
     private[this] val useHacks066 =
       useHacks065 || sourceVersion == "0.6.6"
+    private[this] val useHacks068 =
+      true // useHacks066 || sourceVersion == "0.6.8"
 
     private[this] val input = new DataInputStream(stream)
 
@@ -638,7 +640,11 @@ object Serializers {
         case TagNew             => New(readClassType(), readIdent(), readTrees())
         case TagLoadModule      => LoadModule(readClassType())
         case TagStoreModule     => StoreModule(readClassType(), readTree())
-        case TagSelect          => Select(readTree(), readIdent())(readType())
+        case TagSelect          =>
+          val qual = readTree()
+          val ident = readIdent()
+          val tpe = readType()
+          Select(qual, patchFieldIdent068(ident, tpe))(tpe)
         case TagApply           => Apply(readTree(), readIdent(), readTrees())(readType())
         case TagApplyStatically =>
           val result1 =
@@ -731,7 +737,10 @@ object Serializers {
           ClassDef(name, kind, superClass, parents, jsName, defs)(optimizerHints)
 
         case TagFieldDef =>
-          FieldDef(readIdent(), readType(), readBoolean())
+          val ident = readIdent()
+          val tpe = readType()
+          val mutable = readBoolean()
+          FieldDef(patchFieldIdent068(ident, tpe), tpe, mutable)
         case TagStringLitFieldDef =>
           /* TODO Merge this into TagFieldDef and use readPropertyName()
            * when we can break binary compatibility.
@@ -784,6 +793,19 @@ object Serializers {
       result
     }
 
+    private def patchFieldIdent068(ident: Ident, tpe: Type): Ident = {
+      if (useHacks068) {
+        (ident, tpe) match {
+          case (Ident("$$outer$f", origName), ClassType(cls)) =>
+            Ident("$$outer$" + cls + "$f", origName)(ident.pos)
+          case _ =>
+            ident
+        }
+      } else {
+        ident
+      }
+    }
+
     def readTrees(): List[Tree] =
       List.fill(input.readInt())(readTree())
 

diff --git a/test-suite/shared/src/test/scala/org/scalajs/testsuite/compiler/OuterClassTest.scala b/test-suite/shared/src/test/scala/org/scalajs/testsuite/compiler/OuterClassTest.scala
@@ -0,0 +1,144 @@
+package org.scalajs.testsuite.compiler
+
+import org.junit.Test
+import org.junit.Assert._
+
+class OuterClassTest {
+
+  @Test def `Test code variant 1 from #2382`(): Unit = {
+    val b1 = new B1 {}
+    val y1 = new b1.Y1
+    val z1 = new y1.Z1
+
+    assertEquals(1, b1.a)
+    assertEquals(1, z1.z1)
+    assertEquals(1, z1.z2)
+    assertEquals(2, y1.y)
+  }
+
+  @Test def `Test code variant 2 from #2382`(): Unit = {
+    val b2 = new B2 {}
+    val y2 = new b2.Y2
+    val z2 = new y2.Z2
+
+    assertEquals(1, b2.a)
+    assertEquals(1, z2.z1)
+    assertEquals(2, y2.y)
+    assertEquals(2, z2.z2)
+  }
+
+  @Test def `Test code variant 3 from #2382`(): Unit = {
+    val a3 = new A3 {}
+    val y3 = new a3.Y3
+    val z3 = new y3.Z3
+
+    assertEquals(1, a3.a)
+    assertEquals(1, z3.b)
+  }
+
+  @Test def `Test code variant 4 from #2382`(): Unit = {
+    val a4 = new A4 {}
+    val y4 = new a4.Y4
+    val z4 = new y4.Z4
+
+    assertEquals(1, a4.a)
+    assertEquals(1, z4.b)
+  }
+
+  @Test def `Test code variant 5 from #2382`(): Unit = {
+    val a5 = new A5 {}
+    val y5 = new a5.Y5
+    val z5 = new y5.Z5
+
+    assertEquals(1, a5.a)
+    assertEquals(1, z5.b)
+    assertEquals(2, z5.c)
+    assertEquals(2, z5.d)
+    assertEquals(3, z5.e)
+    assertEquals(3, z5.f)
+  }
+}
+
+// Code from issue #2382 variant 1
+
+trait A1 {
+  val a: Int = 1
+  class X1 {
+    def x: Int = a
+  }
+
+}
+
+trait B1 extends A1 {
+  class Y1 {
+    def y: Int = 2
+    class Z1 extends X1 {
+      def z1: Int = a
+      def z2: Int = x
+    }
+  }
+}
+
+// Code from issue #2382 variant 2
+
+trait A2 {
+  val a: Int = 1
+  class X2
+}
+
+trait B2 extends A2 {
+  class Y2 {
+    def y: Int = 2
+    class Z2 extends X2 {
+      def z1: Int = a
+      def z2: Int = y
+    }
+  }
+}
+
+// Code from issue #2382 variant 2
+
+trait A3 {
+  val a: Int = 1
+  class X3
+  class Y3 {
+    class Z3 extends X3 {
+      def b: Int = a
+    }
+  }
+}
+
+// Code from issue #2382 variant 4
+
+class A4 {
+  val a: Int = 1
+  class X4
+  class Y4 {
+    class Z4 extends X4 {
+      def b: Int = a
+    }
+  }
+}
+
+// Code from issue #2382 variant 5
+
+class A5 {
+  val a: Int = 1
+  trait B5 {
+    def c: Int = 2
+  }
+  trait C5 extends B5
+  trait D5 extends C5
+  class X5 {
+    def e: Int = 3
+  }
+  trait U5 extends X5
+  trait S5 extends U5
+  class Y5 extends S5 {
+    class Z5 extends X5 with D5 {
+      def b: Int = a
+      def d: Int = c
+      def f: Int = e
+    }
+  }
+}
diff --git a/tools/shared/src/main/scala/org/scalajs/core/tools/linker/checker/IRChecker.scala b/tools/shared/src/main/scala/org/scalajs/core/tools/linker/checker/IRChecker.scala
@@ -11,7 +11,7 @@ package org.scalajs.core.tools.linker.checker
 
 import scala.language.implicitConversions
 
-import scala.annotation.switch
+import scala.annotation.{switch, tailrec}
 
 import scala.collection.mutable
 
@@ -155,7 +155,25 @@ private final class IRChecker(unit: LinkingUnit, logger: Logger) {
 
     name match {
       case _: Ident =>
-        // ok
+        /* Check that the field is not already defined in one of the super
+         * classes.
+         * FIXME This should be enabled when we can break binary compatibility.
+         */
+        if (false) {
+          @tailrec def checkClass(superClass: CheckedClass): Unit = {
+            if (superClass.fields.contains(name.name)) {
+              reportError(s"FieldDef '$name' of '${classDef.name.name}' is " +
+                  s"already defined in '${superClass.name}'.")
+            } else {
+              superClass.superClass match {
+                case Some(cls) => checkClass(cls)
+                case None      => // ok
+              }
+            }
+          }
+          classDef.superClass.foreach(cls => checkClass(lookupClass(cls.name)))
+        }
+
       case _: StringLiteral =>
         if (!classDef.kind.isJSClass)
           reportError(s"FieldDef '$name' cannot have a string literal name")