From f507a26a12ffe206fd9c71c8b3fa2bdb257969ac Mon Sep 17 00:00:00 2001 From: Ilmir Usmanov Date: Wed, 28 Mar 2018 15:46:03 +0300 Subject: [PATCH] Generate decomposed lambda params in suspend lambda's local variables Unlike ordinary lambdas, suspend lambdas do the computation in doResume(Ljava/lang/Object;Ljava/lang/Throwable;)Ljava/lang/Object; method. As you can see, there are no decomposed parameters. As a result, they used not to be generated. To fix the issue, I add decomposed parameters to value parameters while generating local variables table. In addition, when generating suspend lambda for inline, the codegen does not take this kind of parameters into account. This is also fixed. #KT-18576: Fixed --- .../kotlin/codegen/FunctionCodegen.java | 64 +++++++++++++++++-- .../destructuringInSuspendLambda/dataClass.kt | 19 ++++++ .../extensionComponents.kt | 26 ++++++++ .../destructuringInSuspendLambda/generic.kt | 13 ++++ .../destructuringInSuspendLambda/inline.kt | 15 +++++ .../otherParameters.kt | 13 ++++ .../underscoreNames.kt | 17 +++++ .../inlineSeparateFiles.kt | 10 +++ .../codegen/BytecodeTextTestGenerated.java | 18 ++++++ ...CheckLocalVariablesTableTestGenerated.java | 43 +++++++++++++ 10 files changed, 234 insertions(+), 4 deletions(-) create mode 100644 compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/dataClass.kt create mode 100644 compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/extensionComponents.kt create mode 100644 compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/generic.kt create mode 100644 compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/inline.kt create mode 100644 compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/otherParameters.kt create mode 100644 compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/underscoreNames.kt create mode 100644 compiler/testData/codegen/bytecodeText/coroutines/destructuringInLambda/inlineSeparateFiles.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java index ba002c58f74..5fb8db2d433 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java @@ -35,6 +35,7 @@ import org.jetbrains.kotlin.load.java.JvmAbi; import org.jetbrains.kotlin.load.java.SpecialBuiltinMembers; import org.jetbrains.kotlin.load.java.descriptors.JavaClassDescriptor; import org.jetbrains.kotlin.name.FqName; +import org.jetbrains.kotlin.name.Name; import org.jetbrains.kotlin.psi.*; import org.jetbrains.kotlin.resolve.BindingContext; import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils; @@ -383,6 +384,7 @@ public class FunctionCodegen { new Label(), contextKind, typeMapper, + Collections.emptyList(), 0); mv.visitEnd(); @@ -633,9 +635,26 @@ public class FunctionCodegen { typeMapper.getBindingContext() ); } + + List destructuredParametersForSuspendLambda = new ArrayList<>(); + if (context.getParentContext() instanceof ClosureContext) { + if (context instanceof InlineLambdaContext) { + CallableMemberDescriptor lambdaDescriptor = context.getContextDescriptor(); + if (lambdaDescriptor instanceof FunctionDescriptor && + ((FunctionDescriptor) lambdaDescriptor).isSuspend()) { + destructuredParametersForSuspendLambda.addAll(lambdaDescriptor.getValueParameters()); + } + } else { + FunctionDescriptor lambdaDescriptor = ((ClosureContext) context.getParentContext()).getOriginalSuspendLambdaDescriptor(); + if (lambdaDescriptor != null && functionDescriptor.getName().equals(Name.identifier("doResume"))) { + destructuredParametersForSuspendLambda.addAll(lambdaDescriptor.getValueParameters()); + } + } + } + generateLocalVariableTable( mv, signature, functionDescriptor, thisType, methodBegin, methodEnd, context.getContextKind(), typeMapper, - (functionFakeIndex >= 0 ? 1 : 0) + (lambdaFakeIndex >= 0 ? 1 : 0) + destructuredParametersForSuspendLambda, (functionFakeIndex >= 0 ? 1 : 0) + (lambdaFakeIndex >= 0 ? 1 : 0) ); //TODO: it's best to move all below logic to 'generateLocalVariableTable' method @@ -680,6 +699,7 @@ public class FunctionCodegen { @NotNull Label methodEnd, @NotNull OwnerKind ownerKind, @NotNull KotlinTypeMapper typeMapper, + @NotNull List destructuredParametersForSuspendLambda, int shiftForDestructuringVariables ) { if (functionDescriptor.isSuspend()) { @@ -698,7 +718,8 @@ public class FunctionCodegen { ) ), unwrapped, - thisType, methodBegin, methodEnd, ownerKind, typeMapper, shiftForDestructuringVariables + thisType, methodBegin, methodEnd, ownerKind, typeMapper, destructuredParametersForSuspendLambda, + shiftForDestructuringVariables ); return; } @@ -707,6 +728,7 @@ public class FunctionCodegen { generateLocalVariablesForParameters(mv, jvmMethodSignature, thisType, methodBegin, methodEnd, functionDescriptor.getValueParameters(), + destructuredParametersForSuspendLambda, AsmUtil.isStaticMethod(ownerKind, functionDescriptor), typeMapper, shiftForDestructuringVariables ); } @@ -722,8 +744,8 @@ public class FunctionCodegen { KotlinTypeMapper typeMapper ) { generateLocalVariablesForParameters( - mv, jvmMethodSignature, thisType, methodBegin, methodEnd, valueParameters, isStatic, typeMapper, 0 - ); + mv, jvmMethodSignature, thisType, methodBegin, methodEnd, valueParameters, Collections.emptyList(), isStatic, typeMapper, + 0); } private static void generateLocalVariablesForParameters( @@ -733,6 +755,7 @@ public class FunctionCodegen { @NotNull Label methodBegin, @NotNull Label methodEnd, Collection valueParameters, + @NotNull List destructuredParametersForSuspendLambda, boolean isStatic, KotlinTypeMapper typeMapper, int shiftForDestructuringVariables @@ -779,6 +802,19 @@ public class FunctionCodegen { } shift += shiftForDestructuringVariables; + shift = generateDestructuredParameterEntries(mv, methodBegin, methodEnd, valueParameters, typeMapper, shift); + shift = generateDestructuredParametersForSuspendLambda(mv, methodBegin, methodEnd, typeMapper, shift, destructuredParametersForSuspendLambda); + generateDestructuredParameterEntries(mv, methodBegin, methodEnd, destructuredParametersForSuspendLambda, typeMapper, shift); + } + + private static int generateDestructuredParameterEntries( + @NotNull MethodVisitor mv, + @NotNull Label methodBegin, + @NotNull Label methodEnd, + Collection valueParameters, + KotlinTypeMapper typeMapper, + int shift + ) { for (ValueParameterDescriptor parameter : valueParameters) { List destructuringVariables = ValueParameterDescriptorImpl.getDestructuringVariablesOrNull(parameter); if (destructuringVariables == null) continue; @@ -789,6 +825,26 @@ public class FunctionCodegen { shift += type.getSize(); } } + return shift; + } + + private static int generateDestructuredParametersForSuspendLambda( + @NotNull MethodVisitor mv, + @NotNull Label methodBegin, + @NotNull Label methodEnd, + KotlinTypeMapper typeMapper, + int shift, + List destructuredParametersForSuspendLambda + ) { + for (ValueParameterDescriptor parameter : destructuredParametersForSuspendLambda) { + List destructuringVariables = ValueParameterDescriptorImpl.getDestructuringVariablesOrNull(parameter); + if (destructuringVariables == null) continue; + + Type type = typeMapper.mapType(parameter.getType()); + mv.visitLocalVariable("$" + joinParameterNames(destructuringVariables), type.getDescriptor(), null, methodBegin, methodEnd, shift); + shift += type.getSize(); + } + return shift; } private static String computeParameterName(int i, ValueParameterDescriptor parameter) { diff --git a/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/dataClass.kt b/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/dataClass.kt new file mode 100644 index 00000000000..163b5a3262a --- /dev/null +++ b/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/dataClass.kt @@ -0,0 +1,19 @@ +data class Data(val x: String, val y: Int) + +suspend fun test() { + foo(Data("A", 1)) { (x_param, y_param) -> + "$x_param / $y_param" + } +} + +suspend fun foo(data: Data, body: suspend (Data) -> Unit) { + body(data) +} + +// METHOD : DataClassKt$test$2.doResume(Ljava/lang/Object;Ljava/lang/Throwable;)Ljava/lang/Object; +// VARIABLE : NAME=this TYPE=LDataClassKt$test$2; INDEX=0 +// VARIABLE : NAME=data TYPE=Ljava/lang/Object; INDEX=1 +// VARIABLE : NAME=throwable TYPE=Ljava/lang/Throwable; INDEX=2 +// VARIABLE : NAME=$x_param_y_param TYPE=LData; INDEX=3 +// VARIABLE : NAME=x_param TYPE=Ljava/lang/String; INDEX=4 +// VARIABLE : NAME=y_param TYPE=I INDEX=5 diff --git a/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/extensionComponents.kt b/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/extensionComponents.kt new file mode 100644 index 00000000000..c8d17d929de --- /dev/null +++ b/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/extensionComponents.kt @@ -0,0 +1,26 @@ +class A(val x: String, val y: String, val z: T) + +suspend fun foo(a: A, block: suspend (A) -> String): String = block(a) + +operator fun A<*>.component1() = x + +object B { + operator fun A<*>.component2() = y +} + +suspend fun B.bar(): String { + operator fun A.component3() = z + + return foo(A("O", "K", 123)) { (x_param, y_param, z_param) -> x_param + y_param + z_param.toString() } +} + +suspend fun test() = B.bar() + +// METHOD : ExtensionComponentsKt$bar$3.doResume(Ljava/lang/Object;Ljava/lang/Throwable;)Ljava/lang/Object; +// VARIABLE : NAME=this TYPE=LExtensionComponentsKt$bar$3; INDEX=0 +// VARIABLE : NAME=data TYPE=Ljava/lang/Object; INDEX=1 +// VARIABLE : NAME=throwable TYPE=Ljava/lang/Throwable; INDEX=2 +// VARIABLE : NAME=$x_param_y_param_z_param TYPE=LA; INDEX=3 +// VARIABLE : NAME=x_param TYPE=Ljava/lang/String; INDEX=4 +// VARIABLE : NAME=y_param TYPE=Ljava/lang/String; INDEX=5 +// VARIABLE : NAME=z_param TYPE=I INDEX=6 diff --git a/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/generic.kt b/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/generic.kt new file mode 100644 index 00000000000..13de9f3f934 --- /dev/null +++ b/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/generic.kt @@ -0,0 +1,13 @@ +data class A(val x: T, val y: F) + +suspend fun foo(a: A, block: suspend (A) -> String) = block(a) + +suspend fun test() = foo(A("OK", 1)) { (x_param, y_param) -> x_param + (y_param.toString()) } + +// METHOD : GenericKt$test$2.doResume(Ljava/lang/Object;Ljava/lang/Throwable;)Ljava/lang/Object; +// VARIABLE : NAME=this TYPE=LGenericKt$test$2; INDEX=0 +// VARIABLE : NAME=data TYPE=Ljava/lang/Object; INDEX=1 +// VARIABLE : NAME=throwable TYPE=Ljava/lang/Throwable; INDEX=2 +// VARIABLE : NAME=$x_param_y_param TYPE=LA; INDEX=3 +// VARIABLE : NAME=x_param TYPE=Ljava/lang/String; INDEX=4 +// VARIABLE : NAME=y_param TYPE=I INDEX=5 diff --git a/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/inline.kt b/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/inline.kt new file mode 100644 index 00000000000..81842fb31d6 --- /dev/null +++ b/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/inline.kt @@ -0,0 +1,15 @@ +data class A(val x: String, val y: String) + +suspend inline fun foo(a: A, block: suspend (A) -> String): String = block(a) + +suspend fun test() = foo(A("O", "K")) { (x_param, y_param) -> x_param + y_param } + +// METHOD : InlineKt.test(Lkotlin/coroutines/experimental/Continuation;)Ljava/lang/Object; +// VARIABLE : NAME= TYPE=LA; INDEX=3 +// VARIABLE : NAME=continuation TYPE=Lkotlin/coroutines/experimental/Continuation; INDEX=2 +// VARIABLE : NAME=$x_param_y_param TYPE=LA; INDEX=5 +// VARIABLE : NAME=x_param TYPE=Ljava/lang/String; INDEX=6 +// VARIABLE : NAME=y_param TYPE=Ljava/lang/String; INDEX=7 +// VARIABLE : NAME=$i$a$2$foo TYPE=I INDEX=2 +// VARIABLE : NAME=a$iv TYPE=LA; INDEX=1 +// VARIABLE : NAME=$i$f$foo TYPE=I INDEX=8 diff --git a/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/otherParameters.kt b/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/otherParameters.kt new file mode 100644 index 00000000000..d92891e2ddf --- /dev/null +++ b/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/otherParameters.kt @@ -0,0 +1,13 @@ +data class A(val x: String, val y: String) + +suspend fun foo(a: A, block: suspend (Int, A, String) -> String): String = block(1, a, "#") + +suspend fun test() = foo(A("O", "K")) { i_param, (x_param, y_param), v_param -> i_param.toString() + x_param + y_param + v_param } + +// METHOD : OtherParametersKt$test$2.doResume(Ljava/lang/Object;Ljava/lang/Throwable;)Ljava/lang/Object; +// VARIABLE : NAME=this TYPE=LOtherParametersKt$test$2; INDEX=0 +// VARIABLE : NAME=data TYPE=Ljava/lang/Object; INDEX=1 +// VARIABLE : NAME=throwable TYPE=Ljava/lang/Throwable; INDEX=2 +// VARIABLE : NAME=$x_param_y_param TYPE=LA; INDEX=3 +// VARIABLE : NAME=x_param TYPE=Ljava/lang/String; INDEX=4 +// VARIABLE : NAME=y_param TYPE=Ljava/lang/String; INDEX=5 diff --git a/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/underscoreNames.kt b/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/underscoreNames.kt new file mode 100644 index 00000000000..0fcd8294522 --- /dev/null +++ b/compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/underscoreNames.kt @@ -0,0 +1,17 @@ +class A { + operator fun component1() = "O" + operator fun component2(): String = throw RuntimeException("fail 0") + operator fun component3() = "K" +} + +suspend fun foo(a: A, block: suspend (A) -> String): String = block(a) + +suspend fun test() = foo(A()) { (x_param, _, y_param) -> x_param + y_param } + +// METHOD : UnderscoreNamesKt$test$2.doResume(Ljava/lang/Object;Ljava/lang/Throwable;)Ljava/lang/Object; +// VARIABLE : NAME=this TYPE=LUnderscoreNamesKt$test$2; INDEX=0 +// VARIABLE : NAME=data TYPE=Ljava/lang/Object; INDEX=1 +// VARIABLE : NAME=throwable TYPE=Ljava/lang/Throwable; INDEX=2 +// VARIABLE : NAME=$x_param_$_$_y_param TYPE=LA; INDEX=3 +// VARIABLE : NAME=x_param TYPE=Ljava/lang/String; INDEX=4 +// VARIABLE : NAME=y_param TYPE=Ljava/lang/String; INDEX=5 diff --git a/compiler/testData/codegen/bytecodeText/coroutines/destructuringInLambda/inlineSeparateFiles.kt b/compiler/testData/codegen/bytecodeText/coroutines/destructuringInLambda/inlineSeparateFiles.kt new file mode 100644 index 00000000000..b06cccf8236 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/coroutines/destructuringInLambda/inlineSeparateFiles.kt @@ -0,0 +1,10 @@ +data class A(val x: String, val y: String) + +suspend inline fun foo(a: A, block: suspend (A) -> String): String = block(a) + +// FILE: test.kt +suspend fun test() = foo(A("O", "K")) { (x_param, y_param) -> x_param + y_param } + +// @TestKt.class: +// 1 LOCALVARIABLE x_param Ljava/lang/String; +// 1 LOCALVARIABLE y_param Ljava/lang/String; \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index 07836de9b6c..120350a172e 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -1081,6 +1081,24 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { doTestWithCoroutinesPackageReplacement(fileName, "kotlin.coroutines"); } + @TestMetadata("compiler/testData/codegen/bytecodeText/coroutines/destructuringInLambda") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class DestructuringInLambda extends AbstractBytecodeTextTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); + } + + public void testAllFilesPresentInDestructuringInLambda() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/bytecodeText/coroutines/destructuringInLambda"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("inlineSeparateFiles.kt") + public void testInlineSeparateFiles() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/coroutines/destructuringInLambda/inlineSeparateFiles.kt"); + } + } + @TestMetadata("compiler/testData/codegen/bytecodeText/coroutines/intLikeVarSpilling") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/CheckLocalVariablesTableTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/CheckLocalVariablesTableTestGenerated.java index 1ad1d44b173..18606ef6a96 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/CheckLocalVariablesTableTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/CheckLocalVariablesTableTestGenerated.java @@ -108,4 +108,47 @@ public class CheckLocalVariablesTableTestGenerated extends AbstractCheckLocalVar public void testUnderscoreNames() throws Exception { runTest("compiler/testData/checkLocalVariablesTable/underscoreNames.kt"); } + + @TestMetadata("compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class DestructuringInSuspendLambda extends AbstractCheckLocalVariablesTableTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); + } + + public void testAllFilesPresentInDestructuringInSuspendLambda() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("dataClass.kt") + public void testDataClass() throws Exception { + runTest("compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/dataClass.kt"); + } + + @TestMetadata("extensionComponents.kt") + public void testExtensionComponents() throws Exception { + runTest("compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/extensionComponents.kt"); + } + + @TestMetadata("generic.kt") + public void testGeneric() throws Exception { + runTest("compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/generic.kt"); + } + + @TestMetadata("inline.kt") + public void testInline() throws Exception { + runTest("compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/inline.kt"); + } + + @TestMetadata("otherParameters.kt") + public void testOtherParameters() throws Exception { + runTest("compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/otherParameters.kt"); + } + + @TestMetadata("underscoreNames.kt") + public void testUnderscoreNames() throws Exception { + runTest("compiler/testData/checkLocalVariablesTable/destructuringInSuspendLambda/underscoreNames.kt"); + } + } }