From da6841163b87332a399c2f7a8ab4e92dcece0986 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 2 Oct 2017 17:29:20 +0300 Subject: [PATCH] Maintain evaluation order for suspend calls in constructor arguments Insert 'Class.forName(...)' in place of NEW instruction, so that the corresponding class will be initialized if required. --- .../coroutines/processUninitializedStores.kt | 46 +++++++-------- .../UninitializedStoresMethodTransformer.kt | 2 +- ...ddleOfObjectConstructionEvaluationOrder.kt | 56 +++++++++++++++++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 6 ++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++ .../LightAnalysisModeTestGenerated.java | 6 ++ .../semantics/JsCodegenBoxTestGenerated.java | 6 ++ 7 files changed, 105 insertions(+), 23 deletions(-) create mode 100644 compiler/testData/codegen/box/coroutines/suspendInTheMiddleOfObjectConstructionEvaluationOrder.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/processUninitializedStores.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/processUninitializedStores.kt index d4c808162e0..d026e787a43 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/processUninitializedStores.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/processUninitializedStores.kt @@ -70,10 +70,16 @@ import org.jetbrains.org.objectweb.asm.tree.analysis.Interpreter * - restore constructor arguments */ internal fun processUninitializedStores(methodNode: MethodNode) { - UninitializedStoresProcessor(methodNode, forCoroutines = true).run() + UninitializedStoresProcessor(methodNode).run() } -class UninitializedStoresProcessor(private val methodNode: MethodNode, private val forCoroutines: Boolean) { +class UninitializedStoresProcessor(private val methodNode: MethodNode) { + // method is "special", because it will invoke from this class or from a base class for #0 + // + // method is "special", because for singleton objects is generated as: + // NEW + // INVOKESPECIAL . ()V + // and the newly created value is dropped. private val isInSpecialMethod = methodNode.name == "" || methodNode.name == "" fun run() { @@ -97,26 +103,22 @@ class UninitializedStoresProcessor(private val methodNode: MethodNode, private v methodNode.instructions.run { removeAll(copyUsages) - if (forCoroutines) { - remove(newInsn) - } - else { - // Replace 'NEW C' instruction with "manual" initialization of class 'C': - // LDC [typeName for C] - // INVOKESTATIC java/lang/Class.forName (Ljava/lang/String;)Ljava/lang/Class; - // POP - val typeNameForClass = newInsn.desc.replace('/', '.') - insertBefore(newInsn, LdcInsnNode(typeNameForClass)) - insertBefore( - newInsn, - MethodInsnNode( - Opcodes.INVOKESTATIC, - "java/lang/Class", "forName", "(Ljava/lang/String;)Ljava/lang/Class;", - false - ) - ) - set(newInsn, InsnNode(Opcodes.POP)) - } + + // Replace 'NEW C' instruction with "manual" initialization of class 'C': + // LDC [typeName for C] + // INVOKESTATIC java/lang/Class.forName (Ljava/lang/String;)Ljava/lang/Class; + // POP + val typeNameForClass = newInsn.desc.replace('/', '.') + insertBefore(newInsn, LdcInsnNode(typeNameForClass)) + insertBefore( + newInsn, + MethodInsnNode( + Opcodes.INVOKESTATIC, + "java/lang/Class", "forName", "(Ljava/lang/String;)Ljava/lang/Class;", + false + ) + ) + set(newInsn, InsnNode(Opcodes.POP)) } val indexOfConstructorArgumentFromTopOfStack = Type.getArgumentTypes((insn as MethodInsnNode).desc).size diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/UninitializedStoresMethodTransformer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/UninitializedStoresMethodTransformer.kt index aab689820fa..6f81188b8ec 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/UninitializedStoresMethodTransformer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/UninitializedStoresMethodTransformer.kt @@ -22,6 +22,6 @@ import org.jetbrains.org.objectweb.asm.tree.MethodNode class UninitializedStoresMethodTransformer : MethodTransformer() { override fun transform(internalClassName: String, methodNode: MethodNode) { - UninitializedStoresProcessor(methodNode, forCoroutines = false).run() + UninitializedStoresProcessor(methodNode).run() } } \ No newline at end of file diff --git a/compiler/testData/codegen/box/coroutines/suspendInTheMiddleOfObjectConstructionEvaluationOrder.kt b/compiler/testData/codegen/box/coroutines/suspendInTheMiddleOfObjectConstructionEvaluationOrder.kt new file mode 100644 index 00000000000..4cbc82839d8 --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/suspendInTheMiddleOfObjectConstructionEvaluationOrder.kt @@ -0,0 +1,56 @@ +// WITH_RUNTIME +// WITH_COROUTINES +import helpers.* +import kotlin.coroutines.experimental.* +import kotlin.coroutines.experimental.intrinsics.* + +class Controller { + suspend fun suspendHere(): String = suspendCoroutineOrReturn { x -> + x.resume("K") + COROUTINE_SUSPENDED + } +} + +fun builder(c: suspend Controller.() -> Unit) { + c.startCoroutine(Controller(), EmptyContinuation) +} + +val logger = StringBuilder() + +class A(val first: String, val second: String) { + init { + logger.append("A.;") + } + + override fun toString() = "$first$second" + + companion object { + init { + logger.append("A.;") + } + } +} + +inline fun logged(message: String, result: () -> T): T { + logger.append(message) + return result() +} + +fun box(): String { + var result = "OK" + + builder { + var local: Any = A(logged("args;") { "O" }, suspendHere()) + + if (local.toString() != "OK") { + result = "fail 1: $local" + return@builder + } + } + + if (logger.toString() != "A.;args;A.;") { + return "Fail: '$logger'" + } + + return result +} diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 4584c79d688..506c09b0fd7 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -5498,6 +5498,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes doTest(fileName); } + @TestMetadata("suspendInTheMiddleOfObjectConstructionEvaluationOrder.kt") + public void testSuspendInTheMiddleOfObjectConstructionEvaluationOrder() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendInTheMiddleOfObjectConstructionEvaluationOrder.kt"); + doTest(fileName); + } + @TestMetadata("suspensionInsideSafeCall.kt") public void testSuspensionInsideSafeCall() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspensionInsideSafeCall.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 7ad0b3590e2..c20040b770e 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -5498,6 +5498,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("suspendInTheMiddleOfObjectConstructionEvaluationOrder.kt") + public void testSuspendInTheMiddleOfObjectConstructionEvaluationOrder() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendInTheMiddleOfObjectConstructionEvaluationOrder.kt"); + doTest(fileName); + } + @TestMetadata("suspensionInsideSafeCall.kt") public void testSuspensionInsideSafeCall() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspensionInsideSafeCall.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index bcac1e48bcb..9a08e4a2c52 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -5498,6 +5498,12 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes doTest(fileName); } + @TestMetadata("suspendInTheMiddleOfObjectConstructionEvaluationOrder.kt") + public void testSuspendInTheMiddleOfObjectConstructionEvaluationOrder() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendInTheMiddleOfObjectConstructionEvaluationOrder.kt"); + doTest(fileName); + } + @TestMetadata("suspensionInsideSafeCall.kt") public void testSuspensionInsideSafeCall() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspensionInsideSafeCall.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index 445d959cd07..8d14cf7ae64 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -6146,6 +6146,12 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { doTest(fileName); } + @TestMetadata("suspendInTheMiddleOfObjectConstructionEvaluationOrder.kt") + public void testSuspendInTheMiddleOfObjectConstructionEvaluationOrder() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendInTheMiddleOfObjectConstructionEvaluationOrder.kt"); + doTest(fileName); + } + @TestMetadata("suspensionInsideSafeCall.kt") public void testSuspensionInsideSafeCall() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspensionInsideSafeCall.kt");