Generate correct linenumber for the debugger to see the return value

of suspend function. #KT-20322 Fixed
2018-09-20 19:53:40 +03:00
parent 9bf55d81ca
commit 1777849ff3
14 changed files with 277 additions and 32 deletions
@@ -13,7 +13,6 @@ import org.jetbrains.kotlin.codegen.StackValue
 import org.jetbrains.kotlin.codegen.TransformationMethodVisitor
 import org.jetbrains.kotlin.codegen.inline.*
 import org.jetbrains.kotlin.codegen.optimization.DeadCodeEliminationMethodTransformer
-import org.jetbrains.kotlin.codegen.optimization.boxing.isPrimitiveUnboxing
 import org.jetbrains.kotlin.codegen.optimization.common.*
 import org.jetbrains.kotlin.codegen.optimization.fixStack.FixStackMethodTransformer
 import org.jetbrains.kotlin.codegen.optimization.fixStack.top
@@ -25,8 +24,6 @@ import org.jetbrains.kotlin.psi.KtElement
 import org.jetbrains.kotlin.resolve.jvm.AsmTypes
 import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm
 import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOrigin
-import org.jetbrains.kotlin.utils.addToStdlib.cast
-import org.jetbrains.kotlin.utils.addToStdlib.safeAs
 import org.jetbrains.kotlin.utils.sure
 import org.jetbrains.org.objectweb.asm.Label
 import org.jetbrains.org.objectweb.asm.MethodVisitor
@@ -670,7 +667,7 @@ class CoroutineTransformerMethodVisitor(
        val continuationLabel = LabelNode()
        val continuationLabelAfterLoadedResult = LabelNode()
        val suspendElementLineNumber = lineNumber
-        val nextLineNumberNode = suspension.suspensionCallEnd.findNextOrNull { it is LineNumberNode } as? LineNumberNode
+        var nextLineNumberNode = suspension.suspensionCallEnd.findNextOrNull { it is LineNumberNode } as? LineNumberNode
        with(methodNode.instructions) {
            // Save state
            insertBefore(
@@ -720,34 +717,18 @@ class CoroutineTransformerMethodVisitor(

                // Extend next instruction linenumber. Can't use line number of suspension point here because both non-suspended execution
                // and re-entering after suspension passes this label.
-
-                // However, for primitives we generate it separately
-                if (possibleTryCatchBlockStart.next?.isUnboxingSequence() != true) {
+                if (possibleTryCatchBlockStart.next?.opcode?.let {
+                        it != Opcodes.ASTORE && it != Opcodes.CHECKCAST && it != Opcodes.INVOKESTATIC &&
+                                it != Opcodes.INVOKEVIRTUAL && it != Opcodes.INVOKEINTERFACE
+                    } == true
+                ) {
                    visitLineNumber(afterSuspensionPointLineNumber, continuationLabelAfterLoadedResult.label)
+                } else {
+                    // But keep the linenumber if the result of the call is is used afterwards
+                    nextLineNumberNode = null
                }
            })

-            // In code like val a = suspendReturnsInt()
-            // `a` is coerced from Object to int, and coercion happens before scopeStart's mark:
-            //  LL
-            //   CHECKCAST java/lang/Number
-            //   INVOKEVIRTUAL java/lang/Number.intValue ()I
-            //   ISTORE N
-            //  LM
-            //   /* put lineNumber here */
-            //   ...
-            //  LOCALVARIABLE name LM LK N
-            if (continuationLabelAfterLoadedResult.label.info.safeAs<AbstractInsnNode>()?.next?.isUnboxingSequence() == true) {
-                // Find next label after unboxing and put linenumber there
-                var current = (continuationLabelAfterLoadedResult.label.info as AbstractInsnNode).next
-                while (current != null && current !is LabelNode) {
-                    current = current.next
-                }
-                if (current != null) {
-                    insert(current, LineNumberNode(afterSuspensionPointLineNumber, current.cast()))
-                }
-            }
-
            if (nextLineNumberNode != null) {
                // Remove the line number instruction as it now covered with line number on continuation label.
                // If both linenumber are present in bytecode, debugger will trigger line specific events twice.
@@ -758,10 +739,6 @@ class CoroutineTransformerMethodVisitor(
        return continuationLabel
    }

-    private fun AbstractInsnNode.isUnboxingSequence(): Boolean {
-        return opcode == Opcodes.CHECKCAST && next?.isPrimitiveUnboxing() == true
-    }
-
    // It's necessary to preserve some sensible invariants like there should be no jump in the middle of try-catch-block
    // Also it's important that spilled variables are being restored outside of TCB,
    // otherwise they would be treated as uninitialized within catch-block while they can be used there
@@ -0,0 +1,32 @@
+package anyUpdateInvokeStatic
+
+import kotlin.sequences.*
+import kotlin.coroutines.*
+
+fun builder(c: suspend () -> Unit) {
+    c.startCoroutine(object : Continuation<Unit>{
+        override val context: CoroutineContext
+            get() = EmptyCoroutineContext
+
+        override fun resumeWith(result: Result<Unit>) {
+            result.getOrThrow()
+        }
+    })
+}
+
+object A {
+    @JvmStatic var s: Any? = "aabb"
+}
+
+fun main(args: Array<String>) {
+    builder {
+        A.s = strChanger(A.s)
+        //Breakpoint!
+        println(A.s) // (1)
+    }
+}
+
+suspend fun strChanger(str: Any?): Any? = (str as String).filter { it !in "a" }
+
+// EXPRESSION: A.s
+// RESULT: "bb": Ljava/lang/String;
@@ -0,0 +1,9 @@
+LineBreakpoint created at anyUpdateInvokeStatic.kt:25
+Run Java
+Connected to the target VM
+anyUpdateInvokeStatic.kt:25
+Compile bytecode for A.s
+Disconnected from the target VM
+
+Process finished with exit code 0
+bb
@@ -0,0 +1,29 @@
+package anyUpdateVariable
+
+import kotlin.sequences.*
+import kotlin.coroutines.*
+
+fun builder(c: suspend () -> Unit) {
+    c.startCoroutine(object : Continuation<Unit>{
+        override val context: CoroutineContext
+            get() = EmptyCoroutineContext
+
+        override fun resumeWith(result: Result<Unit>) {
+            result.getOrThrow()
+        }
+    })
+}
+
+fun main(args: Array<String>) {
+    builder {
+        var s:Any? = "aabb"
+        s = strChanger(s)
+        //Breakpoint!
+        println(s) // (1)
+    }
+}
+
+suspend fun strChanger(str: Any?): Any? = (str as String).filter { it !in "a" }
+
+// EXPRESSION: s
+// RESULT: "bb": Ljava/lang/String;
@@ -0,0 +1,9 @@
+LineBreakpoint created at anyUpdateVariable.kt:22
+Run Java
+Connected to the target VM
+anyUpdateVariable.kt:22
+Compile bytecode for s
+Disconnected from the target VM
+
+Process finished with exit code 0
+bb
@@ -0,0 +1,32 @@
+package stringUpdateInvokeStatic
+
+import kotlin.sequences.*
+import kotlin.coroutines.*
+
+fun builder(c: suspend () -> Unit) {
+    c.startCoroutine(object : Continuation<Unit>{
+        override val context: CoroutineContext
+            get() = EmptyCoroutineContext
+
+        override fun resumeWith(result: Result<Unit>) {
+            result.getOrThrow()
+        }
+    })
+}
+
+object A {
+    @JvmStatic var s: String = "aabb"
+}
+
+fun main(args: Array<String>) {
+    builder {
+        A.s = strChanger(A.s)
+        //Breakpoint!
+        println(A.s) // (1)
+    }
+}
+
+suspend fun strChanger(str: String): String = str.filter { it !in "a" }
+
+// EXPRESSION: A.s
+// RESULT: "bb": Ljava/lang/String;
@@ -0,0 +1,9 @@
+LineBreakpoint created at stringUpdateInvokeStatic.kt:25
+Run Java
+Connected to the target VM
+stringUpdateInvokeStatic.kt:25
+Compile bytecode for A.s
+Disconnected from the target VM
+
+Process finished with exit code 0
+bb
@@ -0,0 +1,31 @@
+package stringUpdateInvokeVirtual
+
+import kotlin.sequences.*
+import kotlin.coroutines.*
+
+fun builder(c: suspend () -> Unit) {
+    c.startCoroutine(object : Continuation<Unit>{
+        override val context: CoroutineContext
+            get() = EmptyCoroutineContext
+
+        override fun resumeWith(result: Result<Unit>) {
+            result.getOrThrow()
+        }
+    })
+}
+
+class A(var s: String)
+
+fun main(args: Array<String>) {
+    builder {
+        var a = A("aabb")
+        a.s = strChanger(a.s)
+        //Breakpoint!
+        println(a.s) // (1)
+    }
+}
+
+suspend fun strChanger(str: String): String = str.filter { it !in "a" }
+
+// EXPRESSION: a.s
+// RESULT: "bb": Ljava/lang/String;
@@ -0,0 +1,9 @@
+LineBreakpoint created at stringUpdateInvokeVirtual.kt:24
+Run Java
+Connected to the target VM
+stringUpdateInvokeVirtual.kt:24
+Compile bytecode for a.s
+Disconnected from the target VM
+
+Process finished with exit code 0
+bb
@@ -0,0 +1,31 @@
+package stringUpdatePutField
+
+import kotlin.sequences.*
+import kotlin.coroutines.*
+
+fun builder(c: suspend () -> Unit) {
+    c.startCoroutine(object : Continuation<Unit>{
+        override val context: CoroutineContext
+            get() = EmptyCoroutineContext
+
+        override fun resumeWith(result: Result<Unit>) {
+            result.getOrThrow()
+        }
+    })
+}
+
+class A(@JvmField var s: String)
+
+fun main(args: Array<String>) {
+    builder {
+        var a = A("aabb")
+        a.s = strChanger(a.s)
+        //Breakpoint!
+        println(a.s) // (1)
+    }
+}
+
+suspend fun strChanger(str: String): String = str.filter { it !in "a" }
+
+// EXPRESSION: a.s
+// RESULT: "bb": Ljava/lang/String;
@@ -0,0 +1,9 @@
+LineBreakpoint created at stringUpdatePutField.kt:24
+Run Java
+Connected to the target VM
+stringUpdatePutField.kt:24
+Compile bytecode for a.s
+Disconnected from the target VM
+
+Process finished with exit code 0
+bb
@@ -0,0 +1,29 @@
+package stringUpdateVariable
+
+import kotlin.sequences.*
+import kotlin.coroutines.*
+
+fun builder(c: suspend () -> Unit) {
+    c.startCoroutine(object : Continuation<Unit>{
+        override val context: CoroutineContext
+            get() = EmptyCoroutineContext
+
+        override fun resumeWith(result: Result<Unit>) {
+            result.getOrThrow()
+        }
+    })
+}
+
+fun main(args: Array<String>) {
+    builder {
+        var s = "aabb"
+        s = strChanger(s)
+        //Breakpoint!
+        println(s) // (1)
+    }
+}
+
+suspend fun strChanger(str: String): String = str.filter { it !in "a" }
+
+// EXPRESSION: s
+// RESULT: "bb": Ljava/lang/String;
@@ -0,0 +1,9 @@
+LineBreakpoint created at stringUpdateVariable.kt:22
+Run Java
+Connected to the target VM
+stringUpdateVariable.kt:22
+Compile bytecode for s
+Disconnected from the target VM
+
+Process finished with exit code 0
+bb
@@ -446,10 +446,40 @@ public class KotlinEvaluateExpressionTestGenerated extends AbstractKotlinEvaluat
                KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/coroutines"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
            }

+            @TestMetadata("anyUpdateInvokeStatic.kt")
+            public void testAnyUpdateInvokeStatic() throws Exception {
+                runTest("idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/coroutines/anyUpdateInvokeStatic.kt");
+            }
+
+            @TestMetadata("anyUpdateVariable.kt")
+            public void testAnyUpdateVariable() throws Exception {
+                runTest("idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/coroutines/anyUpdateVariable.kt");
+            }
+
            @TestMetadata("primitivesCoertion.kt")
            public void testPrimitivesCoertion() throws Exception {
                runTest("idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/coroutines/primitivesCoertion.kt");
            }
+
+            @TestMetadata("stringUpdateInvokeStatic.kt")
+            public void testStringUpdateInvokeStatic() throws Exception {
+                runTest("idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/coroutines/stringUpdateInvokeStatic.kt");
+            }
+
+            @TestMetadata("stringUpdateInvokeVirtual.kt")
+            public void testStringUpdateInvokeVirtual() throws Exception {
+                runTest("idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/coroutines/stringUpdateInvokeVirtual.kt");
+            }
+
+            @TestMetadata("stringUpdatePutField.kt")
+            public void testStringUpdatePutField() throws Exception {
+                runTest("idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/coroutines/stringUpdatePutField.kt");
+            }
+
+            @TestMetadata("stringUpdateVariable.kt")
+            public void testStringUpdateVariable() throws Exception {
+                runTest("idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/coroutines/stringUpdateVariable.kt");
+            }
        }

        @TestMetadata("idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/createExpression")