diff options
16 files changed, 368 insertions, 149 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala b/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala index 4fc05cafdc..4eb24d13e3 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala @@ -1,7 +1,7 @@ package scala.tools.nsc package backend.jvm -import scala.tools.asm.tree.{AbstractInsnNode, MethodNode} +import scala.tools.asm.tree.{InvokeDynamicInsnNode, AbstractInsnNode, MethodNode} import scala.tools.nsc.backend.jvm.BTypes.InternalName import scala.reflect.internal.util.Position import scala.tools.nsc.settings.ScalaSettings @@ -246,6 +246,11 @@ object BackendReporting { case class ResultingMethodTooLarge(calleeDeclarationClass: InternalName, name: String, descriptor: String, callsiteClass: InternalName, callsiteName: String, callsiteDesc: String) extends CannotInlineWarning + case object UnknownInvokeDynamicInstruction extends OptimizerWarning { + override def toString = "The callee contains an InvokeDynamic instruction with an unknown bootstrap method (not a LambdaMetaFactory)." + def emitWarning(settings: ScalaSettings): Boolean = settings.YoptWarningEmitAtInlineFailed + } + /** * Used in `rewriteClosureApplyInvocations` when a closure apply callsite cannot be rewritten * to the closure body method. diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala index cd36fd8bba..df8dcc690a 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala @@ -104,6 +104,8 @@ object BytecodeUtils { def isStrictfpMethod(methodNode: MethodNode): Boolean = (methodNode.access & Opcodes.ACC_STRICT) != 0 + def isReference(t: Type) = t.getSort == Type.OBJECT || t.getSort == Type.ARRAY + def nextExecutableInstruction(instruction: AbstractInsnNode, alsoKeep: AbstractInsnNode => Boolean = Set()): Option[AbstractInsnNode] = { var result = instruction do { result = result.getNext } 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 8abecdb261..96455c0e38 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala @@ -9,7 +9,7 @@ package opt import scala.reflect.internal.util.{NoPosition, Position} import scala.tools.asm.tree.analysis.{Value, Analyzer, BasicInterpreter} -import scala.tools.asm.{Opcodes, Type} +import scala.tools.asm.{Opcodes, Type, Handle} import scala.tools.asm.tree._ import scala.collection.concurrent import scala.collection.convert.decorateAsScala._ @@ -24,7 +24,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) { val callsites: concurrent.Map[MethodInsnNode, Callsite] = recordPerRunCache(concurrent.TrieMap.empty) - val closureInstantiations: concurrent.Map[InvokeDynamicInsnNode, (MethodNode, ClassBType)] = recordPerRunCache(concurrent.TrieMap.empty) + val closureInstantiations: concurrent.Map[InvokeDynamicInsnNode, ClosureInstantiation] = recordPerRunCache(concurrent.TrieMap.empty) def addClass(classNode: ClassNode): Unit = { val classType = classBTypeFromClassNode(classNode) @@ -33,14 +33,14 @@ class CallGraph[BT <: BTypes](val btypes: BT) { (calls, closureInits) = analyzeCallsites(m, classType) } { calls foreach (callsite => callsites(callsite.callsiteInstruction) = callsite) - closureInits foreach (indy => closureInstantiations(indy) = (m, classType)) + closureInits foreach (lmf => closureInstantiations(lmf.indy) = ClosureInstantiation(lmf, m, classType)) } } /** * Returns a list of callsites in the method, plus a list of closure instantiation indy instructions. */ - def analyzeCallsites(methodNode: MethodNode, definingClass: ClassBType): (List[Callsite], List[InvokeDynamicInsnNode]) = { + def analyzeCallsites(methodNode: MethodNode, definingClass: ClassBType): (List[Callsite], List[LambdaMetaFactoryCall]) = { case class CallsiteInfo(safeToInline: Boolean, safeToRewrite: Boolean, annotatedInline: Boolean, annotatedNoInline: Boolean, @@ -129,7 +129,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) { } val callsites = new collection.mutable.ListBuffer[Callsite] - val closureInstantiations = new collection.mutable.ListBuffer[InvokeDynamicInsnNode] + val closureInstantiations = new collection.mutable.ListBuffer[LambdaMetaFactoryCall] methodNode.instructions.iterator.asScala foreach { case call: MethodInsnNode => @@ -173,8 +173,8 @@ class CallGraph[BT <: BTypes](val btypes: BT) { callsitePosition = callsitePositions.getOrElse(call, NoPosition) ) - case indy: InvokeDynamicInsnNode => - if (closureOptimizer.isClosureInstantiation(indy)) closureInstantiations += indy + case LambdaMetaFactoryCall(indy, samMethodType, implMethod, instantiatedMethodType) => + closureInstantiations += LambdaMetaFactoryCall(indy, samMethodType, implMethod, instantiatedMethodType) case _ => } @@ -236,4 +236,82 @@ class CallGraph[BT <: BTypes](val btypes: BT) { calleeInfoWarning: Option[CalleeInfoWarning]) { assert(!(safeToInline && safeToRewrite), s"A callee of ${callee.name} can be either safeToInline or safeToRewrite, but not both.") } + + final case class ClosureInstantiation(lambdaMetaFactoryCall: LambdaMetaFactoryCall, ownerMethod: MethodNode, ownerClass: ClassBType) { + override def toString = s"ClosureInstantiation($lambdaMetaFactoryCall, ${ownerMethod.name + ownerMethod.desc}, $ownerClass)" + } + final case class LambdaMetaFactoryCall(indy: InvokeDynamicInsnNode, samMethodType: Type, implMethod: Handle, instantiatedMethodType: Type) + + object LambdaMetaFactoryCall { + private val lambdaMetaFactoryInternalName: InternalName = "java/lang/invoke/LambdaMetafactory" + + private val metafactoryHandle = { + val metafactoryMethodName: String = "metafactory" + val metafactoryDesc: String = "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;" + new Handle(Opcodes.H_INVOKESTATIC, lambdaMetaFactoryInternalName, metafactoryMethodName, metafactoryDesc) + } + + private val altMetafactoryHandle = { + val altMetafactoryMethodName: String = "altMetafactory" + val altMetafactoryDesc: String = "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;" + new Handle(Opcodes.H_INVOKESTATIC, lambdaMetaFactoryInternalName, altMetafactoryMethodName, altMetafactoryDesc) + } + + def unapply(insn: AbstractInsnNode): Option[(InvokeDynamicInsnNode, Type, Handle, Type)] = insn match { + case indy: InvokeDynamicInsnNode if indy.bsm == metafactoryHandle || indy.bsm == altMetafactoryHandle => + indy.bsmArgs match { + case Array(samMethodType: Type, implMethod: Handle, instantiatedMethodType: Type, xs@_*) => // xs binding because IntelliJ gets confused about _@_* + // LambdaMetaFactory performs a number of automatic adaptations when invoking the lambda + // implementation method (casting, boxing, unboxing, and primitive widening, see Javadoc). + // + // The closure optimizer supports only one of those adaptations: it will cast arguments + // to the correct type when re-writing a closure call to the body method. Example: + // + // val fun: String => String = l => l + // val l = List("") + // fun(l.head) + // + // The samMethodType of Function1 is `(Object)Object`, while the instantiatedMethodType + // is `(String)String`. The return type of `List.head` is `Object`. + // + // The implMethod has the signature `C$anonfun(String)String`. + // + // At the closure callsite, we have an `INVOKEINTERFACE Function1.apply (Object)Object`, + // so the object returned by `List.head` can be directly passed into the call (no cast). + // + // The closure object will cast the object to String before passing it to the implMethod. + // + // When re-writing the closure callsite to the implMethod, we have to insert a cast. + // + // The check below ensures that + // (1) the implMethod type has the expected singature (captured types plus argument types + // from instantiatedMethodType) + // (2) the receiver of the implMethod matches the first captured type + // (3) all parameters that are not the same in samMethodType and instantiatedMethodType + // are reference types, so that we can insert casts to perform the same adaptation + // that the closure object would. + + val isStatic = implMethod.getTag == Opcodes.H_INVOKESTATIC + val indyParamTypes = Type.getArgumentTypes(indy.desc) + val instantiatedMethodArgTypes = instantiatedMethodType.getArgumentTypes + val expectedImplMethodType = { + val paramTypes = (if (isStatic) indyParamTypes else indyParamTypes.tail) ++ instantiatedMethodArgTypes + Type.getMethodType(instantiatedMethodType.getReturnType, paramTypes: _*) + } + + val isIndyLambda = ( + Type.getType(implMethod.getDesc) == expectedImplMethodType // (1) + && (isStatic || implMethod.getOwner == indyParamTypes(0).getInternalName) // (2) + && samMethodType.getArgumentTypes.corresponds(instantiatedMethodArgTypes)((samArgType, instArgType) => + samArgType == instArgType || isReference(samArgType) && isReference(instArgType)) // (3) + ) + + if (isIndyLambda) Some((indy, samMethodType, implMethod, instantiatedMethodType)) + else None + + case _ => None + } + case _ => None + } + } } diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala index 743a454678..86536ff0d2 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala @@ -8,6 +8,7 @@ package backend.jvm package opt import scala.annotation.switch +import scala.collection.mutable import scala.reflect.internal.util.NoPosition import scala.tools.asm.{Handle, Type, Opcodes} import scala.tools.asm.tree._ @@ -24,85 +25,28 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { import callGraph._ def rewriteClosureApplyInvocations(): Unit = { - closureInstantiations foreach { - case (indy, (methodNode, ownerClass)) => - val warnings = rewriteClosureApplyInvocations(indy, methodNode, ownerClass) - warnings.foreach(w => backendReporting.inlinerWarning(w.pos, w.toString)) - } - } + implicit object closureInitOrdering extends Ordering[ClosureInstantiation] { + override def compare(x: ClosureInstantiation, y: ClosureInstantiation): Int = { + val cls = x.ownerClass.internalName compareTo y.ownerClass.internalName + if (cls != 0) return cls - private val lambdaMetaFactoryInternalName: InternalName = "java/lang/invoke/LambdaMetafactory" + val mName = x.ownerMethod.name compareTo y.ownerMethod.name + if (mName != 0) return mName - private val metafactoryHandle = { - val metafactoryMethodName: String = "metafactory" - val metafactoryDesc: String = "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;" - new Handle(H_INVOKESTATIC, lambdaMetaFactoryInternalName, metafactoryMethodName, metafactoryDesc) - } - - private val altMetafactoryHandle = { - val altMetafactoryMethodName: String = "altMetafactory" - val altMetafactoryDesc: String = "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;" - new Handle(H_INVOKESTATIC, lambdaMetaFactoryInternalName, altMetafactoryMethodName, altMetafactoryDesc) - } + val mDesc = x.ownerMethod.desc compareTo y.ownerMethod.desc + if (mDesc != 0) return mDesc - def isClosureInstantiation(indy: InvokeDynamicInsnNode): Boolean = { - (indy.bsm == metafactoryHandle || indy.bsm == altMetafactoryHandle) && - { - indy.bsmArgs match { - case Array(samMethodType: Type, implMethod: Handle, instantiatedMethodType: Type, xs @ _*) => - // LambdaMetaFactory performs a number of automatic adaptations when invoking the lambda - // implementation method (casting, boxing, unboxing, and primitive widening, see Javadoc). - // - // The closure optimizer supports only one of those adaptations: it will cast arguments - // to the correct type when re-writing a closure call to the body method. Example: - // - // val fun: String => String = l => l - // val l = List("") - // fun(l.head) - // - // The samMethodType of Function1 is `(Object)Object`, while the instantiatedMethodType - // is `(String)String`. The return type of `List.head` is `Object`. - // - // The implMethod has the signature `C$anonfun(String)String`. - // - // At the closure callsite, we have an `INVOKEINTERFACE Function1.apply (Object)Object`, - // so the object returned by `List.head` can be directly passed into the call (no cast). - // - // The closure object will cast the object to String before passing it to the implMethod. - // - // When re-writing the closure callsite to the implMethod, we have to insert a cast. - // - // The check below ensures that - // (1) the implMethod type has the expected singature (captured types plus argument types - // from instantiatedMethodType) - // (2) the receiver of the implMethod matches the first captured type - // (3) all parameters that are not the same in samMethodType and instantiatedMethodType - // are reference types, so that we can insert casts to perform the same adaptation - // that the closure object would. - - val isStatic = implMethod.getTag == H_INVOKESTATIC - val indyParamTypes = Type.getArgumentTypes(indy.desc) - val instantiatedMethodArgTypes = instantiatedMethodType.getArgumentTypes - val expectedImplMethodType = { - val paramTypes = (if (isStatic) indyParamTypes else indyParamTypes.tail) ++ instantiatedMethodArgTypes - Type.getMethodType(instantiatedMethodType.getReturnType, paramTypes: _*) - } + def pos(inst: ClosureInstantiation) = inst.ownerMethod.instructions.indexOf(inst.lambdaMetaFactoryCall.indy) + pos(x) - pos(y) + } + } - { - Type.getType(implMethod.getDesc) == expectedImplMethodType // (1) - } && { - isStatic || implMethod.getOwner == indyParamTypes(0).getInternalName // (2) - } && { - def isReference(t: Type) = t.getSort == Type.OBJECT || t.getSort == Type.ARRAY - (samMethodType.getArgumentTypes, instantiatedMethodArgTypes).zipped forall { - case (samArgType, instArgType) => - samArgType == instArgType || isReference(samArgType) && isReference(instArgType) // (3) - } - } + val sorted = mutable.TreeSet.empty[ClosureInstantiation] + sorted ++= closureInstantiations.values - case _ => - false - } + for (closureInst <- sorted) { + val warnings = rewriteClosureApplyInvocations(closureInst) + warnings.foreach(w => backendReporting.inlinerWarning(w.pos, w.toString)) } } @@ -131,9 +75,10 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { * Stores the values captured by a closure creation into fresh local variables. * Returns the list of locals holding the captured values. */ - private def storeCaptures(indy: InvokeDynamicInsnNode, methodNode: MethodNode): LocalsList = { + private def storeCaptures(closureInit: ClosureInstantiation): LocalsList = { + val indy = closureInit.lambdaMetaFactoryCall.indy val capturedTypes = Type.getArgumentTypes(indy.desc) - val firstCaptureLocal = methodNode.maxLocals + val firstCaptureLocal = closureInit.ownerMethod.maxLocals // This could be optimized: in many cases the captured values are produced by LOAD instructions. // If the variable is not modified within the method, we could avoid introducing yet another @@ -144,10 +89,10 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { // This is checked in `isClosureInstantiation`: the types of the captured variables in the indy // instruction match exactly the corresponding parameter types in the body method. val localsForCaptures = LocalsList.fromTypes(firstCaptureLocal, capturedTypes, castLoadTypes = _ => None) - methodNode.maxLocals = firstCaptureLocal + localsForCaptures.size + closureInit.ownerMethod.maxLocals = firstCaptureLocal + localsForCaptures.size - insertStoreOps(indy, methodNode, localsForCaptures) - insertLoadOps(indy, methodNode, localsForCaptures) + insertStoreOps(indy, closureInit.ownerMethod, localsForCaptures) + insertLoadOps(indy, closureInit.ownerMethod, localsForCaptures) localsForCaptures } @@ -184,22 +129,24 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { } } - def rewriteClosureApplyInvocations(indy: InvokeDynamicInsnNode, methodNode: MethodNode, ownerClass: ClassBType): List[RewriteClosureApplyToClosureBodyFailed] = { - val lambdaBodyHandle = indy.bsmArgs(1).asInstanceOf[Handle] // safe, checked in isClosureInstantiation + def rewriteClosureApplyInvocations(closureInit: ClosureInstantiation): List[RewriteClosureApplyToClosureBodyFailed] = { + val lambdaBodyHandle = closureInit.lambdaMetaFactoryCall.implMethod + val ownerMethod = closureInit.ownerMethod + val ownerClass = closureInit.ownerClass // Kept as a lazy val to make sure the analysis is only computed if it's actually needed. // ProdCons is used to identify closure body invocations (see isSamInvocation), but only if the // callsite has the right name and signature. If the method has no invcation instruction with // the right name and signature, the analysis is not executed. - lazy val prodCons = new ProdConsAnalyzer(methodNode, ownerClass.internalName) + lazy val prodCons = new ProdConsAnalyzer(ownerMethod, ownerClass.internalName) // First collect all callsites without modifying the instructions list yet. // Once we start modifying the instruction list, prodCons becomes unusable. // A list of callsites and stack heights. If the invocation cannot be rewritten, a warning // message is stored in the stack height value. - val invocationsToRewrite: List[(MethodInsnNode, Either[RewriteClosureApplyToClosureBodyFailed, Int])] = methodNode.instructions.iterator.asScala.collect({ - case invocation: MethodInsnNode if isSamInvocation(invocation, indy, prodCons) => + val invocationsToRewrite: List[(MethodInsnNode, Either[RewriteClosureApplyToClosureBodyFailed, Int])] = ownerMethod.instructions.iterator.asScala.collect({ + case invocation: MethodInsnNode if isSamInvocation(invocation, closureInit.lambdaMetaFactoryCall.indy, prodCons) => val bodyAccessible: Either[OptimizerWarning, Boolean] = for { (bodyMethodNode, declClass) <- byteCodeRepository.methodNode(lambdaBodyHandle.getOwner, lambdaBodyHandle.getName, lambdaBodyHandle.getDesc): Either[OptimizerWarning, (MethodNode, InternalName)] isAccessible <- inliner.memberIsAccessible(bodyMethodNode.access, classBTypeFromParsedClassfile(declClass), classBTypeFromParsedClassfile(lambdaBodyHandle.getOwner), ownerClass) @@ -222,17 +169,17 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { // lazy val to make sure locals for captures and arguments are only allocated if there's // effectively a callsite to rewrite. lazy val (localsForCapturedValues, argumentLocalsList) = { - val captureLocals = storeCaptures(indy, methodNode) + val captureLocals = storeCaptures(closureInit) // allocate locals for storing the arguments of the closure apply callsites. // if there are multiple callsites, the same locals are re-used. - val argTypes = indy.bsmArgs(0).asInstanceOf[Type].getArgumentTypes // safe, checked in isClosureInstantiation - val firstArgLocal = methodNode.maxLocals + val argTypes = closureInit.lambdaMetaFactoryCall.samMethodType.getArgumentTypes + val firstArgLocal = ownerMethod.maxLocals // The comment in `isClosureInstantiation` explains why we have to introduce casts for // arguments that have different types in samMethodType and instantiatedMethodType. val castLoadTypes = { - val instantiatedMethodType = indy.bsmArgs(2).asInstanceOf[Type] + val instantiatedMethodType = closureInit.lambdaMetaFactoryCall.instantiatedMethodType (argTypes, instantiatedMethodType.getArgumentTypes).zipped map { case (samArgType, instantiatedArgType) if samArgType != instantiatedArgType => // isClosureInstantiation ensures that the two types are reference types, so we don't @@ -243,7 +190,7 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { } } val argLocals = LocalsList.fromTypes(firstArgLocal, argTypes, castLoadTypes) - methodNode.maxLocals = firstArgLocal + argLocals.size + ownerMethod.maxLocals = firstArgLocal + argLocals.size (captureLocals, argLocals) } @@ -253,20 +200,20 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { case (invocation, Right(stackHeight)) => // store arguments - insertStoreOps(invocation, methodNode, argumentLocalsList) + insertStoreOps(invocation, ownerMethod, argumentLocalsList) // drop the closure from the stack - methodNode.instructions.insertBefore(invocation, new InsnNode(POP)) + ownerMethod.instructions.insertBefore(invocation, new InsnNode(POP)) // load captured values and arguments - insertLoadOps(invocation, methodNode, localsForCapturedValues) - insertLoadOps(invocation, methodNode, argumentLocalsList) + insertLoadOps(invocation, ownerMethod, localsForCapturedValues) + insertLoadOps(invocation, ownerMethod, argumentLocalsList) // update maxStack val capturesStackSize = localsForCapturedValues.size val invocationStackHeight = stackHeight + capturesStackSize - 1 // -1 because the closure is gone - if (invocationStackHeight > methodNode.maxStack) - methodNode.maxStack = invocationStackHeight + if (invocationStackHeight > ownerMethod.maxStack) + ownerMethod.maxStack = invocationStackHeight // replace the callsite with a new call to the body method val bodyOpcode = (lambdaBodyHandle.getTag: @switch) match { @@ -275,19 +222,19 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { case H_INVOKESPECIAL => INVOKESPECIAL case H_INVOKEINTERFACE => INVOKEINTERFACE case H_NEWINVOKESPECIAL => - val insns = methodNode.instructions + val insns = ownerMethod.instructions insns.insertBefore(invocation, new TypeInsnNode(NEW, lambdaBodyHandle.getOwner)) insns.insertBefore(invocation, new InsnNode(DUP)) INVOKESPECIAL } val isInterface = bodyOpcode == INVOKEINTERFACE val bodyInvocation = new MethodInsnNode(bodyOpcode, lambdaBodyHandle.getOwner, lambdaBodyHandle.getName, lambdaBodyHandle.getDesc, isInterface) - methodNode.instructions.insertBefore(invocation, bodyInvocation) + ownerMethod.instructions.insertBefore(invocation, bodyInvocation) val returnType = Type.getReturnType(lambdaBodyHandle.getDesc) - fixLoadedNothingOrNullValue(returnType, bodyInvocation, methodNode, btypes) // see comment of that method + fixLoadedNothingOrNullValue(returnType, bodyInvocation, ownerMethod, btypes) // see comment of that method - methodNode.instructions.remove(invocation) + ownerMethod.instructions.remove(invocation) // update the call graph val originalCallsite = callGraph.callsites.remove(invocation) @@ -297,7 +244,7 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { def bodyMethodIsBeingCompiled = byteCodeRepository.classNodeAndSource(lambdaBodyHandle.getOwner).map(_._2 == CompilationUnit).getOrElse(false) val bodyMethodCallsite = Callsite( callsiteInstruction = bodyInvocation, - callsiteMethod = methodNode, + callsiteMethod = ownerMethod, callsiteClass = ownerClass, callee = bodyMethod.map({ case (bodyMethodNode, bodyMethodDeclClass) => Callee( 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 e8e848161c..8477f5461a 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala @@ -9,6 +9,7 @@ package opt import scala.annotation.tailrec import scala.tools.asm +import asm.Handle import asm.Opcodes._ import asm.tree._ import scala.collection.convert.decorateAsScala._ @@ -455,9 +456,9 @@ class Inliner[BT <: BTypes](val btypes: BT) { case indy: InvokeDynamicInsnNode => callGraph.closureInstantiations.get(indy) match { - case Some((methodNode, ownerClass)) => + case Some(closureInit) => val newIndy = instructionMap(indy).asInstanceOf[InvokeDynamicInsnNode] - callGraph.closureInstantiations(newIndy) = (callsiteMethod, callsiteClass) + callGraph.closureInstantiations(newIndy) = ClosureInstantiation(closureInit.lambdaMetaFactoryCall.copy(indy = newIndy), callsiteMethod, callsiteClass) case None => } @@ -687,9 +688,67 @@ class Inliner[BT <: BTypes](val btypes: BT) { } } - case ivd: InvokeDynamicInsnNode => - // TODO @lry check necessary conditions to inline an indy, instead of giving up - Right(false) + case _: InvokeDynamicInsnNode if destinationClass == calleeDeclarationClass => + // within the same class, any indy instruction can be inlined + Right(true) + + // does the InvokeDynamicInsnNode call LambdaMetaFactory? + case LambdaMetaFactoryCall(_, _, implMethod, _) => + // an indy instr points to a "call site specifier" (CSP) [1] + // - a reference to a bootstrap method [2] + // - bootstrap method name + // - references to constant arguments, which can be: + // - constant (string, long, int, float, double) + // - class + // - method type (without name) + // - method handle + // - a method name+type + // + // execution [3] + // - resolve the CSP, yielding the boostrap method handle, the static args and the name+type + // - resolution entails accessibility checking [4] + // - execute the `invoke` method of the boostrap method handle (which is signature polymorphic, check its javadoc) + // - the descriptor for the call is made up from the actual arguments on the stack: + // - the first parameters are "MethodHandles.Lookup, String, MethodType", then the types of the constant arguments, + // - the return type is CallSite + // - the values for the call are + // - the bootstrap method handle of the CSP is the receiver + // - the Lookup object for the class in which the callsite occurs (obtained as through calling MethodHandles.lookup()) + // - the method name of the CSP + // - the method type of the CSP + // - the constants of the CSP (primitives are not boxed) + // - the resulting `CallSite` object + // - has as `type` the method type of the CSP + // - is popped from the operand stack + // - the `invokeExact` method (signature polymorphic!) of the `target` method handle of the CallSite is invoked + // - the method descriptor is that of the CSP + // - the receiver is the target of the CallSite + // - the other argument values are those that were on the operand stack at the indy instruction (indyLambda: the captured values) + // + // [1] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.4.10 + // [2] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.23 + // [3] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.invokedynamic + // [4] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-5.html#jvms-5.4.3 + + // We cannot generically check if an `invokedynamic` instruction can be safely inlined into + // a different class, that depends on the bootstrap method. The Lookup object passed to the + // bootstrap method is a capability to access private members of the callsite class. We can + // only move the invokedynamic to a new class if we know that the bootstrap method doesn't + // use this capability for otherwise non-accessible members. + // In the case of indyLambda, it depends on the visibility of the implMethod handle. If + // the implMethod is public, lambdaMetaFactory doesn't use the Lookup object's extended + // capability, and we can safely inline the instruction into a different class. + + val methodRefClass = classBTypeFromParsedClassfile(implMethod.getOwner) + for { + (methodNode, methodDeclClassNode) <- byteCodeRepository.methodNode(methodRefClass.internalName, implMethod.getName, implMethod.getDesc): Either[OptimizerWarning, (MethodNode, InternalName)] + methodDeclClass = classBTypeFromParsedClassfile(methodDeclClassNode) + res <- memberIsAccessible(methodNode.access, methodDeclClass, methodRefClass, destinationClass) + } yield { + res + } + + case _: InvokeDynamicInsnNode => Left(UnknownInvokeDynamicInstruction) case ci: LdcInsnNode => ci.cst match { case t: asm.Type => classIsAccessible(bTypeForDescriptorOrInternalNameFromClassfile(t.getInternalName), destinationClass) diff --git a/src/partest-extras/scala/tools/partest/ASMConverters.scala b/src/partest-extras/scala/tools/partest/ASMConverters.scala index f6e2d2a9ec..e8d327d352 100644 --- a/src/partest-extras/scala/tools/partest/ASMConverters.scala +++ b/src/partest-extras/scala/tools/partest/ASMConverters.scala @@ -58,21 +58,24 @@ object ASMConverters { case class Method(instructions: List[Instruction], handlers: List[ExceptionHandler], localVars: List[LocalVariable]) - case class Field (opcode: Int, owner: String, name: String, desc: String) extends Instruction - case class Incr (opcode: Int, `var`: Int, incr: Int) extends Instruction - case class Op (opcode: Int) extends Instruction - case class IntOp (opcode: Int, operand: Int) extends Instruction - case class Jump (opcode: Int, label: Label) extends Instruction - case class Ldc (opcode: Int, cst: Any) extends Instruction - case class LookupSwitch(opcode: Int, dflt: Label, keys: List[Int], labels: List[Label]) extends Instruction - case class TableSwitch (opcode: Int, min: Int, max: Int, dflt: Label, labels: List[Label]) extends Instruction - case class Invoke (opcode: Int, owner: String, name: String, desc: String, itf: Boolean) extends Instruction - case class NewArray (opcode: Int, desc: String, dims: Int) extends Instruction - case class TypeOp (opcode: Int, desc: String) extends Instruction - case class VarOp (opcode: Int, `var`: Int) extends Instruction - case class Label (offset: Int) extends Instruction { def opcode: Int = -1 } - case class FrameEntry (`type`: Int, local: List[Any], stack: List[Any]) extends Instruction { def opcode: Int = -1 } - case class LineNumber (line: Int, start: Label) extends Instruction { def opcode: Int = -1 } + case class Field (opcode: Int, owner: String, name: String, desc: String) extends Instruction + case class Incr (opcode: Int, `var`: Int, incr: Int) extends Instruction + case class Op (opcode: Int) extends Instruction + case class IntOp (opcode: Int, operand: Int) extends Instruction + case class Jump (opcode: Int, label: Label) extends Instruction + case class Ldc (opcode: Int, cst: Any) extends Instruction + case class LookupSwitch (opcode: Int, dflt: Label, keys: List[Int], labels: List[Label]) extends Instruction + case class TableSwitch (opcode: Int, min: Int, max: Int, dflt: Label, labels: List[Label]) extends Instruction + case class Invoke (opcode: Int, owner: String, name: String, desc: String, itf: Boolean) extends Instruction + case class InvokeDynamic(opcode: Int, name: String, desc: String, bsm: MethodHandle, bsmArgs: List[AnyRef]) extends Instruction + case class NewArray (opcode: Int, desc: String, dims: Int) extends Instruction + case class TypeOp (opcode: Int, desc: String) extends Instruction + case class VarOp (opcode: Int, `var`: Int) extends Instruction + case class Label (offset: Int) extends Instruction { def opcode: Int = -1 } + case class FrameEntry (`type`: Int, local: List[Any], stack: List[Any]) extends Instruction { def opcode: Int = -1 } + case class LineNumber (line: Int, start: Label) extends Instruction { def opcode: Int = -1 } + + case class MethodHandle(tag: Int, owner: String, name: String, desc: String) case class ExceptionHandler(start: Label, end: Label, handler: Label, desc: Option[String]) case class LocalVariable(name: String, desc: String, signature: Option[String], start: Label, end: Label, index: Int) @@ -111,6 +114,7 @@ object ASMConverters { case i: t.LookupSwitchInsnNode => LookupSwitch (op(i), applyLabel(i.dflt), lst(i.keys) map (x => x: Int), lst(i.labels) map applyLabel) case i: t.TableSwitchInsnNode => TableSwitch (op(i), i.min, i.max, applyLabel(i.dflt), lst(i.labels) map applyLabel) case i: t.MethodInsnNode => Invoke (op(i), i.owner, i.name, i.desc, i.itf) + case i: t.InvokeDynamicInsnNode => InvokeDynamic(op(i), i.name, i.desc, convertMethodHandle(i.bsm), convertBsmArgs(i.bsmArgs)) case i: t.MultiANewArrayInsnNode => NewArray (op(i), i.desc, i.dims) case i: t.TypeInsnNode => TypeOp (op(i), i.desc) case i: t.VarInsnNode => VarOp (op(i), i.`var`) @@ -119,6 +123,13 @@ object ASMConverters { case i: t.LineNumberNode => LineNumber (i.line, applyLabel(i.start)) } + private def convertBsmArgs(a: Array[Object]): List[Object] = a.map({ + case h: asm.Handle => convertMethodHandle(h) + case _ => a // can be: Class, method Type, primitive constant + })(collection.breakOut) + + private def convertMethodHandle(h: asm.Handle): MethodHandle = MethodHandle(h.getTag, h.getOwner, h.getName, h.getDesc) + private def convertHandlers(method: t.MethodNode): List[ExceptionHandler] = { method.tryCatchBlocks.asScala.map(h => ExceptionHandler(applyLabel(h.start), applyLabel(h.end), applyLabel(h.handler), Option(h.`type`)))(collection.breakOut) } @@ -197,21 +208,28 @@ object ASMConverters { case x => x.asInstanceOf[Object] } + def unconvertMethodHandle(h: MethodHandle): asm.Handle = new asm.Handle(h.tag, h.owner, h.name, h.desc) + def unconvertBsmArgs(a: List[Object]): Array[Object] = a.map({ + case h: MethodHandle => unconvertMethodHandle(h) + case o => o + })(collection.breakOut) + private def visitMethod(method: t.MethodNode, instruction: Instruction, asmLabel: Map[Label, asm.Label]): Unit = instruction match { - case Field(op, owner, name, desc) => method.visitFieldInsn(op, owner, name, desc) - case Incr(op, vr, incr) => method.visitIincInsn(vr, incr) - case Op(op) => method.visitInsn(op) - case IntOp(op, operand) => method.visitIntInsn(op, operand) - case Jump(op, label) => method.visitJumpInsn(op, asmLabel(label)) - case Ldc(op, cst) => method.visitLdcInsn(cst) - case LookupSwitch(op, dflt, keys, labels) => method.visitLookupSwitchInsn(asmLabel(dflt), keys.toArray, (labels map asmLabel).toArray) - case TableSwitch(op, min, max, dflt, labels) => method.visitTableSwitchInsn(min, max, asmLabel(dflt), (labels map asmLabel).toArray: _*) - case Invoke(op, owner, name, desc, itf) => method.visitMethodInsn(op, owner, name, desc, itf) - case NewArray(op, desc, dims) => method.visitMultiANewArrayInsn(desc, dims) - case TypeOp(op, desc) => method.visitTypeInsn(op, desc) - case VarOp(op, vr) => method.visitVarInsn(op, vr) - case l: Label => method.visitLabel(asmLabel(l)) - case FrameEntry(tp, local, stack) => method.visitFrame(tp, local.length, frameTypesToAsm(local, asmLabel).toArray, stack.length, frameTypesToAsm(stack, asmLabel).toArray) - case LineNumber(line, start) => method.visitLineNumber(line, asmLabel(start)) + case Field(op, owner, name, desc) => method.visitFieldInsn(op, owner, name, desc) + case Incr(op, vr, incr) => method.visitIincInsn(vr, incr) + case Op(op) => method.visitInsn(op) + case IntOp(op, operand) => method.visitIntInsn(op, operand) + case Jump(op, label) => method.visitJumpInsn(op, asmLabel(label)) + case Ldc(op, cst) => method.visitLdcInsn(cst) + case LookupSwitch(op, dflt, keys, labels) => method.visitLookupSwitchInsn(asmLabel(dflt), keys.toArray, (labels map asmLabel).toArray) + case TableSwitch(op, min, max, dflt, labels) => method.visitTableSwitchInsn(min, max, asmLabel(dflt), (labels map asmLabel).toArray: _*) + case Invoke(op, owner, name, desc, itf) => method.visitMethodInsn(op, owner, name, desc, itf) + case InvokeDynamic(op, name, desc, bsm, bsmArgs) => method.visitInvokeDynamicInsn(name, desc, unconvertMethodHandle(bsm), unconvertBsmArgs(bsmArgs)) + case NewArray(op, desc, dims) => method.visitMultiANewArrayInsn(desc, dims) + case TypeOp(op, desc) => method.visitTypeInsn(op, desc) + case VarOp(op, vr) => method.visitVarInsn(op, vr) + case l: Label => method.visitLabel(asmLabel(l)) + case FrameEntry(tp, local, stack) => method.visitFrame(tp, local.length, frameTypesToAsm(local, asmLabel).toArray, stack.length, frameTypesToAsm(stack, asmLabel).toArray) + case LineNumber(line, start) => method.visitLineNumber(line, asmLabel(start)) } } diff --git a/test/files/neg/inlineIndyLambdaPrivate.check b/test/files/neg/inlineIndyLambdaPrivate.check new file mode 100644 index 0000000000..dbd142f59e --- /dev/null +++ b/test/files/neg/inlineIndyLambdaPrivate.check @@ -0,0 +1,16 @@ +Test_2.scala:2: warning: A_1::test()Ljava/lang/String; could not be inlined: +The callee A_1::test()Ljava/lang/String; contains the instruction INVOKEDYNAMIC m()LA_1$Fun; [ + // handle kind 0x6 : INVOKESTATIC + java/lang/invoke/LambdaMetafactory.metafactory(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite; + // arguments: + (Ljava/lang/String;)Ljava/lang/String;, + // handle kind 0x6 : INVOKESTATIC + A_1.lambda$test$0(Ljava/lang/String;)Ljava/lang/String;, + (Ljava/lang/String;)Ljava/lang/String; + ] +that would cause an IllegalAccessError when inlined into class Test. + def foo = A_1.test + ^ +error: No warnings can be incurred under -Xfatal-warnings. +one warning found +one error found diff --git a/test/files/neg/inlineIndyLambdaPrivate.flags b/test/files/neg/inlineIndyLambdaPrivate.flags new file mode 100644 index 0000000000..01b466bd8c --- /dev/null +++ b/test/files/neg/inlineIndyLambdaPrivate.flags @@ -0,0 +1 @@ +-Yopt:l:classpath -Yopt-inline-heuristics:everything -Yopt-warnings:_ -Xfatal-warnings
\ No newline at end of file diff --git a/test/files/neg/inlineIndyLambdaPrivate/A_1.java b/test/files/neg/inlineIndyLambdaPrivate/A_1.java new file mode 100644 index 0000000000..a9144a9fa6 --- /dev/null +++ b/test/files/neg/inlineIndyLambdaPrivate/A_1.java @@ -0,0 +1,9 @@ +public class A_1 { + interface Fun { + String m(String s); + } + public static final String test() { + Fun f = s -> s.trim(); + return f.m(" eh "); + } +} diff --git a/test/files/neg/inlineIndyLambdaPrivate/Test_2.scala b/test/files/neg/inlineIndyLambdaPrivate/Test_2.scala new file mode 100644 index 0000000000..dd59c05176 --- /dev/null +++ b/test/files/neg/inlineIndyLambdaPrivate/Test_2.scala @@ -0,0 +1,3 @@ +class Test { + def foo = A_1.test +} diff --git a/test/files/presentation/t7678/Runner.scala b/test/files/presentation/t7678/Runner.scala index e45f057ff1..14d6dc2a70 100644 --- a/test/files/presentation/t7678/Runner.scala +++ b/test/files/presentation/t7678/Runner.scala @@ -1,6 +1,3 @@ -/* - * filter: inliner warnings; re-run with - */ import scala.tools.nsc.interactive.tests._ import scala.reflect.internal.util._ diff --git a/test/files/run/noInlineUnknownIndy.check b/test/files/run/noInlineUnknownIndy.check new file mode 100644 index 0000000000..7cc6d1b675 --- /dev/null +++ b/test/files/run/noInlineUnknownIndy.check @@ -0,0 +1,13 @@ +newSource1.scala:1: warning: A_1::test()Ljava/lang/String; could not be inlined: +Failed to check if A_1::test()Ljava/lang/String; can be safely inlined to T without causing an IllegalAccessError. Checking instruction INVOKEDYNAMIC m()LA_1$Fun; [ + // handle kind 0x6 : INVOKESTATIC + not/java/lang/SomeLambdaMetafactory.notAMetaFactoryMethod(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite; + // arguments: + (Ljava/lang/String;)Ljava/lang/String;, + // handle kind 0x6 : INVOKESTATIC + A_1.lambda$test$0(Ljava/lang/String;)Ljava/lang/String;, + (Ljava/lang/String;)Ljava/lang/String; + ] failed: +The callee contains an InvokeDynamic instruction with an unknown bootstrap method (not a LambdaMetaFactory). +class T { def foo = A_1.test } + ^ diff --git a/test/files/run/noInlineUnknownIndy/A_1.java b/test/files/run/noInlineUnknownIndy/A_1.java new file mode 100644 index 0000000000..a9144a9fa6 --- /dev/null +++ b/test/files/run/noInlineUnknownIndy/A_1.java @@ -0,0 +1,9 @@ +public class A_1 { + interface Fun { + String m(String s); + } + public static final String test() { + Fun f = s -> s.trim(); + return f.m(" eh "); + } +} diff --git a/test/files/run/noInlineUnknownIndy/Test.scala b/test/files/run/noInlineUnknownIndy/Test.scala new file mode 100644 index 0000000000..16d8126543 --- /dev/null +++ b/test/files/run/noInlineUnknownIndy/Test.scala @@ -0,0 +1,28 @@ +import java.io.File + +import scala.collection.convert.decorateAsScala._ +import scala.tools.asm.tree.{ClassNode, InvokeDynamicInsnNode} +import scala.tools.asm.{Handle, Opcodes} +import scala.tools.partest.BytecodeTest.modifyClassFile +import scala.tools.partest._ + +object Test extends DirectTest { + def code = ??? + + def compileCode(code: String) = { + val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator") + compileString(newCompiler("-cp", classpath, "-d", testOutput.path, "-Yopt:l:classpath", "-Yopt-inline-heuristics:everything", "-Yopt-warnings:_"))(code) + } + + def show(): Unit = { + val unknownBootstrapMethod = new Handle(Opcodes.H_INVOKESTATIC, "not/java/lang/SomeLambdaMetafactory", "notAMetaFactoryMethod", "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;") + modifyClassFile(new File(testOutput.toFile, "A_1.class"))((cn: ClassNode) => { + val testMethod = cn.methods.iterator.asScala.find(_.name == "test").head + val indy = testMethod.instructions.iterator.asScala.collect({ case i: InvokeDynamicInsnNode => i }).next() + indy.bsm = unknownBootstrapMethod + cn + }) + + compileCode("class T { def foo = A_1.test }") + } +} diff --git a/test/files/run/t8029.scala b/test/files/run/t8029.scala index 62629d51bc..dbd5c41387 100644 --- a/test/files/run/t8029.scala +++ b/test/files/run/t8029.scala @@ -1,6 +1,3 @@ -/* - * filter: inliner warning; re-run with - */ import scala.tools.partest._ import scala.tools.nsc._ 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 0309bb97cc..617eced560 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala @@ -991,4 +991,41 @@ class InlinerTest extends ClearAfterClass { assert(2 == t.collect({case Ldc(_, "hai!") => }).size) // twice the body of f assert(1 == t.collect({case Jump(IFNONNULL, _) => }).size) // one single null check } + + @Test + def inlineIndyLambda(): Unit = { + val code = + """object M { + | @inline def m(s: String) = { + | val f = (x: String) => x.trim + | f(s) + | } + |} + |class C { + | @inline final def m(s: String) = { + | val f = (x: String) => x.trim + | f(s) + | } + | def t1 = m("foo") + | def t2 = M.m("bar") + |} + """.stripMargin + + val List(c, _, _) = compile(code) + + val t1 = getSingleMethod(c, "t1") + assert(t1.instructions exists { + case _: InvokeDynamic => true + case _ => false + }) + // the indy call is inlined into t, and the closure elimination rewrites the closure invocation to the body method + assertInvoke(t1, "C", "C$$$anonfun$2") + + val t2 = getSingleMethod(c, "t2") + assert(t2.instructions exists { + case _: InvokeDynamic => true + case _ => false + }) + assertInvoke(t2, "M$", "M$$$anonfun$1") + } } |