Looking up the ClassNode for an InternalName returns an Option

The `ByteCodeRepository.classNode(InternalName)` method now returns an
option. Concretely, in mixed compilation, the compiler does not create
a ClassNode for Java classes, and they may not exist on the classpath
either.

9 files changed, 58 insertions, 44 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
index b4de5cf52f..32a421c570 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
@@ -129,7 +129,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
 
       if (settings.YoptInlinerEnabled) {
         // The inliner needs to find all classes in the code repo, also those being compiled
-        byteCodeRepository.classes(cnode.name) = (cnode, ByteCodeRepository.CompilationUnit)
+        byteCodeRepository.classes(cnode.name) = Some((cnode, ByteCodeRepository.CompilationUnit))
       }
 
       assert(cd.symbol == claszSymbol, "Someone messed up BCodePhase.claszSymbol during genPlainClass().")
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala
index c93496fb49..f07c7b7764 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala
@@ -106,7 +106,13 @@ abstract class BTypes {
    * Parse the classfile for `internalName` and construct the [[ClassBType]].
    */
   def classBTypeFromParsedClassfile(internalName: InternalName): ClassBType = {
-    classBTypeFromClassNode(byteCodeRepository.classNode(internalName))
+    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")
+    }
+    classBTypeFromClassNode(classNode)
   }
 
   /**
@@ -141,11 +147,15 @@ abstract class BTypes {
      * For local and anonymous classes, innerClassNode.outerName is null. Such classes are required
      * to have an EnclosingMethod attribute declaring the outer class. So we keep those local and
      * anonymous classes whose outerClass is classNode.name.
-     *
      */
     def nestedInCurrentClass(innerClassNode: InnerClassNode): Boolean = {
       (innerClassNode.outerName != null && innerClassNode.outerName == classNode.name) ||
-      (innerClassNode.outerName == null && byteCodeRepository.classNode(innerClassNode.name).outerClass == classNode.name)
+      (innerClassNode.outerName == null && {
+        val classNodeForInnerClass = byteCodeRepository.classNode(innerClassNode.name) getOrElse {
+          assertionError(s"Could not find bytecode for class ${innerClassNode.name}")
+        }
+        classNodeForInnerClass.outerClass == classNode.name
+      })
     }
 
     val nestedClasses: List[ClassBType] = classNode.innerClasses.asScala.collect({
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala
index 9ed7b3174b..d94bd77851 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala
@@ -36,7 +36,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
   val coreBTypes = new CoreBTypesProxy[this.type](this)
   import coreBTypes._
 
-  val byteCodeRepository = new ByteCodeRepository(global.classPath, recordPerRunCache(collection.concurrent.TrieMap.empty[InternalName, (ClassNode, Source)]))
+  val byteCodeRepository = new ByteCodeRepository(global.classPath, recordPerRunCache(collection.concurrent.TrieMap.empty))
 
   val inliner: Inliner[this.type] = new Inliner(this)
 
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala
index b3ac06877b..ea4dd0c032 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala
@@ -25,21 +25,23 @@ import BTypes.InternalName
  * @param classes   Cache for parsed ClassNodes. Also stores the source of the bytecode:
  *                  [[Classfile]] if read from `classPath`, [[CompilationUnit]] if the bytecode
  *                  corresponds to a class being compiled.
+ *                  For Java classes in mixed compilation, the map contains `None`: there is no
+ *                  ClassNode generated by the backend and also no classfile that could be parsed.
  */
-class ByteCodeRepository(val classPath: ClassFileLookup[AbstractFile], val classes: collection.concurrent.Map[InternalName, (ClassNode, Source)]) {
+class ByteCodeRepository(val classPath: ClassFileLookup[AbstractFile], val classes: collection.concurrent.Map[InternalName, Option[(ClassNode, Source)]]) {
   /**
    * The class node and source for an internal name. If the class node is not yet available, it is
    * parsed from the classfile on the compile classpath.
    */
-  def classNodeAndSource(internalName: InternalName): (ClassNode, Source) = {
-    classes.getOrElseUpdate(internalName, (parseClass(internalName), Classfile))
+  def classNodeAndSource(internalName: InternalName): Option[(ClassNode, Source)] = {
+    classes.getOrElseUpdate(internalName, parseClass(internalName).map((_, Classfile)))
   }
 
   /**
    * The class node for an internal name. If the class node is not yet available, it is parsed from
    * the classfile on the compile classpath.
    */
-  def classNode(internalName: InternalName) = classNodeAndSource(internalName)._1
+  def classNode(internalName: InternalName): Option[ClassNode] = classNodeAndSource(internalName).map(_._1)
 
   /**
    * The field node for a field matching `name` and `descriptor`, accessed in class `classInternalName`.
@@ -48,10 +50,10 @@ class ByteCodeRepository(val classPath: ClassFileLookup[AbstractFile], val class
    * @return The [[FieldNode]] of the requested field and the [[InternalName]] of its declaring class.
    */
   def fieldNode(classInternalName: InternalName, name: String, descriptor: String): Option[(FieldNode, InternalName)] = {
-    val c = classNode(classInternalName)
-    c.fields.asScala.find(f => f.name == name && f.desc == descriptor).map((_, classInternalName)) orElse {
-      Option(c.superName).flatMap(n => fieldNode(n, name, descriptor))
-    }
+    classNode(classInternalName).flatMap(c =>
+      c.fields.asScala.find(f => f.name == name && f.desc == descriptor).map((_, classInternalName)) orElse {
+        Option(c.superName).flatMap(n => fieldNode(n, name, descriptor))
+      })
   }
 
   /**
@@ -64,16 +66,16 @@ class ByteCodeRepository(val classPath: ClassFileLookup[AbstractFile], val class
     // In a MethodInsnNode, the `owner` field may be an array descriptor, for exmple when invoking `clone`.
     if (ownerInternalNameOrArrayDescriptor.charAt(0) == '[') None
     else {
-      val c = classNode(ownerInternalNameOrArrayDescriptor)
-      c.methods.asScala.find(m => m.name == name && m.desc == descriptor).map((_, ownerInternalNameOrArrayDescriptor)) orElse {
-        val parents = Option(c.superName) ++ c.interfaces.asScala
-        // `view` to stop at the first result
-        parents.view.flatMap(methodNode(_, name, descriptor)).headOption
-      }
+      classNode(ownerInternalNameOrArrayDescriptor).flatMap(c =>
+        c.methods.asScala.find(m => m.name == name && m.desc == descriptor).map((_, ownerInternalNameOrArrayDescriptor)) orElse {
+          val parents = Option(c.superName) ++ c.interfaces.asScala
+          // `view` to stop at the first result
+          parents.view.flatMap(methodNode(_, name, descriptor)).headOption
+        })
     }
   }
 
-  private def parseClass(internalName: InternalName): ClassNode = {
+  private def parseClass(internalName: InternalName): Option[ClassNode] = {
     val fullName = internalName.replace('/', '.')
     classPath.findClassFile(fullName) map { classFile =>
       val classNode = new asm.tree.ClassNode()
@@ -90,8 +92,6 @@ class ByteCodeRepository(val classPath: ClassFileLookup[AbstractFile], val class
       //   https://jcp.org/aboutJava/communityprocess/final/jsr045/index.html
       removeLineNumberNodes(classNode)
       classNode
-    } getOrElse {
-      inlineFailure(s"Class file for class $fullName not found.")
     }
   }
 }
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala
index bfaa67004c..ac40ab8904 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala
@@ -34,27 +34,30 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
     methodNode.instructions.iterator.asScala.collect({
       case call: MethodInsnNode =>
         // TODO: log an inliner warning if the callee method cannot be found in the code repo? eg it's not on the classpath.
-        val callee = byteCodeRepository.methodNode(call.owner, call.name, call.desc) map {
+        val callee = byteCodeRepository.methodNode(call.owner, call.name, call.desc) flatMap {
           case (method, declarationClass) =>
-            val (declarationClassNode, source) = byteCodeRepository.classNodeAndSource(declarationClass)
-            val declarationClassBType          = classBTypeFromClassNode(declarationClassNode)
-            val methodSignature                = method.name + method.desc
-            val (safeToInline, annotatedInline, annotatedNoInline) = declarationClassBType.info.inlineInfos.get(methodSignature) match {
-              case Some(inlineInfo) =>
-                val canInlineFromSource = inlineGlobalEnabled || source == ByteCodeRepository.CompilationUnit
-                // TODO: for now, we consider a callee safeToInline only if it's final
-                // type analysis can render more calls safeToInline (e.g. when the precise receiver type is known)
-                (canInlineFromSource && inlineInfo.effectivelyFinal, Some(inlineInfo.annotatedInline), Some(inlineInfo.annotatedNoInline))
-              case None =>
-                (false, None, None)
+            // TODO: log inliner warning if callee decl class cannot be found?
+            byteCodeRepository.classNodeAndSource(declarationClass) map {
+              case (declarationClassNode, source) =>
+                val declarationClassBType = classBTypeFromClassNode(declarationClassNode)
+                val methodSignature = method.name + method.desc
+                val (safeToInline, annotatedInline, annotatedNoInline) = declarationClassBType.info.inlineInfos.get(methodSignature) match {
+                  case Some(inlineInfo) =>
+                    val canInlineFromSource = inlineGlobalEnabled || source == ByteCodeRepository.CompilationUnit
+                    // TODO: for now, we consider a callee safeToInline only if it's final
+                    // type analysis can render more calls safeToInline (e.g. when the precise receiver type is known)
+                    (canInlineFromSource && inlineInfo.effectivelyFinal, Some(inlineInfo.annotatedInline), Some(inlineInfo.annotatedNoInline))
+                  case None =>
+                    (false, None, None)
+                }
+                Callee(
+                  callee = method,
+                  calleeDeclarationClass = declarationClassBType,
+                  safeToInline = safeToInline,
+                  annotatedInline = annotatedInline,
+                  annotatedNoInline = annotatedNoInline
+                )
             }
-            Callee(
-              callee = method,
-              calleeDeclarationClass = declarationClassBType,
-              safeToInline = safeToInline,
-              annotatedInline = annotatedInline,
-              annotatedNoInline = annotatedNoInline
-            )
         }
 
         val argInfos = if (callee.isEmpty) Nil else {
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/OptimizerReporting.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/OptimizerReporting.scala
index a918e13534..53c00c7724 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/OptimizerReporting.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/OptimizerReporting.scala
@@ -21,6 +21,7 @@ object OptimizerReporting {
     classInternalName + "::" + method.name + method.desc
   }
 
+  // TODO: clean up reporting of the inliner, test inline failure warnings, etc
   def inlineFailure(reason: String): Nothing = MissingRequirementError.signal(reason)
   def assertionError(message: String): Nothing = throw new AssertionError(message)
 }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala
index 5946f50f0c..69bd92b4ba 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala
@@ -70,7 +70,7 @@ class CallGraphTest {
 
     // Get the ClassNodes from the code repo (don't use the unparsed ClassNodes returned by compile).
     // The callGraph.callsites map is indexed by instructions of those ClassNodes.
-    val List(cCls, cMod, dCls, testCls) = compile(code).map(c => byteCodeRepository.classNode(c.name))
+    val List(cCls, cMod, dCls, testCls) = compile(code).map(c => byteCodeRepository.classNode(c.name).get)
 
     val List(cf1, cf2, cf3, cf4, cf5, cf6, cf7) = cCls.methods.iterator.asScala.filter(_.name.startsWith("f")).toList.sortBy(_.name)
     val List(df1, df3) = dCls.methods.iterator.asScala.filter(_.name.startsWith("f")).toList.sortBy(_.name)
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 36f297767e..40dc990c0f 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala
@@ -31,7 +31,7 @@ class InlinerIllegalAccessTest extends ClearAfterClass {
   val compiler = InlinerIllegalAccessTest.compiler
   import compiler.genBCode.bTypes._
 
-  def addToRepo(cls: List[ClassNode]): Unit = for (c <- cls) byteCodeRepository.classes(c.name) = (c, ByteCodeRepository.Classfile)
+  def addToRepo(cls: List[ClassNode]): Unit = for (c <- cls) byteCodeRepository.classes(c.name) = Some((c, ByteCodeRepository.Classfile))
   def assertEmpty(ins: Option[AbstractInsnNode]) = for (i <- ins) throw new AssertionError(textify(i))
 
   @Test
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 819252841e..240d106f5c 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
@@ -47,7 +47,7 @@ class InlinerTest extends ClearAfterClass {
   def checkCallsite(callsite: callGraph.Callsite, callee: MethodNode) = {
     assert(callsite.callsiteMethod.instructions.contains(callsite.callsiteInstruction), instructionsFromMethod(callsite.callsiteMethod))
 
-    val callsiteClassNode = byteCodeRepository.classNode(callsite.callsiteClass.internalName)
+    val callsiteClassNode = byteCodeRepository.classNode(callsite.callsiteClass.internalName).get
     assert(callsiteClassNode.methods.contains(callsite.callsiteMethod), callsiteClassNode.methods.asScala.map(_.name).toList)
 
     assert(callsite.callee.get.callee == callee, callsite.callee.get.callee.name)