From 10f0a2f66040ceb6df277b019e5f4f75783efb7f Mon Sep 17 00:00:00 2001 From: Ilmir Usmanov Date: Tue, 18 Dec 2018 14:57:32 +0300 Subject: [PATCH] Obtain correct captured suspend local function if we call the function inside, for example, lambda. #KT-28844 Fixed --- .../jetbrains/kotlin/codegen/StackValue.java | 14 ++++--- .../codegen/box/coroutines/kt28844.kt | 39 +++++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 10 +++++ .../LightAnalysisModeTestGenerated.java | 10 +++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 10 +++++ .../IrJsCodegenBoxTestGenerated.java | 5 +++ .../semantics/JsCodegenBoxTestGenerated.java | 10 +++++ 7 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 compiler/testData/codegen/box/coroutines/kt28844.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java index 019b77d0fc7..b482129191e 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java @@ -885,6 +885,7 @@ public abstract class StackValue { // 2) call using callable reference: in this case it is not local, but rather captured value // 3) recursive call: we are in the middle of defining it, but, thankfully, we can simply call `this.invoke` to // create new coroutine + // 4) Normal call, but the value is captured // First, check whether this is a normal call int index = codegen.lookupLocalIndex(callee); @@ -898,24 +899,27 @@ public abstract class StackValue { Type calleeType = CodegenBinding.asmTypeForAnonymousClass(bindingContext, callee); if (codegen.context.hasThisDescriptor()) { ClassDescriptor thisDescriptor = codegen.context.getThisDescriptor(); + ClassDescriptor classDescriptor = bindingContext.get(CLASS_FOR_CALLABLE, callee); if (thisDescriptor instanceof SyntheticClassDescriptorForLambda && ((SyntheticClassDescriptorForLambda) thisDescriptor).isCallableReference()) { // Call is inside a callable reference // if it is call to recursive local, just return this$0 Boolean isRecursive = bindingContext.get(RECURSIVE_SUSPEND_CALLABLE_REFERENCE, thisDescriptor); if (isRecursive != null && isRecursive) { - ClassDescriptor classDescriptor = - bindingContext.get(CLASS_FOR_CALLABLE, callee); assert classDescriptor != null : "No CLASS_FOR_CALLABLE" + callee; return thisOrOuter(codegen, classDescriptor, false, false); } // Otherwise, just call constructor of the closure return codegen.findCapturedValue(callee); } + if (classDescriptor == thisDescriptor) { + // Recursive suspend local function, just call invoke on this, it will create new coroutine automatically + codegen.v.visitVarInsn(ALOAD, 0); + return onStack(calleeType); + } } - // Recursive suspend local function, just call invoke on this, it will create new coroutine automatically - codegen.v.visitVarInsn(ALOAD, 0); - return onStack(calleeType); + // Otherwise, this is captured value + return codegen.findCapturedValue(callee); } private static StackValue platformStaticCallIfPresent(@NotNull StackValue resultReceiver, @NotNull CallableDescriptor descriptor) { diff --git a/compiler/testData/codegen/box/coroutines/kt28844.kt b/compiler/testData/codegen/box/coroutines/kt28844.kt new file mode 100644 index 00000000000..331db670ef5 --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/kt28844.kt @@ -0,0 +1,39 @@ +// WITH_RUNTIME +// WITH_COROUTINES +// COMMON_COROUTINES_TEST +// IGNORE_BACKEND: JVM_IR, NATIVE + +import helpers.* +import COROUTINES_PACKAGE.* + +fun builder(block: suspend Unit.() -> Unit) { + block.startCoroutine(Unit, EmptyContinuation) +} + +var res = "FAIL 1" + +fun testOuterJobIsCancelled() = builder { + suspend fun callJobScoped() = "OK" + + val outerJob = suspend { + res = callJobScoped() + } + outerJob() +} + +fun testOuterJobIsCancelled2() = builder { + suspend fun callJobScoped() = "OK" + + suspend fun outerJob() { + res = callJobScoped() + } + outerJob() +} + +fun box(): String { + testOuterJobIsCancelled() + if (res != "OK") return res + res = "FAIL 2" + testOuterJobIsCancelled2() + return res +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 9b1552bf0c6..f8b37bbc1eb 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -5906,6 +5906,16 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt25912.kt", "kotlin.coroutines"); } + @TestMetadata("kt28844.kt") + public void testKt28844_1_2() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt28844.kt", "kotlin.coroutines.experimental"); + } + + @TestMetadata("kt28844.kt") + public void testKt28844_1_3() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt28844.kt", "kotlin.coroutines"); + } + @TestMetadata("lastExpressionIsLoop.kt") public void testLastExpressionIsLoop_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/lastExpressionIsLoop.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 3f44395327c..75f8e04d472 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -5906,6 +5906,16 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt25912.kt", "kotlin.coroutines"); } + @TestMetadata("kt28844.kt") + public void testKt28844_1_2() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt28844.kt", "kotlin.coroutines.experimental"); + } + + @TestMetadata("kt28844.kt") + public void testKt28844_1_3() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt28844.kt", "kotlin.coroutines"); + } + @TestMetadata("lastExpressionIsLoop.kt") public void testLastExpressionIsLoop_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/lastExpressionIsLoop.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 0120c831a4d..0072ed97b85 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -5906,6 +5906,16 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt25912.kt", "kotlin.coroutines"); } + @TestMetadata("kt28844.kt") + public void testKt28844_1_2() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt28844.kt", "kotlin.coroutines.experimental"); + } + + @TestMetadata("kt28844.kt") + public void testKt28844_1_3() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt28844.kt", "kotlin.coroutines"); + } + @TestMetadata("lastExpressionIsLoop.kt") public void testLastExpressionIsLoop_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/lastExpressionIsLoop.kt", "kotlin.coroutines.experimental"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java index 40dc39ed5f0..0e5a1f47c81 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java @@ -4851,6 +4851,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt25912.kt", "kotlin.coroutines"); } + @TestMetadata("kt28844.kt") + public void testKt28844_1_3() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt28844.kt", "kotlin.coroutines"); + } + @TestMetadata("lastExpressionIsLoop.kt") public void testLastExpressionIsLoop_1_3() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/lastExpressionIsLoop.kt", "kotlin.coroutines"); 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 47d8fc8a998..b7e68ac8627 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 @@ -5046,6 +5046,16 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt25912.kt", "kotlin.coroutines"); } + @TestMetadata("kt28844.kt") + public void testKt28844_1_2() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt28844.kt", "kotlin.coroutines.experimental"); + } + + @TestMetadata("kt28844.kt") + public void testKt28844_1_3() throws Exception { + runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/kt28844.kt", "kotlin.coroutines"); + } + @TestMetadata("lastExpressionIsLoop.kt") public void testLastExpressionIsLoop_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/box/coroutines/lastExpressionIsLoop.kt", "kotlin.coroutines.experimental");