[JVM_IR] Fix stepping behavior for assignments to local variables.

compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/CapturedVarsOptimizationMethodTransformer.kt

@@ -242,6 +242,17 @@ class CapturedVarsOptimizationMethodTransformer : MethodTransformer() {
             methodNode.removeUnusedLocalVariables()
         }
 
+        // Be careful to not remove instructions that are the only instruction for a line number. That will
+        // break debugging. If the previous instruction is a line number and the following instruction is
+        // a label followed by a line number, insert a nop instead of deleting the instruction.
+        private fun InsnList.removeOrReplaceByNop(insn: AbstractInsnNode) {
+            if (insn.previous is LineNumberNode && insn.next is LabelNode && insn.next.next is LineNumberNode) {
+                set(insn, InsnNode(Opcodes.NOP))
+            } else {
+                remove(insn)
+            }
+        }
+
         private fun rewriteRefValue(capturedVar: CapturedVarDescriptor) {
             methodNode.instructions.run {
                 val localVar = capturedVar.localVar!!
@@ -259,9 +270,10 @@ class CapturedVarsOptimizationMethodTransformer : MethodTransformer() {
 
                 remove(capturedVar.newInsn)
                 remove(capturedVar.initCallInsn!!)
-                capturedVar.stackInsns.forEach { remove(it) }
-                capturedVar.aloadInsns.forEach { remove(it) }
-                capturedVar.astoreInsns.forEach { remove(it) }
+
+                capturedVar.stackInsns.forEach { removeOrReplaceByNop(it) }
+                capturedVar.aloadInsns.forEach { removeOrReplaceByNop(it) }
+                capturedVar.astoreInsns.forEach { removeOrReplaceByNop(it) }
                 capturedVar.getFieldInsns.forEach { set(it, VarInsnNode(loadOpcode, capturedVar.localVarIndex)) }
                 capturedVar.putFieldInsns.forEach { set(it, VarInsnNode(storeOpcode, capturedVar.localVarIndex)) }
 
@@ -273,6 +285,7 @@ class CapturedVarsOptimizationMethodTransformer : MethodTransformer() {
                 }
             }
         }
+
     }
 }
 

compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt

@@ -406,6 +406,8 @@ class ExpressionCodegen(
                 mv.dup()
             }
             expression is IrDelegatingConstructorCall -> {
+                expression.markLineNumber(startOffset = true)
+
                 // In this case the receiver is `this` (not specified in IR) and the return value is discarded anyway.
                 mv.load(0, OBJECT_TYPE)
 
@@ -502,13 +504,12 @@ class ExpressionCodegen(
         val varType = typeMapper.mapType(declaration)
         val index = frameMap.enter(declaration.symbol, varType)
 
-        declaration.markLineNumber(startOffset = true)
-
         val initializer = declaration.initializer
         if (initializer != null) {
             var value = initializer.accept(this, data)
             initializer.markLineNumber(startOffset = true)
             value.materializeAt(varType, declaration.type)
+            declaration.markLineNumber(startOffset = true)
             mv.store(index, varType)
         } else if (declaration.isVisibleInLVT) {
             pushDefaultValueOnStack(varType, mv)
@@ -569,6 +570,7 @@ class ExpressionCodegen(
                 value.materializeAt(fieldType, callee.type)
             }
 
+            expression.markLineNumber(startOffset = true)
             when {
                 isStatic -> mv.putstatic(ownerName, fieldName, fieldType.descriptor)
                 else -> mv.putfield(ownerName, fieldName, fieldType.descriptor)

compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/descriptors/SharedVariablesManager.kt

@@ -10,7 +10,6 @@ import org.jetbrains.kotlin.backend.common.ir.createImplicitParameterDeclaration
 import org.jetbrains.kotlin.backend.jvm.JvmSymbols
 import org.jetbrains.kotlin.backend.jvm.lower.inlineclasses.InlineClassAbi
 import org.jetbrains.kotlin.descriptors.ModuleDescriptor
-import org.jetbrains.kotlin.descriptors.impl.EmptyPackageFragmentDescriptor
 import org.jetbrains.kotlin.ir.IrStatement
 import org.jetbrains.kotlin.ir.builders.declarations.*
 import org.jetbrains.kotlin.ir.declarations.*
@@ -21,7 +20,6 @@ import org.jetbrains.kotlin.ir.descriptors.WrappedVariableDescriptor
 import org.jetbrains.kotlin.ir.expressions.*
 import org.jetbrains.kotlin.ir.expressions.impl.*
 import org.jetbrains.kotlin.ir.symbols.IrVariableSymbol
-import org.jetbrains.kotlin.ir.symbols.impl.IrExternalPackageFragmentSymbolImpl
 import org.jetbrains.kotlin.ir.symbols.impl.IrVariableSymbolImpl
 import org.jetbrains.kotlin.ir.types.*
 import org.jetbrains.kotlin.name.FqName
@@ -87,7 +85,9 @@ class JvmSharedVariablesManager(
         val provider = getProvider(InlineClassAbi.unboxType(valueType) ?: valueType)
         val typeArguments = provider.refClass.typeParameters.map { valueType }
         val refType = provider.refClass.typeWith(typeArguments)
-        val refConstructorCall = IrConstructorCallImpl.fromSymbolOwner(refType, provider.refConstructor.symbol).apply {
+        val refConstructorCall = IrConstructorCallImpl.fromSymbolOwner(
+            originalDeclaration.startOffset, originalDeclaration.startOffset, refType, provider.refConstructor.symbol
+        ).apply {
             typeArguments.forEachIndexed(::putTypeArgument)
         }
         return with(originalDeclaration) {
@@ -110,8 +110,8 @@ class JvmSharedVariablesManager(
             // The field is preinitialized to the default value, so an explicit set is not required.
             return sharedVariableDeclaration
         }
-        val initializationStatement = with(initializer) {
-            IrSetVariableImpl(startOffset, endOffset, irBuiltIns.unitType, originalDeclaration.symbol, this, null)
+        val initializationStatement = with (originalDeclaration) {
+            IrSetVariableImpl(startOffset, endOffset, irBuiltIns.unitType, symbol, initializer, null)
         }
         val sharedVariableInitialization = setSharedValue(sharedVariableDeclaration.symbol, initializationStatement)
         return with(originalDeclaration) {

compiler/testData/debug/stepping/assertion.kt

@@ -1,5 +1,5 @@
-// IGNORE_BACKEND: JVM_IR
 // FILE: test.kt
+
 public val MASSERTIONS_ENABLED: Boolean = true
 
 public inline fun massert(value: Boolean, lazyMessage: () -> String) {
@@ -29,6 +29,12 @@ fun box(): String {
     return "OK"
 }
 
+// JVM_IR steps on line 15 both on the way in and on the way out
+// of the massert method. This is consistent with what would
+// happen if massert was not inline and we used force-step-into
+// to step through the $default method. JVM only hits line 15 on
+// the way in.
+
 // LINENUMBERS
 // test.kt:24 box
 // test.kt:15 box
@@ -37,6 +43,9 @@ fun box(): String {
 // test.kt:16 box
 // test.kt:17 box
 // test.kt:21 box
+// LINENUMBERS JVM_IR
+// test.kt:15 box
+// LINENUMBERS
 // test.kt:25 box
 // test.kt:6 box
 // test.kt:3 getMASSERTIONS_ENABLED

compiler/testData/debug/stepping/compileTimeConstant.kt

@@ -5,9 +5,6 @@ fun box() {
             42
 }
 
-// IGNORE_BACKEND: JVM_IR
-// The JVM_IR backend does not hit line 4. That should be fixed.
-
 // LINENUMBERS
 // test.kt:5 box
 // test.kt:4 box

compiler/testData/debug/stepping/functionCallWithDefault.kt

@@ -11,11 +11,10 @@ fun foo(i: Int = 1) {
 inline fun bar(i: Int = 1) {
 }
 
-// TODO: The JVM_IR backend has line number 11 for the inlined
+// The JVM_IR backend has line number 11 for the inlined
 // default argument handling both before and after the actual
 // body of bar. That is consistent with what happens with the
-// $default method in the non-inlined case, but it is not what
-// happens with the JVM backend.
+// $default method in the non-inlined case.
 
 // FORCE_STEP_INTO
 // LINENUMBERS

compiler/testData/debug/stepping/inlineCallableReference.kt

@@ -4,6 +4,11 @@ fun box() {
     f {
         x = true
     }
+    var y =
+        true
+    f {
+        y = false
+    }
 }
 
 inline fun f(block: () -> Unit) {
@@ -13,8 +18,16 @@ inline fun f(block: () -> Unit) {
 // LINENUMBERS
 // test.kt:3 box
 // test.kt:4 box
-// test.kt:10 box
+// test.kt:15 box
 // test.kt:5 box
 // test.kt:6 box
-// test.kt:11 box
+// test.kt:16 box
 // test.kt:7 box
+// test.kt:8 box
+// test.kt:7 box
+// test.kt:9 box
+// test.kt:15 box
+// test.kt:10 box
+// test.kt:11 box
+// test.kt:16 box
+// test.kt:12 box

compiler/testData/debug/stepping/simpleDefaultArgWithInline.kt

@@ -1,15 +1,32 @@
-// IGNORE_BACKEND: JVM_IR
 // FILE: test.kt
+
 inline fun ifoo(ok: String = "OK"): String {
     return ok
 }
 
-fun box(): String {
-    return ifoo()
+fun ifoo2(ok: String = "OK"): String {
+    return ok
 }
-// TODO: IR Backend behaves like simpleDefaultArg: 8,3,4, _3_ ,8
+
+fun box(): String {
+    ifoo()
+    return ifoo2()
+}
+
+// JVM_IR backend has the same stepping behavior as
+// the non-inlined case when using force step into to step
+// through $default method. JVM does not.
+
+// FORCE_STEP_INTO
 // LINENUMBERS
-// test.kt:8 box
+// test.kt:12 box
 // test.kt:3 box
 // test.kt:4 box
-// test.kt:8 box
+// LINENUMBERS JVM_IR
+// test.kt:3 box
+// LINENUMBERS
+// test.kt:13 box
+// test.kt:7 ifoo2$default (synthetic)
+// test.kt:8 ifoo2
+// test.kt:7 ifoo2$default (synthetic)
+// test.kt:13 box

compiler/testData/debug/stepping/variablesWithoutInitializer.kt

@@ -0,0 +1,29 @@
+// FILE: test.kt
+
+fun box() {
+    var x: String
+    var y: Int
+    var z: Boolean
+    z = false
+    y = 42
+    if (!z) {
+        x = y.toString()
+    }
+}
+
+// The JVM IR backend does not generate line number information for the
+// declaration of local variables without initializer. Stepping through
+// those does not seem useful. This is consistent with javac behavior
+// as well. The JVM backend does generate these line numbers.
+
+// LINENUMBERS
+// LINENUMBERS JVM
+// test.kt:4 box
+// test.kt:5 box
+// test.kt:6 box
+// LINENUMBERS
+// test.kt:7 box
+// test.kt:8 box
+// test.kt:9 box
+// test.kt:10 box
+// test.kt:12 box

compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/IrSteppingTestGenerated.java

@@ -391,6 +391,12 @@ public class IrSteppingTestGenerated extends AbstractIrSteppingTest {
         runTest("compiler/testData/debug/stepping/tryFinally.kt");
     }
 
+    @Test
+    @TestMetadata("variablesWithoutInitializer.kt")
+    public void testVariablesWithoutInitializer() throws Exception {
+        runTest("compiler/testData/debug/stepping/variablesWithoutInitializer.kt");
+    }
+
     @Test
     @TestMetadata("voidLambdaStepInline.kt")
     public void testVoidLambdaStepInline() throws Exception {

compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/SteppingTestGenerated.java

@@ -391,6 +391,12 @@ public class SteppingTestGenerated extends AbstractSteppingTest {
         runTest("compiler/testData/debug/stepping/tryFinally.kt");
     }
 
+    @Test
+    @TestMetadata("variablesWithoutInitializer.kt")
+    public void testVariablesWithoutInitializer() throws Exception {
+        runTest("compiler/testData/debug/stepping/variablesWithoutInitializer.kt");
+    }
+
     @Test
     @TestMetadata("voidLambdaStepInline.kt")
     public void testVoidLambdaStepInline() throws Exception {