From 9d7db101e66a89f08d119479a930cce790f2967d Mon Sep 17 00:00:00 2001 From: pyos Date: Thu, 26 Mar 2020 09:18:36 +0100 Subject: [PATCH] JVM: refactor SMAP stuff some more Just trying to keep the entropy down. --- .../kotlin/codegen/AbstractClassBuilder.java | 4 +- .../kotlin/codegen/inline/MethodInliner.kt | 3 - .../jetbrains/kotlin/codegen/inline/SMAP.kt | 151 ++++++------------ .../kotlin/codegen/inline/SMAPParser.kt | 4 +- 4 files changed, 48 insertions(+), 114 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/AbstractClassBuilder.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/AbstractClassBuilder.java index 048d26836e3..efc0106db6b 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/AbstractClassBuilder.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/AbstractClassBuilder.java @@ -137,9 +137,7 @@ public abstract class AbstractClassBuilder implements ClassBuilder { List fileMappings = smap.getResultMappings(); if (fileMappings.isEmpty()) return; - FileMapping origin = fileMappings.get(0); - SMAPBuilder builder = new SMAPBuilder(origin.getName(), origin.getPath(), fileMappings, backwardsCompatibleSyntax); - visitSource(origin.getName(), builder.build()); + visitSource(fileMappings.get(0).getName(), SMAPBuilder.INSTANCE.build(fileMappings, backwardsCompatibleSyntax)); } @Override diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt index 0ce9cf352c7..e98ea7507b0 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt +++ b/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 diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SMAP.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SMAP.kt index 3ba7bfc8daf..5665865486d 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SMAP.kt +++ b/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 -class SMAPBuilder( - val source: String, - val path: String, - private val fileMappings: List, - 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 - } - +object SMAPBuilder { + fun build(fileMappings: List, backwardsCompatibleSyntax: Boolean): String? { + val realMappings = fileMappings.filter { it.lineMappings.isNotEmpty() && it != FileMapping.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): String { - val fileIds = FILE_SECTION + realMappings.mapIndexed { id, file -> "\n${file.toSMAPFile(id + 1)}" }.joinToString("") - val lineMappings = LINE_SECTION + realMappings.joinToString("") { it.toSMAPMapping() } - return "$STRATA_SECTION $KOTLIN_STRATA_NAME\n$fileIds\n$lineMappings\n" + val fileData = realMappings.mapIndexed { id, file -> file.toSMAPFile(id + 1) }.joinToString("") + val lineData = realMappings.mapIndexed { id, file -> file.toSMAPMapping(id + 1) }.joinToString("") + return "${SMAP.STRATA_SECTION} $KOTLIN_STRATA_NAME\n${SMAP.FILE_SECTION}\n$fileData${SMAP.LINE_SECTION}\n$lineData" } private fun generateDebugStrata(realMappings: List): String { - val combinedMapping = FileMapping(source, path) - realMappings.forEach { fileMapping -> - fileMapping.lineMappings.filter { it.callSiteMarker != null }.forEach { (_, dest, range, callSiteMarker) -> - combinedMapping.addRangeMapping(RangeMapping(callSiteMarker!!.lineNumber, dest, range)) + val combinedMapping = FileMapping(realMappings[0].name, realMappings[0].path) + for (fileMapping in realMappings) { + for ((_, dest, range, callSiteMarker) in fileMapping.lineMappings) { + callSiteMarker?.let { combinedMapping.mapNewInterval(it.lineNumber, dest, range) } } } if (combinedMapping.lineMappings.isEmpty()) return "" - - val newMappings = listOf(combinedMapping) - val fileIds = FILE_SECTION + newMappings.mapIndexed { id, file -> "\n${file.toSMAPFile(id + 1)}" }.joinToString("") - val lineMappings = LINE_SECTION + newMappings.joinToString("") { it.toSMAPMapping() } - return "$STRATA_SECTION $KOTLIN_DEBUG_STRATA_NAME\n$fileIds\n$lineMappings\n" + val fileData = combinedMapping.toSMAPFile(1) + // TODO: this generates entries like `1#2,3:4` which means "map lines 4..6 to lines 1..3 of file #2". + // 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 lineData = combinedMapping.toSMAPMapping(1) + 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 { - return if (range == 1) "$source#$fileId:$dest" else "$source#$fileId,$range:$dest" - } + private fun RangeMapping.toSMAP(fileId: Int): String = + if (range == 1) "$source#$fileId:$dest\n" else "$source#$fileId,$range:$dest\n" - private fun FileMapping.toSMAPFile(id: Int): String { - this.id = id - return "+ $id $name\n$path" - } + private fun FileMapping.toSMAPFile(id: Int): String = + "+ $id $name\n$path\n" - //TODO inline - private fun FileMapping.toSMAPMapping(): String { - return lineMappings.joinToString("") { "\n${it.toSMAP(id)}" } - } + private fun FileMapping.toSMAPMapping(id: Int): String = + lineMappings.joinToString("") { 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, RawFileMapping> = linkedMapOf() + private var fileMappings: LinkedHashMap, FileMapping> = linkedMapOf() var callSiteMarker: CallSiteMarker? = null override val resultMappings: List - 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 { - return fileMappings.getOrPut(name to path) { RawFileMapping(name, path) } + private fun getOrRegisterNewSource(name: String, path: String): FileMapping { + return fileMappings.getOrPut(name to path) { FileMapping(name, path) } } - override fun mapLineNumber(lineNumber: Int): Int { - if (lineNumber < 0) { - //no source information, so just skip this linenumber - return -1 - } - //TODO maybe add assertion that linenumber contained in fileMappings - return lineNumber - } + //TODO maybe add assertion that linenumber contained in fileMappings + override fun mapLineNumber(lineNumber: Int): Int = 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) { 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 -> - if (key.dest in value) 0 else RangeMapping.Comparator.compare(value, key) - }) + val index = intervals.binarySearch { if (lineNumber in it) 0 else it.dest - lineNumber } return if (index < 0) null else intervals[index] } @@ -246,18 +214,11 @@ class SMAP(val fileMappings: List) { data class SMAPAndMethodNode(val node: MethodNode, val classSMAP: SMAP) -class RawFileMapping(val name: String, val path: String) { - private val rangeMappings = arrayListOf() - - fun toFileMapping() = - FileMapping(name, path).apply { - for (range in rangeMappings) { - addRangeMapping(range) - } - } +class FileMapping(val name: String, val path: String) { + val lineMappings = arrayListOf() 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) { - val lineMappings = arrayListOf() - var id = -1 - - fun addRangeMapping(lineMapping: RangeMapping) { - lineMappings.add(lineMapping) - lineMapping.parent = this - } - - object SKIP : FileMapping("no-source-info", "no-source-info") { - init { - addRangeMapping(RangeMapping.SKIP) + companion object { + val SKIP = FileMapping("no-source-info", "no-source-info").apply { + mapNewInterval(-1, -1, 1) } } } -//TODO comparable -data class RangeMapping(val source: Int, val dest: Int, var range: Int = 1, var callSiteMarker: CallSiteMarker? = null) { - var parent: FileMapping? = null +data class RangeMapping( + val source: Int, val dest: Int, var range: Int, val callSiteMarker: CallSiteMarker?, + 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 { - 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 diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SMAPParser.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SMAPParser.kt index 5e6b67c581d..4f4be3ceae6 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SMAPParser.kt +++ b/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())