Merge pull request scala#5202 from lrytz/lineNumbers

Keep line numbers when inlining from the same compilation unit

Author: adriaanm

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))

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)
   }

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,40 +67,45 @@ 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 != "<no file>" => 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))
+    }
   }
 
   /**
    * The class node for an internal name. If the class node is not yet available, it is parsed from
    * 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
-}

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,15 +255,15 @@ 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])
 
   /**
    * 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,26 +305,26 @@ 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),
             warning             = warning)
 
         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]) {

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),

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,11 +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)
-    val keepLineNumbers = callsiteClass == calleeDeclarationClass
-    if (!keepLineNumbers) {
-      removeLineNumberNodes(clonedInstructions)
+    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