JVM: separate the two kinds of source mappers

* a writing source mapper has `mapLineNumber(line, file, class)` that inserts a new SMAP entry and returns a fake line number from it; * a copying source mapper has `mapLineNumber(line)` that uses an existing SMAP to resolve the line number and call the former method on a different source mapper; * those two types are disjoint.
2020-04-01 07:59:01 +02:00
parent 143d8d1520
commit 1fe7ef6521
16 changed files with 90 additions and 179 deletions
@@ -13,9 +13,9 @@ import org.jetbrains.annotations.Nullable;
 import org.jetbrains.kotlin.backend.common.CodegenUtil;
 import org.jetbrains.kotlin.codegen.binding.CodegenBinding;
 import org.jetbrains.kotlin.codegen.context.*;
-import org.jetbrains.kotlin.codegen.inline.DefaultSourceMapper;
 import org.jetbrains.kotlin.codegen.inline.NameGenerator;
 import org.jetbrains.kotlin.codegen.inline.ReifiedTypeParametersUsages;
+import org.jetbrains.kotlin.codegen.inline.SourceMapper;
 import org.jetbrains.kotlin.codegen.state.GenerationState;
 import org.jetbrains.kotlin.codegen.state.KotlinTypeMapper;
 import org.jetbrains.kotlin.codegen.state.TypeMapperUtilsKt;
@@ -88,7 +88,7 @@ public abstract class MemberCodegen<T extends KtPureElement/* TODO: & KtDeclarat
    private NameGenerator inlineNameGenerator;
    private boolean jvmAssertFieldGenerated;

-    private DefaultSourceMapper sourceMapper;
+    private SourceMapper sourceMapper;

    public MemberCodegen(
            @NotNull GenerationState state,
@@ -722,10 +722,10 @@ public abstract class MemberCodegen<T extends KtPureElement/* TODO: & KtDeclarat
    }

    @NotNull
-    public DefaultSourceMapper getOrCreateSourceMapper() {
+    public SourceMapper getOrCreateSourceMapper() {
        if (sourceMapper == null) {
            // note: this is used in InlineCodegen and the element is always physical (KtElement) there
-            sourceMapper = new DefaultSourceMapper(SourceInfo.Companion.createInfo((KtElement)element, getClassName()));
+            sourceMapper = new SourceMapper(SourceInfo.Companion.createInfo((KtElement)element, getClassName()));
        }
        return sourceMapper;
    }
@@ -42,8 +42,7 @@ class AnonymousObjectTransformer(
    private val fieldNames = hashMapOf<String, MutableList<String>>()

    private var constructor: MethodNode? = null
-    private var sourceInfo: String? = null
-    private var debugInfo: String? = null
+    private lateinit var sourceMap: SMAP
    private lateinit var sourceMapper: SourceMapper
    private val languageVersionSettings = inliningContext.state.languageVersionSettings

@@ -56,6 +55,8 @@ class AnonymousObjectTransformer(
        val methodsToTransform = ArrayList<MethodNode>()
        val metadataReader = ReadKotlinClassHeaderAnnotationVisitor()
        lateinit var superClassName: String
+        var sourceInfo: String? = null
+        var debugInfo: String? = null

        createClassReader().accept(object : ClassVisitor(Opcodes.API_VERSION, classBuilder.visitor) {
            override fun visit(version: Int, access: Int, name: String, signature: String?, superName: String, interfaces: Array<String>) {
@@ -113,22 +114,13 @@ class AnonymousObjectTransformer(
            override fun visitEnd() {}
        }, ClassReader.SKIP_FRAMES)

-        if (!inliningContext.isInliningLambda) {
-            sourceMapper = if (debugInfo != null && !debugInfo!!.isEmpty()) {
-                SourceMapper.createFromSmap(SMAPParser.parse(debugInfo!!))
-            } else {
-                //seems we can't do any clever mapping cause we don't know any about original class name
-                IdenticalSourceMapper
-            }
-            if (sourceInfo != null && !GENERATE_SMAP) {
-                classBuilder.visitSource(sourceInfo!!, debugInfo)
-            }
-        } else {
-            if (sourceInfo != null) {
-                classBuilder.visitSource(sourceInfo!!, debugInfo)
-            }
-            sourceMapper = IdenticalSourceMapper
-        }
+        // When regenerating objects in lambdas, we have to pass the SMAP straight through and keep
+        // the original line numbers because SMAPParser does not parse call site markers.
+        val debugInfoToParse = if (inliningContext.isInliningLambda) null else debugInfo
+        sourceMap = SMAPParser.parseOrCreateDefault(debugInfoToParse, sourceInfo, oldObjectType.internalName, 1, 65535)
+        // TODO source info should be for the file into which the function is being inlined, else we cannot
+        //      generate correct call site markers for lambdas inlined into the object
+        sourceMapper = SourceMapper(sourceMap.sourceInfo.takeIf { debugInfoToParse?.isEmpty() == false })

        val allCapturedParamBuilder = ParametersBuilder.newBuilder()
        val constructorParamBuilder = ParametersBuilder.newBuilder()
@@ -183,7 +175,11 @@ class AnonymousObjectTransformer(
            }
        }

-        classBuilder.visitSMAP(sourceMapper, !state.languageVersionSettings.supportsFeature(LanguageFeature.CorrectSourceMappingSyntax))
+        if (GENERATE_SMAP && !inliningContext.isInliningLambda) {
+            classBuilder.visitSMAP(sourceMapper, !state.languageVersionSettings.supportsFeature(LanguageFeature.CorrectSourceMappingSyntax))
+        } else if (sourceInfo != null) {
+            classBuilder.visitSource(sourceInfo!!, debugInfo)
+        }

        val visitor = classBuilder.visitor
        innerClassNodes.forEach { node ->
@@ -296,7 +292,7 @@ class AnonymousObjectTransformer(
            remapper,
            isSameModule,
            "Transformer for " + transformationInfo.oldClassName,
-            sourceMapper,
+            SourceMapCopier(sourceMapper, sourceMap, keepCallSites = inliningContext.isInliningLambda),
            InlineCallSiteInfo(
                transformationInfo.oldClassName,
                sourceNode.name,
@@ -30,14 +30,14 @@ import static org.jetbrains.kotlin.codegen.inline.InlineCodegenUtilsKt.GENERATE_
 import static org.jetbrains.kotlin.codegen.inline.InlineCodegenUtilsKt.getLoadStoreArgSize;

 public class InlineAdapter extends InstructionAdapter {
-    private final SourceMapper sourceMapper;
+    private final SourceMapCopier sourceMapper;
    private final List<CatchBlock> blocks = new ArrayList<>();

    private boolean isLambdaInlining = false;
    private int nextLocalIndex = 0;
    private int nextLocalIndexBeforeInline = -1;

-    public InlineAdapter(@NotNull MethodVisitor mv, int localsSize, @NotNull SourceMapper sourceMapper) {
+    public InlineAdapter(@NotNull MethodVisitor mv, int localsSize, @NotNull SourceMapCopier sourceMapper) {
        super(Opcodes.API_VERSION, mv);
        this.nextLocalIndex = localsSize;
        this.sourceMapper = sourceMapper;
@@ -233,7 +233,7 @@ abstract class InlineCodegen<out T : BaseExpressionCodegen>(
        val inliner = MethodInliner(
            node, parameters, info, FieldRemapper(null, null, parameters), isSameModule,
            "Method inlining " + sourceCompiler.callElementText,
-            NestedSourceMapper(defaultSourceMapper, nodeAndSmap.classSMAP), info.callSiteInfo,
+            SourceMapCopier(defaultSourceMapper, nodeAndSmap.classSMAP), info.callSiteInfo,
            if (functionDescriptor.isInlineOnly()) InlineOnlySmapSkipper(codegen) else null,
            !isInlinedToInlineFunInKotlinRuntime()
        ) //with captured
@@ -42,7 +42,7 @@ class InlineCodegenForDefaultBody(
        val nodeAndSmap = InlineCodegen.createInlineMethodNode(
            function, methodOwner, jvmSignature, callDefault, null, codegen.typeSystem, state, sourceCompilerForInline
        )
-        val childSourceMapper = NestedSourceMapper(sourceMapper, nodeAndSmap.classSMAP, sameFile = true)
+        val childSourceMapper = SourceMapCopier(sourceMapper, nodeAndSmap.classSMAP, keepCallSites = true)

        val node = nodeAndSmap.node
        val transformedMethod = MethodNode(
@@ -54,7 +54,7 @@ class MethodInliner(
    private val nodeRemapper: FieldRemapper,
    private val isSameModule: Boolean,
    private val errorPrefix: String,
-    private val sourceMapper: SourceMapper,
+    private val sourceMapper: SourceMapCopier,
    private val inlineCallSiteInfo: InlineCallSiteInfo,
    private val inlineOnlySmapSkipper: InlineOnlySmapSkipper?, //non null only for root
    private val shouldPreprocessApiVersionCalls: Boolean = false
@@ -288,20 +288,17 @@ class MethodInliner(
                    )

                    setLambdaInlining(true)
-                    val lambdaSMAP = info.node.classSMAP
-
-                    val childSourceMapper =
-                        if (inliningContext.classRegeneration && !inliningContext.isInliningLambda)
-                            NestedSourceMapper(sourceMapper, lambdaSMAP)
-                        else
-                            NestedSourceMapper(sourceMapper.parent!!, lambdaSMAP, sameFile = info !is DefaultLambda)

+                    // TODO we cannot keep call site markers when inlining lambdas into objects because AnonymousObjectTransformer
+                    //      uses the original's source info, thus KotlinDebug would point to the file with the inline function
+                    //      instead of the one with the call.
+                    val sameFile = info !is DefaultLambda && (!inliningContext.classRegeneration || inliningContext.isInliningLambda)
                    val inliner = MethodInliner(
                        info.node.node, lambdaParameters, inliningContext.subInlineLambda(info),
                        newCapturedRemapper,
                        if (info is DefaultLambda) isSameModule else true /*cause all nested objects in same module as lambda*/,
                        "Lambda inlining " + info.lambdaClassType.internalName,
-                        childSourceMapper, inlineCallSiteInfo, null
+                        SourceMapCopier(sourceMapper.parent, info.node.classSMAP, sameFile), inlineCallSiteInfo, null
                    )

                    val varRemapper = LocalVarRemapper(lambdaParameters, valueParamShift)
@@ -63,24 +63,15 @@ object SMAPBuilder {
        lineMappings.joinToString("") { it.toSMAP(id) }
 }

-class NestedSourceMapper(
-    override val parent: SourceMapper, private val smap: SMAP, private val sameFile: Boolean = false
-) : DefaultSourceMapper(smap.sourceInfo) {
-
+class SourceMapCopier(val parent: SourceMapper, private val smap: SMAP, private val keepCallSites: Boolean = false) {
    private val visitedLines = TIntIntHashMap()
-
    private var lastVisitedRange: RangeMapping? = null

-    override fun mapLineNumber(lineNumber: Int): Int {
+    fun mapLineNumber(lineNumber: Int): Int {
        if (lineNumber in JvmAbi.SYNTHETIC_MARKER_LINE_NUMBERS) {
            return lineNumber
        }

-        if (sameFile && lineNumber <= smap.sourceInfo.linesInFile) {
-            // assuming the parent source mapper is for the same file, this line number does not need remapping
-            return lineNumber
-        }
-
        val mappedLineNumber = visitedLines.get(lineNumber)
        if (mappedLineNumber > 0) {
            return mappedLineNumber
@@ -88,99 +79,41 @@ class NestedSourceMapper(

        val range = lastVisitedRange?.takeIf { lineNumber in it }
            ?: smap.findRange(lineNumber)
-            ?: error("Can't find range to map line $lineNumber in ${sourceInfo.source}: ${sourceInfo.pathOrCleanFQN}")
+            ?: error("Can't find range to map line $lineNumber in ${smap.sourceInfo.source}: ${smap.sourceInfo.pathOrCleanFQN}")
        val sourceLineNumber = range.mapDestToSource(lineNumber)
-        val newLineNumber = if (sameFile)
-            parent.mapLineNumber(sourceLineNumber, range.parent.name, range.parent.path, range.callSiteMarker)
-        else
-            parent.mapLineNumber(sourceLineNumber, range.parent.name, range.parent.path)
-        if (newLineNumber > 0) {
-            visitedLines.put(lineNumber, newLineNumber)
+        if (sourceLineNumber < 0) {
+            return -1
        }
+        val callSiteMarker = if (keepCallSites) range.callSiteMarker else parent.callSiteMarker
+        val newLineNumber = parent.mapLineNumber(sourceLineNumber, range.parent.name, range.parent.path, callSiteMarker)
+        visitedLines.put(lineNumber, newLineNumber)
        lastVisitedRange = range
        return newLineNumber
    }
 }

-interface SourceMapper {
-    val resultMappings: List<FileMapping>
-    val parent: SourceMapper?
-        get() = null
-
-    fun mapLineNumber(lineNumber: Int): Int
-
-    fun mapLineNumber(source: Int, sourceName: String, sourcePath: String): Int =
-        mapLineNumber(source, sourceName, sourcePath, null)
-
-    fun mapLineNumber(source: Int, sourceName: String, sourcePath: String, callSiteMarker: CallSiteMarker?): Int
-
-    companion object {
-        fun createFromSmap(smap: SMAP): SourceMapper {
-            return DefaultSourceMapper(smap.sourceInfo, smap.fileMappings)
-        }
-    }
-}
-
-object IdenticalSourceMapper : SourceMapper {
-    override val resultMappings: List<FileMapping>
-        get() = emptyList()
-
-    override val parent: SourceMapper?
-        get() = null
-
-    override fun mapLineNumber(lineNumber: Int) = lineNumber
-
-    override fun mapLineNumber(source: Int, sourceName: String, sourcePath: String, callSiteMarker: CallSiteMarker?): Int =
-        throw UnsupportedOperationException(
-            "IdenticalSourceMapper#mapLineNumber($source, $sourceName, $sourcePath)\n"
-                    + "This mapper should not encounter a line number out of range of the current file.\n"
-                    + "This indicates that SMAP generation is missed somewhere."
-        )
-}
-
 data class CallSiteMarker(val lineNumber: Int)

-open class DefaultSourceMapper(val sourceInfo: SourceInfo) : SourceMapper {
-    private var maxUsedValue: Int = sourceInfo.linesInFile
+class SourceMapper(val sourceInfo: SourceInfo?) {
+    private var maxUsedValue: Int = sourceInfo?.linesInFile ?: 0
    private var fileMappings: LinkedHashMap<Pair<String, String>, FileMapping> = linkedMapOf()

    var callSiteMarker: CallSiteMarker? = null

-    override val resultMappings: List<FileMapping>
+    val resultMappings: List<FileMapping>
        get() = fileMappings.values.toList()

    init {
-        // Explicitly map the file to itself.
-        getOrRegisterNewSource(sourceInfo.source, sourceInfo.pathOrCleanFQN).mapNewInterval(1, 1, sourceInfo.linesInFile)
-    }
-
-    constructor(sourceInfo: SourceInfo, fileMappings: List<FileMapping>) : this(sourceInfo) {
-        // The first mapping is already created in the `init` block above.
-        fileMappings.asSequence().drop(1)
-            .forEach { fileMapping ->
-                val newFileMapping = getOrRegisterNewSource(fileMapping.name, fileMapping.path)
-                fileMapping.lineMappings.forEach {
-                    newFileMapping.mapNewInterval(it.source, it.dest, it.range)
-                    maxUsedValue = max(it.maxDest, maxUsedValue)
-                }
-            }
-    }
-
-    private fun getOrRegisterNewSource(name: String, path: String): FileMapping {
-        return fileMappings.getOrPut(name to path) { FileMapping(name, path) }
-    }
-
-    //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)
-
-    override fun mapLineNumber(source: Int, sourceName: String, sourcePath: String, callSiteMarker: CallSiteMarker?): Int {
-        if (source < 0) {
-            //no source information, so just skip this linenumber
-            return -1
+        sourceInfo?.let {
+            // Explicitly map the file to itself -- we'll probably need a lot of lines from it, so this will produce less ranges.
+            getOrRegisterNewSource(it.source, it.pathOrCleanFQN).mapNewInterval(1, 1, it.linesInFile)
        }
+    }
+
+    private fun getOrRegisterNewSource(name: String, path: String): FileMapping =
+        fileMappings.getOrPut(name to path) { FileMapping(name, path) }
+
+    fun mapLineNumber(source: Int, sourceName: String, sourcePath: String, callSiteMarker: CallSiteMarker?): Int {
        val fileMapping = getOrRegisterNewSource(sourceName, sourcePath)
        val mappedLineIndex = fileMapping.mapNewLineNumber(source, maxUsedValue, callSiteMarker)
        maxUsedValue = max(maxUsedValue, mappedLineIndex)
@@ -188,13 +121,12 @@ open class DefaultSourceMapper(val sourceInfo: SourceInfo) : SourceMapper {
    }
 }

+private fun FileMapping.toSourceInfo(): SourceInfo =
+    SourceInfo(name, path, lineMappings.fold(0) { result, mapping -> max(result, mapping.source + mapping.range - 1) })
+
 class SMAP(val fileMappings: List<FileMapping>) {
-    val sourceInfo: SourceInfo = run {
-        assert(fileMappings.isNotEmpty()) { "File Mappings shouldn't be empty" }
-        val defaultFile = fileMappings.first()
-        val defaultRange = defaultFile.lineMappings.first()
-        SourceInfo(defaultFile.name, defaultFile.path, defaultRange.source + defaultRange.range - 1)
-    }
+    val sourceInfo: SourceInfo
+        get() = fileMappings.firstOrNull()?.toSourceInfo() ?: throw AssertionError("no files mapped")

    // assuming disjoint line mappings (otherwise binary search can't be used anyway)
    private val intervals = fileMappings.flatMap { it.lineMappings }.sortedBy { it.dest }
@@ -218,18 +150,19 @@ class FileMapping(val name: String, val path: String) {
    val lineMappings = arrayListOf<RangeMapping>()

    fun mapNewLineNumber(source: Int, currentIndex: Int, callSiteMarker: CallSiteMarker?): Int {
-        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)
-        ) {
-            // Save some space in the SMAP by reusing (or extending if it's the last one) the existing range.
-            mapping.range = max(mapping.range, source - mapping.source + 1)
-        } else {
-            mapping = mapNewInterval(source, currentIndex + 1, 1, callSiteMarker)
-        }
+        // Save some space in the SMAP by reusing (or extending if it's the last one) the existing range.
+        // TODO some *other* range may already cover `source`; probably too slow to check them all though.
+        //   Maybe keep the list ordered by `source` and use binary search to locate the closest range on the left?
+        val mapping = lineMappings.lastOrNull()?.takeIf { it.canReuseFor(source, currentIndex, callSiteMarker) }
+            ?: lineMappings.firstOrNull()?.takeIf { it.canReuseFor(source, currentIndex, callSiteMarker) }
+            ?: mapNewInterval(source, currentIndex + 1, 1, callSiteMarker)
+        mapping.range = max(mapping.range, source - mapping.source + 1)
        return mapping.mapSourceToDest(source)
    }

+    private fun RangeMapping.canReuseFor(newSource: Int, globalMaxDest: Int, newCallSite: CallSiteMarker?): Boolean =
+        callSiteMarker == newCallSite && (newSource - source) in 0 until range + (if (maxDest == globalMaxDest) 10 else 0)
+
    fun mapNewInterval(source: Int, dest: Int, range: Int, callSiteMarker: CallSiteMarker? = null): RangeMapping =
        RangeMapping(source, dest, range, callSiteMarker, parent = this).also { lineMappings.add(it) }

@@ -23,18 +23,12 @@ object SMAPParser {
        if (mappingInfo != null && mappingInfo.isNotEmpty()) {
            return parse(mappingInfo)
        }
-
-        val mapping =
-            if (source == null || source.isEmpty() || methodStartLine > methodEndLine)
-                FileMapping.SKIP
-            else
-                FileMapping(source, path).apply {
-                    if (methodStartLine <= methodEndLine) {
-                        //one to one
-                        mapNewInterval(methodStartLine, methodStartLine, methodEndLine - methodStartLine + 1)
-                    }
-                }
-
+        if (source == null || source.isEmpty() || methodStartLine > methodEndLine) {
+            return SMAP(listOf(FileMapping.SKIP))
+        }
+        val mapping = FileMapping(source, path).apply {
+            mapNewInterval(methodStartLine, methodStartLine, methodEndLine - methodStartLine + 1)
+        }
        return SMAP(listOf(mapping))
    }

@@ -46,7 +46,7 @@ interface SourceCompilerForInline {

    val inlineCallSiteInfo: InlineCallSiteInfo

-    val lazySourceMapper: DefaultSourceMapper
+    val lazySourceMapper: SourceMapper

    fun generateLambdaBody(lambdaInfo: ExpressionLambda): SMAPAndMethodNode

@@ -212,18 +212,7 @@ class PsiSourceCompilerForInline(private val codegen: ExpressionCodegen, overrid
            codegen.propagateChildReifiedTypeParametersUsages(parentCodegen.reifiedTypeParametersUsages)
        }

-        return createSMAPWithDefaultMapping(expression, parentCodegen.orCreateSourceMapper.resultMappings)
-    }
-
-
-    private fun createSMAPWithDefaultMapping(
-        declaration: KtExpression,
-        mappings: List<FileMapping>
-    ): SMAP {
-        val containingFile = declaration.containingFile
-        CodegenUtil.getLineNumberForElement(containingFile, true) ?: error("Couldn't extract line count in $containingFile")
-
-        return SMAP(mappings)
+        return SMAP(parentCodegen.orCreateSourceMapper.resultMappings)
    }

    @Suppress("UNCHECKED_CAST")
@@ -310,7 +299,7 @@ class PsiSourceCompilerForInline(private val codegen: ExpressionCodegen, overrid
                methodContext, callableDescriptor, maxCalcAdapter, DefaultParameterValueLoader.DEFAULT,
                inliningFunction as KtNamedFunction?, parentCodegen, asmMethod
            )
-            createSMAPWithDefaultMapping(inliningFunction, parentCodegen.orCreateSourceMapper.resultMappings)
+            SMAP(parentCodegen.orCreateSourceMapper.resultMappings)
        } else {
            generateMethodBody(maxCalcAdapter, callableDescriptor, methodContext, inliningFunction!!, jvmSignature, null)
        }