Merge pull request scala#5058 from lrytz/newTraitsInliner

Inline super calls, as they are statically resolved

Author: lrytz

src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala

@@ -88,6 +88,11 @@ object BytecodeUtils {
 
   def isLoadOrStore(instruction: AbstractInsnNode): Boolean = isLoad(instruction) || isStore(instruction)
 
+  def isNonVirtualCall(instruction: AbstractInsnNode): Boolean = {
+    val op = instruction.getOpcode
+    op == INVOKESPECIAL || op == INVOKESTATIC
+  }
+
   def isExecutable(instruction: AbstractInsnNode): Boolean = instruction.getOpcode >= 0
 
   def isConstructor(methodNode: MethodNode): Boolean = {

src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala

@@ -132,7 +132,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
               (declarationClassNode, source) <- byteCodeRepository.classNodeAndSource(declarationClass): Either[OptimizerWarning, (ClassNode, Source)]
             } yield {
                 val declarationClassBType = classBTypeFromClassNode(declarationClassNode)
-                val CallsiteInfo(safeToInline, safeToRewrite, canInlineFromSource, annotatedInline, annotatedNoInline, samParamTypes, warning) = analyzeCallsite(method, declarationClassBType, call.owner, source)
+                val CallsiteInfo(safeToInline, safeToRewrite, canInlineFromSource, annotatedInline, annotatedNoInline, samParamTypes, warning) = analyzeCallsite(method, declarationClassBType, call, source)
                 Callee(
                   callee = method,
                   calleeDeclarationClass = declarationClassBType,
@@ -264,7 +264,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
   /**
    * Analyze a callsite and gather meta-data that can be used for inlining decisions.
    */
-  private def analyzeCallsite(calleeMethodNode: MethodNode, calleeDeclarationClassBType: ClassBType, receiverTypeInternalName: InternalName, calleeSource: Source): CallsiteInfo = {
+  private def analyzeCallsite(calleeMethodNode: MethodNode, calleeDeclarationClassBType: ClassBType, call: MethodInsnNode, calleeSource: Source): CallsiteInfo = {
     val methodSignature = calleeMethodNode.name + calleeMethodNode.desc
 
     try {
@@ -277,7 +277,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
 
           val isAbstract = BytecodeUtils.isAbstractMethod(calleeMethodNode)
 
-          val receiverType = classBTypeFromParsedClassfile(receiverTypeInternalName)
+          val receiverType = classBTypeFromParsedClassfile(call.owner)
           // (1) A non-final method can be safe to inline if the receiver type is a final subclass. Example:
           //   class A { @inline def f = 1 }; object B extends A; B.f  // can be inlined
           //
@@ -295,6 +295,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
           // TODO: type analysis can render more calls statically resolved. Example:
           //   new A.f  // can be inlined, the receiver type is known to be exactly A.
           val isStaticallyResolved: Boolean = {
+            isNonVirtualCall(call) || // SD-86: super calls (invokespecial) can be inlined
             methodInlineInfo.effectivelyFinal ||
               receiverType.info.orThrow.inlineInfo.isEffectivelyFinal // (1)
           }

test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala

@@ -67,14 +67,17 @@ class BTypesFromClassfileTest {
     // there's a separate InlineInfoTest.
 
     val chk1 = sameBTypes(fromSym.superClass, fromClassfile.superClass, checked)
+    
+    // was:
+    // val chk2 = sameBTypes(fromSym.interfaces, fromClassfile.interfaces, chk1)
 
+    // TODO: The new trait encoding emits redundant parents in the backend to avoid linkage errors in invokespecial
+    // Need to give this some more thought, maybe do it earlier so it is reflected in the Symbol's info, too.
     val fromSymInterfaces = fromSym.interfaces
     val fromClassFileInterfaces = fromClassfile.interfaces
     val (matching, other) = fromClassFileInterfaces.partition(x => fromSymInterfaces.exists(_.internalName == x.internalName))
     val chk2 = sameBTypes(fromSym.interfaces, matching, chk1)
     for (redundant <- other) {
-      // TODO SD-86 The new trait encoding emits redundant parents in the backend to avoid linkage errors in invokespecial
-      //            Need to give this some more thought, maybe do it earlier so it is reflected in the Symbol's info, too.
       assert(matching.exists(x => x.isSubtypeOf(redundant).orThrow), redundant)
     }
 

test/junit/scala/tools/nsc/backend/jvm/opt/InlineInfoTest.scala

@@ -59,9 +59,7 @@ class InlineInfoTest extends ClearAfterClass {
         |}
         |class C extends T with U
       """.stripMargin
-//    val classes = compile(code) // SD-86
-    InlineInfoTest.notPerRun.foreach(_.clear())
-    val classes = compileClasses(compiler)(code, allowMessage = _ => true) // SD-86 inline warnings
+    val classes = compile(code)
 
     val fromSyms = classes.map(c => compiler.genBCode.bTypes.classBTypeFromInternalName(c.name).info.get.inlineInfo)
 

test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala

@@ -67,10 +67,10 @@ class InlineWarningTest extends ClearAfterClass {
       "T::m2()I is annotated @inline but cannot be inlined: the method is not final and may be overridden",
       "D::m2()I is annotated @inline but cannot be inlined: the method is not final and may be overridden")
     compile(code, allowMessage = i => {count += 1; warns.exists(i.msg contains _)})
-    assert(count == 5, count) // TODO SD-85: 5th warning
+    assert(count == 4, count)
   }
 
-  // TODO SD-85: no more impl classes. this test (and the warning it tests!) can be removed
+  // TODO SD-86: no more impl classes. this test (and the warning it tests!) can be removed
   @org.junit.Ignore @Test
   def traitMissingImplClass(): Unit = {
     val codeA = "trait T { @inline final def f = 1 }"

test/junit/scala/tools/nsc/backend/jvm/opt/InlinerSeparateCompilationTest.scala

@@ -42,15 +42,10 @@ class InlinerSeparateCompilationTest {
         |}
       """.stripMargin
 
-    val warns = Set(
-      "T::f()I is annotated @inline but cannot be inlined: the method is not final and may be overridden",
-      // TODO SD-85
-      """O$::f()I is annotated @inline but could not be inlined:
-        |The callee O$::f()I contains the instruction INVOKESPECIAL T.f ()I
-        |that would cause an IllegalAccessError when inlined into class C""".stripMargin)
-    val List(c, o, oMod, t) = compileClassesSeparately(List(codeA, codeB), args + " -Yopt-warnings", i => warns.exists(i.msg contains _))
+    val warn = "T::f()I is annotated @inline but cannot be inlined: the method is not final and may be overridden"
+    val List(c, o, oMod, t) = compileClassesSeparately(List(codeA, codeB), args + " -Yopt-warnings", _.msg contains warn)
     assertInvoke(getSingleMethod(c, "t1"), "T", "f")
-//    assertNoInvoke(getSingleMethod(c, "t2")) // SD-85
+    assertNoInvoke(getSingleMethod(c, "t2"))
     assertNoInvoke(getSingleMethod(c, "t3"))
   }
 

test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala

@@ -324,8 +324,6 @@ class InlinerTest extends ClearAfterClass {
         |}
       """.stripMargin
     val List(c, t) = compile(code)
-    println(textify(c))
-    println(textify(t))
     assertNoInvoke(getSingleMethod(c, "g"))
   }
 
@@ -507,8 +505,7 @@ class InlinerTest extends ClearAfterClass {
       "T::f()I is annotated @inline but cannot be inlined: the method is not final and may be overridden")
     var count = 0
     val List(c, t) = compile(code, allowMessage = i => {count += 1; warns.exists(i.msg contains _)})
-    // 3rd warnings because of mixin-method, see SD-86
-    assert(count == 3, count)
+    assert(count == 2, count)
     assertInvoke(getSingleMethod(c, "t1"), "T", "f")
     assertInvoke(getSingleMethod(c, "t2"), "C", "f")
   }
@@ -535,27 +532,22 @@ class InlinerTest extends ClearAfterClass {
         |}
         |object O extends T {
         |  @inline def g = 1
-        |  // mixin generates `def f = T$class.f(this)`, which is inlined here (we get ICONST_0)
+        |  // mixin generates `def f = super[T].f`, which is inlined here (we get ICONST_0)
         |}
         |class C {
         |  def t1 = O.f       // the mixin method of O is inlined, so we directly get the ICONST_0
         |  def t2 = O.g       // object members are inlined
         |  def t3(t: T) = t.f // no inlining here
         |}
       """.stripMargin
-    val warns = Set(
-      "T::f()I is annotated @inline but cannot be inlined: the method is not final and may be overridden",
-      // SD-86 -- once the mixin-method O.f inlines the body of T.f, we can also inline O.g into class C.
-      """O$::f()I is annotated @inline but could not be inlined:
-        |The callee O$::f()I contains the instruction INVOKESPECIAL T.f ()I
-        |that would cause an IllegalAccessError when inlined into class C""".stripMargin)
+    val warn = "T::f()I is annotated @inline but cannot be inlined: the method is not final and may be overridden"
     var count = 0
-    val List(c, oMirror, oModule, t) = compile(code, allowMessage = i => {count += 1; warns.exists(i.msg contains _)})
-    assert(count == 3, count) // SD-86
+    val List(c, oMirror, oModule, t) = compile(code, allowMessage = i => {count += 1; i.msg contains warn})
+    assert(count == 1, count)
 
-//    assertNoInvoke(getSingleMethod(oModule, "f")) // SD-86
+    assertNoInvoke(getSingleMethod(oModule, "f"))
 
-//    assertNoInvoke(getSingleMethod(c, "t1")) // SD-86
+    assertNoInvoke(getSingleMethod(c, "t1"))
     assertNoInvoke(getSingleMethod(c, "t2"))
     assertInvoke(getSingleMethod(c, "t3"), "T", "f")
   }
@@ -569,12 +561,10 @@ class InlinerTest extends ClearAfterClass {
         |}
         |trait Assembly extends T {
         |  @inline final def g = 1
-        |  @inline final def n = m // inlined. (*)
-        |  // (*) the declaration class of m is T. the signature of T$class.m is m(LAssembly;)I. so we need the self type to build the
-        |  //     signature. then we can look up the MethodNode of T$class.m and then rewrite the INVOKEINTERFACE to INVOKESTATIC.
+        |  @inline final def n = m // inlined (m is final)
         |}
         |class C {
-        |  def t1(a: Assembly) = a.f // like above, decl class is T, need self-type of T to rewrite the interface call to static.
+        |  def t1(a: Assembly) = a.f // inlined (f is final)
         |  def t2(a: Assembly) = a.n
         |}
       """.stripMargin
@@ -618,30 +608,30 @@ class InlinerTest extends ClearAfterClass {
     val code =
       """trait T1 {
         |  @inline def f: Int = 0
-        |  @inline def g1 = f     // not inlined: f not final, so T1$class.g1 has an interface call T1.f
+        |  @inline def g1 = f     // not inlined: f not final
         |}
         |
-        |// erased self-type (used in impl class for `self` parameter): T1
+        |// erased self-type: T1
         |trait T2a { self: T1 with T2a =>
         |  @inline override final def f = 1
-        |  @inline def g2a = f    // inlined: resolved as T2a.f, which is re-written to T2a$class.f, so T2a$class.g2a has ICONST_1
+        |  @inline def g2a = f    // inlined: resolved as T2a.f
         |}
         |
         |final class Ca extends T1 with T2a {
-        |  // mixin generates accessors like `def g1 = T1$class.g1`, the impl class method call is inlined into the accessor.
+        |  // mixin generates accessors like `def g1 = super[T1].g1`, the impl super call is inlined into the accessor.
         |
         |  def m1a = g1           // call to accessor, inlined, we get the interface call T1.f
         |  def m2a = g2a          // call to accessor, inlined, we get ICONST_1
         |  def m3a = f            // call to accessor, inlined, we get ICONST_1
         |
-        |  def m4a(t: T1) = t.f   // T1.f is not final, so not inlined, interface call to T1.f
-        |  def m5a(t: T2a) = t.f  // re-written to T2a$class.f, inlined, ICONST_1
+        |  def m4a(t: T1) = t.f   // T1.f is not final, so not inlined, we get an interface call T1.f
+        |  def m5a(t: T2a) = t.f  // inlined, we get ICONST_1
         |}
         |
         |// erased self-type: T2b
         |trait T2b { self: T2b with T1 =>
         |  @inline override final def f = 1
-        |  @inline def g2b = f    // not inlined: resolved as T1.f, so T2b$class.g2b has an interface call T1.f
+        |  @inline def g2b = f    // not inlined: resolved as T1.f, we get an interface call T1.f
         |}
         |
         |final class Cb extends T1 with T2b {
@@ -650,31 +640,26 @@ class InlinerTest extends ClearAfterClass {
         |  def m3b = f            // inlined, we get ICONST_1
         |
         |  def m4b(t: T1) = t.f   // T1.f is not final, so not inlined, interface call to T1.f
-        |  def m5b(t: T2b) = t.f  // re-written to T2b$class.f, inlined, ICONST_1
+        |  def m5b(t: T2b) = t.f  // inlined, ICONST_1
         |}
       """.stripMargin
 
-    val warnings = Set(
-      "T1::f()I is annotated @inline but cannot be inlined: the method is not final and may be overridden",
-      "T2b::g2b()I is annotated @inline but cannot be inlined: the method is not final and may be overridden",
-      "T1::g1()I is annotated @inline but cannot be inlined: the method is not final and may be overridden",
-      "T2a::g2a()I is annotated @inline but cannot be inlined: the method is not final and may be overridden",
-      "T1::g1()I is annotated @inline but cannot be inlined: the method is not final and may be overridden")
+    val warning = "T1::f()I is annotated @inline but cannot be inlined: the method is not final and may be overridden"
     var count = 0
-    val List(ca, cb, t1, t2a, t2b) = compile(code, allowMessage = i => {count += 1; warnings.exists(i.msg contains _)})
-    assert(count == 8, count) // see comments, f is not inlined 4 times, additional warnings due to SD-86
+    val List(ca, cb, t1, t2a, t2b) = compile(code, allowMessage = i => {count += 1; i.msg contains warning})
+    assert(count == 4, count) // see comments, f is not inlined 4 times
 
     assertNoInvoke(getSingleMethod(t2a, "g2a"))
     assertInvoke(getSingleMethod(t2b, "g2b"), "T1", "f")
 
-//    assertInvoke(getSingleMethod(ca, "m1a"), "T1", "f") // disabled due to SD-86: m1a calls the mixin-method g1a, which calls super[T1].g1a. we inline the mixin-method and end up with the super call.
-//    assertNoInvoke(getSingleMethod(ca, "m2a"))            // no invoke, see comment on def g2a // SD-86
+    assertInvoke(getSingleMethod(ca, "m1a"), "T1", "f")
+    assertNoInvoke(getSingleMethod(ca, "m2a"))            // no invoke, see comment on def g2a
     assertNoInvoke(getSingleMethod(ca, "m3a"))
     assertInvoke(getSingleMethod(ca, "m4a"), "T1", "f")
     assertNoInvoke(getSingleMethod(ca, "m5a"))
 
-//    assertInvoke(getSingleMethod(cb, "m1b"), "T1", "f") // SD-86
-//    assertInvoke(getSingleMethod(cb, "m2b"), "T1", "f")  // invoke, see comment on def g2b // SD-86
+    assertInvoke(getSingleMethod(cb, "m1b"), "T1", "f")
+    assertInvoke(getSingleMethod(cb, "m2b"), "T1", "f")  // invoke, see comment on def g2b
     assertNoInvoke(getSingleMethod(cb, "m3b"))
     assertInvoke(getSingleMethod(cb, "m4b"), "T1", "f")
     assertNoInvoke(getSingleMethod(cb, "m5b"))
@@ -772,8 +757,8 @@ class InlinerTest extends ClearAfterClass {
         |  final val d = 3
         |  final val d1: Int = 3
         |
-        |  @noinline def f = 5       // re-written to T$class
-        |  @noinline final def g = 6 // re-written
+        |  @noinline def f = 5
+        |  @noinline final def g = 6
         |
         |  @noinline def h: Int
         |  @inline def i: Int
@@ -786,8 +771,8 @@ class InlinerTest extends ClearAfterClass {
         |  final val d = 3
         |  final val d1: Int = 3
         |
-        |  @noinline def f = 5       // not re-written (not final)
-        |  @noinline final def g = 6 // re-written
+        |  @noinline def f = 5
+        |  @noinline final def g = 6
         |
         |  @noinline def h: Int
         |  @inline def i: Int
@@ -1508,4 +1493,14 @@ class InlinerTest extends ClearAfterClass {
     val List(a, b) = compileClassesSeparately(List(codeA, codeB), extraArgs = "-Yopt:l:project -Yopt-warnings")
     assertInvoke(getSingleMethod(b, "t"), "A", "f")
   }
+
+  @Test
+  def sd86(): Unit = {
+    val code =
+      """trait T { @inline def f = 1 } // note that f is not final
+        |class C extends T
+      """.stripMargin
+    val List(c, t) = compile(code, allowMessage = _ => true)
+    assertSameSummary(getSingleMethod(c, "f"), List(ICONST_1, IRETURN))
+  }
 }