From 1f812e9482855d3fd5a8a5e9118942dc80f22db5 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 30 May 2016 14:38:50 +0200 Subject: Avoid separate traversal in inliner to remove line number nodes --- .../scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala | 10 ++++++---- src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala | 6 +----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala b/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala index 6d3c3f3863..9abd1d8006 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala @@ -98,7 +98,7 @@ class BackendUtils[BT <: BTypes](val btypes: BT) { * a boolean indicating if the instruction list contains an instantiation of a serializable SAM * type. */ - def cloneInstructions(methodNode: MethodNode, labelMap: Map[LabelNode, LabelNode]): (InsnList, Map[AbstractInsnNode, AbstractInsnNode], Boolean) = { + def cloneInstructions(methodNode: MethodNode, labelMap: Map[LabelNode, LabelNode], keepLineNumbers: Boolean): (InsnList, Map[AbstractInsnNode, AbstractInsnNode], Boolean) = { val javaLabelMap = labelMap.asJava val result = new InsnList var map = Map.empty[AbstractInsnNode, AbstractInsnNode] @@ -112,9 +112,11 @@ class BackendUtils[BT <: BTypes](val btypes: BT) { } case _ => } - val cloned = ins.clone(javaLabelMap) - result add cloned - map += ((ins, cloned)) + if (keepLineNumbers || !ins.isInstanceOf[LineNumberNode]) { + val cloned = ins.clone(javaLabelMap) + result add cloned + map += ((ins, cloned)) + } } (result, map, hasSerializableClosureInstantiation) } 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 809b9e310d..d18963ec8b 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala @@ -268,11 +268,7 @@ class Inliner[BT <: BTypes](val btypes: BT) { // New labels for the cloned instructions val labelsMap = cloneLabels(callee) - val (clonedInstructions, instructionMap, hasSerializableClosureInstantiation) = cloneInstructions(callee, labelsMap) - val keepLineNumbers = callsiteClass == calleeDeclarationClass - if (!keepLineNumbers) { - removeLineNumberNodes(clonedInstructions) - } + val (clonedInstructions, instructionMap, hasSerializableClosureInstantiation) = cloneInstructions(callee, labelsMap, keepLineNumbers = callsiteClass == calleeDeclarationClass) // local vars in the callee are shifted by the number of locals at the callsite val localVarShift = callsiteMethod.maxLocals -- cgit v1.2.3 From 037a089ad4fb7137513777ccda6d47e30e151838 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 30 May 2016 19:25:14 +0200 Subject: Store source file paths of classes being compiled in the bytecode repo For classes being compiled (vs. being loaded from classfiles), keep the source file path in the bytecode repo. This will allow to keep line numbers when inlining from one class into another in case the two are defined in the same compilation unit. --- .../scala/tools/nsc/backend/jvm/GenBCode.scala | 25 ++++---- .../nsc/backend/jvm/opt/ByteCodeRepository.scala | 67 ++++++++++------------ .../tools/nsc/backend/jvm/opt/CallGraph.scala | 25 ++++---- .../nsc/backend/jvm/opt/ClosureOptimizer.scala | 8 +-- .../nsc/backend/jvm/opt/InlinerHeuristics.scala | 10 ++-- .../backend/jvm/opt/InlinerIllegalAccessTest.scala | 2 +- .../tools/nsc/backend/jvm/opt/InlinerTest.scala | 2 +- 7 files changed, 67 insertions(+), 72 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala index 02dc2b8ede..584b11d4ed 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala @@ -77,15 +77,16 @@ abstract class GenBCode extends BCodeSyncAndTry { /* ---------------- q2 ---------------- */ - case class Item2(arrivalPos: Int, - mirror: asm.tree.ClassNode, - plain: asm.tree.ClassNode, - bean: asm.tree.ClassNode, - outFolder: scala.tools.nsc.io.AbstractFile) { + case class Item2(arrivalPos: Int, + mirror: asm.tree.ClassNode, + plain: asm.tree.ClassNode, + bean: asm.tree.ClassNode, + sourceFilePath: String, + outFolder: scala.tools.nsc.io.AbstractFile) { def isPoison = { arrivalPos == Int.MaxValue } } - private val poison2 = Item2(Int.MaxValue, null, null, null, null) + private val poison2 = Item2(Int.MaxValue, null, null, null, null, null) private val q2 = new _root_.java.util.LinkedList[Item2] /* ---------------- q3 ---------------- */ @@ -205,6 +206,7 @@ abstract class GenBCode extends BCodeSyncAndTry { val item2 = Item2(arrivalPos, mirrorC, plainC, beanC, + cunit.source.file.canonicalPath, outF) q2 add item2 // at the very end of this method so that no Worker2 thread starts mutating before we're done. @@ -226,10 +228,11 @@ abstract class GenBCode extends BCodeSyncAndTry { // add classes to the bytecode repo before building the call graph: the latter needs to // look up classes and methods in the code repo. if (settings.optAddToBytecodeRepository) q2.asScala foreach { - case Item2(_, mirror, plain, bean, _) => - if (mirror != null) byteCodeRepository.add(mirror, ByteCodeRepository.CompilationUnit) - if (plain != null) byteCodeRepository.add(plain, ByteCodeRepository.CompilationUnit) - if (bean != null) byteCodeRepository.add(bean, ByteCodeRepository.CompilationUnit) + case Item2(_, mirror, plain, bean, sourceFilePath, _) => + val someSourceFilePath = Some(sourceFilePath) + if (mirror != null) byteCodeRepository.add(mirror, someSourceFilePath) + if (plain != null) byteCodeRepository.add(plain, someSourceFilePath) + if (bean != null) byteCodeRepository.add(bean, someSourceFilePath) } if (settings.optBuildCallGraph) q2.asScala foreach { item => // skip call graph for mirror / bean: wd don't inline into tem, and they are not used in the plain class @@ -286,7 +289,7 @@ abstract class GenBCode extends BCodeSyncAndTry { cw.toByteArray } - val Item2(arrivalPos, mirror, plain, bean, outFolder) = item + val Item2(arrivalPos, mirror, plain, bean, _, outFolder) = item val mirrorC = if (mirror == null) null else SubItem3(mirror.name, getByteArray(mirror)) val plainC = SubItem3(plain.name, getByteArray(plain)) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala index 16590ec75c..78acd72dba 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala @@ -15,7 +15,6 @@ import scala.tools.asm.Attribute import scala.tools.nsc.backend.jvm.BackendReporting._ import scala.tools.nsc.util.ClassPath import BytecodeUtils._ -import ByteCodeRepository._ import BTypes.InternalName import java.util.concurrent.atomic.AtomicLong @@ -29,9 +28,10 @@ class ByteCodeRepository[BT <: BTypes](val classPath: ClassPath, val btypes: BT) import btypes._ /** - * ClassNodes for classes being compiled in the current compilation run. + * Contains ClassNodes and the canonical path of the source file path of classes being compiled in + * the current compilation run. */ - val compilingClasses: concurrent.Map[InternalName, ClassNode] = recordPerRunCache(concurrent.TrieMap.empty) + val compilingClasses: concurrent.Map[InternalName, (ClassNode, String)] = recordPerRunCache(concurrent.TrieMap.empty) /** * Cache for parsed ClassNodes. @@ -67,20 +67,35 @@ class ByteCodeRepository[BT <: BTypes](val classPath: ClassPath, val btypes: BT) } } - def add(classNode: ClassNode, source: Source) = { - if (source == CompilationUnit) compilingClasses(classNode.name) = classNode - else parsedClasses(classNode.name) = Right((classNode, lruCounter.incrementAndGet())) + def add(classNode: ClassNode, sourceFilePath: Option[String]) = sourceFilePath match { + case Some(path) if path != "" => compilingClasses(classNode.name) = (classNode, path) + case _ => parsedClasses(classNode.name) = Right((classNode, lruCounter.incrementAndGet())) + } + + private def parsedClassNode(internalName: InternalName): Either[ClassNotFound, ClassNode] = { + val r = parsedClasses.get(internalName) match { + case Some(l @ Left(_)) => l + case Some(r @ Right((classNode, _))) => + parsedClasses(internalName) = Right((classNode, lruCounter.incrementAndGet())) + r + case None => + limitCacheSize() + val res = parseClass(internalName).map((_, lruCounter.incrementAndGet())) + parsedClasses(internalName) = res + res + } + r.map(_._1) } /** - * The class node and source for an internal name. If the class node is not yet available, it is - * parsed from the classfile on the compile classpath. + * The class node and source file path (if the class is being compiled) for an internal name. If + * the class node is not yet available, it is parsed from the classfile on the compile classpath. */ - def classNodeAndSource(internalName: InternalName): Either[ClassNotFound, (ClassNode, Source)] = { - classNode(internalName) map (n => { - val source = if (compilingClasses contains internalName) CompilationUnit else Classfile - (n, source) - }) + def classNodeAndSourceFilePath(internalName: InternalName): Either[ClassNotFound, (ClassNode, Option[String])] = { + compilingClasses.get(internalName) match { + case Some((c, p)) => Right((c, Some(p))) + case _ => parsedClassNode(internalName).map((_, None)) + } } /** @@ -88,19 +103,9 @@ class ByteCodeRepository[BT <: BTypes](val classPath: ClassPath, val btypes: BT) * the classfile on the compile classpath. */ def classNode(internalName: InternalName): Either[ClassNotFound, ClassNode] = { - compilingClasses.get(internalName).map(Right(_)) getOrElse { - val r = parsedClasses.get(internalName) match { - case Some(l @ Left(_)) => l - case Some(r @ Right((classNode, _))) => - parsedClasses(internalName) = Right((classNode, lruCounter.incrementAndGet())) - r - case None => - limitCacheSize() - val res = parseClass(internalName).map((_, lruCounter.incrementAndGet())) - parsedClasses(internalName) = res - res - } - r.map(_._1) + compilingClasses.get(internalName) match { + case Some((c, _)) => Right(c) + case None => parsedClassNode(internalName) } } @@ -289,13 +294,3 @@ class ByteCodeRepository[BT <: BTypes](val classPath: ClassPath, val btypes: BT) } } } - -object ByteCodeRepository { - /** - * The source of a ClassNode in the ByteCodeRepository. Can be either [[CompilationUnit]] if the - * class is being compiled or [[Classfile]] if the class was parsed from the compilation classpath. - */ - sealed trait Source - object CompilationUnit extends Source - object Classfile extends Source -} 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 d4ff6493a3..40344809bf 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala @@ -16,7 +16,6 @@ import scala.collection.JavaConverters._ import scala.tools.nsc.backend.jvm.BTypes.InternalName import scala.tools.nsc.backend.jvm.BackendReporting._ import scala.tools.nsc.backend.jvm.analysis._ -import ByteCodeRepository.{Source, CompilationUnit} import BytecodeUtils._ class CallGraph[BT <: BTypes](val btypes: BT) { @@ -128,17 +127,17 @@ class CallGraph[BT <: BTypes](val btypes: BT) { methodNode.instructions.iterator.asScala foreach { case call: MethodInsnNode if a.frameAt(call) != null => // skips over unreachable code val callee: Either[OptimizerWarning, Callee] = for { - (method, declarationClass) <- byteCodeRepository.methodNode(call.owner, call.name, call.desc): Either[OptimizerWarning, (MethodNode, InternalName)] - (declarationClassNode, source) <- byteCodeRepository.classNodeAndSource(declarationClass): Either[OptimizerWarning, (ClassNode, Source)] + (method, declarationClass) <- byteCodeRepository.methodNode(call.owner, call.name, call.desc): Either[OptimizerWarning, (MethodNode, InternalName)] + (declarationClassNode, calleeSourceFilePath) <- byteCodeRepository.classNodeAndSourceFilePath(declarationClass): Either[OptimizerWarning, (ClassNode, Option[String])] } yield { val declarationClassBType = classBTypeFromClassNode(declarationClassNode) - val info = analyzeCallsite(method, declarationClassBType, call, source) + val info = analyzeCallsite(method, declarationClassBType, call, calleeSourceFilePath) import info._ Callee( callee = method, calleeDeclarationClass = declarationClassBType, safeToInline = safeToInline, - canInlineFromSource = canInlineFromSource, + sourceFilePath = sourceFilePath, annotatedInline = annotatedInline, annotatedNoInline = annotatedNoInline, samParamTypes = info.samParamTypes, @@ -256,7 +255,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) { /** * Just a named tuple used as return type of `analyzeCallsite`. */ - private case class CallsiteInfo(safeToInline: Boolean, canInlineFromSource: Boolean, + private case class CallsiteInfo(safeToInline: Boolean, sourceFilePath: Option[String], annotatedInline: Boolean, annotatedNoInline: Boolean, samParamTypes: IntMap[ClassBType], warning: Option[CalleeInfoWarning]) @@ -264,7 +263,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) { /** * Analyze a callsite and gather meta-data that can be used for inlining decisions. */ - private def analyzeCallsite(calleeMethodNode: MethodNode, calleeDeclarationClassBType: ClassBType, call: MethodInsnNode, calleeSource: Source): CallsiteInfo = { + private def analyzeCallsite(calleeMethodNode: MethodNode, calleeDeclarationClassBType: ClassBType, call: MethodInsnNode, calleeSourceFilePath: Option[String]): CallsiteInfo = { val methodSignature = calleeMethodNode.name + calleeMethodNode.desc try { @@ -273,8 +272,6 @@ class CallGraph[BT <: BTypes](val btypes: BT) { // callee, we only check there for the methodInlineInfo, we should find it there. calleeDeclarationClassBType.info.orThrow.inlineInfo.methodInfos.get(methodSignature) match { case Some(methodInlineInfo) => - val canInlineFromSource = compilerSettings.optInlineGlobal || calleeSource == CompilationUnit - val isAbstract = BytecodeUtils.isAbstractMethod(calleeMethodNode) val receiverType = classBTypeFromParsedClassfile(call.owner) @@ -308,13 +305,13 @@ class CallGraph[BT <: BTypes](val btypes: BT) { // static impl method first (safeToRewrite). CallsiteInfo( safeToInline = - canInlineFromSource && + inlinerHeuristics.canInlineFromSource(calleeSourceFilePath) && isStaticallyResolved && // (1) !isAbstract && !BytecodeUtils.isConstructor(calleeMethodNode) && !BytecodeUtils.isNativeMethod(calleeMethodNode) && !BytecodeUtils.hasCallerSensitiveAnnotation(calleeMethodNode), - canInlineFromSource = canInlineFromSource, + sourceFilePath = calleeSourceFilePath, annotatedInline = methodInlineInfo.annotatedInline, annotatedNoInline = methodInlineInfo.annotatedNoInline, samParamTypes = samParamTypes(calleeMethodNode, receiverType), @@ -322,12 +319,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, IntMap.empty, Some(warning)) + CallsiteInfo(false, None, 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, IntMap.empty, Some(warning)) + CallsiteInfo(false, None, false, false, IntMap.empty, Some(warning)) } } @@ -389,7 +386,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) { * gathering the information about this callee. */ final case class Callee(callee: MethodNode, calleeDeclarationClass: btypes.ClassBType, - safeToInline: Boolean, canInlineFromSource: Boolean, + safeToInline: Boolean, sourceFilePath: Option[String], annotatedInline: Boolean, annotatedNoInline: Boolean, samParamTypes: IntMap[btypes.ClassBType], calleeInfoWarning: Option[CalleeInfoWarning]) { 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 7f9858286e..081830d61d 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala @@ -17,7 +17,6 @@ import scala.tools.nsc.backend.jvm.BTypes.InternalName import BytecodeUtils._ import BackendReporting._ import Opcodes._ -import scala.tools.nsc.backend.jvm.opt.ByteCodeRepository.CompilationUnit import scala.collection.JavaConverters._ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { @@ -354,16 +353,15 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { // the method node is needed for building the call graph entry val bodyMethod = byteCodeRepository.methodNode(lambdaBodyHandle.getOwner, lambdaBodyHandle.getName, lambdaBodyHandle.getDesc) - def bodyMethodIsBeingCompiled = byteCodeRepository.classNodeAndSource(lambdaBodyHandle.getOwner).map(_._2 == CompilationUnit).getOrElse(false) + val sourceFilePath = byteCodeRepository.compilingClasses.get(lambdaBodyHandle.getOwner).map(_._2) val callee = bodyMethod.map({ case (bodyMethodNode, bodyMethodDeclClass) => val bodyDeclClassType = classBTypeFromParsedClassfile(bodyMethodDeclClass) - val canInlineFromSource = compilerSettings.optInlineGlobal || bodyMethodIsBeingCompiled Callee( callee = bodyMethodNode, calleeDeclarationClass = bodyDeclClassType, - safeToInline = canInlineFromSource, - canInlineFromSource = canInlineFromSource, + safeToInline = inlinerHeuristics.canInlineFromSource(sourceFilePath), + sourceFilePath = sourceFilePath, annotatedInline = false, annotatedNoInline = false, samParamTypes = callGraph.samParamTypes(bodyMethodNode, bodyDeclClassType), 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 17807fb385..009742501e 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala @@ -22,6 +22,8 @@ class InlinerHeuristics[BT <: BTypes](val bTypes: BT) { for (pr <- post) assert(pr.callsite.callsiteMethod == callsite.callee.get.callee, s"Callsite method mismatch: main $callsite - post ${pr.callsite}") } + def canInlineFromSource(sourceFilePath: Option[String]) = compilerSettings.optInlineGlobal || sourceFilePath.isDefined + /** * Select callsites from the call graph that should be inlined, grouped by the containing method. * Cyclic inlining requests are allowed, the inliner will eliminate requests to break cycles. @@ -32,14 +34,14 @@ class InlinerHeuristics[BT <: BTypes](val bTypes: BT) { // classpath. In order to get only the callsites being compiled, we start at the map of // compilingClasses in the byteCodeRepository. val compilingMethods = for { - classNode <- byteCodeRepository.compilingClasses.valuesIterator - methodNode <- classNode.methods.iterator.asScala + (classNode, _) <- byteCodeRepository.compilingClasses.valuesIterator + methodNode <- classNode.methods.iterator.asScala } yield methodNode 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, sourceFilePath, calleeAnnotatedInline, _, _, callsiteWarning)), _, _, _, pos, _, _) => inlineRequest(callsite, requests) match { case Some(Right(req)) => requests += req case Some(Left(w)) => @@ -50,7 +52,7 @@ class InlinerHeuristics[BT <: BTypes](val bTypes: BT) { } case None => - if (canInlineFromSource && calleeAnnotatedInline && !callsite.annotatedNoInline && bTypes.compilerSettings.optWarningEmitAtInlineFailed) { + 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" 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 3cb1fbdae6..3e0b889e9c 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala @@ -19,7 +19,7 @@ class InlinerIllegalAccessTest extends BytecodeTesting { import compiler._ import global.genBCode.bTypes._ - def addToRepo(cls: List[ClassNode]): Unit = for (c <- cls) byteCodeRepository.add(c, ByteCodeRepository.Classfile) + def addToRepo(cls: List[ClassNode]): Unit = for (c <- cls) byteCodeRepository.add(c, None) def assertEmpty(ins: Option[AbstractInsnNode]) = for (i <- ins) throw new AssertionError(textify(i)) 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 02cd632af1..4023f1fd3a 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala @@ -43,7 +43,7 @@ class InlinerTest extends BytecodeTesting { // Use the class nodes stored in the byteCodeRepository. The ones returned by compileClasses are not the same, // these are created new from the classfile byte array. They are completely separate instances which cannot // be used to look up methods / callsites in the callGraph hash maps for example. - byteCodeRepository.compilingClasses.valuesIterator.toList.sortBy(_.name) + byteCodeRepository.compilingClasses.valuesIterator.map(_._1).toList.sortBy(_.name) } def checkCallsite(callsite: callGraph.Callsite, callee: MethodNode) = { -- cgit v1.2.3 From 59d6dbc0aac912567d235048b5114cccf965c7ce Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Wed, 25 May 2016 15:21:50 +0200 Subject: clear all flags when resetting a symbol this change is a bit scary because it changes code that's not been changed in 11 years https://github.com/scala/scala/commit/7fa7c93#diff-d5789e5ae5061197d782d08324b260dbL214 --- src/reflect/scala/reflect/internal/Flags.scala | 1 - src/reflect/scala/reflect/internal/Symbols.scala | 2 +- .../junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala | 11 +++++++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/reflect/scala/reflect/internal/Flags.scala b/src/reflect/scala/reflect/internal/Flags.scala index 230d30c74e..e2522ef280 100644 --- a/src/reflect/scala/reflect/internal/Flags.scala +++ b/src/reflect/scala/reflect/internal/Flags.scala @@ -235,7 +235,6 @@ class Flags extends ModifierFlags { final val AllFlags = -1L /** These flags can be set when class or module symbol is first created. - * They are the only flags to survive a call to resetFlags(). */ final val TopLevelCreationFlags = MODULE | PACKAGE | FINAL | JAVA diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 6f2d8d802a..ab52a875f8 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -725,7 +725,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => def setFlag(mask: Long): this.type = { _rawflags |= mask ; this } def resetFlag(mask: Long): this.type = { _rawflags &= ~mask ; this } - def resetFlags() { rawflags &= TopLevelCreationFlags } + def resetFlags() { rawflags = 0 } /** Default implementation calls the generic string function, which * will print overloaded flags as . Subclasses diff --git a/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala b/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala index f835e9b140..36bdb759a6 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala @@ -88,4 +88,15 @@ class DirectCompileTest extends BytecodeTesting { def compileErroneous(): Unit = { compileToBytes("class C { def f: String = 1 }", allowMessage = _.msg contains "type mismatch") } + + @Test + def residentRedefineFinalFlag(): Unit = { + val compiler = newCompiler() + val a = "final class C { def c1 = 0 }" + // for re-defined class symbols (C), the compiler did not clear the `final` flag. + // so compiling `D` would give an error `illegal inheritance from final class C`. + val b = "class C; class D extends C" + compiler.compileToBytes(a) + compiler.compileToBytes(b) + } } -- cgit v1.2.3 From e1084299350bcc20f5d412993d77b8f956ba3165 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Wed, 25 May 2016 15:22:19 +0200 Subject: SI-9256 check companions in same compilation unit only if same run --- .../scala/tools/nsc/typechecker/Namers.scala | 1 + .../tools/nsc/backend/jvm/DirectCompileTest.scala | 12 +++++++++++ .../nsc/backend/jvm/opt/InlineWarningTest.scala | 24 +++++++++++----------- .../tools/nsc/backend/jvm/opt/InlinerTest.scala | 10 ++++----- 4 files changed, 30 insertions(+), 17 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 2773ee19cf..9c1ba7ced1 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -431,6 +431,7 @@ trait Namers extends MethodSynthesis { && !(module isCoDefinedWith clazz) && module.exists && clazz.exists + && (currentRun.compiles(clazz) == currentRun.compiles(module)) ) if (fails) { reporter.error(tree.pos, ( diff --git a/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala b/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala index 36bdb759a6..a28599cd92 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala @@ -99,4 +99,16 @@ class DirectCompileTest extends BytecodeTesting { compiler.compileToBytes(a) compiler.compileToBytes(b) } + + @Test + def residentMultipleRunsNotCompanions(): Unit = { + val compiler = newCompiler() + val a = List(("public class A { }", "A.java")) + // when checking that a class and its companion are defined in the same compilation unit, the + // compiler would also emit a warning if the two symbols are defined in separate runs. this + // would lead to an error message when compiling the scala class A. + val b = "class A" + compiler.compileToBytes("", a) + compiler.compileToBytes(b) + } } 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 024cf0c416..5254d7e1f2 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala @@ -97,7 +97,7 @@ class InlineWarningTest extends BytecodeTesting { @Test def cannotInlinePrivateCallIntoDifferentClass(): Unit = { val code = - """class M { + """class A { | @inline final def f = { | @noinline def nested = 0 | nested @@ -106,15 +106,15 @@ class InlineWarningTest extends BytecodeTesting { | def t = f // ok |} | - |class N { - | def t(a: M) = a.f // not possible + |class B { + | def t(a: A) = a.f // not possible |} """.stripMargin val warn = - """M::f()I is annotated @inline but could not be inlined: - |The callee M::f()I contains the instruction INVOKESTATIC M.nested$1 ()I - |that would cause an IllegalAccessError when inlined into class N""".stripMargin + """A::f()I is annotated @inline but could not be inlined: + |The callee A::f()I contains the instruction INVOKESTATIC A.nested$1 ()I + |that would cause an IllegalAccessError when inlined into class B""".stripMargin var c = 0 compileToBytes(code, allowMessage = i => { c += 1; i.msg contains warn }) @@ -124,7 +124,7 @@ class InlineWarningTest extends BytecodeTesting { @Test def dontWarnWhenNotIlnineAnnotated(): Unit = { val code = - """class M { + """class A { | final def f(t: Int => Int) = { | @noinline def nested = 0 | nested + t(1) @@ -132,16 +132,16 @@ class InlineWarningTest extends BytecodeTesting { | def t = f(x => x + 1) |} | - |class N { - | def t(a: M) = a.f(x => x + 1) + |class B { + | def t(a: A) = a.f(x => x + 1) |} """.stripMargin compileToBytes(code, allowMessage = _ => false) // no warnings allowed val warn = - """M::f(Lscala/Function1;)I could not be inlined: - |The callee M::f(Lscala/Function1;)I contains the instruction INVOKESTATIC M.nested$1 ()I - |that would cause an IllegalAccessError when inlined into class N""".stripMargin + """A::f(Lscala/Function1;)I could not be inlined: + |The callee A::f(Lscala/Function1;)I contains the instruction INVOKESTATIC A.nested$1 ()I + |that would cause an IllegalAccessError when inlined into class B""".stripMargin var c = 0 compilerWarnAll.compileToBytes(code, allowMessage = i => { c += 1; i.msg contains warn }) 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 4023f1fd3a..9b538573dc 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala @@ -835,11 +835,11 @@ class InlinerTest extends BytecodeTesting { @Test def inlineInvokeSpecial(): Unit = { val code = - """class Aa { + """class A { | def f1 = 0 |} - |class B extends Aa { - | @inline final override def f1 = 1 + super.f1 // invokespecial Aa.f1 + |class B extends A { + | @inline final override def f1 = 1 + super.f1 // invokespecial A.f1 | | private def f2m = 0 // public B$$f2m in bytecode | @inline final def f2 = f2m // invokevirtual B.B$$f2m @@ -863,13 +863,13 @@ class InlinerTest extends BytecodeTesting { val warn = """B::f1()I is annotated @inline but could not be inlined: - |The callee B::f1()I contains the instruction INVOKESPECIAL Aa.f1 ()I + |The callee B::f1()I contains the instruction INVOKESPECIAL A.f1 ()I |that would cause an IllegalAccessError when inlined into class T.""".stripMargin var c = 0 val List(a, b, t) = compile(code, allowMessage = i => {c += 1; i.msg contains warn}) assert(c == 1, c) - assertInvoke(getMethod(b, "t1"), "Aa", "f1") + assertInvoke(getMethod(b, "t1"), "A", "f1") assertInvoke(getMethod(b, "t2"), "B", "B$$f2m") assertInvoke(getMethod(b, "t3"), "B", "") assertInvoke(getMethod(b, "t4"), "B", "") -- cgit v1.2.3 From 2980c3921f1270f05add25239da93e05f64ad45f Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 31 May 2016 09:45:10 +0200 Subject: Keep line numbers when inlining from the same compilation unit So far, line numbers were kept only when inlining from the same class. We can also keep them when inlining from a different class defined in the same compilation unit. Longer-term we should support JSR-45, see SI-7518 and scala-dev#3. --- .../scala/tools/nsc/backend/jvm/opt/Inliner.scala | 11 ++++- .../tools/nsc/backend/jvm/opt/InlinerTest.scala | 48 ++++++++++++++++++++++ .../scala/tools/testing/BytecodeTesting.scala | 2 +- 3 files changed, 58 insertions(+), 3 deletions(-) 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 d18963ec8b..7b4cfe2a18 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala @@ -253,7 +253,7 @@ class Inliner[BT <: BTypes](val btypes: BT) { def inlineCallsite(callsite: Callsite): Unit = { import callsite.{callsiteClass, callsiteMethod, callsiteInstruction, receiverKnownNotNull, callsiteStackHeight} val Right(callsiteCallee) = callsite.callee - import callsiteCallee.{callee, calleeDeclarationClass} + import callsiteCallee.{callee, calleeDeclarationClass, sourceFilePath} // Inlining requires the callee not to have unreachable code, the analyzer used below should not // return any `null` frames. Note that inlining a method can create unreachable code. Example: @@ -268,7 +268,14 @@ class Inliner[BT <: BTypes](val btypes: BT) { // New labels for the cloned instructions val labelsMap = cloneLabels(callee) - val (clonedInstructions, instructionMap, hasSerializableClosureInstantiation) = cloneInstructions(callee, labelsMap, keepLineNumbers = callsiteClass == calleeDeclarationClass) + val sameSourceFile = sourceFilePath match { + case Some(calleeSource) => byteCodeRepository.compilingClasses.get(callsiteClass.internalName) match { + case Some((_, `calleeSource`)) => true + case _ => false + } + case _ => false + } + val (clonedInstructions, instructionMap, hasSerializableClosureInstantiation) = cloneInstructions(callee, labelsMap, keepLineNumbers = sameSourceFile) // local vars in the callee are shifted by the number of locals at the callsite val localVarShift = callsiteMethod.maxLocals 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 9b538573dc..9173a1d189 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala @@ -1526,4 +1526,52 @@ class InlinerTest extends BytecodeTesting { val c :: _ = compileClassesSeparately(codes, extraArgs = compilerArgs) assertInvoke(getMethod(c, "t"), "p1/Implicits$RichFunction1$", "toRx$extension") } + + @Test + def keepLineNumbersPerCompilationUnit(): Unit = { + val code1 = + """class A { + | def fx(): Unit = () + | @inline final def ma = { + | fx() + | 1 + | } + |} + """.stripMargin + val code2 = + """class B extends A { + | @inline final def mb = { + | fx() + | 1 + | } + |} + |class C extends B { + | @inline final def mc = { + | fx() + | 1 + | } + | def t1 = ma // no lines, not the same source file + | def t2 = mb // lines + | def t3 = mc // lines + |} + """.stripMargin + notPerRun.foreach(_.clear()) + val run = compiler.newRun + run.compileSources(List(makeSourceFile(code1, "A.scala"), makeSourceFile(code2, "B.scala"))) + val List(_, _, c) = readAsmClasses(getGeneratedClassfiles(global.settings.outputDirs.getSingleOutput.get)) + def is(name: String) = getMethod(c, name).instructions.filterNot(_.isInstanceOf[FrameEntry]) + + assertSameCode(is("t1"), List( + Label(0), LineNumber(12, Label(0)), + VarOp(ALOAD, 0), Invoke(INVOKEVIRTUAL, "A", "fx", "()V", false), + Op(ICONST_1), Op(IRETURN), Label(6))) + + assertSameCode(is("t2"), List( + Label(0), LineNumber(3, Label(0)), VarOp(ALOAD, 0), Invoke(INVOKEVIRTUAL, "B", "fx", "()V", false), + Label(4), LineNumber(4, Label(4)), Op(ICONST_1), Op(IRETURN), Label(8))) + + assertSameCode(is("t3"), List( + Label(0), LineNumber(9, Label(0)), VarOp(ALOAD, 0), Invoke(INVOKEVIRTUAL, "C", "fx", "()V", false), + Label(4), LineNumber(10, Label(4)), Op(ICONST_1), Op(IRETURN), Label(8))) + } } diff --git a/test/junit/scala/tools/testing/BytecodeTesting.scala b/test/junit/scala/tools/testing/BytecodeTesting.scala index 1a0c1e210a..4ddb6580df 100644 --- a/test/junit/scala/tools/testing/BytecodeTesting.scala +++ b/test/junit/scala/tools/testing/BytecodeTesting.scala @@ -29,7 +29,7 @@ class Compiler(val global: Global) { global.settings.outputDirs.setSingleOutput(new VirtualDirectory("(memory)", None)) } - private def newRun: global.Run = { + def newRun: global.Run = { global.reporter.reset() resetOutput() new global.Run() -- cgit v1.2.3 From a1ea0aa0a6136c13baa41268d7dbd4197924d3c9 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 6 Jun 2016 10:50:09 +0200 Subject: Remove TopLevelCreationFlags --- src/reflect/scala/reflect/internal/Flags.scala | 9 ++------- src/reflect/scala/reflect/internal/Mirrors.scala | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/reflect/scala/reflect/internal/Flags.scala b/src/reflect/scala/reflect/internal/Flags.scala index e2522ef280..e06decea6d 100644 --- a/src/reflect/scala/reflect/internal/Flags.scala +++ b/src/reflect/scala/reflect/internal/Flags.scala @@ -234,13 +234,8 @@ class Flags extends ModifierFlags { */ final val AllFlags = -1L - /** These flags can be set when class or module symbol is first created. - */ - final val TopLevelCreationFlags = - MODULE | PACKAGE | FINAL | JAVA - // TODO - there's no call to slap four flags onto every package. - final val PackageFlags = TopLevelCreationFlags + final val PackageFlags = MODULE | PACKAGE | FINAL | JAVA // FINAL not included here due to possibility of object overriding. // In fact, FINAL should not be attached regardless. We should be able @@ -300,7 +295,7 @@ class Flags extends ModifierFlags { final val ConstrFlags = JAVA /** Module flags inherited by their module-class */ - final val ModuleToClassFlags = AccessFlags | TopLevelCreationFlags | CASE | SYNTHETIC + final val ModuleToClassFlags = AccessFlags | PackageFlags | CASE | SYNTHETIC /** These flags are not pickled */ final val FlagsNotPickled = IS_ERROR | OVERLOADED | LIFTED | TRANS_FLAG | LOCKED | TRIEDCOOKING diff --git a/src/reflect/scala/reflect/internal/Mirrors.scala b/src/reflect/scala/reflect/internal/Mirrors.scala index 756300d403..3d1c160d52 100644 --- a/src/reflect/scala/reflect/internal/Mirrors.scala +++ b/src/reflect/scala/reflect/internal/Mirrors.scala @@ -273,7 +273,7 @@ trait Mirrors extends api.Mirrors { // is very beneficial for a handful of bootstrap symbols to have // first class identities sealed trait WellKnownSymbol extends Symbol { - this initFlags (TopLevelCreationFlags | STATIC) + this initFlags (PackageFlags | STATIC) } // Features common to RootClass and RootPackage, the roots of all // type and term symbols respectively. -- cgit v1.2.3