Don't crash the inliner in mixed compilation

In mixed compilation, the bytecode of Java classes is not availalbe:
the Scala compiler does not produce any, and there are no classfiles
yet.

When inlining a (Scala defined) method that contains an invocation to
a Java method, we need the Java method's bytecode in order to check
whether that invocation can be transplanted to the new location
without causing an IllegalAccessError. If the bytecode cannot be
found, inlining won't be allowed.

9 files changed, 146 insertions, 44 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala
index f07c7b7764..e617c86b23 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala
@@ -87,32 +87,29 @@ abstract class BTypes {
    *
    * This method supports both descriptors and internal names.
    */
-  def bTypeForDescriptorOrInternalNameFromClassfile(desc: String): BType = (desc(0): @switch) match {
-    case 'V'                     => UNIT
-    case 'Z'                     => BOOL
-    case 'C'                     => CHAR
-    case 'B'                     => BYTE
-    case 'S'                     => SHORT
-    case 'I'                     => INT
-    case 'F'                     => FLOAT
-    case 'J'                     => LONG
-    case 'D'                     => DOUBLE
-    case '['                     => ArrayBType(bTypeForDescriptorOrInternalNameFromClassfile(desc.substring(1)))
+  def bTypeForDescriptorOrInternalNameFromClassfile(desc: String): Option[BType] = (desc(0): @switch) match {
+    case 'V'                     => Some(UNIT)
+    case 'Z'                     => Some(BOOL)
+    case 'C'                     => Some(CHAR)
+    case 'B'                     => Some(BYTE)
+    case 'S'                     => Some(SHORT)
+    case 'I'                     => Some(INT)
+    case 'F'                     => Some(FLOAT)
+    case 'J'                     => Some(LONG)
+    case 'D'                     => Some(DOUBLE)
+    case '['                     => bTypeForDescriptorOrInternalNameFromClassfile(desc.substring(1)) map ArrayBType
     case 'L' if desc.last == ';' => classBTypeFromParsedClassfile(desc.substring(1, desc.length - 1))
     case _                       => classBTypeFromParsedClassfile(desc)
   }
 
   /**
-   * Parse the classfile for `internalName` and construct the [[ClassBType]].
+   * Parse the classfile for `internalName` and construct the [[ClassBType]]. Returns `None` if the
+   * classfile cannot be found in the `byteCodeRepository`.
    */
-  def classBTypeFromParsedClassfile(internalName: InternalName): ClassBType = {
-    val classNode = byteCodeRepository.classNode(internalName) getOrElse {
-      // There's no way out, we need the ClassBType. I (lry) only know one case byteCodeRepository.classNode
-      // returns None: for Java classes in mixed compilation. In this case we should not end up here,
-      // because there exists a symbol for that Java class, the ClassBType should be built from the symbol.
-      assertionError(s"Could not find bytecode for class $internalName")
+  def classBTypeFromParsedClassfile(internalName: InternalName): Option[ClassBType] = {
+    classBTypeFromInternalName.get(internalName) orElse {
+      byteCodeRepository.classNode(internalName) map classBTypeFromClassNode
     }
-    classBTypeFromClassNode(classNode)
   }
 
   /**
@@ -120,20 +117,31 @@ abstract class BTypes {
    */
   def classBTypeFromClassNode(classNode: ClassNode): ClassBType = {
     classBTypeFromInternalName.getOrElse(classNode.name, {
-      setClassInfo(classNode, ClassBType(classNode.name))
+      setClassInfoFromParsedClassfile(classNode, ClassBType(classNode.name))
     })
   }
 
-  private def setClassInfo(classNode: ClassNode, classBType: ClassBType): ClassBType = {
+  private def setClassInfoFromParsedClassfile(classNode: ClassNode, classBType: ClassBType): ClassBType = {
+    def ensureClassBTypeFromParsedClassfile(internalName: InternalName): ClassBType = {
+      classBTypeFromParsedClassfile(internalName) getOrElse {
+        // When building a ClassBType from a parsed classfile, we need the ClassBTypes for all
+        // referenced types.
+        // TODO: make this more robust with respect to incomplete classpaths.
+        // Maybe not those parts of the ClassBType that require the missing class are not actually
+        // queried during the backend, so every part of a ClassBType that requires parsing a
+        // (potentially missing) classfile should be computed lazily.
+        assertionError(s"Could not find bytecode for class $internalName")
+      }
+    }
     val superClass = classNode.superName match {
       case null =>
         assert(classNode.name == ObjectReference.internalName, s"class with missing super type: ${classNode.name}")
         None
       case superName =>
-        Some(classBTypeFromParsedClassfile(superName))
+        Some(ensureClassBTypeFromParsedClassfile(superName))
     }
 
-    val interfaces: List[ClassBType] = classNode.interfaces.asScala.map(classBTypeFromParsedClassfile)(collection.breakOut)
+    val interfaces: List[ClassBType] = classNode.interfaces.asScala.map(ensureClassBTypeFromParsedClassfile)(collection.breakOut)
 
     val flags = classNode.access
 
@@ -159,7 +167,7 @@ abstract class BTypes {
     }
 
     val nestedClasses: List[ClassBType] = classNode.innerClasses.asScala.collect({
-      case i if nestedInCurrentClass(i) => classBTypeFromParsedClassfile(i.name)
+      case i if nestedInCurrentClass(i) => ensureClassBTypeFromParsedClassfile(i.name)
     })(collection.breakOut)
 
     // if classNode is a nested class, it has an innerClass attribute for itself. in this
@@ -169,11 +177,11 @@ abstract class BTypes {
         val enclosingClass =
           if (innerEntry.outerName != null) {
             // if classNode is a member class, the outerName is non-null
-            classBTypeFromParsedClassfile(innerEntry.outerName)
+            ensureClassBTypeFromParsedClassfile(innerEntry.outerName)
           } else {
             // for anonymous or local classes, the outerName is null, but the enclosing class is
             // stored in the EnclosingMethod attribute (which ASM encodes in classNode.outerClass).
-            classBTypeFromParsedClassfile(classNode.outerClass)
+            ensureClassBTypeFromParsedClassfile(classNode.outerClass)
           }
         val staticFlag = (innerEntry.access & Opcodes.ACC_STATIC) != 0
         NestedInfo(enclosingClass, Option(innerEntry.outerName), Option(innerEntry.innerName), staticFlag)
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 b74c7ba86c..2ca8e8b8c4 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
@@ -353,6 +353,11 @@ class Inliner[BT <: BTypes](val btypes: BT) {
     }
   }
 
+  /**
+   * Returns the first instruction in the `instructions` list that would cause a
+   * [[java.lang.IllegalAccessError]] when inlined into the `destinationClass`. Returns `None` if
+   * all instructions can be legally transplanted.
+   */
   def findIllegalAccess(instructions: InsnList, destinationClass: ClassBType): Option[AbstractInsnNode] = {
 
     /**
@@ -413,36 +418,50 @@ class Inliner[BT <: BTypes](val btypes: BT) {
       }
     }
 
+    /**
+     * Check if `instruction` can be transplanted to `destinationClass`.
+     *
+     * If the instruction references a class, method or field that cannot be found in the
+     * byteCodeRepository, it is considered as not legal. This is known to happen in mixed
+     * compilation: for Java classes there is no classfile that could be parsed, nor does the
+     * compiler generate any bytecode.
+     */
     def isLegal(instruction: AbstractInsnNode): Boolean = instruction match {
       case ti: TypeInsnNode  =>
         // NEW, ANEWARRAY, CHECKCAST or INSTANCEOF. For these instructions, the reference
         // "must be a symbolic reference to a class, array, or interface type" (JVMS 6), so
         // it can be an internal name, or a full array descriptor.
-        classIsAccessible(bTypeForDescriptorOrInternalNameFromClassfile(ti.desc))
+        bTypeForDescriptorOrInternalNameFromClassfile(ti.desc).exists(classIsAccessible(_))
 
       case ma: MultiANewArrayInsnNode =>
         // "a symbolic reference to a class, array, or interface type"
-        classIsAccessible(bTypeForDescriptorOrInternalNameFromClassfile(ma.desc))
+        bTypeForDescriptorOrInternalNameFromClassfile(ma.desc).exists(classIsAccessible(_))
 
       case fi: FieldInsnNode =>
-        val fieldRefClass = classBTypeFromParsedClassfile(fi.owner)
-        val (fieldNode, fieldDeclClass) = byteCodeRepository.fieldNode(fieldRefClass.internalName, fi.name, fi.desc).get
-        memberIsAccessible(fieldNode.access, classBTypeFromParsedClassfile(fieldDeclClass), fieldRefClass)
+        (for {
+          fieldRefClass <- classBTypeFromParsedClassfile(fi.owner)
+          (fieldNode, fieldDeclClassNode) <- byteCodeRepository.fieldNode(fieldRefClass.internalName, fi.name, fi.desc)
+          fieldDeclClass <- classBTypeFromParsedClassfile(fieldDeclClassNode)
+        } yield {
+          memberIsAccessible(fieldNode.access, fieldDeclClass, fieldRefClass)
+        }) getOrElse false
 
       case mi: MethodInsnNode =>
         if (mi.owner.charAt(0) == '[') true // array methods are accessible
-        else {
-          val methodRefClass = classBTypeFromParsedClassfile(mi.owner)
-          val (methodNode, methodDeclClass) = byteCodeRepository.methodNode(methodRefClass.internalName, mi.name, mi.desc).get
-          memberIsAccessible(methodNode.access, classBTypeFromParsedClassfile(methodDeclClass), methodRefClass)
-        }
+        else (for {
+          methodRefClass <- classBTypeFromParsedClassfile(mi.owner)
+          (methodNode, methodDeclClassNode) <- byteCodeRepository.methodNode(methodRefClass.internalName, mi.name, mi.desc)
+          methodDeclClass <- classBTypeFromParsedClassfile(methodDeclClassNode)
+        } yield {
+          memberIsAccessible(methodNode.access, methodDeclClass, methodRefClass)
+        }) getOrElse false
 
       case ivd: InvokeDynamicInsnNode =>
         // TODO @lry check necessary conditions to inline an indy, instead of giving up
         false
 
       case ci: LdcInsnNode => ci.cst match {
-        case t: asm.Type => classIsAccessible(bTypeForDescriptorOrInternalNameFromClassfile(t.getInternalName))
+        case t: asm.Type => bTypeForDescriptorOrInternalNameFromClassfile(t.getInternalName).exists(classIsAccessible(_))
         case _           => true
       }
 
diff --git a/test/files/run/bcodeInlinerMixed.flags b/test/files/run/bcodeInlinerMixed.flags
new file mode 100644
index 0000000000..63b5558cfd
--- /dev/null
+++ b/test/files/run/bcodeInlinerMixed.flags
@@ -0,0 +1 @@
+-Ybackend:GenBCode -Yopt:l:classpath
\ No newline at end of file
diff --git a/test/files/run/bcodeInlinerMixed/A_1.java b/test/files/run/bcodeInlinerMixed/A_1.java
new file mode 100644
index 0000000000..44d7d88eeb
--- /dev/null
+++ b/test/files/run/bcodeInlinerMixed/A_1.java
@@ -0,0 +1,3 @@
+public class A_1 {
+  public static final int bar() { return 100; }
+}
diff --git a/test/files/run/bcodeInlinerMixed/B_1.scala b/test/files/run/bcodeInlinerMixed/B_1.scala
new file mode 100644
index 0000000000..2aadeccb82
--- /dev/null
+++ b/test/files/run/bcodeInlinerMixed/B_1.scala
@@ -0,0 +1,20 @@
+// Partest does proper mixed compilation:
+//   1. scalac *.scala *.java
+//   2. javac *.java
+//   3. scalc *.scala
+//
+// In the second scalc round, the classfile for A_1 is on the classpath.
+// Therefore the inliner has access to the bytecode of `bar`, which means
+// it can verify that the invocation to `bar` can be safely inlined.
+//
+// So both callsites of `flop` are inlined.
+//
+// In a single mixed compilation, `flop` cannot be inlined, see JUnit InlinerTest.scala, def mixedCompilationNoInline.
+
+class B {
+  @inline final def flop = A_1.bar
+  def g = flop
+}
+class C {
+  def h(b: B) = b.flop
+}
diff --git a/test/files/run/bcodeInlinerMixed/Test.scala b/test/files/run/bcodeInlinerMixed/Test.scala
new file mode 100644
index 0000000000..c8c7a9fe2a
--- /dev/null
+++ b/test/files/run/bcodeInlinerMixed/Test.scala
@@ -0,0 +1,16 @@
+import scala.tools.partest.{BytecodeTest, ASMConverters}
+import ASMConverters._
+
+object Test extends BytecodeTest {
+  def show: Unit = {
+    val gIns = instructionsFromMethod(getMethod(loadClassNode("B"), "g"))
+    val hIns = instructionsFromMethod(getMethod(loadClassNode("C"), "h"))
+    // val invocation = Invoke(INVOKESTATIC, A_1, bar, ()I, false)
+    for (i <- List(gIns, hIns)) {
+      assert(i exists {
+        case Invoke(_, _, "bar", "()I", _) => true
+        case _ => false
+      }, i mkString "\n")
+    }
+  }
+}
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala
index 65c96226ff..f7c9cab284 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala
@@ -90,7 +90,7 @@ class BTypesFromClassfileTest {
     clearCache()
     val fromSymbol = classBTypeFromSymbol(classSym)
     clearCache()
-    val fromClassfile = bTypes.classBTypeFromParsedClassfile(fromSymbol.internalName)
+    val fromClassfile = bTypes.classBTypeFromParsedClassfile(fromSymbol.internalName).get
     sameBType(fromSymbol, fromClassfile)
   }
 
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 40dc990c0f..ef0f6bcd77 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)))
+        test(inliner.findIllegalAccess(m.instructions, classBTypeFromParsedClassfile(classNode.name).get))
     }
 
     check(cClass, assertEmpty)
@@ -153,7 +153,7 @@ 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)))
+      test(inliner.findIllegalAccess(method.instructions, classBTypeFromParsedClassfile(dest.name).get))
     }
 
     val cOrDOwner = (_: Option[AbstractInsnNode] @unchecked) match {
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 240d106f5c..4e7a2399a2 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
@@ -6,12 +6,15 @@ import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
 import org.junit.Test
 import scala.collection.generic.Clearable
+import scala.collection.mutable.ListBuffer
+import scala.reflect.internal.util.BatchSourceFile
 import scala.tools.asm.Opcodes._
 import org.junit.Assert._
 
 import scala.tools.asm.tree._
 import scala.tools.asm.tree.analysis._
 import scala.tools.nsc.backend.jvm.opt.BytecodeUtils.BasicAnalyzer
+import scala.tools.nsc.io._
 import scala.tools.testing.AssertUtil._
 
 import CodeGenTools._
@@ -57,7 +60,7 @@ class InlinerTest extends ClearAfterClass {
   def inlineTest(code: String, mod: ClassNode => Unit = _ => ()): (MethodNode, Option[String]) = {
     val List(cls) = compile(code)
     mod(cls)
-    val clsBType = classBTypeFromParsedClassfile(cls.name)
+    val clsBType = classBTypeFromParsedClassfile(cls.name).get
 
     val List(f, g) = cls.methods.asScala.filter(m => Set("f", "g")(m.name)).toList.sortBy(_.name)
     val fCall = g.instructions.iterator.asScala.collect({ case i: MethodInsnNode if i.name == "f" => i }).next()
@@ -191,8 +194,8 @@ class InlinerTest extends ClearAfterClass {
 
     val List(c, d) = compile(code)
 
-    val cTp = classBTypeFromParsedClassfile(c.name)
-    val dTp = classBTypeFromParsedClassfile(d.name)
+    val cTp = classBTypeFromParsedClassfile(c.name).get
+    val dTp = classBTypeFromParsedClassfile(d.name).get
 
     val g = c.methods.asScala.find(_.name == "g").get
     val h = d.methods.asScala.find(_.name == "h").get
@@ -350,7 +353,7 @@ class InlinerTest extends ClearAfterClass {
     val List(c) = compile(code)
     val f = c.methods.asScala.find(_.name == "f").get
     val callsiteIns = f.instructions.iterator().asScala.collect({ case c: MethodInsnNode => c }).next()
-    val clsBType = classBTypeFromParsedClassfile(c.name)
+    val clsBType = classBTypeFromParsedClassfile(c.name).get
     val analyzer = new BasicAnalyzer(f, clsBType.internalName)
 
     val integerClassBType = classBTypeFromInternalName("java/lang/Integer")
@@ -414,4 +417,36 @@ class InlinerTest extends ClearAfterClass {
     // like maxLocals for g1 / f1, but no return value
     assert(g2.maxLocals == 4 && f2.maxLocals == 3, s"${g2.maxLocals} - ${f2.maxLocals}")
   }
+
+  @Test
+  def mixedCompilationNoInline(): Unit = {
+    // The inliner checks if the invocation `A.bar` can be safely inlined. For that it needs to have
+    // the bytecode of the invoked method. In mixed compilation, there's no classfile available for
+    // A, so `flop` cannot be inlined, we cannot check if it's safe.
+
+    val javaCode =
+      """public class A {
+        |  public static final int bar() { return 100; }
+        |}
+      """.stripMargin
+
+    val scalaCode =
+      """class B {
+        |  @inline final def flop = A.bar
+        |  def g = flop
+        |}
+      """.stripMargin
+
+    InlinerTest.notPerRun.foreach(_.clear())
+    compiler.reporter.reset()
+    compiler.settings.outputDirs.setSingleOutput(new VirtualDirectory("(memory)", None))
+    val run = new compiler.Run()
+    run.compileSources(List(new BatchSourceFile("A.java", javaCode), new BatchSourceFile("B.scala", scalaCode)))
+    val outDir = compiler.settings.outputDirs.getSingleOutput.get
+
+    val List(b) = outDir.iterator.map(f => AsmUtils.readClass(f.toByteArray)).toList.sortBy(_.name)
+    val ins = getSingleMethod(b, "g").instructions
+    val invokeFlop = Invoke(INVOKEVIRTUAL, "B", "flop", "()I", false)
+    assert(ins contains invokeFlop, ins mkString "\n")
+  }
 }