From 7c14f4c6aec0d6399929a5f6585b2dfde524a49e Mon Sep 17 00:00:00 2001 From: Ilmir Usmanov Date: Mon, 15 Apr 2019 20:29:04 +0300 Subject: [PATCH] Do not duplicate $$forInline counterpart of inline capturing function --- .../SuspendFunctionGenerationStrategy.kt | 6 +- .../inline/coroutines/CoroutineTransformer.kt | 4 +- .../javaInterop/objectWithSeveralSuspends.kt | 192 ++++++++++++++++++ .../coroutines/javaInterop/returnLambda.kt | 1 + .../coroutines/javaInterop/returnObject.kt | 1 + .../codegen/BlackBoxCodegenTestGenerated.java | 10 + .../LightAnalysisModeTestGenerated.java | 10 + .../ir/IrBlackBoxCodegenTestGenerated.java | 10 + .../IrJsCodegenBoxTestGenerated.java | 10 - .../semantics/JsCodegenBoxTestGenerated.java | 20 -- 10 files changed, 229 insertions(+), 35 deletions(-) create mode 100644 compiler/testData/codegen/box/coroutines/javaInterop/objectWithSeveralSuspends.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/SuspendFunctionGenerationStrategy.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/SuspendFunctionGenerationStrategy.kt index 795a0e96698..2acdd95ad2e 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/SuspendFunctionGenerationStrategy.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/SuspendFunctionGenerationStrategy.kt @@ -65,8 +65,10 @@ open class SuspendFunctionGenerationStrategy( ) val forInline = state.bindingContext[CodegenBinding.CAPTURES_CROSSINLINE_LAMBDA, originalSuspendDescriptor] == true - // Yegor Bugayenko style - return if (forInline) + // Both capturing and inline functions share the same suffix, however, inline functions can also be capturing + // they are already covered by SuspendInlineFunctionGenerationStrategy, thus, if we generate yet another copy, + // we will get name+descriptor clash + return if (forInline && !originalSuspendDescriptor.isInline) AddConstructorCallForCoroutineRegeneration( MethodNodeCopyingMethodVisitor( SurroundSuspendLambdaCallsWithSuspendMarkersMethodVisitor( diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/coroutines/CoroutineTransformer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/coroutines/CoroutineTransformer.kt index a7ef66be78e..cb174ce7b5a 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/coroutines/CoroutineTransformer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/coroutines/CoroutineTransformer.kt @@ -48,8 +48,6 @@ class CoroutineTransformer( fun shouldGenerateStateMachine(node: MethodNode): Boolean { // Continuations are similar to lambdas from bird's view, but we should never generate state machine for them if (isContinuationNotLambda()) return false - // The method does not have state-machine, but should. Generate it - if (node.name.endsWith(FOR_INLINE_SUFFIX)) return true // there can be suspend lambdas inside inline functions, which do not // capture crossinline lambdas, thus, there is no need to transform them return isSuspendFunctionWithFakeConstructorCall(node) || (isSuspendLambda(node) && !isStateMachine(node)) @@ -225,7 +223,7 @@ class SurroundSuspendLambdaCallsWithSuspendMarkersMethodVisitor( if (insn.opcode != Opcodes.INVOKEINTERFACE) continue insn as MethodInsnNode if (!isInvokeOnLambda(insn.owner, insn.name)) continue - val frame = sourceFrames[insn.index()] + val frame = sourceFrames[insn.index()] ?: continue val receiver = findReceiverOfInvoke(frame, insn).takeIf { it?.isSuspendLambda(insn) == true } as? FieldInsnNode ?: continue val aload = receiver.findPreviousOrNull { it.opcode != Opcodes.GETFIELD } ?: error("GETFIELD cannot be the first instruction") assert(aload.opcode == Opcodes.ALOAD) { "Before GETFIELD there shall be ALOAD" } diff --git a/compiler/testData/codegen/box/coroutines/javaInterop/objectWithSeveralSuspends.kt b/compiler/testData/codegen/box/coroutines/javaInterop/objectWithSeveralSuspends.kt new file mode 100644 index 00000000000..45d6255c449 --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/javaInterop/objectWithSeveralSuspends.kt @@ -0,0 +1,192 @@ +// IGNORE_BACKEND: JVM_IR +// TARGET_BACKEND: JVM +// COMMON_COROUTINES_TEST +// WITH_RUNTIME +// WITH_COROUTINES +// NO_CHECK_LAMBDA_INLINING +// CHECK_STATE_MACHINE + +// FILE: inlineMe.kt + +package test + +import helpers.* + +interface SuspendRunnable { + suspend fun run() + suspend fun run1() + suspend fun run2() +} + +inline fun inlineMe(crossinline c: suspend () -> Unit) = object : SuspendRunnable { + override suspend fun run() { + c(); c() + } + override suspend fun run1() { + c(); c() + } + override suspend fun run2() { + inlineMeInner() + } + inline suspend fun inlineMeInner() { + StateMachineChecker.suspendHere() + StateMachineChecker.suspendHere() + } + // TODO: call it from run1 + inline suspend fun inlineMeCapturing() { + c(); c() + } +} + +inline fun inlineMe2(crossinline c: suspend () -> Unit) = inlineMe { c(); c() } + +inline fun inlineMe3(crossinline c: suspend () -> Unit) = object: SuspendRunnable { + override suspend fun run() { + var sr = inlineMe { + c() + c() + } + sr.run() + sr.run1() + sr.run2() + } + + override suspend fun run1() { + } + override suspend fun run2() { + } +} + +// FILE: A.java + +import test.InlineMeKt; +import helpers.CoroutineUtilKt; +import helpers.EmptyContinuation; +import kotlin.Unit; + +public class A { + public static Object call() { + return InlineMeKt.inlineMe((continuation) -> CoroutineUtilKt.getStateMachineChecker().suspendHere(continuation)); + } + public static Object call2() { + return InlineMeKt.inlineMe2((continuation) -> CoroutineUtilKt.getStateMachineChecker().suspendHere(continuation)); + } + public static Object call3() { + return InlineMeKt.inlineMe3((continuation) -> CoroutineUtilKt.getStateMachineChecker().suspendHere(continuation)); + } +} + +// FILE: box.kt + +import test.* +import helpers.* +import COROUTINES_PACKAGE.* + +fun builder(c: suspend () -> Unit) { + c.startCoroutine(CheckStateMachineContinuation) +} + +fun box(): String { + builder { + (A.call() as SuspendRunnable).run() + } + StateMachineChecker.check(2) + StateMachineChecker.reset() + + builder { + (A.call() as SuspendRunnable).run1() + } + StateMachineChecker.check(2) + StateMachineChecker.reset() + + builder { + (A.call() as SuspendRunnable).run2() + } + StateMachineChecker.check(2) + StateMachineChecker.reset() + + builder { + (A.call2() as SuspendRunnable).run() + } + StateMachineChecker.check(4) + StateMachineChecker.reset() + + builder { + (A.call2() as SuspendRunnable).run1() + } + StateMachineChecker.check(4) + StateMachineChecker.reset() + + builder { + (A.call2() as SuspendRunnable).run2() + } + StateMachineChecker.check(2) + StateMachineChecker.reset() + + builder { + (A.call3() as SuspendRunnable).run() + } + StateMachineChecker.check(10) + StateMachineChecker.reset() + + builder { + inlineMe { + StateMachineChecker.suspendHere() + StateMachineChecker.suspendHere() + }.run() + } + StateMachineChecker.check(4) + StateMachineChecker.reset() + builder { + inlineMe { + StateMachineChecker.suspendHere() + StateMachineChecker.suspendHere() + }.run1() + } + StateMachineChecker.check(4) + StateMachineChecker.reset() + builder { + inlineMe { + StateMachineChecker.suspendHere() + StateMachineChecker.suspendHere() + }.run2() + } + StateMachineChecker.check(2) + StateMachineChecker.reset() + + builder { + inlineMe2 { + StateMachineChecker.suspendHere() + StateMachineChecker.suspendHere() + }.run() + } + StateMachineChecker.check(8) + StateMachineChecker.reset() + + builder { + inlineMe2 { + StateMachineChecker.suspendHere() + StateMachineChecker.suspendHere() + }.run1() + } + StateMachineChecker.check(8) + StateMachineChecker.reset() + builder { + inlineMe2 { + StateMachineChecker.suspendHere() + StateMachineChecker.suspendHere() + }.run2() + } + StateMachineChecker.check(2) + StateMachineChecker.reset() + + builder { + inlineMe3 { + StateMachineChecker.suspendHere() + StateMachineChecker.suspendHere() + }.run() + } + StateMachineChecker.check(18) + + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/coroutines/javaInterop/returnLambda.kt b/compiler/testData/codegen/box/coroutines/javaInterop/returnLambda.kt index 4a4c7b0a38f..bb0a70bebad 100644 --- a/compiler/testData/codegen/box/coroutines/javaInterop/returnLambda.kt +++ b/compiler/testData/codegen/box/coroutines/javaInterop/returnLambda.kt @@ -1,4 +1,5 @@ // IGNORE_BACKEND: JVM_IR +// TARGET_BACKEND: JVM // COMMON_COROUTINES_TEST // WITH_RUNTIME // WITH_COROUTINES diff --git a/compiler/testData/codegen/box/coroutines/javaInterop/returnObject.kt b/compiler/testData/codegen/box/coroutines/javaInterop/returnObject.kt index 01fc200d080..bcb1dad733b 100644 --- a/compiler/testData/codegen/box/coroutines/javaInterop/returnObject.kt +++ b/compiler/testData/codegen/box/coroutines/javaInterop/returnObject.kt @@ -1,4 +1,5 @@ // IGNORE_BACKEND: JVM_IR +// TARGET_BACKEND: JVM // COMMON_COROUTINES_TEST // WITH_RUNTIME // WITH_COROUTINES diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 75f1f7932ac..6455ce90148 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -7397,6 +7397,16 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/javaInterop"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("objectWithSeveralSuspends.kt") + public void testObjectWithSeveralSuspends_1_2() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/objectWithSeveralSuspends.kt", "kotlin.coroutines.experimental"); + } + + @TestMetadata("objectWithSeveralSuspends.kt") + public void testObjectWithSeveralSuspends_1_3() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/objectWithSeveralSuspends.kt", "kotlin.coroutines"); + } + @TestMetadata("returnLambda.kt") public void testReturnLambda_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/returnLambda.kt", "kotlin.coroutines.experimental"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 94f23f67c69..11f0b727be0 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -7397,6 +7397,16 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/javaInterop"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("objectWithSeveralSuspends.kt") + public void testObjectWithSeveralSuspends_1_2() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/objectWithSeveralSuspends.kt", "kotlin.coroutines.experimental"); + } + + @TestMetadata("objectWithSeveralSuspends.kt") + public void testObjectWithSeveralSuspends_1_3() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/objectWithSeveralSuspends.kt", "kotlin.coroutines"); + } + @TestMetadata("returnLambda.kt") public void testReturnLambda_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/returnLambda.kt", "kotlin.coroutines.experimental"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 32745e88ec8..78d8f932f28 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -7397,6 +7397,16 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/javaInterop"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM_IR, true); } + @TestMetadata("objectWithSeveralSuspends.kt") + public void testObjectWithSeveralSuspends_1_2() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/objectWithSeveralSuspends.kt", "kotlin.coroutines.experimental"); + } + + @TestMetadata("objectWithSeveralSuspends.kt") + public void testObjectWithSeveralSuspends_1_3() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/objectWithSeveralSuspends.kt", "kotlin.coroutines"); + } + @TestMetadata("returnLambda.kt") public void testReturnLambda_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/returnLambda.kt", "kotlin.coroutines.experimental"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java index 90e1cf1e246..3161784f89e 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java @@ -5716,16 +5716,6 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { public void testAllFilesPresentInJavaInterop() throws Exception { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/javaInterop"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JS_IR, true); } - - @TestMetadata("returnLambda.kt") - public void testReturnLambda_1_3() throws Exception { - runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/returnLambda.kt", "kotlin.coroutines"); - } - - @TestMetadata("returnObject.kt") - public void testReturnObject_1_3() throws Exception { - runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/returnObject.kt", "kotlin.coroutines"); - } } @TestMetadata("compiler/testData/codegen/box/coroutines/localFunctions") 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 028a7c05623..e03eaa84ce4 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 @@ -6456,26 +6456,6 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { public void testAllFilesPresentInJavaInterop() throws Exception { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/javaInterop"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JS, true); } - - @TestMetadata("returnLambda.kt") - public void testReturnLambda_1_2() throws Exception { - runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/returnLambda.kt", "kotlin.coroutines.experimental"); - } - - @TestMetadata("returnLambda.kt") - public void testReturnLambda_1_3() throws Exception { - runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/returnLambda.kt", "kotlin.coroutines"); - } - - @TestMetadata("returnObject.kt") - public void testReturnObject_1_2() throws Exception { - runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/returnObject.kt", "kotlin.coroutines.experimental"); - } - - @TestMetadata("returnObject.kt") - public void testReturnObject_1_3() throws Exception { - runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/javaInterop/returnObject.kt", "kotlin.coroutines"); - } } @TestMetadata("compiler/testData/codegen/box/coroutines/localFunctions")