diff options
14 files changed, 578 insertions, 270 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index 151926b8e7..121091fe4f 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala @@ -123,10 +123,19 @@ abstract class BTypes { * has the method. */ val indyLambdaImplMethods: mutable.AnyRefMap[InternalName, mutable.LinkedHashSet[asm.Handle]] = recordPerRunCache(mutable.AnyRefMap()) - def addIndyLambdaImplMethod(hostClass: InternalName, handle: Seq[asm.Handle]): Unit = { + def addIndyLambdaImplMethod(hostClass: InternalName, handle: Seq[asm.Handle]): Seq[asm.Handle] = { + if (handle.isEmpty) Nil else { + val set = indyLambdaImplMethods.getOrElseUpdate(hostClass, mutable.LinkedHashSet()) + val added = handle.filterNot(set) + set ++= handle + added + } + } + def removeIndyLambdaImplMethod(hostClass: InternalName, handle: Seq[asm.Handle]): Unit = { if (handle.nonEmpty) - indyLambdaImplMethods.getOrElseUpdate(hostClass, mutable.LinkedHashSet()) ++= handle + indyLambdaImplMethods.getOrElseUpdate(hostClass, mutable.LinkedHashSet()) --= handle } + def getIndyLambdaImplMethods(hostClass: InternalName): Iterable[asm.Handle] = { indyLambdaImplMethods.getOrNull(hostClass) match { case null => Nil diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala b/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala index 72a371cabc..e6ae073a2a 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala @@ -185,53 +185,61 @@ object BackendReporting { def name: String def descriptor: String - def calleeMethodSig = BackendReporting.methodSignature(calleeDeclarationClass, name, descriptor) - - override def toString = this match { - case IllegalAccessInstruction(_, _, _, callsiteClass, instruction) => - s"The callee $calleeMethodSig contains the instruction ${AsmUtils.textify(instruction)}" + - s"\nthat would cause an IllegalAccessError when inlined into class $callsiteClass." - - case IllegalAccessCheckFailed(_, _, _, callsiteClass, instruction, cause) => - s"Failed to check if $calleeMethodSig can be safely inlined to $callsiteClass without causing an IllegalAccessError. Checking instruction ${AsmUtils.textify(instruction)} failed:\n" + cause + /** Either the callee or the callsite is annotated @inline */ + def annotatedInline: Boolean - case MethodWithHandlerCalledOnNonEmptyStack(_, _, _, callsiteClass, callsiteName, callsiteDesc) => - s"""The operand stack at the callsite in ${BackendReporting.methodSignature(callsiteClass, callsiteName, callsiteDesc)} contains more values than the - |arguments expected by the callee $calleeMethodSig. These values would be discarded - |when entering an exception handler declared in the inlined method.""".stripMargin - - case SynchronizedMethod(_, _, _) => - s"Method $calleeMethodSig cannot be inlined because it is synchronized." + def calleeMethodSig = BackendReporting.methodSignature(calleeDeclarationClass, name, descriptor) - case StrictfpMismatch(_, _, _, callsiteClass, callsiteName, callsiteDesc) => - s"""The callsite method ${BackendReporting.methodSignature(callsiteClass, callsiteName, callsiteDesc)} - |does not have the same strictfp mode as the callee $calleeMethodSig. + override def toString = { + val annotWarn = if (annotatedInline) " is annotated @inline but" else "" + val warning = s"$calleeMethodSig$annotWarn could not be inlined:\n" + val reason = this match { + case CalleeNotFinal(_, _, _, _) => + s"The method is not final and may be overridden." + case IllegalAccessInstruction(_, _, _, _, callsiteClass, instruction) => + s"The callee $calleeMethodSig contains the instruction ${AsmUtils.textify(instruction)}" + + s"\nthat would cause an IllegalAccessError when inlined into class $callsiteClass." + + case IllegalAccessCheckFailed(_, _, _, _, callsiteClass, instruction, cause) => + s"Failed to check if $calleeMethodSig can be safely inlined to $callsiteClass without causing an IllegalAccessError. Checking instruction ${AsmUtils.textify(instruction)} failed:\n" + cause + + case MethodWithHandlerCalledOnNonEmptyStack(_, _, _, _, callsiteClass, callsiteName, callsiteDesc) => + s"""The operand stack at the callsite in ${BackendReporting.methodSignature(callsiteClass, callsiteName, callsiteDesc)} contains more values than the + |arguments expected by the callee $calleeMethodSig. These values would be discarded + |when entering an exception handler declared in the inlined method.""".stripMargin + + case SynchronizedMethod(_, _, _, _) => + s"Method $calleeMethodSig cannot be inlined because it is synchronized." + + case StrictfpMismatch(_, _, _, _, callsiteClass, callsiteName, callsiteDesc) => + s"""The callsite method ${BackendReporting.methodSignature(callsiteClass, callsiteName, callsiteDesc)} + |does not have the same strictfp mode as the callee $calleeMethodSig. """.stripMargin - case ResultingMethodTooLarge(_, _, _, callsiteClass, callsiteName, callsiteDesc) => - s"""The size of the callsite method ${BackendReporting.methodSignature(callsiteClass, callsiteName, callsiteDesc)} - |would exceed the JVM method size limit after inlining $calleeMethodSig. + case ResultingMethodTooLarge(_, _, _, _, callsiteClass, callsiteName, callsiteDesc) => + s"""The size of the callsite method ${BackendReporting.methodSignature(callsiteClass, callsiteName, callsiteDesc)} + |would exceed the JVM method size limit after inlining $calleeMethodSig. """.stripMargin + } + warning + reason } - def emitWarning(settings: ScalaSettings): Boolean = this match { - case _: IllegalAccessInstruction | _: MethodWithHandlerCalledOnNonEmptyStack | _: SynchronizedMethod | _: StrictfpMismatch | _: ResultingMethodTooLarge => - settings.optWarnings.contains(settings.optWarningsChoices.anyInlineFailed) - - case IllegalAccessCheckFailed(_, _, _, _, _, cause) => - cause.emitWarning(settings) + def emitWarning(settings: ScalaSettings): Boolean = { + settings.optWarnings.contains(settings.optWarningsChoices.anyInlineFailed) || + annotatedInline && settings.optWarningEmitAtInlineFailed } } - case class IllegalAccessInstruction(calleeDeclarationClass: InternalName, name: String, descriptor: String, + case class CalleeNotFinal(calleeDeclarationClass: InternalName, name: String, descriptor: String, annotatedInline: Boolean) extends CannotInlineWarning + case class IllegalAccessInstruction(calleeDeclarationClass: InternalName, name: String, descriptor: String, annotatedInline: Boolean, callsiteClass: InternalName, instruction: AbstractInsnNode) extends CannotInlineWarning - case class IllegalAccessCheckFailed(calleeDeclarationClass: InternalName, name: String, descriptor: String, + case class IllegalAccessCheckFailed(calleeDeclarationClass: InternalName, name: String, descriptor: String, annotatedInline: Boolean, callsiteClass: InternalName, instruction: AbstractInsnNode, cause: OptimizerWarning) extends CannotInlineWarning - case class MethodWithHandlerCalledOnNonEmptyStack(calleeDeclarationClass: InternalName, name: String, descriptor: String, + case class MethodWithHandlerCalledOnNonEmptyStack(calleeDeclarationClass: InternalName, name: String, descriptor: String, annotatedInline: Boolean, callsiteClass: InternalName, callsiteName: String, callsiteDesc: String) extends CannotInlineWarning - case class SynchronizedMethod(calleeDeclarationClass: InternalName, name: String, descriptor: String) extends CannotInlineWarning - case class StrictfpMismatch(calleeDeclarationClass: InternalName, name: String, descriptor: String, + case class SynchronizedMethod(calleeDeclarationClass: InternalName, name: String, descriptor: String, annotatedInline: Boolean) extends CannotInlineWarning + case class StrictfpMismatch(calleeDeclarationClass: InternalName, name: String, descriptor: String, annotatedInline: Boolean, callsiteClass: InternalName, callsiteName: String, callsiteDesc: String) extends CannotInlineWarning - case class ResultingMethodTooLarge(calleeDeclarationClass: InternalName, name: String, descriptor: String, + case class ResultingMethodTooLarge(calleeDeclarationClass: InternalName, name: String, descriptor: String, annotatedInline: Boolean, callsiteClass: InternalName, callsiteName: String, callsiteDesc: String) extends CannotInlineWarning // TODO: this should be a subtype of CannotInlineWarning 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 e0fd77bb54..9c0dfb0ee2 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala @@ -137,7 +137,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) { Callee( callee = method, calleeDeclarationClass = declarationClassBType, - safeToInline = safeToInline, + isStaticallyResolved = isStaticallyResolved, sourceFilePath = sourceFilePath, annotatedInline = annotatedInline, annotatedNoInline = annotatedNoInline, @@ -256,7 +256,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) { /** * Just a named tuple used as return type of `analyzeCallsite`. */ - private case class CallsiteInfo(safeToInline: Boolean, sourceFilePath: Option[String], + private case class CallsiteInfo(isStaticallyResolved: Boolean, sourceFilePath: Option[String], annotatedInline: Boolean, annotatedNoInline: Boolean, samParamTypes: IntMap[ClassBType], warning: Option[CalleeInfoWarning]) @@ -293,7 +293,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) { // TODO: type analysis can render more calls statically resolved. Example: // new A.f // can be inlined, the receiver type is known to be exactly A. val isStaticallyResolved: Boolean = { - isNonVirtualCall(call) || // SD-86: super calls (invokespecial) can be inlined + isNonVirtualCall(call) || // SD-86: super calls (invokespecial) can be inlined -- TODO: check if that's still needed, and if it's correct: scala-dev#143 methodInlineInfo.effectivelyFinal || receiverType.info.orThrow.inlineInfo.isEffectivelyFinal // (1) } @@ -301,22 +301,13 @@ class CallGraph[BT <: BTypes](val btypes: BT) { val warning = calleeDeclarationClassBType.info.orThrow.inlineInfo.warning.map( MethodInlineInfoIncomplete(calleeDeclarationClassBType.internalName, calleeMethodNode.name, calleeMethodNode.desc, _)) - // (1) For invocations of final trait methods, the callee isStaticallyResolved but also - // abstract. Such a callee is not safe to inline - it needs to be re-written to the - // static impl method first (safeToRewrite). CallsiteInfo( - safeToInline = - inlinerHeuristics.canInlineFromSource(calleeSourceFilePath) && - isStaticallyResolved && // (1) - !isAbstract && - !BytecodeUtils.isConstructor(calleeMethodNode) && - !BytecodeUtils.isNativeMethod(calleeMethodNode) && - !BytecodeUtils.hasCallerSensitiveAnnotation(calleeMethodNode), - sourceFilePath = calleeSourceFilePath, - annotatedInline = methodInlineInfo.annotatedInline, - annotatedNoInline = methodInlineInfo.annotatedNoInline, - samParamTypes = samParamTypes(calleeMethodNode, receiverType), - warning = warning) + isStaticallyResolved = isStaticallyResolved, + sourceFilePath = calleeSourceFilePath, + annotatedInline = methodInlineInfo.annotatedInline, + annotatedNoInline = methodInlineInfo.annotatedNoInline, + samParamTypes = samParamTypes(calleeMethodNode, receiverType), + warning = warning) case None => val warning = MethodInlineInfoMissing(calleeDeclarationClassBType.internalName, calleeMethodNode.name, calleeMethodNode.desc, calleeDeclarationClassBType.info.orThrow.inlineInfo.warning) @@ -353,6 +344,10 @@ class CallGraph[BT <: BTypes](val btypes: BT) { */ val inlinedClones = mutable.Set.empty[ClonedCallsite] + // an annotation at the callsite takes precedence over an annotation at the definition site + def isInlineAnnotated = annotatedInline || (callee.get.annotatedInline && !annotatedNoInline) + def isNoInlineAnnotated = annotatedNoInline || (callee.get.annotatedNoInline && !annotatedInline) + override def toString = "Invocation of" + s" ${callee.map(_.calleeDeclarationClass.internalName).getOrElse("?")}.${callsiteInstruction.name + callsiteInstruction.desc}" + @@ -378,8 +373,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) { * virtual calls, an override of the callee might be invoked. Also, * the callee can be abstract. * @param calleeDeclarationClass The class in which the callee is declared - * @param safeToInline True if the callee can be safely inlined: it cannot be overridden, - * and the inliner settings (project / global) allow inlining it. + * @param isStaticallyResolved True if the callee cannot be overridden * @param annotatedInline True if the callee is annotated @inline * @param annotatedNoInline True if the callee is annotated @noinline * @param samParamTypes A map from parameter positions to SAM parameter types @@ -387,11 +381,17 @@ class CallGraph[BT <: BTypes](val btypes: BT) { * gathering the information about this callee. */ final case class Callee(callee: MethodNode, calleeDeclarationClass: btypes.ClassBType, - safeToInline: Boolean, sourceFilePath: Option[String], + isStaticallyResolved: Boolean, sourceFilePath: Option[String], annotatedInline: Boolean, annotatedNoInline: Boolean, samParamTypes: IntMap[btypes.ClassBType], calleeInfoWarning: Option[CalleeInfoWarning]) { override def toString = s"Callee($calleeDeclarationClass.${callee.name})" + + def canInlineFromSource = inlinerHeuristics.canInlineFromSource(sourceFilePath) + def isAbstract = isAbstractMethod(callee) + def isSpecialMethod = isConstructor(callee) || isNativeMethod(callee) || hasCallerSensitiveAnnotation(callee) + + def safeToInline = isStaticallyResolved && canInlineFromSource && !isAbstract && !isSpecialMethod } /** 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 35ee5ba13d..2fca8991ab 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala @@ -359,7 +359,7 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { Callee( callee = bodyMethodNode, calleeDeclarationClass = bodyDeclClassType, - safeToInline = inlinerHeuristics.canInlineFromSource(sourceFilePath), + isStaticallyResolved = true, sourceFilePath = sourceFilePath, annotatedInline = false, annotatedNoInline = false, @@ -392,7 +392,7 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { // (x: T) => ??? has return type Nothing$, and an ATHROW is added (see fixLoadedNothingOrNullValue). unreachableCodeEliminated -= ownerMethod - if (hasAdaptedImplMethod(closureInit) && inliner.canInlineBody(bodyMethodCallsite).isEmpty) + if (hasAdaptedImplMethod(closureInit) && inliner.canInlineCallsite(bodyMethodCallsite).isEmpty) inliner.inlineCallsite(bodyMethodCallsite) } 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 64638ca34d..b9f593a4d8 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala @@ -25,49 +25,110 @@ class Inliner[BT <: BTypes](val btypes: BT) { import inlinerHeuristics._ import backendUtils._ - case class InlineLog(request: InlineRequest, sizeBefore: Int, sizeAfter: Int, sizeInlined: Int, warning: Option[CannotInlineWarning]) - var inlineLog: List[InlineLog] = Nil + sealed trait InlineLog { + def request: InlineRequest + } + final case class InlineLogSuccess(request: InlineRequest, sizeBefore: Int, sizeInlined: Int) extends InlineLog { + var downstreamLog: mutable.Buffer[InlineLog] = mutable.ListBuffer.empty + } + final case class InlineLogFail(request: InlineRequest, warning: CannotInlineWarning) extends InlineLog + final case class InlineLogRollback(request: InlineRequest, warnings: List[CannotInlineWarning]) extends InlineLog + + object InlineLog { + private def shouldLog(request: InlineRequest): Boolean = { + def logEnabled = compilerSettings.YoptLogInline.isSetByUser + def matchesName = { + val prefix = compilerSettings.YoptLogInline.value match { + case "_" => "" + case p => p + } + val name: String = request.callsite.callsiteClass.internalName + "." + request.callsite.callsiteMethod.name + name startsWith prefix + } + logEnabled && (upstream != null || (isTopLevel && matchesName)) + } - def runInliner(): Unit = { - for (request <- collectAndOrderInlineRequests) { - val Right(callee) = request.callsite.callee // collectAndOrderInlineRequests returns callsites with a known callee + // indexed by callsite method + private val logs = mutable.Map.empty[MethodNode, mutable.LinkedHashSet[InlineLog]] - // TODO: if the request has downstream requests, create a snapshot to which we could roll back in case some downstream callsite cannot be inlined - // (Needs to revert modifications to the callee method, but also the call graph) - // (This assumes that inlining a request only makes sense if its downstream requests are satisfied - sync with heuristics!) + private var upstream: InlineLogSuccess = _ + private var isTopLevel = true - val warnings = inline(request) - for (warning <- warnings) { - if ((callee.annotatedInline && btypes.compilerSettings.optWarningEmitAtInlineFailed) || warning.emitWarning(compilerSettings)) { - val annotWarn = if (callee.annotatedInline) " is annotated @inline but" else "" - val msg = s"${BackendReporting.methodSignature(callee.calleeDeclarationClass.internalName, callee.callee)}$annotWarn could not be inlined:\n$warning" - backendReporting.inlinerWarning(request.callsite.callsitePosition, msg) - } + def withInlineLogging[T](request: InlineRequest)(inlineRequest: => Unit)(inlinePost: => T): T = { + def doInlinePost(): T = { + val savedIsTopLevel = isTopLevel + isTopLevel = false + try inlinePost + finally isTopLevel = savedIsTopLevel + } + if (shouldLog(request)) { + val sizeBefore = request.callsite.callsiteMethod.instructions.size + inlineRequest + val log = InlineLogSuccess(request, sizeBefore, request.callsite.callee.get.callee.instructions.size) + apply(log) + + val savedUpstream = upstream + upstream = log + try doInlinePost() + finally upstream = savedUpstream + } else { + inlineRequest + doInlinePost() } } - if (compilerSettings.YoptLogInline.isSetByUser) { - val methodPrefix = { val p = compilerSettings.YoptLogInline.value; if (p == "_") "" else p } - val byCallsiteMethod = inlineLog.groupBy(_.request.callsite.callsiteMethod).toList.sortBy(_._2.head.request.callsite.callsiteClass.internalName) - for ((m, mLogs) <- byCallsiteMethod) { - val initialSize = mLogs.minBy(_.sizeBefore).sizeBefore - val firstLog = mLogs.head - val methodName = s"${firstLog.request.callsite.callsiteClass.internalName}.${m.name}" - if (methodName.startsWith(methodPrefix)) { - println(s"Inlining into $methodName (initially $initialSize instructions, ultimately ${m.instructions.size}):") - val byCallee = mLogs.groupBy(_.request.callsite.callee.get).toList.sortBy(_._2.length).reverse - for ((c, cLogs) <- byCallee) { - val first = cLogs.head - if (first.warning.isEmpty) { - val num = if (cLogs.tail.isEmpty) "" else s" ${cLogs.length} times" - println(s" - Inlined ${c.calleeDeclarationClass.internalName}.${c.callee.name} (${first.sizeInlined} instructions)$num: ${first.request.reason}") - } else - println(s" - Failed to inline ${c.calleeDeclarationClass.internalName}.${c.callee.name} (${first.request.reason}): ${first.warning.get}") - } - println() - } + def apply(log: => InlineLog): Unit = if (shouldLog(log.request)) { + if (upstream != null) upstream.downstreamLog += log + else { + val methodLogs = logs.getOrElseUpdate(log.request.callsite.callsiteMethod, mutable.LinkedHashSet.empty) + methodLogs += log + } + } + + def entryString(log: InlineLog, indent: Int = 0): String = { + val callee = log.request.callsite.callee.get + val calleeString = callee.calleeDeclarationClass.internalName + "." + callee.callee.name + val indentString = " " * indent + log match { + case s @ InlineLogSuccess(_, sizeBefore, sizeInlined) => + val self = s"${indentString}inlined $calleeString. Before: $sizeBefore ins, inlined: $sizeInlined ins." + if (s.downstreamLog.isEmpty) self + else s.downstreamLog.iterator.map(entryString(_, indent + 2)).mkString(self + "\n", "\n", "") + + case InlineLogFail(_, w) => + s"${indentString}failed $calleeString. ${w.toString.replace('\n', ' ')}" + + case InlineLogRollback(_, _) => + s"${indentString}rolling back, nested inline failed." + } + } + + def print(): Unit = if (compilerSettings.YoptLogInline.isSetByUser) { + val byClassAndMethod: List[(InternalName, mutable.Map[MethodNode, mutable.LinkedHashSet[InlineLog]])] = { + logs. + groupBy(_._2.head.request.callsite.callsiteClass.internalName). + toList.sortBy(_._1) + } + for { + (c, methodLogs) <- byClassAndMethod + (m, mLogs) <- methodLogs.toList.sortBy(_._1.name) + mLog <- mLogs // insertion order + } { + println(s"Inline into $c.${m.name}: ${entryString(mLog)}") + } + } + } + + def runInliner(): Unit = { + for (request <- collectAndOrderInlineRequests) { + val Right(callee) = request.callsite.callee // collectAndOrderInlineRequests returns callsites with a known callee + val warnings = inline(request) + for (warning <- warnings) { + if (warning.emitWarning(compilerSettings)) + backendReporting.inlinerWarning(request.callsite.callsitePosition, warning.toString) } } + InlineLog.print() } /** @@ -221,26 +282,79 @@ class Inliner[BT <: BTypes](val btypes: BT) { impl(post, mainCallsite) } + class UndoLog(active: Boolean = true) { + import java.util.{ ArrayList => JArrayList } + + private var actions = List.empty[() => Unit] + private var methodStateSaved = false + + def apply(a: => Unit): Unit = if (active) actions = (() => a) :: actions + def rollback(): Unit = if (active) actions.foreach(_.apply()) + + def saveMethodState(methodNode: MethodNode): Unit = if (active && !methodStateSaved) { + methodStateSaved = true + val currentInstructions = methodNode.instructions.toArray + val currentLocalVariables = new JArrayList(methodNode.localVariables) + val currentTryCatchBlocks = new JArrayList(methodNode.tryCatchBlocks) + val currentMaxLocals = methodNode.maxLocals + val currentMaxStack = methodNode.maxStack + + apply { + // `methodNode.instructions.clear()` doesn't work: it keeps the `prev` / `next` / `index` of + // instruction nodes. `instructions.removeAll(true)` would work, but is not public. + methodNode.instructions.iterator.asScala.toList.foreach(methodNode.instructions.remove) + for (i <- currentInstructions) methodNode.instructions.add(i) + + methodNode.localVariables.clear() + methodNode.localVariables.addAll(currentLocalVariables) + + methodNode.tryCatchBlocks.clear() + methodNode.tryCatchBlocks.addAll(currentTryCatchBlocks) + + methodNode.maxLocals = currentMaxLocals + methodNode.maxStack = currentMaxStack + } + } + } + + val NoUndoLogging = new UndoLog(active = false) /** * Inline the callsite of an inlining request and its post-inlining requests. * * @return An inliner warning for each callsite that could not be inlined. */ - def inline(request: InlineRequest): List[CannotInlineWarning] = canInlineBody(request.callsite) match { - case Some(w) => - if (compilerSettings.YoptLogInline.isSetByUser) { - val size = request.callsite.callsiteMethod.instructions.size - inlineLog ::= InlineLog(request, size, size, 0, Some(w)) + def inline(request: InlineRequest, undo: UndoLog = NoUndoLogging): List[CannotInlineWarning] = { + def doInline(undo: UndoLog, callRollback: Boolean = false): List[CannotInlineWarning] = { + InlineLog.withInlineLogging(request) { + inlineCallsite(request.callsite, undo) + } { + val postRequests = request.post.flatMap(adaptPostRequestForMainCallsite(_, request.callsite)) + val warnings = postRequests.flatMap(inline(_, undo)) + if (callRollback && warnings.nonEmpty) { + undo.rollback() + InlineLog(InlineLogRollback(request, warnings)) + } + warnings } - List(w) - case None => - val sizeBefore = request.callsite.callsiteMethod.instructions.size - inlineCallsite(request.callsite) - if (compilerSettings.YoptLogInline.isSetByUser) - inlineLog ::= InlineLog(request, sizeBefore, request.callsite.callsiteMethod.instructions.size, request.callsite.callee.get.callee.instructions.size, None) - val postRequests = request.post.flatMap(adaptPostRequestForMainCallsite(_, request.callsite)) - postRequests flatMap inline + } + + def inlinedByPost(insns: List[AbstractInsnNode]): Boolean = + insns.nonEmpty && insns.forall(ins => request.post.exists(_.callsite.callsiteInstruction == ins)) + + canInlineCallsite(request.callsite) match { + case None => + doInline(undo) + + case Some((_, illegalAccessInsns)) if inlinedByPost(illegalAccessInsns) => + // speculatively inline, roll back if an illegalAccessInsn cannot be eliminated + if (undo == NoUndoLogging) doInline(new UndoLog(), callRollback = true) + else doInline(undo) + + case Some((w, _)) => + InlineLog(InlineLogFail(request, w)) + List(w) + } } /** @@ -253,7 +367,7 @@ class Inliner[BT <: BTypes](val btypes: BT) { * @return A map associating instruction nodes of the callee with the corresponding cloned * instruction in the callsite method. */ - def inlineCallsite(callsite: Callsite): Unit = { + def inlineCallsite(callsite: Callsite, undo: UndoLog = NoUndoLogging): Unit = { import callsite.{callsiteClass, callsiteMethod, callsiteInstruction, receiverKnownNotNull, callsiteStackHeight} val Right(callsiteCallee) = callsite.callee import callsiteCallee.{callee, calleeDeclarationClass, sourceFilePath} @@ -380,6 +494,8 @@ class Inliner[BT <: BTypes](val btypes: BT) { clonedInstructions.insert(postCallLabel, retVarLoad) } + undo.saveMethodState(callsiteMethod) + callsiteMethod.instructions.insert(callsiteInstruction, clonedInstructions) callsiteMethod.instructions.remove(callsiteInstruction) @@ -406,7 +522,8 @@ class Inliner[BT <: BTypes](val btypes: BT) { callsiteMethod.maxStack = math.max(callsiteMethod.maxStack, math.max(stackHeightAtNullCheck, maxStackOfInlinedCode)) - addIndyLambdaImplMethod(callsiteClass.internalName, targetHandles) + val added = addIndyLambdaImplMethod(callsiteClass.internalName, targetHandles) + undo { removeIndyLambdaImplMethod(callsiteClass.internalName, added) } callGraph.addIfMissing(callee, calleeDeclarationClass) @@ -426,8 +543,13 @@ class Inliner[BT <: BTypes](val btypes: BT) { argInfos = argInfos, callsiteStackHeight = callsiteStackHeight + originalCallsite.callsiteStackHeight ) - originalCallsite.inlinedClones += ClonedCallsite(newCallsite, callsite) + val clonedCallsite = ClonedCallsite(newCallsite, callsite) + originalCallsite.inlinedClones += clonedCallsite callGraph.addCallsite(newCallsite) + undo { + originalCallsite.inlinedClones -= clonedCallsite + callGraph.removeCallsite(newCallsite.callsiteInstruction, newCallsite.callsiteMethod) + } } callGraph.closureInstantiations(callee).valuesIterator foreach { originalClosureInit => @@ -440,10 +562,14 @@ class Inliner[BT <: BTypes](val btypes: BT) { capturedArgInfos) originalClosureInit.inlinedClones += newClosureInit callGraph.addClosureInstantiation(newClosureInit) + undo { + callGraph.removeClosureInstantiation(newClosureInit.lambdaMetaFactoryCall.indy, newClosureInit.ownerMethod) + } } // Remove the elided invocation from the call graph callGraph.removeCallsite(callsiteInstruction, callsiteMethod) + undo { callGraph.addCallsite(callsite) } // Inlining a method body can render some code unreachable, see example above in this method. unreachableCodeEliminated -= callsiteMethod @@ -467,10 +593,10 @@ class Inliner[BT <: BTypes](val btypes: BT) { if (isSynchronizedMethod(callee)) { // Could be done by locking on the receiver, wrapping the inlined code in a try and unlocking // in finally. But it's probably not worth the effort, scala never emits synchronized methods. - Some(SynchronizedMethod(calleeDeclarationClass.internalName, callee.name, callee.desc)) + Some(SynchronizedMethod(calleeDeclarationClass.internalName, callee.name, callee.desc, callsite.isInlineAnnotated)) } else if (isStrictfpMethod(callsiteMethod) != isStrictfpMethod(callee)) { Some(StrictfpMismatch( - calleeDeclarationClass.internalName, callee.name, callee.desc, + calleeDeclarationClass.internalName, callee.name, callee.desc, callsite.isInlineAnnotated, callsiteClass.internalName, callsiteMethod.name, callsiteMethod.desc)) } else None @@ -486,9 +612,14 @@ class Inliner[BT <: BTypes](val btypes: BT) { * we don't query it while traversing the call graph and selecting callsites to inline - it might * rule out callsites that can be inlined just fine. * - * @return `Some(message)` if inlining cannot be performed, `None` otherwise + * Returns + * - `None` if the callsite can be inlined + * - `Some((message, Nil))` if there was an issue performing the access checks, for example + * because of a missing classfile + * - `Some((message, instructions))` if inlining `instructions` into the callsite method would + * cause an IllegalAccessError */ - def canInlineBody(callsite: Callsite): Option[CannotInlineWarning] = { + def canInlineCallsite(callsite: Callsite): Option[(CannotInlineWarning, List[AbstractInsnNode])] = { import callsite.{callsiteInstruction, callsiteMethod, callsiteClass, callsiteStackHeight} val Right(callsiteCallee) = callsite.callee import callsiteCallee.{callee, calleeDeclarationClass} @@ -519,23 +650,30 @@ class Inliner[BT <: BTypes](val btypes: BT) { } if (codeSizeOKForInlining(callsiteMethod, callee)) { - Some(ResultingMethodTooLarge( - calleeDeclarationClass.internalName, callee.name, callee.desc, - callsiteClass.internalName, callsiteMethod.name, callsiteMethod.desc)) + val warning = ResultingMethodTooLarge( + calleeDeclarationClass.internalName, callee.name, callee.desc, callsite.isInlineAnnotated, + callsiteClass.internalName, callsiteMethod.name, callsiteMethod.desc) + Some((warning, Nil)) } else if (!callee.tryCatchBlocks.isEmpty && stackHasNonParameters) { - Some(MethodWithHandlerCalledOnNonEmptyStack( - calleeDeclarationClass.internalName, callee.name, callee.desc, - callsiteClass.internalName, callsiteMethod.name, callsiteMethod.desc)) - } else findIllegalAccess(callee.instructions, calleeDeclarationClass, callsiteClass) map { - case (illegalAccessIns, None) => - IllegalAccessInstruction( - calleeDeclarationClass.internalName, callee.name, callee.desc, - callsiteClass.internalName, illegalAccessIns) - - case (illegalAccessIns, Some(warning)) => - IllegalAccessCheckFailed( - calleeDeclarationClass.internalName, callee.name, callee.desc, - callsiteClass.internalName, illegalAccessIns, warning) + val warning = MethodWithHandlerCalledOnNonEmptyStack( + calleeDeclarationClass.internalName, callee.name, callee.desc, callsite.isInlineAnnotated, + callsiteClass.internalName, callsiteMethod.name, callsiteMethod.desc) + Some((warning, Nil)) + } else findIllegalAccess(callee.instructions, calleeDeclarationClass, callsiteClass) match { + case Right(Nil) => + None + + case Right(illegalAccessInsns) => + val warning = IllegalAccessInstruction( + calleeDeclarationClass.internalName, callee.name, callee.desc, callsite.isInlineAnnotated, + callsiteClass.internalName, illegalAccessInsns.head) + Some((warning, illegalAccessInsns)) + + case Left((illegalAccessIns, cause)) => + val warning = IllegalAccessCheckFailed( + calleeDeclarationClass.internalName, callee.name, callee.desc, callsite.isInlineAnnotated, + callsiteClass.internalName, illegalAccessIns, cause) + Some((warning, Nil)) } } @@ -624,13 +762,14 @@ 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`. - * - * If validity of some instruction could not be checked because an error occurred, the instruction - * is returned together with a warning message that describes the problem. + * Returns + * - `Right(Nil)` if all instructions can be safely inlined + * - `Right(insns)` if inlining any of `insns` would cause a [[java.lang.IllegalAccessError]] + * when inlined into the `destinationClass` + * - `Left((insn, warning))` if validity of some instruction could not be checked because an + * error occurred */ - def findIllegalAccess(instructions: InsnList, calleeDeclarationClass: ClassBType, destinationClass: ClassBType): Option[(AbstractInsnNode, Option[OptimizerWarning])] = { + def findIllegalAccess(instructions: InsnList, calleeDeclarationClass: ClassBType, destinationClass: ClassBType): Either[(AbstractInsnNode, OptimizerWarning), List[AbstractInsnNode]] = { /** * Check if `instruction` can be transplanted to `destinationClass`. * @@ -759,17 +898,15 @@ class Inliner[BT <: BTypes](val btypes: BT) { } val it = instructions.iterator.asScala - @tailrec def find: Option[(AbstractInsnNode, Option[OptimizerWarning])] = { - if (!it.hasNext) None // all instructions are legal - else { - val i = it.next() - isLegal(i) match { - case Left(warning) => Some((i, Some(warning))) // checking isLegal for i failed - case Right(false) => Some((i, None)) // an illegal instruction was found - case _ => find - } + val illegalAccess = mutable.ListBuffer.empty[AbstractInsnNode] + while (it.hasNext) { + val i = it.next() + isLegal(i) match { + case Left(warning) => return Left((i, warning)) // checking isLegal for i failed + case Right(false) => illegalAccess += i // an illegal instruction was found + case _ => } } - find + Right(illegalAccess.toList) } } diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala index 79e74f3eb7..929e8b5ca4 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala @@ -7,17 +7,18 @@ package scala.tools.nsc package backend.jvm package opt +import scala.annotation.tailrec import scala.collection.JavaConverters._ import scala.tools.asm.Opcodes -import scala.tools.asm.tree.{MethodInsnNode, MethodNode} +import scala.tools.asm.tree.{AbstractInsnNode, MethodInsnNode, MethodNode} import scala.tools.nsc.backend.jvm.BTypes.InternalName -import scala.tools.nsc.backend.jvm.BackendReporting.OptimizerWarning +import scala.tools.nsc.backend.jvm.BackendReporting.{CalleeNotFinal, OptimizerWarning} class InlinerHeuristics[BT <: BTypes](val bTypes: BT) { import bTypes._ import callGraph._ - case class InlineRequest(callsite: Callsite, post: List[InlineRequest], reason: String) { + final case class InlineRequest(callsite: Callsite, post: List[InlineRequest], reason: String) { // invariant: all post inline requests denote callsites in the callee of the main callsite for (pr <- post) assert(pr.callsite.callsiteMethod == callsite.callee.get.callee, s"Callsite method mismatch: main $callsite - post ${pr.callsite}") } @@ -41,30 +42,18 @@ class InlinerHeuristics[BT <: BTypes](val bTypes: BT) { compilingMethods.map(methodNode => { var requests = Set.empty[InlineRequest] callGraph.callsites(methodNode).valuesIterator foreach { - case callsite @ Callsite(_, _, _, Right(Callee(callee, calleeDeclClass, safeToInline, sourceFilePath, calleeAnnotatedInline, _, _, callsiteWarning)), _, _, _, pos, _, _) => + case callsite @ Callsite(_, _, _, Right(Callee(callee, _, _, _, _, _, _, callsiteWarning)), _, _, _, pos, _, _) => inlineRequest(callsite, requests) match { case Some(Right(req)) => requests += req - case Some(Left(w)) => - if ((calleeAnnotatedInline && bTypes.compilerSettings.optWarningEmitAtInlineFailed) || w.emitWarning(compilerSettings)) { - val annotWarn = if (calleeAnnotatedInline) " is annotated @inline but" else "" - val msg = s"${BackendReporting.methodSignature(calleeDeclClass.internalName, callee)}$annotWarn could not be inlined:\n$w" - backendReporting.inlinerWarning(callsite.callsitePosition, msg) + + case Some(Left(w)) => + if (w.emitWarning(compilerSettings)) { + backendReporting.inlinerWarning(callsite.callsitePosition, w.toString) } case None => - if (canInlineFromSource(sourceFilePath) && calleeAnnotatedInline && !callsite.annotatedNoInline && bTypes.compilerSettings.optWarningEmitAtInlineFailed) { - // if the callsite is annotated @inline, we report an inline warning even if the underlying - // reason is, for example, mixed compilation (which has a separate -opt-warning flag). - def initMsg = s"${BackendReporting.methodSignature(calleeDeclClass.internalName, callee)} is annotated @inline but cannot be inlined" - def warnMsg = callsiteWarning.map(" Possible reason:\n" + _).getOrElse("") - if (!safeToInline) - backendReporting.inlinerWarning(pos, s"$initMsg: the method is not final and may be overridden." + warnMsg) - else - backendReporting.inlinerWarning(pos, s"$initMsg." + warnMsg) - } else if (callsiteWarning.isDefined && callsiteWarning.get.emitWarning(compilerSettings)) { - // when annotatedInline is false, and there is some warning, the callsite metadata is possibly incomplete. + if (callsiteWarning.isDefined && callsiteWarning.get.emitWarning(compilerSettings)) backendReporting.inlinerWarning(pos, s"there was a problem determining if method ${callee.name} can be inlined: \n"+ callsiteWarning.get) - } } case Callsite(ins, _, _, Left(warning), _, _, _, pos, _, _) => @@ -75,6 +64,42 @@ class InlinerHeuristics[BT <: BTypes](val bTypes: BT) { }).filterNot(_._2.isEmpty).toMap } + private def isTraitStaticSuperAccessorName(s: String) = s.endsWith("$") + + private def isTraitSuperAccessor(method: MethodNode, owner: ClassBType): Boolean = { + owner.isInterface == Right(true) && BytecodeUtils.isStaticMethod(method) && isTraitStaticSuperAccessorName(method.name) + } + + private def findCall(method: MethodNode, such: MethodInsnNode => Boolean): Option[MethodInsnNode] = { + @tailrec def noMoreInvoke(insn: AbstractInsnNode): Boolean = { + insn == null || (!insn.isInstanceOf[MethodInsnNode] && noMoreInvoke(insn.getNext)) + } + @tailrec def find(insn: AbstractInsnNode): Option[MethodInsnNode] = { + if (insn == null) None + else insn match { + case mi: MethodInsnNode => + if (such(mi) && noMoreInvoke(insn.getNext)) Some(mi) + else None + case _ => + find(insn.getNext) + } + } + find(method.instructions.getFirst) + } + private def superAccessorInvocation(method: MethodNode): Option[MethodInsnNode] = + findCall(method, mi => mi.itf && mi.getOpcode == Opcodes.INVOKESTATIC && isTraitStaticSuperAccessorName(mi.name)) + + private def isMixinForwarder(method: MethodNode, owner: ClassBType): Boolean = { + owner.isInterface == Right(false) && + !BytecodeUtils.isStaticMethod(method) && + superAccessorInvocation(method).nonEmpty + } + + private def isTraitSuperAccessorOrMixinForwarder(method: MethodNode, owner: ClassBType): Boolean = { + isTraitSuperAccessor(method, owner) || isMixinForwarder(method, owner) + } + + /** * Returns the inline request for a callsite if the callsite should be inlined according to the * current heuristics (`-Yopt-inline-heuristics`). @@ -90,81 +115,89 @@ class InlinerHeuristics[BT <: BTypes](val bTypes: BT) { * `Some(Right)` if the callsite should be and can be inlined */ def inlineRequest(callsite: Callsite, selectedRequestsForCallee: Set[InlineRequest]): Option[Either[OptimizerWarning, InlineRequest]] = { - val callee = callsite.callee.get - def requestIfCanInline(callsite: Callsite, reason: String): Either[OptimizerWarning, InlineRequest] = inliner.earlyCanInlineCheck(callsite) match { - case Some(w) => Left(w) - case None => - val callee = callsite.callee.get - val postInlineRequest: List[InlineRequest] = callee.calleeDeclarationClass.isInterface match { - case Right(true) => - // Treat the pair of trait interface method and static method as one for the purposes of inlining: - // if we inline invokeinterface, invoke the invokestatic, too. - val calls = callee.callee.instructions.iterator().asScala.filter(BytecodeUtils.isCall).take(2).toList - calls match { - case List(x: MethodInsnNode) if x.getOpcode == Opcodes.INVOKESTATIC && x.name == (callee.callee.name + "$") => - callGraph.addIfMissing(callee.callee, callee.calleeDeclarationClass) - val maybeNodeToCallsite1 = callGraph.findCallSite(callee.callee, x) - maybeNodeToCallsite1.toList.flatMap(x => requestIfCanInline(x, reason).right.toOption) - case _ => - Nil - - } - case _ => Nil - } - - Right(InlineRequest(callsite, postInlineRequest, reason)) - + def requestIfCanInline(callsite: Callsite, reason: String): Option[Either[OptimizerWarning, InlineRequest]] = { + val callee = callsite.callee.get + if (!callee.safeToInline) { + if (callsite.isInlineAnnotated && callee.canInlineFromSource) { + // By default, we only emit inliner warnings for methods annotated @inline. However, we don't + // want to be unnecessarily noisy with `-opt-warnings:_`: for example, the inliner heuristic + // would attempty to inline `Function1.apply$sp$II`, as it's higher-order (the receiver is + // a function), and it's concrete (forwards to `apply`). But because it's non-final, it cannot + // be inlined. So we only create warnings here for methods annotated @inline. + Some(Left(CalleeNotFinal( + callee.calleeDeclarationClass.internalName, + callee.callee.name, + callee.callee.desc, + callsite.isInlineAnnotated))) + } else None + } else inliner.earlyCanInlineCheck(callsite) match { + case Some(w) => Some(Left(w)) + case None => + val postInlineRequest: List[InlineRequest] = { + val postCall = + if (isTraitSuperAccessor(callee.callee, callee.calleeDeclarationClass)) { + // scala-dev#259: when inlining a trait super accessor, also inline the callsite to the default method + val implName = callee.callee.name.dropRight(1) + findCall(callee.callee, mi => mi.itf && mi.getOpcode == Opcodes.INVOKESPECIAL && mi.name == implName) + } else { + // scala-dev#259: when inlining a mixin forwarder, also inline the callsite to the static super accessor + superAccessorInvocation(callee.callee) + } + postCall.flatMap(call => { + callGraph.addIfMissing(callee.callee, callee.calleeDeclarationClass) + val maybeCallsite = callGraph.findCallSite(callee.callee, call) + maybeCallsite.flatMap(requestIfCanInline(_, reason).flatMap(_.right.toOption)) + }).toList + } + Some(Right(InlineRequest(callsite, postInlineRequest, reason))) + } } - compilerSettings.YoptInlineHeuristics.value match { - case "everything" => - if (callee.safeToInline) { + // scala-dev#259: don't inline into static accessors and mixin forwarders + if (isTraitSuperAccessorOrMixinForwarder(callsite.callsiteMethod, callsite.callsiteClass)) None + else { + val callee = callsite.callee.get + compilerSettings.YoptInlineHeuristics.value match { + case "everything" => val reason = if (compilerSettings.YoptLogInline.isSetByUser) "the inline strategy is \"everything\"" else null - Some(requestIfCanInline(callsite, reason)) - } - else None + requestIfCanInline(callsite, reason) - case "at-inline-annotated" => - if (callee.safeToInline && callee.annotatedInline) { - val reason = if (compilerSettings.YoptLogInline.isSetByUser) { - val what = if (callee.safeToInline) "callee" else "callsite" + case "at-inline-annotated" => + def reason = if (!compilerSettings.YoptLogInline.isSetByUser) null else { + val what = if (callee.annotatedInline) "callee" else "callsite" s"the $what is annotated `@inline`" - } else null - Some(requestIfCanInline(callsite, reason)) - } - else None + } + if (callsite.isInlineAnnotated && !callsite.isNoInlineAnnotated) requestIfCanInline(callsite, reason) + else None - case "default" => - if (callee.safeToInline && !callee.annotatedNoInline && !callsite.annotatedNoInline) { - def shouldInlineHO = callee.samParamTypes.nonEmpty && (callee.samParamTypes exists { - case (index, _) => callsite.argInfos.contains(index) - }) - if (callee.annotatedInline || callsite.annotatedInline || shouldInlineHO) { - val reason = if (compilerSettings.YoptLogInline.isSetByUser) { - if (callee.annotatedInline || callsite.annotatedInline) { - val what = if (callee.safeToInline) "callee" else "callsite" - s"the $what is annotated `@inline`" - } else { - val paramNames = Option(callee.callee.parameters).map(_.asScala.map(_.name).toVector) - def param(i: Int) = { - def syn = s"<param $i>" - paramNames.fold(syn)(v => v.applyOrElse(i, (_: Int) => syn)) - } - def samInfo(i: Int, sam: String, arg: String) = s"the argument for parameter (${param(i)}: $sam) is a $arg" - val argInfos = for ((i, sam) <- callee.samParamTypes; info <- callsite.argInfos.get(i)) yield { - val argKind = info match { - case FunctionLiteral => "function literal" - case ForwardedParam(_) => "parameter of the callsite method" - } - samInfo(i, sam.internalName.split('/').last, argKind) + case "default" => + def reason = if (!compilerSettings.YoptLogInline.isSetByUser) null else { + if (callsite.isInlineAnnotated) { + val what = if (callee.annotatedInline) "callee" else "callsite" + s"the $what is annotated `@inline`" + } else { + val paramNames = Option(callee.callee.parameters).map(_.asScala.map(_.name).toVector) + def param(i: Int) = { + def syn = s"<param $i>" + paramNames.fold(syn)(v => v.applyOrElse(i, (_: Int) => syn)) + } + def samInfo(i: Int, sam: String, arg: String) = s"the argument for parameter (${param(i)}: $sam) is a $arg" + val argInfos = for ((i, sam) <- callee.samParamTypes; info <- callsite.argInfos.get(i)) yield { + val argKind = info match { + case FunctionLiteral => "function literal" + case ForwardedParam(_) => "parameter of the callsite method" } - s"the callee is a higher-order method, ${argInfos.mkString(", ")}" + samInfo(i, sam.internalName.split('/').last, argKind) } - } else null - Some(requestIfCanInline(callsite, reason)) + s"the callee is a higher-order method, ${argInfos.mkString(", ")}" + } } + def shouldInlineHO = callee.samParamTypes.nonEmpty && (callee.samParamTypes exists { + case (index, _) => callsite.argInfos.contains(index) + }) + if (!callsite.isNoInlineAnnotated && (callsite.isInlineAnnotated || shouldInlineHO)) requestIfCanInline(callsite, reason) else None - } else None + } } } diff --git a/src/library/scala/inline.scala b/src/library/scala/inline.scala index f6d7c7569e..f188ccab07 100644 --- a/src/library/scala/inline.scala +++ b/src/library/scala/inline.scala @@ -23,7 +23,7 @@ package scala * def t2 = f2(1) // not inlined * def t3 = f3(1) // may be inlined (heuristics) * def t4 = f1(1): @noinline // not inlined (override at callsite) - * def t5 = f2(1): @inline // not inlined (cannot override the @noinline at f2's definition) + * def t5 = f2(1): @inline // inlined if possible (override at callsite) * def t6 = f3(1): @inline // inlined if possible * def t7 = f3(1): @noinline // not inlined * } diff --git a/src/library/scala/noinline.scala b/src/library/scala/noinline.scala index 0cd5ef9f64..6c21ed667d 100644 --- a/src/library/scala/noinline.scala +++ b/src/library/scala/noinline.scala @@ -23,7 +23,7 @@ package scala * def t2 = f2(1) // not inlined * def t3 = f3(1) // may be inlined (heuristics) * def t4 = f1(1): @noinline // not inlined (override at callsite) - * def t5 = f2(1): @inline // not inlined (cannot override the @noinline at f2's definition) + * def t5 = f2(1): @inline // inlined if possible (override at callsite) * def t6 = f3(1): @inline // inlined if possible * def t7 = f3(1): @noinline // not inlined * } diff --git a/test/files/neg/sealed-final-neg.check b/test/files/neg/sealed-final-neg.check index e135f38f8b..5e47c69ed8 100644 --- a/test/files/neg/sealed-final-neg.check +++ b/test/files/neg/sealed-final-neg.check @@ -1,7 +1,9 @@ -sealed-final-neg.scala:17: warning: neg1/Foo::bar(I)I is annotated @inline but cannot be inlined: the method is not final and may be overridden. +sealed-final-neg.scala:17: warning: neg1/Foo::bar(I)I is annotated @inline but could not be inlined: +The method is not final and may be overridden. def f = Foo.mkFoo() bar 10 ^ -sealed-final-neg.scala:37: warning: neg2/Foo::bar(I)I is annotated @inline but cannot be inlined: the method is not final and may be overridden. +sealed-final-neg.scala:37: warning: neg2/Foo::bar(I)I is annotated @inline but could not be inlined: +The method is not final and may be overridden. def f = Foo.mkFoo() bar 10 ^ error: No warnings can be incurred under -Xfatal-warnings. 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 80fbba133e..a74e73afc9 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala @@ -72,7 +72,7 @@ class CallGraphTest extends BytecodeTesting { | @noinline def f5 = try { 0 } catch { case _: Throwable => 1 } | @noinline final def f6 = try { 0 } catch { case _: Throwable => 1 } | - | @inline @noinline def f7 = try { 0 } catch { case _: Throwable => 1 } + | @inline @noinline def f7 = try { 0 } catch { case _: Throwable => 1 } // no warning, @noinline takes precedence |} |class D extends C { | @inline override def f1 = try { 0 } catch { case _: Throwable => 1 } @@ -91,18 +91,17 @@ class CallGraphTest extends BytecodeTesting { // The callGraph.callsites map is indexed by instructions of those ClassNodes. val ok = Set( - "D::f1()I is annotated @inline but cannot be inlined: the method is not final and may be overridden", // only one warning for D.f1: C.f1 is not annotated @inline - "C::f3()I is annotated @inline but cannot be inlined: the method is not final and may be overridden", // only one warning for C.f3: D.f3 does not have @inline (and it would also be safe to inline) - "C::f7()I is annotated @inline but cannot be inlined: the method is not final and may be overridden", // two warnings (the error message mentions C.f7 even if the receiver type is D, because f7 is inherited from C) - "operand stack at the callsite in Test::t1(LC;)I contains more values", - "operand stack at the callsite in Test::t2(LD;)I contains more values") + "D::f1()I is annotated @inline but could not be inlined:\nThe method is not final and may be overridden.", // only one warning for D.f1: C.f1 is not annotated @inline + "C::f3()I is annotated @inline but could not be inlined:\nThe method is not final and may be overridden.", // only one warning for C.f3: D.f3 does not have @inline (and it would also be safe to inline) + "C::f4()I is annotated @inline but could not be inlined:\nThe operand stack at the callsite in Test::t1(LC;)I contains more values", + "C::f4()I is annotated @inline but could not be inlined:\nThe operand stack at the callsite in Test::t2(LD;)I contains more values") var msgCount = 0 val checkMsg = (m: StoreReporter#Info) => { msgCount += 1 ok exists (m.msg contains _) } val List(cCls, cMod, dCls, testCls) = compile(code, checkMsg) - assert(msgCount == 6, msgCount) + assert(msgCount == 4, msgCount) val List(cf1, cf2, cf3, cf4, cf5, cf6, cf7) = getAsmMethods(cCls, _.startsWith("f")) val List(df1, df3) = getAsmMethods(dCls, _.startsWith("f")) diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala index 95b47f7d04..b1aa27fd27 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala @@ -35,9 +35,9 @@ class InlineWarningTest extends BytecodeTesting { """.stripMargin var count = 0 val warns = Set( - "C::m1()I is annotated @inline but cannot be inlined: the method is not final and may be overridden", - "T::m2()I is annotated @inline but cannot be inlined: the method is not final and may be overridden", - "D::m2()I is annotated @inline but cannot be inlined: the method is not final and may be overridden") + "C::m1()I is annotated @inline but could not be inlined:\nThe method is not final and may be overridden.", + "T::m2()I is annotated @inline but could not be inlined:\nThe method is not final and may be overridden.", + "D::m2()I is annotated @inline but could not be inlined:\nThe method is not final and may be overridden.") compileToBytes(code, allowMessage = i => {count += 1; warns.exists(i.msg contains _)}) assert(count == 4, count) } 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 3e0b889e9c..bf9da0f48f 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala @@ -20,7 +20,7 @@ class InlinerIllegalAccessTest extends BytecodeTesting { import global.genBCode.bTypes._ def addToRepo(cls: List[ClassNode]): Unit = for (c <- cls) byteCodeRepository.add(c, None) - def assertEmpty(ins: Option[AbstractInsnNode]) = for (i <- ins) + def assertEmpty(ins: List[AbstractInsnNode]) = for (i <- ins) throw new AssertionError(textify(i)) @Test @@ -28,7 +28,7 @@ class InlinerIllegalAccessTest extends BytecodeTesting { val code = """package a { | private class C { // the Scala compiler makes all classes public - | def f1 = new C // NEW a/C + | def f1 = new C // NEW a/C, INVOKESPECIAL a/C.<init> ()V | def f2 = new Array[C](0) // ANEWARRAY a/C | def f3 = new Array[Array[C]](0) // ANEWARRAY [La/C; | } @@ -46,9 +46,9 @@ class InlinerIllegalAccessTest extends BytecodeTesting { val methods = cClass.methods.asScala.filter(_.name(0) == 'f').toList - def check(classNode: ClassNode, test: Option[AbstractInsnNode] => Unit) = { + def check(classNode: ClassNode, test: List[AbstractInsnNode] => Unit) = { for (m <- methods) - test(inliner.findIllegalAccess(m.instructions, classBTypeFromParsedClassfile(cClass.name), classBTypeFromParsedClassfile(classNode.name)).map(_._1)) + test(inliner.findIllegalAccess(m.instructions, classBTypeFromParsedClassfile(cClass.name), classBTypeFromParsedClassfile(classNode.name)).right.get) } check(cClass, assertEmpty) @@ -65,7 +65,11 @@ class InlinerIllegalAccessTest extends BytecodeTesting { check(cClass, assertEmpty) check(dClass, assertEmpty) // accessing a private class in the same package is OK check(eClass, { - case Some(ti: TypeInsnNode) if Set("a/C", "[La/C;")(ti.desc) => () + case (ti: TypeInsnNode) :: is if Set("a/C", "[La/C;")(ti.desc) => + is match { + case List(mi: MethodInsnNode) => assert(mi.owner == "a/C" && mi.name == "<init>") + case Nil => + } // MatchError otherwise }) } @@ -141,12 +145,12 @@ class InlinerIllegalAccessTest extends BytecodeTesting { val List(rbD, rcD, rfD, rgD) = dCl.methods.asScala.toList.filter(_.name(0) == 'r').sortBy(_.name) - def check(method: MethodNode, decl: ClassNode, dest: ClassNode, test: Option[AbstractInsnNode] => Unit): Unit = { - test(inliner.findIllegalAccess(method.instructions, classBTypeFromParsedClassfile(decl.name), classBTypeFromParsedClassfile(dest.name)).map(_._1)) + def check(method: MethodNode, decl: ClassNode, dest: ClassNode, test: List[AbstractInsnNode] => Unit): Unit = { + test(inliner.findIllegalAccess(method.instructions, classBTypeFromParsedClassfile(decl.name), classBTypeFromParsedClassfile(dest.name)).right.get) } - val cOrDOwner = (_: Option[AbstractInsnNode] @unchecked) match { - case Some(mi: MethodInsnNode) if Set("a/C", "a/D")(mi.owner) => () + val cOrDOwner = (_: List[AbstractInsnNode] @unchecked) match { + case List(mi: MethodInsnNode) if Set("a/C", "a/D")(mi.owner) => () // MatchError otherwise } diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerSeparateCompilationTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerSeparateCompilationTest.scala index 5362585642..9b1609a130 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerSeparateCompilationTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerSeparateCompilationTest.scala @@ -31,7 +31,7 @@ class InlinerSeparateCompilationTest { |} """.stripMargin - val warn = "T::f()I is annotated @inline but cannot be inlined: the method is not final and may be overridden" + val warn = "T::f()I is annotated @inline but could not be inlined:\nThe method is not final and may be overridden." val List(c, o, oMod, t) = compileClassesSeparately(List(codeA, codeB), args + " -opt-warnings", _.msg contains warn) assertInvoke(getMethod(c, "t1"), "T", "f") assertNoInvoke(getMethod(c, "t2")) 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 a844c20a7f..90a938be35 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala @@ -67,7 +67,7 @@ class InlinerTest extends BytecodeTesting { def canInlineTest(code: String, mod: ClassNode => Unit = _ => ()): Option[OptimizerWarning] = { val cs = gMethAndFCallsite(code, mod)._2 - inliner.earlyCanInlineCheck(cs) orElse inliner.canInlineBody(cs) + inliner.earlyCanInlineCheck(cs) orElse inliner.canInlineCallsite(cs).map(_._1) } def inlineTest(code: String, mod: ClassNode => Unit = _ => ()): MethodNode = { @@ -199,8 +199,8 @@ class InlinerTest extends BytecodeTesting { val List(c, d) = compile(code) val hMeth = getAsmMethod(d, "h") val gCall = getCallsite(hMeth, "g") - val r = inliner.canInlineBody(gCall) - assert(r.nonEmpty && r.get.isInstanceOf[IllegalAccessInstruction], r) + val r = inliner.canInlineCallsite(gCall) + assert(r.nonEmpty && r.get._1.isInstanceOf[IllegalAccessInstruction], r) } @Test @@ -340,7 +340,7 @@ class InlinerTest extends BytecodeTesting { val fMeth = getAsmMethod(c, "f") val call = getCallsite(fMeth, "lowestOneBit") - val warning = inliner.canInlineBody(call) + val warning = inliner.canInlineCallsite(call) assert(warning.isEmpty, warning) inliner.inline(InlineRequest(call, Nil, null)) @@ -475,7 +475,7 @@ class InlinerTest extends BytecodeTesting { | def t2 = this.f |} """.stripMargin - val warn = "::f()I is annotated @inline but cannot be inlined: the method is not final and may be overridden" + val warn = "::f()I is annotated @inline but could not be inlined:\nThe method is not final and may be overridden." var count = 0 val List(c, t) = compile(code, allowMessage = i => {count += 1; i.msg contains warn}) assert(count == 2, count) @@ -513,7 +513,7 @@ class InlinerTest extends BytecodeTesting { | def t3(t: T) = t.f // no inlining here |} """.stripMargin - val warn = "T::f()I is annotated @inline but cannot be inlined: the method is not final and may be overridden" + val warn = "T::f()I is annotated @inline but could not be inlined:\nThe method is not final and may be overridden." var count = 0 val List(c, oMirror, oModule, t) = compile(code, allowMessage = i => {count += 1; i.msg contains warn}) assert(count == 1, count) @@ -617,7 +617,7 @@ class InlinerTest extends BytecodeTesting { |} """.stripMargin - val warning = "T1::f()I is annotated @inline but cannot be inlined: the method is not final and may be overridden" + val warning = "T1::f()I is annotated @inline but could not be inlined:\nThe method is not final and may be overridden." var count = 0 val List(ca, cb, t1, t2a, t2b) = compile(code, allowMessage = i => {count += 1; i.msg contains warning}) assert(count == 4, count) // see comments, f is not inlined 4 times @@ -698,7 +698,7 @@ class InlinerTest extends BytecodeTesting { | def t1(c: C) = c.foo |} """.stripMargin - val warn = "C::foo()I is annotated @inline but cannot be inlined: the method is not final and may be overridden" + val warn = "C::foo()I is annotated @inline but could not be inlined:\nThe method is not final and may be overridden." var c = 0 compile(code, allowMessage = i => {c += 1; i.msg contains warn}) assert(c == 1, c) @@ -762,7 +762,7 @@ class InlinerTest extends BytecodeTesting { |} """.stripMargin - val List(c, t, u) = compile(code, allowMessage = _.msg contains "i()I is annotated @inline but cannot be inlined") + val List(c, t, u) = compile(code, allowMessage = _.msg contains "::i()I is annotated @inline but could not be inlined:\nThe method is not final and may be overridden.") val m1 = getMethod(c, "m1") assertInvoke(m1, "T", "a") assertInvoke(m1, "T", "b") @@ -969,7 +969,7 @@ class InlinerTest extends BytecodeTesting { val gCall = getCallsite(hMeth, "g") val hCall = getCallsite(iMeth, "h") - val warning = inliner.canInlineBody(gCall) + val warning = inliner.canInlineCallsite(gCall) assert(warning.isEmpty, warning) inliner.inline(InlineRequest(hCall, @@ -1053,7 +1053,7 @@ class InlinerTest extends BytecodeTesting { | def t1 = f1(1) // inlined | def t2 = f2(1) // not inlined | def t3 = f1(1): @noinline // not inlined - | def t4 = f2(1): @inline // not inlined (cannot override the def-site @noinline) + | def t4 = f2(1): @inline // inlined | def t5 = f3(1): @inline // inlined | def t6 = f3(1): @noinline // not inlined | @@ -1067,7 +1067,7 @@ class InlinerTest extends BytecodeTesting { assertNoInvoke(getMethod(c, "t1")) assertInvoke(getMethod(c, "t2"), "C", "f2") assertInvoke(getMethod(c, "t3"), "C", "f1") - assertInvoke(getMethod(c, "t4"), "C", "f2") + assertNoInvoke(getMethod(c, "t4")) assertNoInvoke(getMethod(c, "t5")) assertInvoke(getMethod(c, "t6"), "C", "f3") assertNoInvoke(getMethod(c, "t7")) @@ -1469,8 +1469,8 @@ class InlinerTest extends BytecodeTesting { |class C extends T1 with T2 """.stripMargin val List(c, t1, t2) = compile(code, allowMessage = _ => true) - // the forwarder C.f is inlined, so there's no invocation - assertSameSummary(getMethod(c, "f"), List(ICONST_1, IRETURN)) + // we never inline into mixin forwarders, see scala-dev#259 + assertInvoke(getMethod(c, "f"), "T2", "f$") } @Test @@ -1622,4 +1622,120 @@ class InlinerTest extends BytecodeTesting { ("oneLastMethodWithVeryVeryLongNam_yetAnotherMethodWithVeryVeryLong_oneMoreMethodWithVeryVeryLongNam_anotherMethodWithVeryVeryLongNam_param",10), ("oneLastMethodWithVeryVery_yetAnotherMethodWithVeryV_oneMoreMethodWithVeryVery_anotherMethodWithVeryVery_methodWithVeryVeryLongNam_param",11))) } + + @Test + def sd259(): Unit = { + // - trait methods are not inlined into their static super accessors, and also not into mixin forwarders. + // - inlining an invocation of a mixin forwarder also inlines the static accessor and the trait method body. + val code = + """trait T { + | def m1a = 1 + | final def m1b = 1 + | + | @inline def m2a = 2 + | @inline final def m2b = 2 + | + | def m3a(f: Int => Int) = f(1) + | final def m3b(f: Int => Int) = f(1) + |} + |final class A extends T + |class C { + | def t1(t: T) = t.m1a + | def t2(t: T) = t.m1b + | def t3(t: T) = t.m2a + | def t4(t: T) = t.m2b + | def t5(t: T) = t.m3a(x => x) + | def t6(t: T) = t.m3b(x => x) + | + | def t7(a: A) = a.m1a + | def t8(a: A) = a.m1b + | def t9(a: A) = a.m2a + | def t10(a: A) = a.m2b + | def t11(a: A) = a.m3a(x => x) + | def t12(a: A) = a.m3b(x => x) + |} + """.stripMargin + val warn = "T::m2a()I is annotated @inline but could not be inlined:\nThe method is not final and may be overridden." + var count = 0 + val List(a, c, t) = compile(code, allowMessage = i => {count += 1; i.msg contains warn}) + assert(count == 1) + + assertInvoke(getMethod(t, "m1a$"), "T", "m1a") + assertInvoke(getMethod(t, "m1b$"), "T", "m1b") + assertInvoke(getMethod(t, "m2a$"), "T", "m2a") + assertInvoke(getMethod(t, "m2b$"), "T", "m2b") + assertInvoke(getMethod(t, "m3a$"), "T", "m3a") + assertInvoke(getMethod(t, "m3b$"), "T", "m3b") + + assertInvoke(getMethod(a, "m1a"), "T", "m1a$") + assertInvoke(getMethod(a, "m1b"), "T", "m1b$") + assertInvoke(getMethod(a, "m2a"), "T", "m2a$") + assertInvoke(getMethod(a, "m2b"), "T", "m2b$") + assertInvoke(getMethod(a, "m3a"), "T", "m3a$") + assertInvoke(getMethod(a, "m3b"), "T", "m3b$") + + assertInvoke(getMethod(c, "t1"), "T", "m1a") + assertInvoke(getMethod(c, "t2"), "T", "m1b") + + assertInvoke(getMethod(c, "t3"), "T", "m2a") // could not inline + assertNoInvoke(getMethod(c, "t4")) + + assertInvoke(getMethod(c, "t5"), "T", "m3a") // could not inline + assertInvoke(getMethod(c, "t6"), "C", "$anonfun$t6$1") // both forwarders inlined, closure eliminated + + assertInvoke(getMethod(c, "t7"), "A", "m1a") + assertInvoke(getMethod(c, "t8"), "A", "m1b") + + assertNoInvoke(getMethod(c, "t9")) + assertNoInvoke(getMethod(c, "t10")) + + assertInvoke(getMethod(c, "t11"), "C", "$anonfun$t11$1") // both forwarders inlined, closure eliminated + assertInvoke(getMethod(c, "t12"), "C", "$anonfun$t12$1") // both forwarders inlined, closure eliminated + } + + @Test + def sd259b(): Unit = { + val code = + """trait T { + | def get = 1 + | @inline final def m = try { get } catch { case _: Throwable => 1 } + |} + |class A extends T + |class C { + | def t(a: A) = 1 + a.m // cannot inline a try block onto a non-empty stack + |} + """.stripMargin + val warn = + """T::m()I is annotated @inline but could not be inlined: + |The operand stack at the callsite in C::t(LA;)I contains more values than the + |arguments expected by the callee T::m()I. These values would be discarded + |when entering an exception handler declared in the inlined method.""".stripMargin + val List(a, c, t) = compile(code, allowMessage = _.msg contains warn) + + // inlinig of m$ is rolled back, because <invokespecial T.m> is not legal in class C. + assertInvoke(getMethod(c, "t"), "T", "m$") + } + + @Test + def sd259c(): Unit = { + val code = + """trait T { + | def bar = 1 + | @inline final def m = { + | def impl = bar // private, non-static method + | impl + | } + |} + |class A extends T + |class C { + | def t(a: A) = a.m + |} + """.stripMargin + val warn = + """T::m()I is annotated @inline but could not be inlined: + |The callee T::m()I contains the instruction INVOKESPECIAL T.impl$1 ()I + |that would cause an IllegalAccessError when inlined into class C.""".stripMargin + val List(a, c, t) = compile(code, allowMessage = _.msg contains warn) + assertInvoke(getMethod(c, "t"), "T", "m$") + } } |