Fix InlineInfo attribute for nested module accessors

2 files changed, 74 insertions, 34 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
index 99d4390873..7786dbed7a 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
@@ -263,11 +263,13 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
           val name      = methodSym.javaSimpleName.toString // same as in genDefDef
           val signature = name + methodSymToDescriptor(methodSym)
 
-          // Some detours are required here because of changing flags (lateDEFERRED):
-          // 1. Why the phase travel? Concrete trait methods obtain the lateDEFERRED flag in Mixin.
-          //    This makes isEffectivelyFinalOrNotOverridden false, which would prevent non-final
-          //    but non-overridden methods of sealed traits from being inlined.
-          val effectivelyFinal = exitingPickler(methodSym.isEffectivelyFinalOrNotOverridden) && !(methodSym.owner.isTrait && methodSym.isModule)
+          // In `trait T { object O }`, `oSym.isEffectivelyFinalOrNotOverridden` is true, but the
+          // method is abstract in bytecode, `defDef.rhs.isEmpty`. Abstract methods are excluded
+          // so they are not marked final in the InlineInfo attribute.
+          //
+          // However, due to https://github.com/scala/scala-dev/issues/126, this currently does not
+          // work, the abstract accessor for O will be marked effectivelyFinal.
+          val effectivelyFinal = methodSym.isEffectivelyFinalOrNotOverridden && !methodSym.isDeferred
 
           val info = MethodInlineInfo(
             effectivelyFinal                    = effectivelyFinal,
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala
index f449b8bb45..4db2657c1b 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala
@@ -29,12 +29,24 @@ class ScalaInlineInfoTest extends ClearAfterClass {
 
   def inlineInfo(c: ClassNode): InlineInfo = c.attrs.asScala.collect({ case a: InlineInfoAttribute => a.inlineInfo }).head
 
+  def mapDiff[A, B](a: Map[A, B], b: Map[A, B]) = {
+    val r = new StringBuilder
+    for ((a, av) <- a) {
+      if (!b.contains(a)) r.append(s"missing in b: $a\n")
+      else if (av != b(a)) r.append(s"different for $a: $av != ${b(a)}\n")
+    }
+    for (b <- b.keys.toList diff a.keys.toList) {
+      r.append(s"missing in a: $b\n")
+    }
+    r.toString
+  }
+
   @Test
   def traitMembersInlineInfo(): Unit = {
     val code =
       """trait T {
         |  def f1 = 1                   // concrete method
-        |  private def f2 = 1           // implOnly method (does not end up in the interface)
+        |  private def f2 = 1           // default method only (not in subclass)
         |  def f3 = {
         |    def nest = 0               // nested method (does not end up in the interface)
         |    nest
@@ -44,13 +56,13 @@ class ScalaInlineInfoTest extends ClearAfterClass {
         |  def f4 = super.toString      // super accessor
         |
         |  object O                     // module accessor (method is generated)
-        |  def f5 = {
+        |  final def f5 = {
         |    object L { val x = 0 }     // nested module (just flattened out)
         |    L.x
         |  }
         |
         |  @noinline
-        |  def f6: Int                  // abstract method (not in impl class)
+        |  def f6: Int                  // abstract method
         |
         |  // fields
         |
@@ -61,39 +73,65 @@ class ScalaInlineInfoTest extends ClearAfterClass {
         |
         |  final val x5 = 0
         |}
+        |class C extends T {
+        |  def f6 = 0
+        |  var x3 = 0
+        |}
       """.stripMargin
 
-    val cs @ List(t, tl, to) = compileClasses(compiler)(code)
-    val info = inlineInfo(t)
-    val expect = InlineInfo (
+    val cs @ List(c, t, tl, to) = compileClasses(compiler)(code)
+    val infoT = inlineInfo(t)
+    val expectT = InlineInfo (
       false, // final class
       None, // not a sam
       Map(
-        // TODO SD-86: the module accessor used to be `effectivelyFinal` before nuke-impl-classes
-        ("O()LT$O$;",                                                MethodInlineInfo(false,false,false)),
-        ("T$$super$toString()Ljava/lang/String;",                    MethodInlineInfo(false,false,false)),
-        ("T$_setter_$x1_$eq(I)V",                                    MethodInlineInfo(false,false,false)),
-        ("f1()I",                                                    MethodInlineInfo(false,false,false)),
-        ("f3()I",                                                    MethodInlineInfo(false,false,false)),
-        ("f4()Ljava/lang/String;",                                   MethodInlineInfo(false,true, false)),
-        ("f5()I",                                                    MethodInlineInfo(false,false,false)),
-        ("f6()I",                                                    MethodInlineInfo(false,false,true )),
-        ("x1()I",                                                    MethodInlineInfo(false,false,false)),
-        ("x3()I",                                                    MethodInlineInfo(false,false,false)),
-        ("x3_$eq(I)V",                                               MethodInlineInfo(false,false,false)),
-        ("x4()I",                                                    MethodInlineInfo(false,false,false)),
-        ("x5()I",                                                    MethodInlineInfo(true, false,false)),
-        ("y2()I",                                                    MethodInlineInfo(false,false,false)),
-        ("y2_$eq(I)V",                                               MethodInlineInfo(false,false,false)),
-        ("f2()I",                                                    MethodInlineInfo(true, false,false)),
-        ("L$lzycompute$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;",MethodInlineInfo(true, false,false)),
-        // TODO SD-86: should probably be effectivelyFinal
-        ("L$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;",           MethodInlineInfo(false,false,false)),
-        ("nest$1()I",                                                MethodInlineInfo(true, false,false)),
-        ("$init$()V",                                                MethodInlineInfo(false,false,false))),
+        ("O()LT$O$;",                                                 MethodInlineInfo(true ,false,false)), // the accessor is abstract in bytecode, but still effectivelyFinal because there's no (late)DEFERRED flag, https://github.com/scala/scala-dev/issues/126
+        ("T$$super$toString()Ljava/lang/String;",                     MethodInlineInfo(true ,false,false)),
+        ("T$_setter_$x1_$eq(I)V",                                     MethodInlineInfo(false,false,false)),
+        ("f1()I",                                                     MethodInlineInfo(false,false,false)),
+        ("f2()I",                                                     MethodInlineInfo(true, false,false)),
+        ("f3()I",                                                     MethodInlineInfo(false,false,false)),
+        ("f4()Ljava/lang/String;",                                    MethodInlineInfo(false,true, false)),
+        ("f5()I",                                                     MethodInlineInfo(true ,false,false)),
+        ("f6()I",                                                     MethodInlineInfo(false,false,true )),
+        ("x1()I",                                                     MethodInlineInfo(false,false,false)),
+        ("y2()I",                                                     MethodInlineInfo(false,false,false)),
+        ("y2_$eq(I)V",                                                MethodInlineInfo(false,false,false)),
+        ("x3()I",                                                     MethodInlineInfo(false,false,false)),
+        ("x3_$eq(I)V",                                                MethodInlineInfo(false,false,false)),
+        ("x4()I",                                                     MethodInlineInfo(false,false,false)),
+        ("x5()I",                                                     MethodInlineInfo(true, false,false)),
+        ("L$lzycompute$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;", MethodInlineInfo(true, false,false)),
+        ("L$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;",            MethodInlineInfo(true ,false,false)),
+        ("nest$1()I",                                                 MethodInlineInfo(true, false,false)),
+        ("$init$()V",                                                 MethodInlineInfo(false,false,false))),
       None // warning
     )
-    assert(info == expect, info)
+
+    assert(infoT == expectT, mapDiff(expectT.methodInfos, infoT.methodInfos) + infoT)
+
+    val infoC = inlineInfo(c)
+    val expectC = InlineInfo(false, None, Map(
+      "O()LT$O$;"                             -> MethodInlineInfo(true ,false,false),
+      "f1()I"                                 -> MethodInlineInfo(false,false,false),
+      "f3()I"                                 -> MethodInlineInfo(false,false,false),
+      "f4()Ljava/lang/String;"                -> MethodInlineInfo(false,true ,false),
+      "f5()I"                                 -> MethodInlineInfo(true ,false,false),
+      "f6()I"                                 -> MethodInlineInfo(false,false,false),
+      "x1()I"                                 -> MethodInlineInfo(false,false,false),
+      "T$_setter_$x1_$eq(I)V"                 -> MethodInlineInfo(false,false,false),
+      "y2()I"                                 -> MethodInlineInfo(false,false,false),
+      "y2_$eq(I)V"                            -> MethodInlineInfo(false,false,false),
+      "x3()I"                                 -> MethodInlineInfo(false,false,false),
+      "x3_$eq(I)V"                            -> MethodInlineInfo(false,false,false),
+      "x4$lzycompute()I"                      -> MethodInlineInfo(true ,false,false),
+      "x4()I"                                 -> MethodInlineInfo(false,false,false),
+      "x5()I"                                 -> MethodInlineInfo(true ,false,false),
+      "T$$super$toString()Ljava/lang/String;" -> MethodInlineInfo(true ,false,false),
+      "<init>()V"                             -> MethodInlineInfo(false,false,false)),
+      None)
+
+    assert(infoC == expectC, mapDiff(expectC.methodInfos, infoC.methodInfos) + infoC)
   }
 
   @Test