From 84df9962049357f7858b070ddd070f7f6d167e37 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 15 Sep 2020 16:25:12 +0200 Subject: [PATCH] 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. --- .../backend/jvm/codegen/ExpressionCodegen.kt | 21 ++++-- .../backend/jvm/codegen/IrCallGenerator.kt | 7 +- .../backend/jvm/codegen/IrInlineCodegen.kt | 11 +-- .../lineNumbers/inlineCondition.kt | 4 +- .../debug/stepping/ifWithInlineInCondition.kt | 56 +++++++++++++++ .../stepping/whenWithInlineInCondition.kt | 71 +++++++++++++++++++ .../IrSteppingTestGenerated.java | 12 ++++ .../SteppingTestGenerated.java | 12 ++++ .../test/IrKotlinSteppingTestGenerated.java | 5 ++ .../test/KotlinSteppingTestGenerated.java | 5 ++ .../stepping/stepOver/soInlineLibFun.out | 1 - .../stepOver/soInlineLibFunInWhen.ir.out | 11 +++ .../stepping/stepOver/soInlineLibFunInWhen.kt | 17 +++++ .../stepOver/soInlineLibFunInWhen.out | 13 ++++ .../stepOver/soSimpleInlineIfCondition.kt | 2 + .../stepOver/soSimpleInlineIfCondition.out | 2 + 16 files changed, 238 insertions(+), 12 deletions(-) create mode 100644 compiler/testData/debug/stepping/ifWithInlineInCondition.kt create mode 100644 compiler/testData/debug/stepping/whenWithInlineInCondition.kt create mode 100644 idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.ir.out create mode 100644 idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.kt create mode 100644 idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.out diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt index cf3045bcd99..b1eb4e17e86 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt @@ -131,6 +131,8 @@ class ExpressionCodegen( override var lastLineNumber: Int = -1 var noLineNumberScope: Boolean = false + private var isInsideCondition = false + private val closureReifiedMarkers = hashMapOf() 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 { diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrCallGenerator.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrCallGenerator.kt index f73ffc53010..dea73c5cdb8 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrCallGenerator.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrCallGenerator.kt @@ -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) } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrInlineCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrInlineCodegen.kt index 742c471eefa..3f4a80d4ddc 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrInlineCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrInlineCodegen.kt @@ -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 { diff --git a/compiler/testData/codegen/bytecodeText/lineNumbers/inlineCondition.kt b/compiler/testData/codegen/bytecodeText/lineNumbers/inlineCondition.kt index 4abd4ae5805..4ca2079a094 100644 --- a/compiler/testData/codegen/bytecodeText/lineNumbers/inlineCondition.kt +++ b/compiler/testData/codegen/bytecodeText/lineNumbers/inlineCondition.kt @@ -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 Iterable.myAny(predicate: (T) -> Boolean): Boolean { return false } -// 3 LINENUMBER 5 \ No newline at end of file +// 3 LINENUMBER 5 diff --git a/compiler/testData/debug/stepping/ifWithInlineInCondition.kt b/compiler/testData/debug/stepping/ifWithInlineInCondition.kt new file mode 100644 index 00000000000..ecdf3f67817 --- /dev/null +++ b/compiler/testData/debug/stepping/ifWithInlineInCondition.kt @@ -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 diff --git a/compiler/testData/debug/stepping/whenWithInlineInCondition.kt b/compiler/testData/debug/stepping/whenWithInlineInCondition.kt new file mode 100644 index 00000000000..e57e77c07d9 --- /dev/null +++ b/compiler/testData/debug/stepping/whenWithInlineInCondition.kt @@ -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 diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/IrSteppingTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/IrSteppingTestGenerated.java index fe0a3eea11c..1b73f38c4ff 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/IrSteppingTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/IrSteppingTestGenerated.java @@ -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 { diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/SteppingTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/SteppingTestGenerated.java index d3290ff140d..069c8e91331 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/SteppingTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/SteppingTestGenerated.java @@ -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 { diff --git a/idea/jvm-debugger/jvm-debugger-test/test/org/jetbrains/kotlin/idea/debugger/test/IrKotlinSteppingTestGenerated.java b/idea/jvm-debugger/jvm-debugger-test/test/org/jetbrains/kotlin/idea/debugger/test/IrKotlinSteppingTestGenerated.java index 59d83d7e7e4..6bbd6b16013 100644 --- a/idea/jvm-debugger/jvm-debugger-test/test/org/jetbrains/kotlin/idea/debugger/test/IrKotlinSteppingTestGenerated.java +++ b/idea/jvm-debugger/jvm-debugger-test/test/org/jetbrains/kotlin/idea/debugger/test/IrKotlinSteppingTestGenerated.java @@ -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"); diff --git a/idea/jvm-debugger/jvm-debugger-test/test/org/jetbrains/kotlin/idea/debugger/test/KotlinSteppingTestGenerated.java b/idea/jvm-debugger/jvm-debugger-test/test/org/jetbrains/kotlin/idea/debugger/test/KotlinSteppingTestGenerated.java index 7a90fcf89d6..7c68964a9f1 100644 --- a/idea/jvm-debugger/jvm-debugger-test/test/org/jetbrains/kotlin/idea/debugger/test/KotlinSteppingTestGenerated.java +++ b/idea/jvm-debugger/jvm-debugger-test/test/org/jetbrains/kotlin/idea/debugger/test/KotlinSteppingTestGenerated.java @@ -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"); diff --git a/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFun.out b/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFun.out index 673e8060b00..4afba816a4c 100644 --- a/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFun.out +++ b/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFun.out @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR LineBreakpoint created at soInlineLibFun.kt:6 Run Java Connected to the target VM diff --git a/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.ir.out b/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.ir.out new file mode 100644 index 00000000000..dff27001f5b --- /dev/null +++ b/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.ir.out @@ -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 diff --git a/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.kt b/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.kt new file mode 100644 index 00000000000..26598789220 --- /dev/null +++ b/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.kt @@ -0,0 +1,17 @@ +// FILE: soInlineLibFunInWhen.kt +package soInlineLibFunInWhen + +fun main(args: Array) { + //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. diff --git a/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.out b/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.out new file mode 100644 index 00000000000..7ae281dcba9 --- /dev/null +++ b/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soInlineLibFunInWhen.out @@ -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 diff --git a/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soSimpleInlineIfCondition.kt b/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soSimpleInlineIfCondition.kt index 2b83b63ba34..6c2e5790c27 100644 --- a/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soSimpleInlineIfCondition.kt +++ b/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soSimpleInlineIfCondition.kt @@ -16,3 +16,5 @@ inline fun foo(f: () -> Boolean): Boolean = f() fun test(i: Int): Boolean = true fun bar() {} + +// STEP_OVER: 5 diff --git a/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soSimpleInlineIfCondition.out b/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soSimpleInlineIfCondition.out index 2931962a946..8b5fa9761ef 100644 --- a/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soSimpleInlineIfCondition.out +++ b/idea/jvm-debugger/jvm-debugger-test/testData/stepping/stepOver/soSimpleInlineIfCondition.out @@ -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