JVM: refactor SMAP stuff some more

Just trying to keep the entropy down.

compiler/backend/src/org/jetbrains/kotlin/codegen/AbstractClassBuilder.java

@@ -137,9 +137,7 @@ public abstract class AbstractClassBuilder implements ClassBuilder {
         List<FileMapping> fileMappings = smap.getResultMappings();
         if (fileMappings.isEmpty()) return;
 
-        FileMapping origin = fileMappings.get(0);
+        visitSource(fileMappings.get(0).getName(), SMAPBuilder.INSTANCE.build(fileMappings, backwardsCompatibleSyntax));
-        SMAPBuilder builder = new SMAPBuilder(origin.getName(), origin.getPath(), fileMappings, backwardsCompatibleSyntax);
-        visitSource(origin.getName(), builder.build());
     }
 
     @Override

compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt

@@ -132,8 +132,6 @@ class MethodInliner(
         processReturns(resultNode, returnLabelOwner, remapReturn, end)
         //flush transformed node to output
         resultNode.accept(SkipMaxAndEndVisitor(adapter))
-
-        sourceMapper.endMapping()
         return result
     }
 
@@ -307,7 +305,6 @@ class MethodInliner(
                         .put(OBJECT_TYPE, erasedInvokeFunction.returnType, this)
                     setLambdaInlining(false)
                     addInlineMarker(this, false)
-                    childSourceMapper.endMapping()
                     inlineOnlySmapSkipper?.markCallSiteLineNumber(remappingMethodAdapter)
                 } else if (isAnonymousConstructorCall(owner, name)) { //TODO add method
                     //TODO add proper message

compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SMAP.kt

@@ -7,10 +7,6 @@ package org.jetbrains.kotlin.codegen.inline
 
 import gnu.trove.TIntIntHashMap
 import org.jetbrains.kotlin.codegen.SourceInfo
-import org.jetbrains.kotlin.codegen.inline.SMAP.Companion.END
-import org.jetbrains.kotlin.codegen.inline.SMAP.Companion.FILE_SECTION
-import org.jetbrains.kotlin.codegen.inline.SMAP.Companion.LINE_SECTION
-import org.jetbrains.kotlin.codegen.inline.SMAP.Companion.STRATA_SECTION
 import org.jetbrains.kotlin.load.java.JvmAbi
 import org.jetbrains.org.objectweb.asm.tree.MethodNode
 import java.util.*
@@ -19,70 +15,52 @@ import kotlin.math.max
 const val KOTLIN_STRATA_NAME = "Kotlin"
 const val KOTLIN_DEBUG_STRATA_NAME = "KotlinDebug"
 
-//TODO join parameter
+object SMAPBuilder {
-class SMAPBuilder(
+    fun build(fileMappings: List<FileMapping>, backwardsCompatibleSyntax: Boolean): String? {
-    val source: String,
+        val realMappings = fileMappings.filter { it.lineMappings.isNotEmpty() && it != FileMapping.SKIP }
-    val path: String,
-    private val fileMappings: List<FileMapping>,
-    private val backwardsCompatibleSyntax: Boolean
-) {
-    private val header = "SMAP\n$source\nKotlin"
-
-    fun build(): String? {
-        val realMappings = fileMappings.filter {
-            val mappings = it.lineMappings
-            mappings.isNotEmpty() && mappings.first() != RangeMapping.SKIP
-        }
-
         if (realMappings.isEmpty()) {
             return null
         }
-
         val defaultStrata = generateDefaultStrata(realMappings)
         val debugStrata = generateDebugStrata(realMappings)
         if (backwardsCompatibleSyntax && defaultStrata.isNotEmpty() && debugStrata.isNotEmpty()) {
             // Old versions of kotlinc might fail if there is no END between defaultStrata and debugStrata.
             // This is not actually correct syntax according to JSR-045.
-            return "$header\n$defaultStrata$END\n$debugStrata$END\n"
+            return "SMAP\n${fileMappings[0].name}\n$KOTLIN_STRATA_NAME\n$defaultStrata${SMAP.END}\n$debugStrata${SMAP.END}\n"
         }
-        return "$header\n$defaultStrata$debugStrata$END\n"
+        return "SMAP\n${fileMappings[0].name}\n$KOTLIN_STRATA_NAME\n$defaultStrata$debugStrata${SMAP.END}\n"
     }
 
     private fun generateDefaultStrata(realMappings: List<FileMapping>): String {
-        val fileIds = FILE_SECTION + realMappings.mapIndexed { id, file -> "\n${file.toSMAPFile(id + 1)}" }.joinToString("")
+        val fileData = realMappings.mapIndexed { id, file -> file.toSMAPFile(id + 1) }.joinToString("")
-        val lineMappings = LINE_SECTION + realMappings.joinToString("") { it.toSMAPMapping() }
+        val lineData = realMappings.mapIndexed { id, file -> file.toSMAPMapping(id + 1) }.joinToString("")
-        return "$STRATA_SECTION $KOTLIN_STRATA_NAME\n$fileIds\n$lineMappings\n"
+        return "${SMAP.STRATA_SECTION} $KOTLIN_STRATA_NAME\n${SMAP.FILE_SECTION}\n$fileData${SMAP.LINE_SECTION}\n$lineData"
     }
 
     private fun generateDebugStrata(realMappings: List<FileMapping>): String {
-        val combinedMapping = FileMapping(source, path)
+        val combinedMapping = FileMapping(realMappings[0].name, realMappings[0].path)
-        realMappings.forEach { fileMapping ->
+        for (fileMapping in realMappings) {
-            fileMapping.lineMappings.filter { it.callSiteMarker != null }.forEach { (_, dest, range, callSiteMarker) ->
+            for ((_, dest, range, callSiteMarker) in fileMapping.lineMappings) {
-                combinedMapping.addRangeMapping(RangeMapping(callSiteMarker!!.lineNumber, dest, range))
+                callSiteMarker?.let { combinedMapping.mapNewInterval(it.lineNumber, dest, range) }
             }
         }
 
         if (combinedMapping.lineMappings.isEmpty()) return ""
-
+        val fileData = combinedMapping.toSMAPFile(1)
-        val newMappings = listOf(combinedMapping)
+        // TODO: this generates entries like `1#2,3:4` which means "map lines 4..6 to lines 1..3 of file #2".
-        val fileIds = FILE_SECTION + newMappings.mapIndexed { id, file -> "\n${file.toSMAPFile(id + 1)}" }.joinToString("")
+        //   What we want is `1#2:4,3`, i.e. "map lines 4..6 to line 1 of #2", but currently IDEA cannot handle that.
-        val lineMappings = LINE_SECTION + newMappings.joinToString("") { it.toSMAPMapping() }
+        val lineData = combinedMapping.toSMAPMapping(1)
-        return "$STRATA_SECTION $KOTLIN_DEBUG_STRATA_NAME\n$fileIds\n$lineMappings\n"
+        return "${SMAP.STRATA_SECTION} $KOTLIN_DEBUG_STRATA_NAME\n${SMAP.FILE_SECTION}\n$fileData${SMAP.LINE_SECTION}\n$lineData"
     }
 
-    private fun RangeMapping.toSMAP(fileId: Int): String {
+    private fun RangeMapping.toSMAP(fileId: Int): String =
-        return if (range == 1) "$source#$fileId:$dest" else "$source#$fileId,$range:$dest"
+        if (range == 1) "$source#$fileId:$dest\n" else "$source#$fileId,$range:$dest\n"
-    }
 
-    private fun FileMapping.toSMAPFile(id: Int): String {
+    private fun FileMapping.toSMAPFile(id: Int): String =
-        this.id = id
+        "+ $id $name\n$path\n"
-        return "+ $id $name\n$path"
-    }
 
-    //TODO inline
+    private fun FileMapping.toSMAPMapping(id: Int): String =
-    private fun FileMapping.toSMAPMapping(): String {
+        lineMappings.joinToString("") { it.toSMAP(id) }
-        return lineMappings.joinToString("") { "\n${it.toSMAP(id)}" }
-    }
 }
 
 class NestedSourceMapper(
@@ -113,9 +91,9 @@ class NestedSourceMapper(
             ?: error("Can't find range to map line $lineNumber in ${sourceInfo.source}: ${sourceInfo.pathOrCleanFQN}")
         val sourceLineNumber = range.mapDestToSource(lineNumber)
         val newLineNumber = if (sameFile)
-            parent.mapLineNumber(sourceLineNumber, range.parent!!.name, range.parent!!.path, range.callSiteMarker)
+            parent.mapLineNumber(sourceLineNumber, range.parent.name, range.parent.path, range.callSiteMarker)
         else
-            parent.mapLineNumber(sourceLineNumber, range.parent!!.name, range.parent!!.path)
+            parent.mapLineNumber(sourceLineNumber, range.parent.name, range.parent.path)
         if (newLineNumber > 0) {
             visitedLines.put(lineNumber, newLineNumber)
         }
@@ -136,9 +114,6 @@ interface SourceMapper {
 
     fun mapLineNumber(source: Int, sourceName: String, sourcePath: String, callSiteMarker: CallSiteMarker?): Int
 
-    fun endMapping() {
-    }
-
     companion object {
         fun createFromSmap(smap: SMAP): SourceMapper {
             return DefaultSourceMapper(smap.sourceInfo, smap.fileMappings)
@@ -167,12 +142,12 @@ data class CallSiteMarker(val lineNumber: Int)
 
 open class DefaultSourceMapper(val sourceInfo: SourceInfo) : SourceMapper {
     private var maxUsedValue: Int = sourceInfo.linesInFile
-    private var fileMappings: LinkedHashMap<Pair<String, String>, RawFileMapping> = linkedMapOf()
+    private var fileMappings: LinkedHashMap<Pair<String, String>, FileMapping> = linkedMapOf()
 
     var callSiteMarker: CallSiteMarker? = null
 
     override val resultMappings: List<FileMapping>
-        get() = fileMappings.values.map { it.toFileMapping() }
+        get() = fileMappings.values.toList()
 
     init {
         // Explicitly map the file to itself.
@@ -191,18 +166,12 @@ open class DefaultSourceMapper(val sourceInfo: SourceInfo) : SourceMapper {
             }
     }
 
-    private fun getOrRegisterNewSource(name: String, path: String): RawFileMapping {
+    private fun getOrRegisterNewSource(name: String, path: String): FileMapping {
-        return fileMappings.getOrPut(name to path) { RawFileMapping(name, path) }
+        return fileMappings.getOrPut(name to path) { FileMapping(name, path) }
     }
 
-    override fun mapLineNumber(lineNumber: Int): Int {
+    //TODO maybe add assertion that linenumber contained in fileMappings
-        if (lineNumber < 0) {
+    override fun mapLineNumber(lineNumber: Int): Int = lineNumber
-            //no source information, so just skip this linenumber
-            return -1
-        }
-        //TODO maybe add assertion that linenumber contained in fileMappings
-        return lineNumber
-    }
 
     override fun mapLineNumber(source: Int, sourceName: String, sourcePath: String): Int =
         mapLineNumber(source, sourceName, sourcePath, callSiteMarker)
@@ -227,12 +196,11 @@ class SMAP(val fileMappings: List<FileMapping>) {
         SourceInfo(defaultFile.name, defaultFile.path, defaultRange.source + defaultRange.range - 1)
     }
 
-    private val intervals = fileMappings.flatMap { it.lineMappings }.sortedWith(RangeMapping.Comparator)
+    // assuming disjoint line mappings (otherwise binary search can't be used anyway)
+    private val intervals = fileMappings.flatMap { it.lineMappings }.sortedBy { it.dest }
 
     fun findRange(lineNumber: Int): RangeMapping? {
-        val index = intervals.binarySearch(RangeMapping(lineNumber, lineNumber, 1), Comparator { value, key ->
+        val index = intervals.binarySearch { if (lineNumber in it) 0 else it.dest - lineNumber }
-            if (key.dest in value) 0 else RangeMapping.Comparator.compare(value, key)
-        })
         return if (index < 0) null else intervals[index]
     }
 
@@ -246,18 +214,11 @@ class SMAP(val fileMappings: List<FileMapping>) {
 
 data class SMAPAndMethodNode(val node: MethodNode, val classSMAP: SMAP)
 
-class RawFileMapping(val name: String, val path: String) {
+class FileMapping(val name: String, val path: String) {
-    private val rangeMappings = arrayListOf<RangeMapping>()
+    val lineMappings = arrayListOf<RangeMapping>()
-
-    fun toFileMapping() =
-        FileMapping(name, path).apply {
-            for (range in rangeMappings) {
-                addRangeMapping(range)
-            }
-        }
 
     fun mapNewLineNumber(source: Int, currentIndex: Int, callSiteMarker: CallSiteMarker?): Int {
-        var mapping = rangeMappings.lastOrNull()
+        var mapping = lineMappings.lastOrNull()
         if (mapping != null && mapping.callSiteMarker == callSiteMarker &&
             (source - mapping.source) in 0 until mapping.range + (if (mapping.maxDest == currentIndex) 10 else 0)
         ) {
@@ -270,28 +231,19 @@ class RawFileMapping(val name: String, val path: String) {
     }
 
     fun mapNewInterval(source: Int, dest: Int, range: Int, callSiteMarker: CallSiteMarker? = null): RangeMapping =
-        RangeMapping(source, dest, range, callSiteMarker).also { rangeMappings.add(it) }
+        RangeMapping(source, dest, range, callSiteMarker, parent = this).also { lineMappings.add(it) }
-}
 
-open class FileMapping(val name: String, val path: String) {
+    companion object {
-    val lineMappings = arrayListOf<RangeMapping>()
+        val SKIP = FileMapping("no-source-info", "no-source-info").apply {
-    var id = -1
+            mapNewInterval(-1, -1, 1)
-
-    fun addRangeMapping(lineMapping: RangeMapping) {
-        lineMappings.add(lineMapping)
-        lineMapping.parent = this
-    }
-
-    object SKIP : FileMapping("no-source-info", "no-source-info") {
-        init {
-            addRangeMapping(RangeMapping.SKIP)
         }
     }
 }
 
-//TODO comparable
+data class RangeMapping(
-data class RangeMapping(val source: Int, val dest: Int, var range: Int = 1, var callSiteMarker: CallSiteMarker? = null) {
+    val source: Int, val dest: Int, var range: Int, val callSiteMarker: CallSiteMarker?,
-    var parent: FileMapping? = null
+    val parent: FileMapping
+) {
     private val skip = source == -1 && dest == -1
 
     val maxDest: Int
@@ -312,19 +264,6 @@ data class RangeMapping(val source: Int, val dest: Int, var range: Int = 1, var
     fun mapSourceToDest(sourceLine: Int): Int {
         return if (skip) -1 else dest + (sourceLine - source)
     }
-
-    object Comparator : java.util.Comparator<RangeMapping> {
-        override fun compare(o1: RangeMapping, o2: RangeMapping): Int {
-            if (o1 == o2) return 0
-
-            val res = o1.dest - o2.dest
-            return if (res == 0) o1.range - o2.range else res
-        }
-    }
-
-    companion object {
-        val SKIP = RangeMapping(-1, -1, 1)
-    }
 }
 
 val RangeMapping.toRange: IntRange

compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SMAPParser.kt

@@ -31,7 +31,7 @@ object SMAPParser {
                 FileMapping(source, path).apply {
                     if (methodStartLine <= methodEndLine) {
                         //one to one
-                        addRangeMapping(RangeMapping(methodStartLine, methodStartLine, methodEndLine - methodStartLine + 1))
+                        mapNewInterval(methodStartLine, methodStartLine, methodEndLine - methodStartLine + 1)
                     }
                 }
 
@@ -74,7 +74,7 @@ object SMAPParser {
 
             val fileIndex = lineMapping.substring(fileSeparator + 1, rangeSeparator).toInt()
             val targetIndex = lineMapping.substring(targetSplit + 1).toInt()
-            fileMappings[fileIndex]!!.addRangeMapping(RangeMapping(originalIndex, targetIndex, range))
+            fileMappings[fileIndex]!!.mapNewInterval(originalIndex, targetIndex, range)
         }
 
         return SMAP(fileMappings.values.toList())