diff options
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") + } } |