summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLukas Rytz <lukas.rytz@gmail.com>2016-11-15 15:11:58 +0100
committerLukas Rytz <lukas.rytz@gmail.com>2016-11-25 11:53:43 +0100
commit8020cd66c8b30126bbba1dc1e87f7daafb3f2dd7 (patch)
tree520e397d27b0d453df664c8d9abab2fc9f80de34
parenteb7d907f6283f897f6b248bae170bade57969519 (diff)
downloadscala-8020cd66c8b30126bbba1dc1e87f7daafb3f2dd7.tar.gz
scala-8020cd66c8b30126bbba1dc1e87f7daafb3f2dd7.tar.bz2
scala-8020cd66c8b30126bbba1dc1e87f7daafb3f2dd7.zip
Better inliner support for 2.12 trait encoding
Some changes to the trait encoding came late in the 2.12 cycle, and the inliner was not adapted to support it in the best possible way. In 2.12.0 concrete trait methods are encoded as interface T { default int m() { return 1 } static int m$(T $this) { <invokespecial $this.m()> } } class C implements T { public int m() { return T.m$(this) } } If a trait method is selected for inlining, the 2.12.0 inliner would copy its body into the static super accessor `T.m$`, and from there into the mixin forwarder `C.m`. This commit special-cases the inliner: - We don't inline into static super accessors and mixin forwarders. - Insted, when inlining an invocation of a mixin forwarder, the inliner also follows through the two forwarders and inlines the trait method body. There was a difficulty implementing this: inlining the static static super accessor would copy an `invokespecial` instruction into a different classfile, which is not legal / may change semantics. That `invokespecial` is supposed to disappear when inlining the actual default method body. However, this last step may fail, for example because the trait method body itself contains instructions that are not legal in a different classfile. It is very difficult to perform all necessary checks ahead of time. So instead, this commit implements the ability to speculatively inline a callsite and roll back if necessary. The commit also cleans up the implementation of inliner warnings a little. The previous code would always emit a warning when a method annotated `@inline` was not picked by the heuristics - this was a problem when the callsite in the static super accessor was no longer chosen.
-rw-r--r--src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala13
-rw-r--r--src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala76
-rw-r--r--src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala42
-rw-r--r--src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala4
-rw-r--r--src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala189
-rw-r--r--src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala205
-rw-r--r--src/library/scala/inline.scala2
-rw-r--r--src/library/scala/noinline.scala2
-rw-r--r--test/files/neg/sealed-final-neg.check6
-rw-r--r--test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala13
-rw-r--r--test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala6
-rw-r--r--test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala22
-rw-r--r--test/junit/scala/tools/nsc/backend/jvm/opt/InlinerSeparateCompilationTest.scala2
-rw-r--r--test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala144
14 files changed, 484 insertions, 242 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..c520bb9d9e 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
@@ -31,18 +31,10 @@ class Inliner[BT <: BTypes](val btypes: BT) {
def runInliner(): Unit = {
for (request <- collectAndOrderInlineRequests) {
val Right(callee) = request.callsite.callee // collectAndOrderInlineRequests returns callsites with a known callee
-
- // 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!)
-
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)
- }
+ if (warning.emitWarning(compilerSettings))
+ backendReporting.inlinerWarning(request.callsite.callsitePosition, warning.toString)
}
}
@@ -221,26 +213,82 @@ class Inliner[BT <: BTypes](val btypes: BT) {
impl(post, mainCallsite)
}
+ class UndoLog(active: Boolean = true) {
+ private var actions = List.empty[() => Unit]
+ private var methodStateSaved = false
+
+ def apply(a: => Unit): Unit = if (active) actions = (() => a) :: actions
+ def run(): Unit = if (active) actions.foreach(_.apply())
+
+ private def arr[T: reflect.ClassTag](l: java.util.List[T]): Array[T] = {
+ val a: Array[T] = new Array[T](l.size)
+ l.toArray(a.asInstanceOf[Array[T with Object]]).asInstanceOf[Array[T]]
+ }
+ private def lst[T](a: Array[T]): java.util.List[T] = java.util.Arrays.asList(a: _*)
+
+ def saveMethodState(methodNode: MethodNode): Unit = if (active && !methodStateSaved) {
+ methodStateSaved = true
+ val currentInstructions = methodNode.instructions.toArray
+ val currentLocalVariables = arr(methodNode.localVariables)
+ val currentTryCatchBlocks = arr(methodNode.tryCatchBlocks)
+ val currentMaxLocals = methodNode.maxLocals
+ val currentMaxStack = methodNode.maxStack
+
+ apply {
+ // this doesn't work: it doesn't reset the `prev` / `next` / `index` of individual instruction nodes
+ // methodNode.instructions.clear()
+ methodNode.instructions.iterator.asScala.toList.foreach(methodNode.instructions.remove)
+ for (i <- currentInstructions) methodNode.instructions.add(i)
+
+ methodNode.localVariables.clear()
+ methodNode.localVariables.addAll(lst(currentLocalVariables))
+
+ methodNode.tryCatchBlocks.clear()
+ methodNode.tryCatchBlocks.addAll(lst(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))
- }
- List(w)
- case None =>
+ def inline(request: InlineRequest, undo: UndoLog = NoUndoLogging): List[CannotInlineWarning] = {
+ def doInline(undo: UndoLog): List[CannotInlineWarning] = {
val sizeBefore = request.callsite.callsiteMethod.instructions.size
- inlineCallsite(request.callsite)
+ inlineCallsite(request.callsite, undo)
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
+ postRequests.flatMap(inline(_, undo))
+ }
+
+ canInlineCallsite(request.callsite) match {
+ case None =>
+ doInline(undo)
+
+ case Some((w, illegalAccessInsns)) if illegalAccessInsns.nonEmpty && illegalAccessInsns.forall(ins => request.post.exists(_.callsite.callsiteInstruction == ins)) =>
+ // speculatively inline, roll back if an illegalAccessInsn cannot be eliminated
+ if (undo == NoUndoLogging) {
+ val undoLog = new UndoLog()
+ val warnings = doInline(undoLog)
+ if (warnings.nonEmpty) undoLog.run()
+ warnings
+ } else doInline(undo)
+
+ case Some((w, _)) =>
+ if (compilerSettings.YoptLogInline.isSetByUser) {
+ val size = request.callsite.callsiteMethod.instructions.size
+ inlineLog ::= InlineLog(request, size, size, 0, Some(w))
+ }
+ List(w)
+ }
}
/**
@@ -253,7 +301,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 +428,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 +456,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 +477,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 +496,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 +527,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 +546,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 +584,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 +696,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 +832,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$")
+ }
}