JVM IR: fix "step over" for inline function calls in conditions

Reimplement the same hacky approach used in the old backend (see cc2fe6b0c6). Previously, the debugger incorrectly stepped into Collections.kt on "step over" inline function calls from stdlib like 'any'. Since `if` and `when` expressions are represented the same way in IR, the behavior is fixed for both of them. It's not the case in the old JVM backend, where stepping over `when` conditions still suffers from the same problem, which the newly added test checks.
2020-09-15 16:25:12 +02:00
parent 4144adf4ee
commit 84df996204
16 changed files with 238 additions and 12 deletions
@@ -131,6 +131,8 @@ class ExpressionCodegen(
    override var lastLineNumber: Int = -1
    var noLineNumberScope: Boolean = false

+    private var isInsideCondition = false
+
    private val closureReifiedMarkers = hashMapOf<IrClass, ReifiedTypeParametersUsages>()

    private val IrType.asmType: Type
@@ -429,7 +431,7 @@ class ExpressionCodegen(
        val generatorForActualCall =
            // Do not inline callee to continuation, instead, call it
            if (irFunction.isInvokeSuspendOfContinuation()) IrCallGenerator.DefaultCallGenerator else callGenerator
-        generatorForActualCall.genCall(callable, this, expression)
+        generatorForActualCall.genCall(callable, this, expression, isInsideCondition)

        if (isSuspensionPoint != SuspensionPointKind.NEVER) {
            addSuspendMarker(mv, isStartNotEnd = false, isSuspensionPoint == SuspensionPointKind.NOT_INLINE)
@@ -829,7 +831,10 @@ class ExpressionCodegen(
                if (branch.condition.isFalseConst())
                    continue // The branch body is dead code.
            } else {
+                val oldIsInsideCondition = isInsideCondition
+                isInsideCondition = true
                branch.condition.accept(this, data).coerceToBoolean().jumpIfFalse(elseLabel)
+                isInsideCondition = oldIsInsideCondition
            }
            val result = branch.result.accept(this, data)
            if (!exhaustive) {
@@ -1250,9 +1255,17 @@ class ExpressionCodegen(
    }

    override fun markLineNumberAfterInlineIfNeeded(registerLineNumberAfterwards: Boolean) {
-        // Inline function has its own line number which is in a separate instance of codegen,
-        // therefore we need to reset lastLineNumber to force a line number generation after visiting inline function.
-        lastLineNumber = -1
+        if (noLineNumberScope || registerLineNumberAfterwards) {
+            if (lastLineNumber > -1) {
+                val label = Label()
+                v.visitLabel(label)
+                v.visitLineNumber(lastLineNumber, label)
+            }
+        } else {
+            // Inline function has its own line number which is in a separate instance of codegen,
+            // therefore we need to reset lastLineNumber to force a line number generation after visiting inline function.
+            lastLineNumber = -1
+        }
    }

    fun isFinallyMarkerRequired(): Boolean {
@@ -22,7 +22,12 @@ import org.jetbrains.kotlin.ir.expressions.IrFunctionAccessExpression
 import org.jetbrains.org.objectweb.asm.Type

 interface IrCallGenerator {
-    fun genCall(callableMethod: IrCallableMethod, codegen: ExpressionCodegen, expression: IrFunctionAccessExpression) {
+    fun genCall(
+        callableMethod: IrCallableMethod,
+        codegen: ExpressionCodegen,
+        expression: IrFunctionAccessExpression,
+        isInsideIfCondition: Boolean,
+    ) {
        with(callableMethod) {
            codegen.mv.visitMethodInsn(invokeOpcode, owner.internalName, asmMethod.name, asmMethod.descriptor, isInterfaceMethod)
        }
@@ -5,10 +5,10 @@

 package org.jetbrains.kotlin.backend.jvm.codegen

-import org.jetbrains.kotlin.codegen.*
 import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
 import org.jetbrains.kotlin.backend.jvm.ir.isInlineParameter
 import org.jetbrains.kotlin.backend.jvm.lower.suspendFunctionOriginal
+import org.jetbrains.kotlin.codegen.*
 import org.jetbrains.kotlin.codegen.inline.*
 import org.jetbrains.kotlin.codegen.state.GenerationState
 import org.jetbrains.kotlin.descriptors.FunctionDescriptor
@@ -17,7 +17,9 @@ import org.jetbrains.kotlin.ir.declarations.*
 import org.jetbrains.kotlin.ir.descriptors.toIrBasedDescriptor
 import org.jetbrains.kotlin.ir.descriptors.toIrBasedKotlinType
 import org.jetbrains.kotlin.ir.expressions.*
-import org.jetbrains.kotlin.ir.types.*
+import org.jetbrains.kotlin.ir.types.IrType
+import org.jetbrains.kotlin.ir.types.classOrNull
+import org.jetbrains.kotlin.ir.types.classifierOrFail
 import org.jetbrains.kotlin.ir.util.*
 import org.jetbrains.kotlin.resolve.jvm.AsmTypes
 import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodSignature
@@ -143,7 +145,8 @@ class IrInlineCodegen(
    override fun genCall(
        callableMethod: IrCallableMethod,
        codegen: ExpressionCodegen,
-        expression: IrFunctionAccessExpression
+        expression: IrFunctionAccessExpression,
+        isInsideIfCondition: Boolean,
    ) {
        val element = codegen.context.psiSourceManager.findPsiElement(expression, codegen.irFunction)
            ?: codegen.context.psiSourceManager.findPsiElement(codegen.irFunction)
@@ -161,7 +164,7 @@ class IrInlineCodegen(
                true,
                false,
                codegen.typeMapper.typeSystem,
-                registerLineNumberAfterwards = false,
+                registerLineNumberAfterwards = isInsideIfCondition,
                isCallOfFunctionInCorrespondingDefaultDispatch = codegen.irFunction == codegen.context.mapping.defaultArgumentsDispatchFunction[function]
            )
        } finally {
@@ -1,5 +1,5 @@
 //FILE: test.kt
-// IGNORE_BACKEND: JVM_IR
+

 fun box() {
    if (listOf(1, 2, 3).myAny { it > 2 }) {
@@ -14,4 +14,4 @@ public inline fun <T> Iterable<T>.myAny(predicate: (T) -> Boolean): Boolean {
    return false
 }

-// 3 LINENUMBER 5
+// 3 LINENUMBER 5
@@ -0,0 +1,56 @@
+// FILE: test.kt
+fun box() {
+    if (inlineFun()) {
+        nop()
+    }
+
+    if (
+        inlineFun().rid() &&
+        inlineFun()
+    ) {
+        nop()
+    }
+
+    if (
+        id(
+            inlineFun()
+        )
+    ) {
+        nop()
+    }
+}
+
+inline fun inlineFun(): Boolean {
+    return true
+}
+
+inline fun id(x: Boolean): Boolean = x
+inline fun Boolean.rid(): Boolean = this
+
+fun nop() {}
+
+// LINENUMBERS
+// test.kt:3 box
+// test.kt:24 box
+// test.kt:3 box
+// test.kt:4 box
+// test.kt:30 nop
+// test.kt:8 box
+// test.kt:24 box
+// test.kt:8 box
+// test.kt:28 box
+// test.kt:8 box
+// test.kt:9 box
+// test.kt:24 box
+// test.kt:9 box
+// test.kt:11 box
+// test.kt:30 nop
+// test.kt:16 box
+// test.kt:24 box
+// test.kt:16 box
+// test.kt:15 box
+// test.kt:27 box
+// test.kt:15 box
+// test.kt:19 box
+// test.kt:30 nop
+// test.kt:21 box
@@ -0,0 +1,71 @@
+// FILE: test.kt
+fun box() {
+    val x = value()
+    when (x) {
+        x0() -> nop()
+        x1() -> nop()
+        x2() -> nop()
+        x3() -> nop()
+        else -> nop()
+    }
+
+    when (x0() + x1()) {
+        x0().rid() -> nop()
+        id(x1()) -> nop()
+        else -> nop()
+    }
+}
+
+fun value(): Int = 2
+inline fun x0(): Int = 0
+inline fun x1(): Int = 1
+inline fun x2(): Int = 2
+inline fun x3(): Int = 3
+
+inline fun id(x: Int): Int = x
+inline fun Int.rid(): Int = this
+
+fun nop() {}
+
+// JVM_IR generates an additional line number for the end of the condition, which is necessary for the correct "step over" behavior.
+
+// LINENUMBERS
+// test.kt:3 box
+// test.kt:19 value
+// test.kt:3 box
+// test.kt:4 box
+// test.kt:5 box
+// test.kt:20 box
+// LINENUMBERS JVM_IR
+// test.kt:5 box
+// LINENUMBERS
+// test.kt:6 box
+// test.kt:21 box
+// LINENUMBERS JVM_IR
+// test.kt:6 box
+// LINENUMBERS
+// test.kt:7 box
+// test.kt:22 box
+// test.kt:7 box
+// test.kt:28 nop
+// test.kt:7 box
+// test.kt:12 box
+// test.kt:20 box
+// test.kt:12 box
+// test.kt:21 box
+// test.kt:12 box
+// test.kt:13 box
+// test.kt:20 box
+// test.kt:13 box
+// test.kt:26 box
+// LINENUMBERS JVM_IR
+// test.kt:13 box
+// LINENUMBERS
+// test.kt:14 box
+// test.kt:21 box
+// test.kt:14 box
+// test.kt:25 box
+// test.kt:14 box
+// test.kt:28 nop
+// test.kt:14 box
+// test.kt:17 box
@@ -187,6 +187,12 @@ public class IrSteppingTestGenerated extends AbstractIrSteppingTest {
        runTest("compiler/testData/debug/stepping/IfTrueThenFalse.kt");
    }

+    @Test
+    @TestMetadata("ifWithInlineInCondition.kt")
+    public void testIfWithInlineInCondition() throws Exception {
+        runTest("compiler/testData/debug/stepping/ifWithInlineInCondition.kt");
+    }
+
    @Test
    @TestMetadata("iincStepping.kt")
    public void testIincStepping() throws Exception {
@@ -475,6 +481,12 @@ public class IrSteppingTestGenerated extends AbstractIrSteppingTest {
        runTest("compiler/testData/debug/stepping/whenSubject2.kt");
    }

+    @Test
+    @TestMetadata("whenWithInlineInCondition.kt")
+    public void testWhenWithInlineInCondition() throws Exception {
+        runTest("compiler/testData/debug/stepping/whenWithInlineInCondition.kt");
+    }
+
    @Test
    @TestMetadata("while.kt")
    public void testWhile() throws Exception {
@@ -187,6 +187,12 @@ public class SteppingTestGenerated extends AbstractSteppingTest {
        runTest("compiler/testData/debug/stepping/IfTrueThenFalse.kt");
    }

+    @Test
+    @TestMetadata("ifWithInlineInCondition.kt")
+    public void testIfWithInlineInCondition() throws Exception {
+        runTest("compiler/testData/debug/stepping/ifWithInlineInCondition.kt");
+    }
+
    @Test
    @TestMetadata("iincStepping.kt")
    public void testIincStepping() throws Exception {
@@ -475,6 +481,12 @@ public class SteppingTestGenerated extends AbstractSteppingTest {
        runTest("compiler/testData/debug/stepping/whenSubject2.kt");
    }

+    @Test
+    @TestMetadata("whenWithInlineInCondition.kt")
+    public void testWhenWithInlineInCondition() throws Exception {
+        runTest("compiler/testData/debug/stepping/whenWithInlineInCondition.kt");
+    }
+
    @Test
    @TestMetadata("while.kt")
    public void testWhile() throws Exception {
@@ -607,6 +607,11 @@ public class IrKotlinSteppingTestGenerated extends AbstractIrKotlinSteppingTest
            runTest("idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFun.kt");
        }

+        @TestMetadata("soInlineLibFunInWhen.kt")
+        public void testSoInlineLibFunInWhen() throws Exception {
+            runTest("idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.kt");
+        }
+
        @TestMetadata("soInlineOperatorIterator.kt")
        public void testSoInlineOperatorIterator() throws Exception {
            runTest("idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineOperatorIterator.kt");
@@ -607,6 +607,11 @@ public class KotlinSteppingTestGenerated extends AbstractKotlinSteppingTest {
            runTest("idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFun.kt");
        }

+        @TestMetadata("soInlineLibFunInWhen.kt")
+        public void testSoInlineLibFunInWhen() throws Exception {
+            runTest("idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.kt");
+        }
+
        @TestMetadata("soInlineOperatorIterator.kt")
        public void testSoInlineOperatorIterator() throws Exception {
            runTest("idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineOperatorIterator.kt");
@@ -1,4 +1,3 @@
-// IGNORE_BACKEND: JVM_IR
 LineBreakpoint created at soInlineLibFun.kt:6
 Run Java
 Connected to the target VM
@@ -0,0 +1,11 @@
+LineBreakpoint created at soInlineLibFunInWhen.kt:6
+Run Java
+Connected to the target VM
+soInlineLibFunInWhen.kt:6
+soInlineLibFunInWhen.kt:8
+soInlineLibFunInWhen.kt:9
+soInlineLibFunInWhen.kt:10
+soInlineLibFunInWhen.kt:12
+Disconnected from the target VM
+
+Process finished with exit code 0
@@ -0,0 +1,17 @@
+// FILE: soInlineLibFunInWhen.kt
+package soInlineLibFunInWhen
+
+fun main(args: Array<String>) {
+    //Breakpoint!
+    val l = listOf(1, 2, 3)
+
+    when {
+        l.any { it > 4 } -> nop()
+        l.any { it > 2 } -> nop()
+    }
+}
+
+fun nop() {}
+
+// STEP_OVER: 7
+// Note: on the old JVM backend, debugger incorrectly steps into Collections.kt even though we're stepping over it. This is fixed in JVM_IR.
@@ -0,0 +1,13 @@
+LineBreakpoint created at soInlineLibFunInWhen.kt:6
+Run Java
+Connected to the target VM
+soInlineLibFunInWhen.kt:6
+soInlineLibFunInWhen.kt:8
+soInlineLibFunInWhen.kt:9
+soInlineLibFunInWhen.kt:10
+_Collections.!EXT!
+soInlineLibFunInWhen.kt:10
+soInlineLibFunInWhen.kt:12
+Disconnected from the target VM
+
+Process finished with exit code 0
@@ -16,3 +16,5 @@ inline fun foo(f: () -> Boolean): Boolean = f()
 fun test(i: Int): Boolean = true

 fun bar() {}
+
+// STEP_OVER: 5
@@ -3,6 +3,8 @@ Run Java
 Connected to the target VM
 soSimpleInlineIfCondition.kt:5
 soSimpleInlineIfCondition.kt:8
+soSimpleInlineIfCondition.kt:11
+soSimpleInlineIfCondition.kt:12
 Disconnected from the target VM

 Process finished with exit code 0