Don't inline methods containing super calls into other classes

Method bodies that contain a super call cannot be inlined into other
classes. The same goes for methods containing calls to private
methods, but that was already ensured before by accessibility checks.

The last case of `invokespecial` instructions is constructor calls.
Those can be safely moved into different classes (as long as the
constructor is accessible at the new location).

Note that scalac never emits methods / constructors as private in
bytecode.

3 files changed, 76 insertions, 16 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
index 0f4c7d5287..e14e57d3ab 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
@@ -480,7 +480,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
       Some(MethodWithHandlerCalledOnNonEmptyStack(
         calleeDeclarationClass.internalName, callee.name, callee.desc,
         callsiteClass.internalName, callsiteMethod.name, callsiteMethod.desc))
-    } else findIllegalAccess(callee.instructions, callsiteClass) map {
+    } else findIllegalAccess(callee.instructions, calleeDeclarationClass, callsiteClass) map {
       case (illegalAccessIns, None) =>
         IllegalAccessInstruction(
           calleeDeclarationClass.internalName, callee.name, callee.desc,
@@ -500,7 +500,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
    * If validity of some instruction could not be checked because an error occurred, the instruction
    * is returned together with a warning message that describes the problem.
    */
-  def findIllegalAccess(instructions: InsnList, destinationClass: ClassBType): Option[(AbstractInsnNode, Option[OptimizerWarning])] = {
+  def findIllegalAccess(instructions: InsnList, calleeDeclarationClass: ClassBType, destinationClass: ClassBType): Option[(AbstractInsnNode, Option[OptimizerWarning])] = {
 
     /**
      * Check if a type is accessible to some class, as defined in JVMS 5.4.4.
@@ -597,11 +597,23 @@ class Inliner[BT <: BTypes](val btypes: BT) {
       case mi: MethodInsnNode =>
         if (mi.owner.charAt(0) == '[') Right(true) // array methods are accessible
         else {
+          def canInlineCall(opcode: Int, methodFlags: Int, methodDeclClass: ClassBType, methodRefClass: ClassBType): Either[OptimizerWarning, Boolean] = {
+            opcode match {
+              case INVOKESPECIAL if mi.name != GenBCode.INSTANCE_CONSTRUCTOR_NAME =>
+                // invokespecial is used for private method calls, super calls and instance constructor calls.
+                // private method and super calls can only be inlined into the same class.
+                Right(destinationClass == calleeDeclarationClass)
+
+              case _ => // INVOKEVIRTUAL, INVOKESTATIC, INVOKEINTERFACE and INVOKESPECIAL of constructors
+                memberIsAccessible(methodFlags, methodDeclClass, methodRefClass)
+            }
+          }
+
           val methodRefClass = classBTypeFromParsedClassfile(mi.owner)
           for {
             (methodNode, methodDeclClassNode) <- byteCodeRepository.methodNode(methodRefClass.internalName, mi.name, mi.desc): Either[OptimizerWarning, (MethodNode, InternalName)]
             methodDeclClass                   =  classBTypeFromParsedClassfile(methodDeclClassNode)
-            res                               <- memberIsAccessible(methodNode.access, methodDeclClass, methodRefClass)
+            res                               <- canInlineCall(mi.getOpcode, methodNode.access, methodDeclClass, methodRefClass)
           } yield {
             res
           }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala
index 03d2f2f108..b4839dcec8 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala
@@ -59,7 +59,7 @@ class InlinerIllegalAccessTest extends ClearAfterClass {
 
     def check(classNode: ClassNode, test: Option[AbstractInsnNode] => Unit) = {
       for (m <- methods)
-        test(inliner.findIllegalAccess(m.instructions, classBTypeFromParsedClassfile(classNode.name)).map(_._1))
+        test(inliner.findIllegalAccess(m.instructions, classBTypeFromParsedClassfile(cClass.name), classBTypeFromParsedClassfile(classNode.name)).map(_._1))
     }
 
     check(cClass, assertEmpty)
@@ -152,8 +152,8 @@ class InlinerIllegalAccessTest extends ClearAfterClass {
 
     val List(rbD, rcD, rfD, rgD) = dCl.methods.asScala.toList.filter(_.name(0) == 'r').sortBy(_.name)
 
-    def check(method: MethodNode, dest: ClassNode, test: Option[AbstractInsnNode] => Unit): Unit = {
-      test(inliner.findIllegalAccess(method.instructions, classBTypeFromParsedClassfile(dest.name)).map(_._1))
+    def check(method: MethodNode, decl: ClassNode, dest: ClassNode, test: Option[AbstractInsnNode] => Unit): Unit = {
+      test(inliner.findIllegalAccess(method.instructions, classBTypeFromParsedClassfile(decl.name), classBTypeFromParsedClassfile(dest.name)).map(_._1))
     }
 
     val cOrDOwner = (_: Option[AbstractInsnNode] @unchecked) match {
@@ -164,35 +164,35 @@ class InlinerIllegalAccessTest extends ClearAfterClass {
     // PUBLIC
 
     // public methods allowed everywhere
-    for (m <- Set(raC, reC); c <- allClasses) check(m, c, assertEmpty)
+    for (m <- Set(raC, reC); c <- allClasses) check(m, cCl, c, assertEmpty)
 
     // DEFAULT ACCESS
 
     // default access OK in same package
-    for (m <- Set(rbC, rfC, rbD, rfD); c <- allClasses) {
-      if (c.name startsWith "a/") check(m, c, assertEmpty)
-      else check(m, c, cOrDOwner)
+    for ((m, declCls) <- Set((rbC, cCl), (rfC, cCl), (rbD, dCl), (rfD, dCl)); c <- allClasses) {
+      if (c.name startsWith "a/") check(m, declCls, c, assertEmpty)
+      else check(m, declCls, c, cOrDOwner)
     }
 
     // PROTECTED
 
     // protected accessed in same class, or protected static accessed in subclass(rgD).
     // can be inlined to subclasses, and classes in the same package (gCl)
-    for (m <- Set(rcC, rgC, rgD); c <- Set(cCl, dCl, eCl, fCl, gCl, hCl)) check(m, c, assertEmpty)
+    for ((m, declCls) <- Set((rcC, cCl), (rgC, cCl), (rgD, dCl)); c <- Set(cCl, dCl, eCl, fCl, gCl, hCl)) check(m, declCls, c, assertEmpty)
 
     // protected in non-subclass and different package
-    for (m <- Set(rcC, rgC)) check(m, iCl, cOrDOwner)
+    for (m <- Set(rcC, rgC)) check(m, cCl, iCl, cOrDOwner)
 
     // non-static protected accessed in subclass (rcD). can be inlined to related class, or classes in the same package
-    for (c <- Set(cCl, dCl, eCl, fCl, gCl)) check(rcD, c, assertEmpty)
+    for (c <- Set(cCl, dCl, eCl, fCl, gCl)) check(rcD, dCl, c, assertEmpty)
 
     // rcD cannot be inlined into non-related classes, if the declaration and destination are not in the same package
-    for (c <- Set(hCl, iCl)) check(rcD, c, cOrDOwner)
+    for (c <- Set(hCl, iCl)) check(rcD, dCl, c, cOrDOwner)
 
     // PRIVATE
 
     // privated method accesses can only be inlined in the same class
-    for (m <- Set(rdC, rhC)) check(m, cCl, assertEmpty)
-    for (m <- Set(rdC, rhC); c <- allClasses.tail) check(m, c, cOrDOwner)
+    for (m <- Set(rdC, rhC)) check(m, cCl, cCl, assertEmpty)
+    for (m <- Set(rdC, rhC); c <- allClasses.tail) check(m, cCl, c, cOrDOwner)
   }
 }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
index 0581bd24fe..39fb28570e 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
@@ -902,4 +902,52 @@ class InlinerTest extends ClearAfterClass {
       allowMessage = i => {c += 1; i.msg contains warn})
     assert(c == 1, c)
   }
+
+  @Test
+  def inlineInvokeSpecial(): Unit = {
+    val code =
+      """class Aa {
+        |  def f1 = 0
+        |}
+        |class B extends Aa {
+        |  @inline final override def f1 = 1 + super.f1 // invokespecial Aa.f1
+        |
+        |  private def f2m = 0                          // public B$$f2m in bytecode
+        |  @inline final def f2 = f2m                   // invokevirtual B.B$$f2m
+        |
+        |  private def this(x: Int) = this()            // public in bytecode
+        |  @inline final def f3 = new B()               // invokespecial B.<init>()
+        |  @inline final def f4 = new B(1)              // invokespecial B.<init>(I)
+        |
+        |  def t1 = f1                                  // inlined
+        |  def t2 = f2                                  // inlined
+        |  def t3 = f3                                  // inlined
+        |  def t4 = f4                                  // inlined
+        |}
+        |class T {
+        |  def t1(b: B) = b.f1                          // cannot inline: contains a super call
+        |  def t2(b: B) = b.f2                          // inlined
+        |  def t3(b: B) = b.f3                          // inlined
+        |  def t4(b: B) = b.f4                          // inlined
+        |}
+      """.stripMargin
+
+    val warn =
+      """B::f1()I is annotated @inline but could not be inlined:
+        |The callee B::f1()I contains the instruction INVOKESPECIAL Aa.f1 ()I
+        |that would cause an IllegalAccessError when inlined into class T.""".stripMargin
+    var c = 0
+    val List(a, b, t) = compile(code, allowMessage = i => {c += 1; i.msg contains warn})
+    assert(c == 1, c)
+
+    assertInvoke(getSingleMethod(b, "t1"), "Aa", "f1")
+    assertInvoke(getSingleMethod(b, "t2"), "B", "B$$f2m")
+    assertInvoke(getSingleMethod(b, "t3"), "B", "<init>")
+    assertInvoke(getSingleMethod(b, "t4"), "B", "<init>")
+
+    assertInvoke(getSingleMethod(t, "t1"), "B", "f1")
+    assertInvoke(getSingleMethod(t, "t2"), "B", "B$$f2m")
+    assertInvoke(getSingleMethod(t, "t3"), "B", "<init>")
+    assertInvoke(getSingleMethod(t, "t4"), "B", "<init>")
+  }
 }