From b36658d257115329cfe25b794685bc85ea1cfc22 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 4 Apr 2016 11:51:38 +0200 Subject: Remove dead code in the optimizer related to trait impl classes --- .../scala/tools/nsc/backend/jvm/BCodeHelpers.scala | 11 +-- .../scala/tools/nsc/backend/jvm/BTypes.scala | 27 +----- .../tools/nsc/backend/jvm/opt/CallGraph.scala | 31 +++--- .../nsc/backend/jvm/opt/ClosureOptimizer.scala | 1 - .../nsc/backend/jvm/opt/InlineInfoAttribute.scala | 46 ++++----- .../scala/tools/nsc/backend/jvm/opt/Inliner.scala | 107 --------------------- .../nsc/backend/jvm/opt/InlinerHeuristics.scala | 6 +- 7 files changed, 41 insertions(+), 188 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala index a2ccce9d21..271146f623 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala @@ -247,14 +247,6 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { * Build the [[InlineInfo]] for a class symbol. */ def buildInlineInfoFromClassSymbol(classSym: Symbol, classSymToInternalName: Symbol => InternalName, methodSymToDescriptor: Symbol => String): InlineInfo = { - val traitSelfType = if (classSym.isTrait) { - // The mixin phase uses typeOfThis for the self parameter in implementation class methods. - val selfSym = classSym.typeOfThis.typeSymbol - if (selfSym != classSym) Some(classSymToInternalName(selfSym)) else None - } else { - None - } - val isEffectivelyFinal = classSym.isEffectivelyFinal val sam = { @@ -291,7 +283,6 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { val info = MethodInlineInfo( effectivelyFinal = effectivelyFinal, - traitMethodWithStaticImplementation = false, annotatedInline = methodSym.hasAnnotation(ScalaInlineClass), annotatedNoInline = methodSym.hasAnnotation(ScalaNoInlineClass) ) @@ -299,7 +290,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { } }).toMap - InlineInfo(traitSelfType, isEffectivelyFinal, sam, methodInlineInfos, warning) + InlineInfo(isEffectivelyFinal, sam, methodInlineInfos, warning) } /* diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index c3f6399901..192c2ee14e 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala @@ -255,13 +255,11 @@ abstract class BTypes { val methodInfos = classNode.methods.asScala.map(methodNode => { val info = MethodInlineInfo( effectivelyFinal = BytecodeUtils.isFinalMethod(methodNode), - traitMethodWithStaticImplementation = false, annotatedInline = false, annotatedNoInline = false) (methodNode.name + methodNode.desc, info) }).toMap InlineInfo( - traitImplClassSelfType = None, isEffectivelyFinal = BytecodeUtils.isFinalClass(classNode), sam = inlinerHeuristics.javaSam(classNode.name), methodInfos = methodInfos, @@ -1139,22 +1137,8 @@ object BTypes { * Note that this class should contain information that can only be obtained from the ClassSymbol. * Information that can be computed from the ClassNode should be added to the call graph instead. * - * @param traitImplClassSelfType `Some(tp)` if this InlineInfo describes a trait, and the `self` - * parameter type of the methods in the implementation class is not - * the trait itself. Example: - * trait T { self: U => def f = 1 } - * Generates something like: - * class T$class { static def f(self: U) = 1 } - * - * In order to inline a trat method call, the INVOKEINTERFACE is - * rewritten to an INVOKESTATIC of the impl class, so we need the - * self type (U) to get the right signature. - * - * `None` if the self type is the interface type, or if this - * InlineInfo does not describe a trait. - * * @param isEffectivelyFinal True if the class cannot have subclasses: final classes, module - * classes, trait impl classes. + * classes. * * @param sam If this class is a SAM type, the SAM's "$name$descriptor". * @@ -1166,26 +1150,21 @@ object BTypes { * InlineInfo, for example if some classfile could not be found on * the classpath. This warning can be reported later by the inliner. */ - final case class InlineInfo(traitImplClassSelfType: Option[InternalName], - isEffectivelyFinal: Boolean, + final case class InlineInfo(isEffectivelyFinal: Boolean, sam: Option[String], methodInfos: Map[String, MethodInlineInfo], warning: Option[ClassInlineInfoWarning]) - val EmptyInlineInfo = InlineInfo(None, false, None, Map.empty, None) + val EmptyInlineInfo = InlineInfo(false, None, Map.empty, None) /** * Metadata about a method, used by the inliner. * * @param effectivelyFinal True if the method cannot be overridden (in Scala) - * @param traitMethodWithStaticImplementation True if the method is an interface method method of - * a trait method and has a static counterpart in the - * implementation class. * @param annotatedInline True if the method is annotated `@inline` * @param annotatedNoInline True if the method is annotated `@noinline` */ final case class MethodInlineInfo(effectivelyFinal: Boolean, - traitMethodWithStaticImplementation: Boolean, annotatedInline: Boolean, annotatedNoInline: Boolean) 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 3267b9b5df..5f06e13560 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala @@ -132,16 +132,16 @@ class CallGraph[BT <: BTypes](val btypes: BT) { (declarationClassNode, source) <- byteCodeRepository.classNodeAndSource(declarationClass): Either[OptimizerWarning, (ClassNode, Source)] } yield { val declarationClassBType = classBTypeFromClassNode(declarationClassNode) - val CallsiteInfo(safeToInline, safeToRewrite, canInlineFromSource, annotatedInline, annotatedNoInline, samParamTypes, warning) = analyzeCallsite(method, declarationClassBType, call, source) + val info = analyzeCallsite(method, declarationClassBType, call, source) + import info._ Callee( callee = method, calleeDeclarationClass = declarationClassBType, safeToInline = safeToInline, - safeToRewrite = false, canInlineFromSource = canInlineFromSource, annotatedInline = annotatedInline, annotatedNoInline = annotatedNoInline, - samParamTypes = samParamTypes, + samParamTypes = info.samParamTypes, calleeInfoWarning = warning) } @@ -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, safeToRewrite: Boolean, canInlineFromSource: Boolean, + private case class CallsiteInfo(safeToInline: Boolean, canInlineFromSource: Boolean, annotatedInline: Boolean, annotatedNoInline: Boolean, samParamTypes: IntMap[ClassBType], warning: Option[CalleeInfoWarning]) @@ -300,16 +300,12 @@ class CallGraph[BT <: BTypes](val btypes: BT) { receiverType.info.orThrow.inlineInfo.isEffectivelyFinal // (1) } - val isRewritableTraitCall = false - 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). - // (2) Final trait methods can be rewritten from the interface to the static implementation - // method to enable inlining. CallsiteInfo( safeToInline = canInlineFromSource && @@ -318,7 +314,6 @@ class CallGraph[BT <: BTypes](val btypes: BT) { !BytecodeUtils.isConstructor(calleeMethodNode) && !BytecodeUtils.isNativeMethod(calleeMethodNode) && !BytecodeUtils.hasCallerSensitiveAnnotation(calleeMethodNode), - safeToRewrite = canInlineFromSource && isRewritableTraitCall, // (2) canInlineFromSource = canInlineFromSource, annotatedInline = methodInlineInfo.annotatedInline, annotatedNoInline = methodInlineInfo.annotatedNoInline, @@ -327,12 +322,12 @@ class CallGraph[BT <: BTypes](val btypes: BT) { case None => val warning = MethodInlineInfoMissing(calleeDeclarationClassBType.internalName, calleeMethodNode.name, calleeMethodNode.desc, calleeDeclarationClassBType.info.orThrow.inlineInfo.warning) - CallsiteInfo(false, false, false, false, false, IntMap.empty, Some(warning)) + CallsiteInfo(false, false, false, false, IntMap.empty, Some(warning)) } } catch { case Invalid(noInfo: NoClassBTypeInfo) => val warning = MethodInlineInfoError(calleeDeclarationClassBType.internalName, calleeMethodNode.name, calleeMethodNode.desc, noInfo) - CallsiteInfo(false, false, false, false, false, IntMap.empty, Some(warning)) + CallsiteInfo(false, false, false, false, IntMap.empty, Some(warning)) } } @@ -387,20 +382,18 @@ class CallGraph[BT <: BTypes](val btypes: BT) { * @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 safeToRewrite True if the callee is the interface method of a concrete trait method - * that can be safely re-written to the static implementation method. * @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 * @param calleeInfoWarning An inliner warning if some information was not available while * gathering the information about this callee. */ - final case class Callee(callee: MethodNode, calleeDeclarationClass: ClassBType, - safeToInline: Boolean, safeToRewrite: Boolean, canInlineFromSource: Boolean, - annotatedInline: Boolean, annotatedNoInline: Boolean, - samParamTypes: IntMap[ClassBType], - calleeInfoWarning: Option[CalleeInfoWarning]) { - assert(!(safeToInline && safeToRewrite), s"A callee of ${callee.name} can be either safeToInline or safeToRewrite, but not both.") + final case class Callee( + callee: MethodNode, calleeDeclarationClass: btypes.ClassBType, + safeToInline: Boolean, canInlineFromSource: Boolean, + annotatedInline: Boolean, annotatedNoInline: Boolean, + samParamTypes: IntMap[btypes.ClassBType], + calleeInfoWarning: Option[CalleeInfoWarning]) { override def toString = s"Callee($calleeDeclarationClass.${callee.name})" } 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 4935b9d1a0..fef15bcf9a 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala @@ -363,7 +363,6 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { callee = bodyMethodNode, calleeDeclarationClass = bodyDeclClassType, safeToInline = canInlineFromSource, - safeToRewrite = false, // the lambda body method is not a trait interface method canInlineFromSource = canInlineFromSource, annotatedInline = false, annotatedNoInline = false, diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/InlineInfoAttribute.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/InlineInfoAttribute.scala index c885a29e16..079a9eec9b 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/InlineInfoAttribute.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/InlineInfoAttribute.scala @@ -27,7 +27,7 @@ import scala.tools.nsc.backend.jvm.BackendReporting.UnknownScalaInlineInfoVersio * In principle we could encode the InlineInfo into a Java annotation (instead of a classfile attribute). * However, an attribute allows us to save many bits. In particular, note that the strings in an * InlineInfo are serialized as references to constants in the constant pool, and those strings - * (traitImplClassSelfType, method names, method signatures) would exist in there anyway. So the + * (method names, method signatures) would exist in there anyway. So the * ScalaInlineAttribute remains relatively compact. */ case class InlineInfoAttribute(inlineInfo: InlineInfo) extends Attribute(InlineInfoAttribute.attributeName) { @@ -48,15 +48,11 @@ case class InlineInfoAttribute(inlineInfo: InlineInfo) extends Attribute(InlineI result.putByte(InlineInfoAttribute.VERSION) var finalSelfSam = 0 - if (inlineInfo.isEffectivelyFinal) finalSelfSam |= 1 - if (inlineInfo.traitImplClassSelfType.isDefined) finalSelfSam |= 2 - if (inlineInfo.sam.isDefined) finalSelfSam |= 4 + if (inlineInfo.isEffectivelyFinal) finalSelfSam |= 1 + // finalSelfSam |= 2 // no longer written + if (inlineInfo.sam.isDefined) finalSelfSam |= 4 result.putByte(finalSelfSam) - for (selfInternalName <- inlineInfo.traitImplClassSelfType) { - result.putShort(cw.newUTF8(selfInternalName)) - } - for (samNameDesc <- inlineInfo.sam) { val (name, desc) = samNameDesc.span(_ != '(') result.putShort(cw.newUTF8(name)) @@ -75,10 +71,10 @@ case class InlineInfoAttribute(inlineInfo: InlineInfo) extends Attribute(InlineI result.putShort(cw.newUTF8(desc)) var inlineInfo = 0 - if (info.effectivelyFinal) inlineInfo |= 1 - if (info.traitMethodWithStaticImplementation) inlineInfo |= 2 - if (info.annotatedInline) inlineInfo |= 4 - if (info.annotatedNoInline) inlineInfo |= 8 + if (info.effectivelyFinal) inlineInfo |= 1 + // inlineInfo |= 2 // no longer written + if (info.annotatedInline) inlineInfo |= 4 + if (info.annotatedNoInline) inlineInfo |= 8 result.putByte(inlineInfo) } @@ -106,10 +102,7 @@ case class InlineInfoAttribute(inlineInfo: InlineInfo) extends Attribute(InlineI val hasSelf = (finalSelfSam & 2) != 0 val hasSam = (finalSelfSam & 4) != 0 - val self = if (!hasSelf) None else { - val selfName = nextUTF8() - Some(selfName) - } + if (hasSelf) nextUTF8() // no longer used val sam = if (!hasSam) None else { val name = nextUTF8() @@ -123,14 +116,14 @@ case class InlineInfoAttribute(inlineInfo: InlineInfo) extends Attribute(InlineI val desc = nextUTF8() val inlineInfo = nextByte() - val isFinal = (inlineInfo & 1) != 0 - val traitMethodWithStaticImplementation = (inlineInfo & 2) != 0 - val isInline = (inlineInfo & 4) != 0 - val isNoInline = (inlineInfo & 8) != 0 - (name + desc, MethodInlineInfo(isFinal, traitMethodWithStaticImplementation, isInline, isNoInline)) + val isFinal = (inlineInfo & 1) != 0 + // val traitMethodWithStaticImplementation = (inlineInfo & 2) != 0 // no longer used + val isInline = (inlineInfo & 4) != 0 + val isNoInline = (inlineInfo & 8) != 0 + (name + desc, MethodInlineInfo(isFinal, isInline, isNoInline)) }).toMap - InlineInfoAttribute(InlineInfo(self, isFinal, sam, infos, None)) + InlineInfoAttribute(InlineInfo(isFinal, sam, infos, None)) } else { val msg = UnknownScalaInlineInfoVersion(cr.getClassName, version) InlineInfoAttribute(BTypes.EmptyInlineInfo.copy(warning = Some(msg))) @@ -140,6 +133,13 @@ case class InlineInfoAttribute(inlineInfo: InlineInfo) extends Attribute(InlineI object InlineInfoAttribute { /** + * Notes: + * - `traitImplClassSelfType` is no longer emitted, `hasTraitImplClassSelfType` is always emitted + * as 0. Similarly, `traitMethodWithStaticImplementation` is always emitted 0. + * - When reading an existing attribute where `hasTraitImplClassSelfType` is 1, the + * `traitImplClassSelfType` is ignored. Also the value of `traitMethodWithStaticImplementation` + * is ignored. + * * [u1] version * [u1] isEffectivelyFinal (<< 0), hasTraitImplClassSelfType (<< 1), hasSam (<< 2) * [u2]? traitImplClassSelfType (reference) @@ -159,4 +159,4 @@ object InlineInfoAttribute { * In order to instruct the ASM framework to de-serialize the ScalaInlineInfo attribute, we need * to pass a prototype instance when running the class reader. */ -object InlineInfoAttributePrototype extends InlineInfoAttribute(InlineInfo(null, false, null, null, null)) +object InlineInfoAttributePrototype extends InlineInfoAttribute(InlineInfo(false, null, null, null)) 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 32106614e3..a0ef74b46a 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala @@ -27,8 +27,6 @@ class Inliner[BT <: BTypes](val btypes: BT) { import backendUtils._ def runInliner(): Unit = { -// rewriteFinalTraitMethodInvocations() - for (request <- collectAndOrderInlineRequests) { val Right(callee) = request.callsite.callee // collectAndOrderInlineRequests returns callsites with a known callee @@ -70,111 +68,6 @@ class Inliner[BT <: BTypes](val btypes: BT) { } } - // Rewriting final trait method callsites to the implementation class enables inlining. - def rewriteFinalTraitMethodInvocations(): Unit = { - // Collect callsites to rewrite before actually rewriting anything. This prevents changing the - // `callsties` map while iterating it. - val toRewrite = mutable.ArrayBuffer.empty[Callsite] - for (css <- callsites.valuesIterator; cs <- css.valuesIterator if doRewriteTraitCallsite(cs)) toRewrite += cs - toRewrite foreach rewriteFinalTraitMethodInvocation - } - - /** - * True for statically resolved trait callsites that should be rewritten to the static implementation method. - */ - def doRewriteTraitCallsite(callsite: Callsite) = callsite.callee match { - case Right(callee) => callee.safeToRewrite - case _ => false - } - - /** - * Rewrite the INVOKEINTERFACE callsite of a final trait method invocation to INVOKESTATIC of the - * corresponding method in the implementation class. This enables inlining final trait methods. - * - * In a final trait method callsite, the callee is safeToInline and the callee method is abstract - * (the receiver type is the interface, so the method is abstract). - */ - def rewriteFinalTraitMethodInvocation(callsite: Callsite): Unit = { - // The analyzer used below needs to have a non-null frame for the callsite instruction - localOpt.minimalRemoveUnreachableCode(callsite.callsiteMethod, callsite.callsiteClass.internalName) - - // If the callsite was eliminated by DCE, do nothing. - if (!callGraph.containsCallsite(callsite)) return - - val Right(Callee(callee, calleeDeclarationClass, _, _, canInlineFromSource, annotatedInline, annotatedNoInline, samParamTypes, infoWarning)) = callsite.callee - - val traitMethodArgumentTypes = asm.Type.getArgumentTypes(callee.desc) - - val implClassInternalName = calleeDeclarationClass.internalName + "$class" - - val selfParamTypeV: Either[OptimizerWarning, ClassBType] = calleeDeclarationClass.info.map(_.inlineInfo.traitImplClassSelfType match { - case Some(internalName) => classBTypeFromParsedClassfile(internalName) - case None => calleeDeclarationClass - }) - - def implClassMethodV(implMethodDescriptor: String): Either[OptimizerWarning, MethodNode] = { - byteCodeRepository.methodNode(implClassInternalName, callee.name, implMethodDescriptor).map(_._1) - } - - // The rewrite reading the implementation class and the implementation method from the bytecode - // repository. If either of the two fails, the rewrite is not performed. - val res = for { - selfParamType <- selfParamTypeV - implMethodDescriptor = asm.Type.getMethodDescriptor(asm.Type.getReturnType(callee.desc), selfParamType.toASMType +: traitMethodArgumentTypes: _*) - implClassMethod <- implClassMethodV(implMethodDescriptor) - implClassBType = classBTypeFromParsedClassfile(implClassInternalName) - selfTypeOk <- calleeDeclarationClass.isSubtypeOf(selfParamType) - } yield { - - // The self parameter type may be incompatible with the trait type. - // trait T { self: S => def foo = 1 } - // The $self parameter type of T$class.foo is S, which may be unrelated to T. If we re-write - // a call to T.foo to T$class.foo, we need to cast the receiver to S, otherwise we get a - // VerifyError. We run a `SourceInterpreter` to find all producer instructions of the - // receiver value and add a cast to the self type after each. - if (!selfTypeOk) { - // We don't need to worry about the method being too large for running an analysis. - // Callsites of large methods are not added to the call graph. - val analyzer = new AsmAnalyzer(callsite.callsiteMethod, callsite.callsiteClass.internalName, new Analyzer(new SourceInterpreter)) - val receiverValue = analyzer.frameAt(callsite.callsiteInstruction).peekStack(traitMethodArgumentTypes.length) - for (i <- receiverValue.insns.asScala) { - val cast = new TypeInsnNode(CHECKCAST, selfParamType.internalName) - callsite.callsiteMethod.instructions.insert(i, cast) - } - } - - val newCallsiteInstruction = new MethodInsnNode(INVOKESTATIC, implClassInternalName, callee.name, implMethodDescriptor, false) - callsite.callsiteMethod.instructions.insert(callsite.callsiteInstruction, newCallsiteInstruction) - callsite.callsiteMethod.instructions.remove(callsite.callsiteInstruction) - - callGraph.removeCallsite(callsite.callsiteInstruction, callsite.callsiteMethod) - val staticCallSamParamTypes = { - if (selfParamType.info.get.inlineInfo.sam.isEmpty) samParamTypes - 0 - else samParamTypes.updated(0, selfParamType) - } - val staticCallsite = callsite.copy( - callsiteInstruction = newCallsiteInstruction, - callee = Right(Callee( - callee = implClassMethod, - calleeDeclarationClass = implClassBType, - safeToInline = true, - safeToRewrite = false, - canInlineFromSource = canInlineFromSource, - annotatedInline = annotatedInline, - annotatedNoInline = annotatedNoInline, - samParamTypes = staticCallSamParamTypes, - calleeInfoWarning = infoWarning)) - ) - callGraph.addCallsite(staticCallsite) - } - - for (warning <- res.left) { - val Right(callee) = callsite.callee - val newCallee = callee.copy(calleeInfoWarning = Some(RewriteTraitCallToStaticImplMethodFailed(calleeDeclarationClass.internalName, callee.callee.name, callee.callee.desc, warning))) - callGraph.addCallsite(callsite.copy(callee = Right(newCallee))) - } - } - /** * Returns the callsites that can be inlined. Ensures that the returned inline request graph does * not contain cycles. 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 7e35d0e7f0..35735794b1 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala @@ -41,7 +41,7 @@ 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, _, canInlineFromSource, calleeAnnotatedInline, _, _, callsiteWarning)), _, _, _, pos, _, _) => + case callsite @ Callsite(_, _, _, Right(Callee(callee, calleeDeclClass, safeToInline, canInlineFromSource, calleeAnnotatedInline, _, _, callsiteWarning)), _, _, _, pos, _, _) => inlineRequest(callsite) match { case Some(Right(req)) => requests += req case Some(Left(w)) => @@ -57,9 +57,7 @@ class InlinerHeuristics[BT <: BTypes](val bTypes: BT) { // reason is, for example, mixed compilation (which has a separate -Yopt-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 (doRewriteTraitCallsite(callsite)) - backendReporting.inlinerWarning(pos, s"$initMsg: the trait method call could not be rewritten to the static implementation method." + warnMsg) - else if (!safeToInline) + if (!safeToInline) backendReporting.inlinerWarning(pos, s"$initMsg: the method is not final and may be overridden." + warnMsg) else backendReporting.inlinerWarning(pos, s"$initMsg." + warnMsg) -- cgit v1.2.3 From 2d62eea9f99411adda6725364690d126dcb12d98 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 4 Apr 2016 13:52:35 +0200 Subject: Remove unused optimizer warnings related to trait impl classes --- .../tools/nsc/backend/jvm/BackendReporting.scala | 6 ------ .../nsc/backend/jvm/opt/InlineWarningTest.scala | 24 ---------------------- 2 files changed, 30 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala b/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala index 13f4738d3d..01206aa6eb 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala @@ -170,9 +170,6 @@ object BackendReporting { case MethodInlineInfoError(_, _, _, cause) => s"Error while computing the inline information for method $warningMessageSignature:\n" + cause - - case RewriteTraitCallToStaticImplMethodFailed(_, _, _, cause) => - cause.toString } def emitWarning(settings: ScalaSettings): Boolean = this match { @@ -182,15 +179,12 @@ object BackendReporting { case MethodInlineInfoMissing(_, _, _, None) => settings.YoptWarningNoInlineMissingBytecode case MethodInlineInfoError(_, _, _, cause) => cause.emitWarning(settings) - - case RewriteTraitCallToStaticImplMethodFailed(_, _, _, cause) => cause.emitWarning(settings) } } case class MethodInlineInfoIncomplete(declarationClass: InternalName, name: String, descriptor: String, cause: ClassInlineInfoWarning) extends CalleeInfoWarning case class MethodInlineInfoMissing(declarationClass: InternalName, name: String, descriptor: String, cause: Option[ClassInlineInfoWarning]) extends CalleeInfoWarning case class MethodInlineInfoError(declarationClass: InternalName, name: String, descriptor: String, cause: NoClassBTypeInfo) extends CalleeInfoWarning - case class RewriteTraitCallToStaticImplMethodFailed(declarationClass: InternalName, name: String, descriptor: String, cause: OptimizerWarning) extends CalleeInfoWarning sealed trait CannotInlineWarning extends OptimizerWarning { def calleeDeclarationClass: InternalName 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 01d97855c8..90236265e6 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala @@ -70,30 +70,6 @@ class InlineWarningTest extends ClearAfterClass { assert(count == 4, count) } - // TODO SD-86: no more impl classes. this test (and the warning it tests!) can be removed - @org.junit.Ignore @Test - def traitMissingImplClass(): Unit = { - val codeA = "trait T { @inline final def f = 1 }" - val codeB = "class C { def t1(t: T) = t.f }" - - val removeImpl = (outDir: AbstractFile) => { - val f = outDir.lookupName("T$class.class", directory = false) - if (f != null) f.delete() - } - - val warn = - """T::f()I is annotated @inline but cannot be inlined: the trait method call could not be rewritten to the static implementation method. Possible reason: - |The method f(LT;)I could not be found in the class T$class or any of its parents. - |Note that the following parent classes could not be found on the classpath: T$class""".stripMargin - - var c = 0 - compileSeparately(List(codeA, codeB), extraArgs = InlineWarningTest.args, afterEach = removeImpl, allowMessage = i => {c += 1; i.msg contains warn}) - assert(c == 1, c) - - // only summary here - compileSeparately(List(codeA, codeB), extraArgs = InlineWarningTest.argsNoWarn, afterEach = removeImpl, allowMessage = _.msg contains "there was one inliner warning") - } - @Test def handlerNonEmptyStack(): Unit = { val code = -- cgit v1.2.3 From 2ed0d2ad63afc4ed9b1a95d85c0b15898ce66e2f Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 4 Apr 2016 13:43:24 +0200 Subject: Remove references to trait impl classes, mostly in doc comments --- .../scala/tools/nsc/backend/jvm/BCodeHelpers.scala | 28 +++++------------ .../tools/nsc/backend/jvm/BCodeSkelBuilder.scala | 7 ----- .../scala/tools/nsc/backend/jvm/BTypes.scala | 21 ++++--------- .../tools/nsc/backend/jvm/BTypesFromSymbols.scala | 36 +++++----------------- src/reflect/scala/reflect/api/Symbols.scala | 6 +--- src/reflect/scala/reflect/internal/Flags.scala | 2 +- src/reflect/scala/reflect/internal/Symbols.scala | 6 +--- src/reflect/scala/reflect/internal/Types.scala | 1 - src/repl/scala/tools/nsc/interpreter/Power.scala | 2 -- test/files/jvm/innerClassAttribute/Classes_1.scala | 19 +++++++++++- test/files/jvm/innerClassAttribute/Test.scala | 6 ++++ 11 files changed, 49 insertions(+), 85 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala index 271146f623..99d4390873 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala @@ -31,8 +31,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { * True for classes generated by the Scala compiler that are considered top-level in terms of * the InnerClass / EnclosingMethod classfile attributes. See comment in BTypes. */ - def considerAsTopLevelImplementationArtifact(classSym: Symbol) = - classSym.isSpecialized + def considerAsTopLevelImplementationArtifact(classSym: Symbol) = classSym.isSpecialized /** * Cache the value of delambdafy == "inline" for each run. We need to query this value many @@ -147,10 +146,10 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { assert(classSym.isClass, classSym) def doesNotExist(method: Symbol) = { - // Value classes. Member methods of value classes exist in the generated box class. However, - // nested methods lifted into a value class are moved to the companion object and don't exist - // in the value class itself. We can identify such nested methods: the initial enclosing class - // is a value class, but the current owner is some other class (the module class). + // Value classes. Member methods of value classes exist in the generated box class. However, + // nested methods lifted into a value class are moved to the companion object and don't exist + // in the value class itself. We can identify such nested methods: the initial enclosing class + // is a value class, but the current owner is some other class (the module class). val enclCls = nextEnclosingClass(method) exitingPickler(enclCls.isDerivedValueClass) && method.owner != enclCls } @@ -172,8 +171,6 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { private def enclosingClassForEnclosingMethodAttribute(classSym: Symbol): Symbol = { assert(classSym.isClass, classSym) val r = nextEnclosingClass(nextEnclosing(classSym)) - // this should be an assertion, but we are more cautious for now as it was introduced before the 2.11.6 minor release - if (considerAsTopLevelImplementationArtifact(r)) devWarning(s"enclosing class of $classSym should not be an implementation artifact class: $r") r } @@ -188,20 +185,11 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { * on the implementation of GenASM / GenBCode, so they need to be passed in. */ def enclosingMethodAttribute(classSym: Symbol, classDesc: Symbol => String, methodDesc: Symbol => String): Option[EnclosingMethodEntry] = { - // trait impl classes are always top-level, see comment in BTypes + // specialized classes are always top-level, see comment in BTypes if (isAnonymousOrLocalClass(classSym) && !considerAsTopLevelImplementationArtifact(classSym)) { val enclosingClass = enclosingClassForEnclosingMethodAttribute(classSym) - val methodOpt = enclosingMethodForEnclosingMethodAttribute(classSym) match { - case some @ Some(m) => - if (m.owner != enclosingClass) { - // This should never happen. In case it does, it prevents emitting an invalid - // EnclosingMethod attribute: if the attribute specifies an enclosing method, - // it needs to exist in the specified enclosing class. - devWarning(s"the owner of the enclosing method ${m.locationString} should be the same as the enclosing class $enclosingClass") - None - } else some - case none => none - } + val methodOpt = enclosingMethodForEnclosingMethodAttribute(classSym) + for (m <- methodOpt) assert(m.owner == enclosingClass, s"the owner of the enclosing method ${m.locationString} should be the same as the enclosing class $enclosingClass") Some(EnclosingMethodEntry( classDesc(enclosingClass), methodOpt.map(_.javaSimpleName.toString).orNull, diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala index 20b1a52818..4c41cfc380 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala @@ -232,13 +232,6 @@ abstract class BCodeSkelBuilder extends BCodeHelpers { } def addClassFields() { - /* Non-method term members are fields, except for module members. Module - * members can only happen on .NET (no flatten) for inner traits. There, - * a module symbol is generated (transformInfo in mixin) which is used - * as owner for the members of the implementation class (so that the - * backend emits them as static). - * No code is needed for this module symbol. - */ for (f <- fieldSymbols(claszSymbol)) { val javagensig = getGenericSignature(f, claszSymbol) val flags = javaFieldFlags(f) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index 192c2ee14e..f6bccca050 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala @@ -790,26 +790,17 @@ abstract class BTypes { * } * * - * Traits Members - * -------------- - * - * Some trait methods don't exist in the generated interface, but only in the implementation class - * (private methods in traits for example). Since EnclosingMethod expresses a source-level property, - * but the source-level enclosing method doesn't exist in the classfile, we the enclosing method - * is null (the enclosing class is still emitted). - * See BCodeAsmCommon.considerAsTopLevelImplementationArtifact - * + * Specialized Classes, Delambdafy:method closure classes + * ------------------------------------------------------ * - * Implementation Classes, Specialized Classes, Delambdafy:method closure classes - * ------------------------------------------------------------------------------ - * - * Trait implementation classes and specialized classes are always considered top-level. Again, - * the InnerClass / EnclosingMethod attributes describe a source-level properties. The impl - * classes are compilation artifacts. + * Specialized classes are always considered top-level, as the InnerClass / EnclosingMethod + * attributes describe a source-level properties. * * The same is true for delambdafy:method closure classes. These classes are generated at * top-level in the delambdafy phase, no special support is required in the backend. * + * See also BCodeHelpers.considerAsTopLevelImplementationArtifact. + * * * Mirror Classes * -------------- diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala index 759b0a615a..d10b6c8dba 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala @@ -96,11 +96,6 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { * scala.Null is mapped to scala.runtime.Null$. This is because there exist no class files * for the Nothing / Null. If used for example as a parameter type, we use the runtime classes * in the classfile method signature. - * - * Note that the referenced class symbol may be an implementation class. For example when - * compiling a mixed-in method that forwards to the static method in the implementation class, - * the class descriptor of the receiver (the implementation class) is obtained by creating the - * ClassBType. */ final def classBTypeFromSymbol(classSym: Symbol): ClassBType = { assert(classSym != NoSymbol, "Cannot create ClassBType from NoSymbol") @@ -159,9 +154,6 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { /** * This method returns the BType for a type reference, for example a parameter type. - * - * If `t` references a class, typeToBType ensures that the class is not an implementation class. - * See also comment on classBTypeFromSymbol, which is invoked for implementation classes. */ final def typeToBType(t: Type): BType = { import definitions.ArrayClass @@ -264,12 +256,12 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { * current phase, for example, after lambdalift, all local classes become member of the enclosing * class. * - * Impl classes are always considered top-level, see comment in BTypes. + * Specialized classes are always considered top-level, see comment in BTypes. */ private def memberClassesForInnerClassTable(classSymbol: Symbol): List[Symbol] = classSymbol.info.decls.collect({ case sym if sym.isClass && !considerAsTopLevelImplementationArtifact(sym) => sym - case sym if sym.isModule && !considerAsTopLevelImplementationArtifact(sym) => // impl classes get the lateMODULE flag in mixin + case sym if sym.isModule && !considerAsTopLevelImplementationArtifact(sym) => val r = exitingPickler(sym.moduleClass) assert(r != NoSymbol, sym.fullLocationString) r @@ -317,7 +309,6 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { ) } - // Check for isImplClass: trait implementation classes have NoSymbol as superClass // Check for hasAnnotationFlag for SI-9393: the classfile / java source parsers add // scala.annotation.Annotation as superclass to java annotations. In reality, java // annotation classfiles have superclass Object (like any interface classfile). @@ -380,8 +371,8 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { } val companionModuleMembers = if (considerAsTopLevelImplementationArtifact(classSym)) Nil else { - // If this is a top-level non-impl (*) class, the member classes of the companion object are - // added as members of the class. For example: + // If this is a top-level class, the member classes of the companion object are added as + // members of the class. For example: // class C { } // object C { // class D @@ -392,11 +383,6 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { // (done by buildNestedInfo). See comment in BTypes. // For consistency, the InnerClass entry for D needs to be present in C - to Java it looks // like D is a member of C, not C$. - // - // (*) We exclude impl classes: if the classfile for the impl class exists on the classpath, - // a linkedClass symbol is found for which isTopLevelModule is true, so we end up searching - // members of that weird impl-class-module-class-symbol. that search probably cannot return - // any classes, but it's better to exclude it. val javaCompatMembers = { if (linkedClass != NoSymbol && isTopLevelModuleClass(linkedClass)) // phase travel to exitingPickler: this makes sure that memberClassesForInnerClassTable only sees member @@ -454,7 +440,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { assert(innerClassSym.isClass, s"Cannot build NestedInfo for non-class symbol $innerClassSym") val isTopLevel = innerClassSym.rawowner.isPackageClass - // impl classes are considered top-level, see comment in BTypes + // specialized classes are considered top-level, see comment in BTypes if (isTopLevel || considerAsTopLevelImplementationArtifact(innerClassSym)) None else if (innerClassSym.rawowner.isTerm) { // This case should never be reached: the lambdalift phase mutates the rawowner field of all @@ -592,17 +578,11 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { /** * True for module classes of modules that are top-level or owned only by objects. Module classes - * for such objects will get a MODULE$ flag and a corresponding static initializer. + * for such objects will get a MODULE$ field and a corresponding static initializer. */ final def isStaticModuleClass(sym: Symbol): Boolean = { - /* (1) Phase travel to to pickler is required to exclude implementation classes; they have the - * lateMODULEs after mixin, so isModuleClass would be true. - * (2) isStaticModuleClass is a source-level property. See comment on isOriginallyStaticOwner. - */ - exitingPickler { // (1) - sym.isModuleClass && - isOriginallyStaticOwner(sym.originalOwner) // (2) - } + sym.isModuleClass && + isOriginallyStaticOwner(sym.originalOwner) // isStaticModuleClass is a source-level property, see comment on isOriginallyStaticOwner } // legacy, to be removed when the @remote annotation gets removed diff --git a/src/reflect/scala/reflect/api/Symbols.scala b/src/reflect/scala/reflect/api/Symbols.scala index 9e9fe5d67b..b9fb323a4c 100644 --- a/src/reflect/scala/reflect/api/Symbols.scala +++ b/src/reflect/scala/reflect/api/Symbols.scala @@ -260,9 +260,6 @@ trait Symbols { self: Universe => * with an object definition (module class in scala compiler parlance)? * If yes, `isType` is also guaranteed to be true. * - * Note to compiler developers: During the "mixin" phase, trait implementation class symbols - * receive the `lateMODULE` flag, hence `isImplClass && isModuleClass` becomes true. - * * @group Tests */ def isModuleClass: Boolean = false @@ -354,8 +351,7 @@ trait Symbols { self: Universe => /******************* tests *******************/ /** Does this symbol represent a synthetic (i.e. a compiler-generated) entity? - * Examples of synthetic entities are accessors for vals and vars - * or mixin constructors in trait implementation classes. + * Examples of synthetic entities are accessors for vals and vars. * * @group Tests */ diff --git a/src/reflect/scala/reflect/internal/Flags.scala b/src/reflect/scala/reflect/internal/Flags.scala index 35c927a5c3..f058acb7c0 100644 --- a/src/reflect/scala/reflect/internal/Flags.scala +++ b/src/reflect/scala/reflect/internal/Flags.scala @@ -50,7 +50,7 @@ package internal // 34: LIFTED // 35: EXISTENTIAL MIXEDIN // 36: EXPANDEDNAME -// 37: IMPLCLASS PRESUPER/M +// 37: PRESUPER/M // 38: TRANS_FLAG // 39: LOCKED // 40: SPECIALIZED diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index ee763df849..ed51414382 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -956,10 +956,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => * method `owner` returns the class C. * * Why not make a stable version of `isStatic`? Maybe some parts of the compiler depend on the - * current implementation. For example - * trait T { def foo = 1 } - * The method `foo` in the implementation class T$impl will be `isStatic`, because trait - * impl classes get the `lateMODULE` flag (T$impl.isStaticOwner is true). + * current implementation. */ def isStatic = (this hasFlag STATIC) || owner.isStaticOwner @@ -2780,7 +2777,6 @@ trait Symbols extends api.Symbols { self: SymbolTable => case DEFAULTPARAM => "" // TRAIT case MIXEDIN => "" // EXISTENTIAL case LABEL => "