From d24d3a73d7b027b921e0229cded60ee0ed7a4156 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Mon, 15 May 2017 10:43:31 +0300 Subject: [PATCH] Support open suspend members and super-calls The problem was that the resume call (from doResume) for open members was based on common INVOKEVIRTUAL to the original function that lead to the invocation of the override when it was expected to be the overridden (after super-call being suspended) The solution is to generate method bodies for open members into the special $suspendImpl synthetic function that may be called from the doResume implementation #KT-17587 Fixed --- .../kotlin/codegen/FunctionCodegen.java | 40 +++++++++++++++++++ .../coroutines/CoroutineCodegenForLambda.kt | 19 ++++++++- .../coroutines/coroutineCodegenUtil.kt | 4 ++ .../suspendFunctionAsCoroutine/superCall.kt | 34 ++++++++++++++++ .../superCallInterface.kt | 38 ++++++++++++++++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 12 ++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 12 ++++++ .../LightAnalysisModeTestGenerated.java | 12 ++++++ .../kotlin/descriptors/descriptorUtil.kt | 4 ++ .../semantics/JsCodegenBoxTestGenerated.java | 12 ++++++ 10 files changed, 185 insertions(+), 2 deletions(-) create mode 100644 compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCall.kt create mode 100644 compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCallInterface.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java index fbdd97e0045..59764fa09cb 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java @@ -201,6 +201,46 @@ public class FunctionCodegen { // Native methods are only defined in facades and do not need package part implementations return; } + + boolean isOpenSuspendInClass = + functionDescriptor.isSuspend() && DescriptorUtilKt.isEffectivelyOpen(functionDescriptor) && + !isInterface(functionDescriptor.getContainingDeclaration()) && + origin.getOriginKind() != JvmDeclarationOriginKind.CLASS_MEMBER_DELEGATION_TO_DEFAULT_IMPL; + + if (isOpenSuspendInClass) { + MethodVisitor mv = + v.newMethod(origin, + flags, + asmMethod.getName(), + asmMethod.getDescriptor(), + jvmSignature.getGenericsSignature(), + getThrownExceptions(functionDescriptor, typeMapper) + ); + + mv.visitCode(); + mv.visitVarInsn(Opcodes.ALOAD, 0); + int index = 1; + for (Type type : asmMethod.getArgumentTypes()) { + mv.visitVarInsn(type.getOpcode(Opcodes.ILOAD), index); + index += type.getSize(); + } + + asmMethod = CoroutineCodegenUtilKt.getImplForOpenMethod(asmMethod, v.getThisName()); + + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, + v.getThisName(), asmMethod.getName(), asmMethod.getDescriptor(), + false + ); + + mv.visitInsn(Opcodes.ARETURN); + mv.visitEnd(); + + flags |= Opcodes.ACC_STATIC | Opcodes.ACC_SYNTHETIC; + flags &= ~getVisibilityAccessFlag(functionDescriptor); + flags |= AsmUtil.NO_FLAG_PACKAGE_PRIVATE; + } + MethodVisitor mv = strategy.wrapMethodVisitor( v.newMethod(origin, diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineCodegenForLambda.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineCodegenForLambda.kt index 6935edbcf29..094e03f46ba 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineCodegenForLambda.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineCodegenForLambda.kt @@ -32,6 +32,7 @@ import org.jetbrains.kotlin.psi.KtDeclarationWithBody import org.jetbrains.kotlin.psi.KtElement import org.jetbrains.kotlin.psi.KtFunction import org.jetbrains.kotlin.psi.KtFunctionLiteral +import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns import org.jetbrains.kotlin.resolve.descriptorUtil.module import org.jetbrains.kotlin.resolve.jvm.AsmTypes @@ -400,14 +401,28 @@ class CoroutineCodegenForNamedFunction private constructor( ).put(captureThisType, codegen.v) } - val callableMethod = typeMapper.mapToCallableMethod(suspendFunctionJvmView, false) + val isInterfaceMethod = DescriptorUtils.isInterface(suspendFunctionJvmView.containingDeclaration) + val callableMethod = + typeMapper.mapToCallableMethod( + suspendFunctionJvmView, + // Obtain default impls method for interfaces + isInterfaceMethod + ) for (argumentType in callableMethod.getAsmMethod().argumentTypes.dropLast(1)) { AsmUtil.pushDefaultValueOnStack(argumentType, codegen.v) } codegen.v.load(0, AsmTypes.OBJECT_TYPE) - callableMethod.genInvokeInstruction(codegen.v) + + if (suspendFunctionJvmView.isEffectivelyOpen() && !isInterfaceMethod && captureThisType != null) { + val owner = captureThisType.internalName + val impl = callableMethod.getAsmMethod().getImplForOpenMethod(owner) + codegen.v.invokestatic(owner, impl.name, impl.descriptor, false) + } + else { + callableMethod.genInvokeInstruction(codegen.v) + } codegen.v.visitInsn(Opcodes.ARETURN) } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/coroutineCodegenUtil.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/coroutineCodegenUtil.kt index e5150cec820..8dd27e7ec71 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/coroutineCodegenUtil.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/coroutineCodegenUtil.kt @@ -49,6 +49,7 @@ import org.jetbrains.kotlin.utils.addToStdlib.safeAs import org.jetbrains.org.objectweb.asm.Opcodes import org.jetbrains.org.objectweb.asm.Type import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter +import org.jetbrains.org.objectweb.asm.commons.Method import org.jetbrains.org.objectweb.asm.tree.MethodNode // These classes do not actually exist at runtime @@ -300,3 +301,6 @@ fun InstructionAdapter.invokeDoResumeWithUnit(thisName: String) { false ) } + +fun Method.getImplForOpenMethod(ownerInternalName: String) = + Method("$name\$suspendImpl", returnType, arrayOf(Type.getObjectType(ownerInternalName)) + argumentTypes) diff --git a/compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCall.kt b/compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCall.kt new file mode 100644 index 00000000000..49a322a5136 --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCall.kt @@ -0,0 +1,34 @@ +// WITH_RUNTIME +// WITH_COROUTINES +import helpers.* +import kotlin.coroutines.experimental.* +import kotlin.coroutines.experimental.intrinsics.* + +open class A(val v: String) { + suspend fun suspendThere(v: String): String = suspendCoroutineOrReturn { x -> + x.resume(v) + COROUTINE_SUSPENDED + } + + open suspend fun suspendHere(): String = suspendThere("O") + suspendThere(v) +} + +class B(v: String) : A(v) { + override suspend fun suspendHere(): String = super.suspendHere() + suspendThere("56") +} + +fun builder(c: suspend A.() -> Unit) { + c.startCoroutine(B("K"), EmptyContinuation) +} + +fun box(): String { + var result = "" + + builder { + result = suspendHere() + } + + if (result != "OK56") return "fail 1: $result" + + return "OK" +} diff --git a/compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCallInterface.kt b/compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCallInterface.kt new file mode 100644 index 00000000000..3913958cdb8 --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCallInterface.kt @@ -0,0 +1,38 @@ +// WITH_RUNTIME +// WITH_COROUTINES +import helpers.* +import kotlin.coroutines.experimental.* +import kotlin.coroutines.experimental.intrinsics.* + +interface A { + val v: String + + suspend fun suspendThere(v: String): String = suspendCoroutineOrReturn { x -> + x.resume(v) + COROUTINE_SUSPENDED + } + + suspend fun suspendHere(): String = suspendThere("O") + suspendThere(v) +} + +interface A2 : A { + override suspend fun suspendHere(): String = super.suspendHere() + suspendThere("56") +} + +class B(override val v: String) : A2 + +fun builder(c: suspend A.() -> Unit) { + c.startCoroutine(B("K"), EmptyContinuation) +} + +fun box(): String { + var result = "" + + builder { + result = suspendHere() + } + + if (result != "OK56") return "fail 1: $result" + + return "OK" +} 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 43fb40c917b..e5137d3fb31 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 @@ -5680,6 +5680,18 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes doTest(fileName); } + @TestMetadata("superCall.kt") + public void testSuperCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCall.kt"); + doTest(fileName); + } + + @TestMetadata("superCallInterface.kt") + public void testSuperCallInterface() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCallInterface.kt"); + doTest(fileName); + } + @TestMetadata("withVariables.kt") public void testWithVariables() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/withVariables.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 4aea6082a3e..4ebb88f3dab 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -5680,6 +5680,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("superCall.kt") + public void testSuperCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCall.kt"); + doTest(fileName); + } + + @TestMetadata("superCallInterface.kt") + public void testSuperCallInterface() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCallInterface.kt"); + doTest(fileName); + } + @TestMetadata("withVariables.kt") public void testWithVariables() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/withVariables.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index b8ee6391973..c7630474285 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -5680,6 +5680,18 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes doTest(fileName); } + @TestMetadata("superCall.kt") + public void testSuperCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCall.kt"); + doTest(fileName); + } + + @TestMetadata("superCallInterface.kt") + public void testSuperCallInterface() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCallInterface.kt"); + doTest(fileName); + } + @TestMetadata("withVariables.kt") public void testWithVariables() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/withVariables.kt"); diff --git a/core/descriptors/src/org/jetbrains/kotlin/descriptors/descriptorUtil.kt b/core/descriptors/src/org/jetbrains/kotlin/descriptors/descriptorUtil.kt index 50f17d019c3..90e93804e5d 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/descriptors/descriptorUtil.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/descriptors/descriptorUtil.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.descriptors import org.jetbrains.kotlin.incremental.components.LookupLocation import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.resolve.DescriptorUtils +import org.jetbrains.kotlin.utils.addToStdlib.safeAs import org.jetbrains.kotlin.utils.sure fun ModuleDescriptor.resolveClassByFqName(fqName: FqName, lookupLocation: LookupLocation): ClassDescriptor? { @@ -37,3 +38,6 @@ fun ModuleDescriptor.findContinuationClassDescriptorOrNull(lookupLocation: Looku fun ModuleDescriptor.findContinuationClassDescriptor(lookupLocation: LookupLocation) = findContinuationClassDescriptorOrNull(lookupLocation).sure { "Continuation interface is not found" } + +fun FunctionDescriptor.isEffectivelyOpen() = + modality == Modality.OPEN && containingDeclaration.safeAs()?.modality == Modality.OPEN 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 81eb0b23f03..a73d409b8b1 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 @@ -6371,6 +6371,18 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { doTest(fileName); } + @TestMetadata("superCall.kt") + public void testSuperCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCall.kt"); + doTest(fileName); + } + + @TestMetadata("superCallInterface.kt") + public void testSuperCallInterface() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/superCallInterface.kt"); + doTest(fileName); + } + @TestMetadata("withVariables.kt") public void testWithVariables() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/withVariables.kt");