From b1189eff233cf640e1243788b82815e22a018245 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 26 May 2016 14:10:26 +0300 Subject: [PATCH] Make suspension calls generation more stable Instead of performing signature change during transformation it's convinient to make just when generating corresponding call, for example there is no need to think about default parameters as they just work as is See comment above replaceSuspensionFunctionViewWithRealDescriptor for clarification --- .../kotlin/codegen/ExpressionCodegen.java | 36 ++++++--- .../CoroutineTransformationClassBuilder.kt | 7 +- .../coroutines/coroutineCodegenUtil.kt | 81 +++++++++++++++++-- .../kotlin/resolve/coroutine/coroutineUtil.kt | 1 + .../coroutines/defaultParametersInSuspend.kt | 41 ++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++ 6 files changed, 149 insertions(+), 23 deletions(-) create mode 100644 compiler/testData/codegen/box/coroutines/defaultParametersInSuspend.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java index 74b3901f982..f20ffa56e56 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java @@ -39,6 +39,7 @@ import org.jetbrains.kotlin.codegen.binding.CodegenBinding; import org.jetbrains.kotlin.codegen.context.*; import org.jetbrains.kotlin.codegen.coroutines.CoroutineCodegen; import org.jetbrains.kotlin.codegen.coroutines.CoroutineCodegenUtilKt; +import org.jetbrains.kotlin.codegen.coroutines.ResolvedCallWithRealDescriptor; import org.jetbrains.kotlin.codegen.extensions.ExpressionCodegenExtension; import org.jetbrains.kotlin.codegen.inline.*; import org.jetbrains.kotlin.codegen.intrinsics.*; @@ -1665,26 +1666,31 @@ public class ExpressionCodegen extends KtVisitor impleme tempVariables.put(argumentExpression, valueToReturn); } - // Currently only handleResult members are supported - ReceiverValue dispatchReceiver = resolvedCall.getDispatchReceiver(); - assert dispatchReceiver != null : "Dispatch receiver is null for handleResult to " + resolvedCall.getResultingDescriptor(); - assert dispatchReceiver instanceof ExtensionReceiver - : "Argument for handleResult call to " + resolvedCall.getResultingDescriptor() + - " should be a coroutine receiver parameter, but " + dispatchReceiver + " found"; - ClassDescriptor coroutineClassDescriptor = - bindingContext.get(CodegenBinding.CLASS_FOR_CALLABLE, ((ExtensionReceiver) dispatchReceiver).getDeclarationDescriptor()); - assert coroutineClassDescriptor != null : "Coroutine class descriptor should not be null"; - - // second argument for handleResult is always Continuation ('this'-object in current implementation) tempVariables.put( resolvedCall.getValueArgumentsByIndex().get(1).getArguments().get(0).getArgumentExpression(), - StackValue.thisOrOuter(this, coroutineClassDescriptor, false, false)); + genCoroutineInstanceValueFromResolvedCall(resolvedCall)); return invokeFunction(resolvedCall, StackValue.none()); } return null; } + @NotNull + private StackValue genCoroutineInstanceValueFromResolvedCall(ResolvedCall resolvedCall) { + // Currently only handleResult/suspend members are supported + ReceiverValue dispatchReceiver = resolvedCall.getDispatchReceiver(); + assert dispatchReceiver != null : "Dispatch receiver is null for handleResult/suspend to " + resolvedCall.getResultingDescriptor(); + assert dispatchReceiver instanceof ExtensionReceiver + : "Argument for handleResult call to " + resolvedCall.getResultingDescriptor() + + " should be a coroutine receiver parameter, but " + dispatchReceiver + " found"; + ClassDescriptor coroutineClassDescriptor = + bindingContext.get(CodegenBinding.CLASS_FOR_CALLABLE, ((ExtensionReceiver) dispatchReceiver).getDeclarationDescriptor()); + assert coroutineClassDescriptor != null : "Coroutine class descriptor should not be null"; + + // second argument for handleResult is always Continuation ('this'-object in current implementation) + return StackValue.thisOrOuter(this, coroutineClassDescriptor, false, false); + } + @NotNull private Type getVariableType(@NotNull VariableDescriptor variableDescriptor) { Type sharedVarType = typeMapper.getSharedVarType(variableDescriptor); @@ -2477,6 +2483,12 @@ public class ExpressionCodegen extends KtVisitor impleme @NotNull public StackValue invokeFunction(@NotNull Call call, @NotNull ResolvedCall resolvedCall, @NotNull StackValue receiver) { + ResolvedCallWithRealDescriptor callWithRealDescriptor = + CoroutineCodegenUtilKt.replaceSuspensionFunctionViewWithRealDescriptor(resolvedCall, state.getProject()); + if (callWithRealDescriptor != null) { + tempVariables.put(callWithRealDescriptor.getFakeThisExpression(), genCoroutineInstanceValueFromResolvedCall(resolvedCall)); + return invokeFunction(callWithRealDescriptor.getResolvedCall(), receiver); + } FunctionDescriptor fd = accessibleFunctionDescriptor(resolvedCall); ClassDescriptor superCallTarget = getSuperCallTarget(call); diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformationClassBuilder.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformationClassBuilder.kt index 5a37733bde9..013ae134c59 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformationClassBuilder.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformationClassBuilder.kt @@ -198,9 +198,7 @@ class CoroutineTransformerMethodVisitor( private fun transformCallAndReturnContinuationLabel(suspension: SuspensionPoint, methodNode: MethodNode): LabelNode { val call = suspension.suspensionCall val method = Method(call.name, call.desc) - val newParameters = method.argumentTypes + CONTINUATION_INTERFACE_ASM_TYPE - - call.desc = Method(method.name, Type.VOID_TYPE, newParameters).descriptor + call.desc = Method(method.name, Type.VOID_TYPE, method.argumentTypes).descriptor val continuationLabel = LabelNode() @@ -213,9 +211,6 @@ class CoroutineTransformerMethodVisitor( FieldInsnNode( Opcodes.PUTFIELD, classBuilder.thisName, COROUTINE_LABEL_FIELD_NAME, Type.INT_TYPE.descriptor))) - // Pass continuation - insertBefore(call, VarInsnNode(Opcodes.ALOAD, 0)) - val nextInsnAfterCall = call.next // Exit 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 45465337bbc..d014c090aed 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/coroutineCodegenUtil.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/coroutineCodegenUtil.kt @@ -16,11 +16,20 @@ package org.jetbrains.kotlin.codegen.coroutines -import org.jetbrains.kotlin.coroutines.CONTINUATION_INTERFACE_FQ_NAME -import org.jetbrains.kotlin.resolve.jvm.JvmClassName -import org.jetbrains.org.objectweb.asm.Type - -val CONTINUATION_INTERFACE_ASM_TYPE = Type.getObjectType(JvmClassName.byFqNameWithoutInnerClasses(CONTINUATION_INTERFACE_FQ_NAME).internalName) +import com.intellij.openapi.project.Project +import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.psi.KtExpression +import org.jetbrains.kotlin.psi.KtPsiFactory +import org.jetbrains.kotlin.resolve.BindingTraceContext +import org.jetbrains.kotlin.resolve.DelegatingBindingTrace +import org.jetbrains.kotlin.resolve.calls.model.ExpressionValueArgument +import org.jetbrains.kotlin.resolve.calls.model.MutableDataFlowInfoForArguments +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCallImpl +import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowInfo +import org.jetbrains.kotlin.resolve.calls.tasks.TracingStrategy +import org.jetbrains.kotlin.types.TypeConstructorSubstitution +import org.jetbrains.kotlin.types.typeUtil.asTypeProjection // These classes do not actually exist at runtime val CONTINUATION_METHOD_ANNOTATION_DESC = "Lkotlin/ContinuationMethod;" @@ -30,3 +39,65 @@ const val SUSPENSION_POINT_MARKER_NAME = "suspensionPoint" const val COROUTINE_CONTROLLER_FIELD_NAME = "controller" const val COROUTINE_LABEL_FIELD_NAME = "label" + +data class ResolvedCallWithRealDescriptor(val resolvedCall: ResolvedCall<*>, val fakeThisExpression: KtExpression) + +// Resolved calls to suspension function contain descriptors as they visible within coroutines: +// E.g. `fun await(f: CompletableFuture): V` instead of `fun await(f: CompletableFuture, machine: Continuation): Unit` +// See `createCoroutineSuspensionFunctionView` and it's usages for clarification +// But for call generation it's convenient to have `machine` (continuation) parameter/argument within resolvedCall. +// So this function returns resolved call with descriptor looking like `fun await(f: CompletableFuture, machine: Continuation): V` +// and fake `this` expression that used as argument for second parameter +fun ResolvedCall<*>.replaceSuspensionFunctionViewWithRealDescriptor( + project: Project +): ResolvedCallWithRealDescriptor? { + val function = candidateDescriptor as? FunctionDescriptor ?: return null + if (!function.isSuspend) return null + + val initialSignatureDescriptor = function.initialSignatureDescriptor ?: return null + val newCandidateDescriptor = + initialSignatureDescriptor.createCustomCopy { + // Here we know that last parameter should be Continuation where T is return type + setReturnType(it.valueParameters.last().type.arguments.single().type) + } + + val newCall = ResolvedCallImpl( + call, + newCandidateDescriptor, + dispatchReceiver, extensionReceiver, explicitReceiverKind, + null, DelegatingBindingTrace(BindingTraceContext().bindingContext, "Temporary trace for unwrapped suspension function"), + TracingStrategy.EMPTY, MutableDataFlowInfoForArguments.WithoutArgumentsCheck(DataFlowInfo.EMPTY)) + + this.valueArguments.forEach { + newCall.recordValueArgument(newCandidateDescriptor.valueParameters[it.key.index], it.value) + } + + val psiFactory = KtPsiFactory(project) + val arguments = psiFactory.createCallArguments("(this)").arguments.single() + val thisExpression = arguments.getArgumentExpression()!! + newCall.recordValueArgument( + newCandidateDescriptor.valueParameters.last(), + ExpressionValueArgument(arguments)) + + newCall.setResultingSubstitutor( + TypeConstructorSubstitution.createByParametersMap(typeArguments.mapValues { it.value.asTypeProjection() }).buildSubstitutor()) + + return ResolvedCallWithRealDescriptor(newCall, thisExpression) +} + +private fun FunctionDescriptor.createCustomCopy( + copySettings: FunctionDescriptor.CopyBuilder.(FunctionDescriptor) -> FunctionDescriptor.CopyBuilder +): FunctionDescriptor { + + val newOriginal = + if (original !== this) + original.createCustomCopy(copySettings) + else + null + + val result = newCopyBuilder().copySettings(this).setOriginal(newOriginal).build()!! + + result.overriddenDescriptors = this.overriddenDescriptors.map { it.createCustomCopy(copySettings) } + + return result +} diff --git a/compiler/resolution/src/org/jetbrains/kotlin/resolve/coroutine/coroutineUtil.kt b/compiler/resolution/src/org/jetbrains/kotlin/resolve/coroutine/coroutineUtil.kt index cddeacfdeb4..7528ea5d84e 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/resolve/coroutine/coroutineUtil.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/resolve/coroutine/coroutineUtil.kt @@ -36,6 +36,7 @@ fun SimpleFunctionDescriptor.createCoroutineSuspensionFunctionView(): SimpleFunc setReturnType(returnType) setOriginal(newOriginal) setValueParameters(valueParameters.subList(0, valueParameters.size - 1)) + setSignatureChange() }.build()!! } diff --git a/compiler/testData/codegen/box/coroutines/defaultParametersInSuspend.kt b/compiler/testData/codegen/box/coroutines/defaultParametersInSuspend.kt new file mode 100644 index 00000000000..8a6b3532842 --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/defaultParametersInSuspend.kt @@ -0,0 +1,41 @@ +class Controller { + suspend fun suspendHere(a: String = "abc", i: Int = 2, x: Continuation) { + x.resume(a + "#" + (i + 1)) + } +} + +fun builder(coroutine c: Controller.() -> Continuation) { + c(Controller()).resume(Unit) +} + +fun box(): String { + var result = "OK" + + builder { + var a = suspendHere() + if (a != "abc#3") { + result = "fail 1: $a" + throw RuntimeException(result) + } + + a = suspendHere("cde") + if (a != "cde#3") { + result = "fail 2: $a" + throw RuntimeException(result) + } + + a = suspendHere(i = 6) + if (a != "abc#7") { + result = "fail 3: $a" + throw RuntimeException(result) + } + + a = suspendHere("xyz", 9) + if (a != "xyz#10") { + result = "fail 4: $a" + throw RuntimeException(result) + } + } + + return result +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 96d7996bdc1..b0d90d20b3b 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -4111,6 +4111,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines"), Pattern.compile("^(.+)\\.kt$"), true); } + @TestMetadata("defaultParametersInSuspend.kt") + public void testDefaultParametersInSuspend() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/defaultParametersInSuspend.kt"); + doTest(fileName); + } + @TestMetadata("generate.kt") public void testGenerate() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/generate.kt");