From e16b0524b66dd50ddc3a6ca5aa0a3fd23afd56a4 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 16 Feb 2017 10:55:07 +0300 Subject: [PATCH] Fix inline codegen on local functions inside inlined lambda The problem was that anonymous classes wasn't regenerated although they capture another anonymous class that is a subject for regeneration #KT-8689 Fixed --- .../kotlin/codegen/inline/MethodInliner.java | 20 +++++++++++--- .../codegen/inline/TransformationInfo.kt | 6 +++-- .../objectInLambdaCapturesAnotherObject.kt | 27 +++++++++++++++++++ .../lambdaInLambdaCapturesAnotherFun.kt | 27 +++++++++++++++++++ .../localFunInLambdaCapturesAnotherFun.kt | 26 ++++++++++++++++++ .../BlackBoxInlineCodegenTestGenerated.java | 18 +++++++++++++ ...otlinAgainstInlineKotlinTestGenerated.java | 18 +++++++++++++ 7 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 compiler/testData/codegen/boxInline/anonymousObject/objectInLambdaCapturesAnotherObject.kt create mode 100644 compiler/testData/codegen/boxInline/localFunInLambda/lambdaInLambdaCapturesAnotherFun.kt create mode 100644 compiler/testData/codegen/boxInline/localFunInLambda/localFunInLambdaCapturesAnotherFun.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java index d62fdc6e3a8..b98ed78688c 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java @@ -463,6 +463,7 @@ public class MethodInliner { Map lambdaMapping = new HashMap(); int offset = 0; + boolean capturesAnonymousObjectThatMustBeRegenerated = false; for (int i = 0; i < paramCount; i++) { SourceValue sourceValue = frame.getStack(firstParameterIndex + i); LambdaInfo lambdaInfo = MethodInlinerUtilKt.getLambdaIfExistsAndMarkInstructions( @@ -471,12 +472,17 @@ public class MethodInliner { if (lambdaInfo != null) { lambdaMapping.put(offset, lambdaInfo); } + else if (i < argTypes.length && isAnonymousClassThatMustBeRegenerated(argTypes[i])) { + capturesAnonymousObjectThatMustBeRegenerated = true; + } offset += i == 0 ? 1 : argTypes[i - 1].getSize(); } transformations.add( - buildConstructorInvocation(owner, desc, lambdaMapping, awaitClassReification) + buildConstructorInvocation( + owner, desc, lambdaMapping, awaitClassReification, capturesAnonymousObjectThatMustBeRegenerated + ) ); awaitClassReification = false; } @@ -538,6 +544,12 @@ public class MethodInliner { return node; } + private boolean isAnonymousClassThatMustBeRegenerated(@Nullable Type type) { + if (type == null || type.getSort() != Type.OBJECT) return false; + AnonymousObjectTransformationInfo info = inliningContext.findAnonymousObjectTransformationInfo(type.getInternalName()); + return info != null && info.shouldRegenerate(true); + } + @NotNull private Frame[] analyzeMethodNodeBeforeInline(@NotNull MethodNode node) { try { @@ -584,7 +596,8 @@ public class MethodInliner { @NotNull String anonymousType, @NotNull String desc, @NotNull Map lambdaMapping, - boolean needReification + boolean needReification, + boolean capturesAnonymousObjectThatMustBeRegenerated ) { boolean memoizeAnonymousObject = inliningContext.findAnonymousObjectTransformationInfo(anonymousType) == null; @@ -594,7 +607,8 @@ public class MethodInliner { isAlreadyRegenerated(anonymousType), desc, false, - inliningContext.nameGenerator + inliningContext.nameGenerator, + capturesAnonymousObjectThatMustBeRegenerated ); if (memoizeAnonymousObject) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TransformationInfo.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TransformationInfo.kt index 3fe22c83549..740faddd7af 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TransformationInfo.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TransformationInfo.kt @@ -68,7 +68,8 @@ class AnonymousObjectTransformationInfo internal constructor( private val alreadyRegenerated: Boolean, val constructorDesc: String?, private val isStaticOrigin: Boolean, - parentNameGenerator: NameGenerator + parentNameGenerator: NameGenerator, + private val capturesAnonymousObjectThatMustBeRegenerated: Boolean = false ) : TransformationInfo { override val nameGenerator by lazy { @@ -93,7 +94,8 @@ class AnonymousObjectTransformationInfo internal constructor( ) : this(ownerInternalName, needReification, hashMapOf(), false, alreadyRegenerated, null, isStaticOrigin, nameGenerator) override fun shouldRegenerate(sameModule: Boolean): Boolean = - !alreadyRegenerated && (!lambdasToInline.isEmpty() || !sameModule || capturedOuterRegenerated || needReification) + !alreadyRegenerated && + (!lambdasToInline.isEmpty() || !sameModule || capturedOuterRegenerated || needReification || capturesAnonymousObjectThatMustBeRegenerated) override fun canRemoveAfterTransformation(): Boolean { // Note: It is unsafe to remove anonymous class that is referenced by GETSTATIC within lambda diff --git a/compiler/testData/codegen/boxInline/anonymousObject/objectInLambdaCapturesAnotherObject.kt b/compiler/testData/codegen/boxInline/anonymousObject/objectInLambdaCapturesAnotherObject.kt new file mode 100644 index 00000000000..165d34191c8 --- /dev/null +++ b/compiler/testData/codegen/boxInline/anonymousObject/objectInLambdaCapturesAnotherObject.kt @@ -0,0 +1,27 @@ +// FILE: 1.kt +package test + +public inline fun myRun(block: () -> Unit) { + return block() +} + +// FILE: 2.kt + +//NO_CHECK_LAMBDA_INLINING +import test.* + +fun box(): String { + var res = "" + myRun { + val x = object { + fun foo() { + res = "OK" + } + } + object { + fun bar() = x.foo() + }.bar() + } + + return res +} diff --git a/compiler/testData/codegen/boxInline/localFunInLambda/lambdaInLambdaCapturesAnotherFun.kt b/compiler/testData/codegen/boxInline/localFunInLambda/lambdaInLambdaCapturesAnotherFun.kt new file mode 100644 index 00000000000..ba32dcfee23 --- /dev/null +++ b/compiler/testData/codegen/boxInline/localFunInLambda/lambdaInLambdaCapturesAnotherFun.kt @@ -0,0 +1,27 @@ +// FILE: 1.kt +package test + +public inline fun myRun(block: () -> Unit) { + return block() +} + +// FILE: 2.kt + +//NO_CHECK_LAMBDA_INLINING +import test.* + +fun box(): String { + var res = "" + myRun { + fun f1() { + res = "OK" + } + val x: () -> Unit = { + f1() + } + + x() + } + + return res +} diff --git a/compiler/testData/codegen/boxInline/localFunInLambda/localFunInLambdaCapturesAnotherFun.kt b/compiler/testData/codegen/boxInline/localFunInLambda/localFunInLambdaCapturesAnotherFun.kt new file mode 100644 index 00000000000..f5ae9ebb3db --- /dev/null +++ b/compiler/testData/codegen/boxInline/localFunInLambda/localFunInLambdaCapturesAnotherFun.kt @@ -0,0 +1,26 @@ +// FILE: 1.kt +package test + +public inline fun myRun(block: () -> Unit) { + return block() +} + +// FILE: 2.kt + +//NO_CHECK_LAMBDA_INLINING +import test.* + +fun box(): String { + var res = "" + myRun { + fun f1() { + res = "OK" + } + fun f2() { + f1() + } + f2() + } + + return res +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java index 7e808128118..269d9edca53 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java @@ -200,6 +200,12 @@ public class BlackBoxInlineCodegenTestGenerated extends AbstractBlackBoxInlineCo doTest(fileName); } + @TestMetadata("objectInLambdaCapturesAnotherObject.kt") + public void testObjectInLambdaCapturesAnotherObject() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/anonymousObject/objectInLambdaCapturesAnotherObject.kt"); + doTest(fileName); + } + @TestMetadata("safeCall.kt") public void testSafeCall() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/anonymousObject/safeCall.kt"); @@ -1151,11 +1157,23 @@ public class BlackBoxInlineCodegenTestGenerated extends AbstractBlackBoxInlineCo KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxInline/localFunInLambda"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("lambdaInLambdaCapturesAnotherFun.kt") + public void testLambdaInLambdaCapturesAnotherFun() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/localFunInLambda/lambdaInLambdaCapturesAnotherFun.kt"); + doTest(fileName); + } + @TestMetadata("localFunInLambda.kt") public void testLocalFunInLambda() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/localFunInLambda/localFunInLambda.kt"); doTest(fileName); } + + @TestMetadata("localFunInLambdaCapturesAnotherFun.kt") + public void testLocalFunInLambdaCapturesAnotherFun() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/localFunInLambda/localFunInLambdaCapturesAnotherFun.kt"); + doTest(fileName); + } } @TestMetadata("compiler/testData/codegen/boxInline/multifileClasses") diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java index 5c323ef5786..b4c5c1a77e5 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java @@ -200,6 +200,12 @@ public class CompileKotlinAgainstInlineKotlinTestGenerated extends AbstractCompi doTest(fileName); } + @TestMetadata("objectInLambdaCapturesAnotherObject.kt") + public void testObjectInLambdaCapturesAnotherObject() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/anonymousObject/objectInLambdaCapturesAnotherObject.kt"); + doTest(fileName); + } + @TestMetadata("safeCall.kt") public void testSafeCall() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/anonymousObject/safeCall.kt"); @@ -1151,11 +1157,23 @@ public class CompileKotlinAgainstInlineKotlinTestGenerated extends AbstractCompi KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxInline/localFunInLambda"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("lambdaInLambdaCapturesAnotherFun.kt") + public void testLambdaInLambdaCapturesAnotherFun() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/localFunInLambda/lambdaInLambdaCapturesAnotherFun.kt"); + doTest(fileName); + } + @TestMetadata("localFunInLambda.kt") public void testLocalFunInLambda() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/localFunInLambda/localFunInLambda.kt"); doTest(fileName); } + + @TestMetadata("localFunInLambdaCapturesAnotherFun.kt") + public void testLocalFunInLambdaCapturesAnotherFun() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/localFunInLambda/localFunInLambdaCapturesAnotherFun.kt"); + doTest(fileName); + } } @TestMetadata("compiler/testData/codegen/boxInline/multifileClasses")