From 02d9c526e2fd5fe07cb73bff538a651c4756e7b5 Mon Sep 17 00:00:00 2001 From: Mikhael Bogdanov Date: Wed, 2 Jan 2019 14:00:43 +0100 Subject: [PATCH] Proper resort variables on inlining lowered ir closures Original problem is that lowered ir closures doesn't meet inliner expectations about captured variable position in inlining method. E.g.: Call 'foo(valueParam) { capturedParam }' to inline function 'foo' with declaration inline fun foo(valueParam: Foo, inlineParamWithCaptured: Bar.() ->) .... is reorganized through inlining to equivalent call foo(valueParam, capturedParam1, cp2 ...). But lowered closure for lambda parameter has totally different parameters order: fun loweredLambda$x(extensionReceiver, captured1, cp2..., valueParam1, vp2...) So before inlining lowered closure should be transformed to fun loweredLambda$x(extensionReceiver, valueParam1, vp2..., captured1, cp2..) #KT-28547 Fixed --- .../kotlin/codegen/inline/MethodInliner.kt | 13 +++-- .../backend/jvm/codegen/IrInlineCodegen.kt | 42 ++++++++------- .../codegen/box/defaultArguments/kt6382.kt | 1 - .../codegen/box/jvmName/classMembers.kt | 1 - .../codegen/box/jvmStatic/convention.kt | 1 - .../objectVsClassInitialization_kt5291.kt | 1 - .../sideEffects.kt | 1 - .../ranges/forInStringWithCustomIterator.kt | 1 - .../boxInline/callableReference/kt15449.kt | 1 - .../codegen/boxInline/complex/swapAndWith.kt | 1 - .../codegen/boxInline/complex/swapAndWith2.kt | 1 - .../localFunInLambda/defaultParam.kt | 1 - .../localFunInLambda/localFunInLambda.kt | 1 - .../codegen/boxInline/simple/kt28547.kt | 52 +++++++++++++++++++ .../codegen/boxInline/simple/kt28547_2.kt | 21 ++++++++ .../codegen/boxInline/simple/safeCall.kt | 1 - .../BlackBoxInlineCodegenTestGenerated.java | 10 ++++ ...otlinAgainstInlineKotlinTestGenerated.java | 10 ++++ .../IrBlackBoxInlineCodegenTestGenerated.java | 10 ++++ 19 files changed, 135 insertions(+), 35 deletions(-) create mode 100644 compiler/testData/codegen/boxInline/simple/kt28547.kt create mode 100644 compiler/testData/codegen/boxInline/simple/kt28547_2.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt index 535e06be3a3..6d01324e42e 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt @@ -418,12 +418,15 @@ class MethodInliner( private val isInliningLambda = nodeRemapper.isInsideInliningLambda private fun getNewIndex(`var`: Int): Int { - if (inliningContext.isInliningLambda && inliningContext.lambdaInfo is IrExpressionLambda) { + val lambdaInfo = inliningContext.lambdaInfo + if (inliningContext.isInliningLambda && lambdaInfo is IrExpressionLambda) { if (`var` < parameters.argsSizeOnStack) { - if (`var` < capturedParamsSize) { - return `var` + realParametersSize - } - else { + val capturedParamsStartIndex = + if (lambdaInfo.isExtensionLambda) lambdaInfo.invokeMethod.argumentTypes[0].size else 0 //shift by extension + val capturedParamsEndIndex = capturedParamsSize + capturedParamsStartIndex - 1 + if (`var` in capturedParamsStartIndex..capturedParamsEndIndex) { + return `var` + realParametersSize - capturedParamsStartIndex //subtract extension + } else if (`var` >= capturedParamsStartIndex) { return `var` - capturedParamsSize } } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrInlineCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrInlineCodegen.kt index eb17f34a0ad..6675450518b 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrInlineCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrInlineCodegen.kt @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.backend.jvm.codegen +import org.jetbrains.kotlin.builtins.isExtensionFunctionType import org.jetbrains.kotlin.codegen.Callable import org.jetbrains.kotlin.codegen.JvmKotlinType import org.jetbrains.kotlin.codegen.StackValue @@ -34,9 +35,8 @@ class IrInlineCodegen( val lambdaInfo = next as IrExpressionLambda activeLambda = lambdaInfo - val argumentTypes = lambdaInfo.loweredMethod.argumentTypes lambdaInfo.reference.getArguments().forEachIndexed { index, (_, ir) -> - putCapturedValueOnStack(ir, argumentTypes[index], index) + putCapturedValueOnStack(ir, lambdaInfo.capturedParamsInDesc[index], index) } activeLambda = null } @@ -70,10 +70,10 @@ class IrInlineCodegen( putArgumentOrCapturedToLocalVal(JvmKotlinType(value.type, value.kotlinType), value, -1, parameterIndex, ValueKind.CAPTURED /*kind*/) } - private fun putCapturedValueOnStack(argumentExpression: IrExpression, valueType: Type, capturedParamindex: Int) { + private fun putCapturedValueOnStack(argumentExpression: IrExpression, valueType: Type, capturedParamIndex: Int) { val onStack = codegen.gen(argumentExpression, valueType, BlockInfo.create()) putArgumentOrCapturedToLocalVal( - JvmKotlinType(onStack.type, onStack.kotlinType), onStack, capturedParamindex, capturedParamindex, ValueKind.CAPTURED + JvmKotlinType(onStack.type, onStack.kotlinType), onStack, capturedParamIndex, capturedParamIndex, ValueKind.CAPTURED ) } @@ -93,10 +93,10 @@ class IrInlineCodegen( private fun rememberClosure(irReference: IrFunctionReference, type: Type, parameter: ValueParameterDescriptor): LambdaInfo { //assert(InlineUtil.isInlinableParameterExpression(ktLambda)) { "Couldn't find inline expression in ${expression.text}" } - val expression = irReference.symbol.owner as IrFunction return IrExpressionLambda( - irReference, expression, typeMapper, parameter.isCrossinline, false/*TODO*/ + irReference, expression, typeMapper, parameter.isCrossinline, false/*TODO*/, + parameter.type.isExtensionFunctionType ).also { lambda -> val closureInfo = invocationParamBuilder.addNextValueParameter(type, true, null, parameter.index) closureInfo.lambda = lambda @@ -110,7 +110,8 @@ class IrExpressionLambda( val function: IrFunction, typeMapper: KotlinTypeMapper, isCrossInline: Boolean, - override val isBoundCallableReference: Boolean + override val isBoundCallableReference: Boolean, + val isExtensionLambda: Boolean ) : ExpressionLambda(typeMapper, isCrossInline) { override fun isMyLabel(name: String): Boolean { @@ -118,30 +119,35 @@ class IrExpressionLambda( return false } - override val lambdaClassType: Type - get() = Type.getObjectType("test123") + override val lambdaClassType: Type = Type.getObjectType("test123") - override val capturedVars: List by lazy { + override val capturedVars: List = arrayListOf().apply { reference.getArguments().forEachIndexed { _, (_, ir) -> val getValue = ir as? IrGetValue ?: error("Unrecognized expression: $ir") add(capturedParamDesc(getValue.descriptor.name.asString(), typeMapper.mapType(getValue.descriptor.type))) } } - } - val loweredMethod: Method - get() = typeMapper.mapAsmMethod(function.descriptor) + private val loweredMethod = typeMapper.mapAsmMethod(function.descriptor) + + val capturedParamsInDesc: List = + loweredMethod.argumentTypes.drop(if (isExtensionLambda) 1 else 0).take(capturedVars.size) override val invokeMethod: Method = loweredMethod.let { - Method(it.name, it.returnType, it.argumentTypes.drop(capturedVars.size).toTypedArray()) + Method( + it.name, + it.returnType, + ( + (if (isExtensionLambda) it.argumentTypes.take(1) else emptyList()) + + it.argumentTypes.drop((if (isExtensionLambda) 1 else 0) + capturedVars.size) + ).toTypedArray() + ) } - override val invokeMethodDescriptor: FunctionDescriptor - get() = function.descriptor + override val invokeMethodDescriptor: FunctionDescriptor = function.descriptor - override val hasDispatchReceiver: Boolean - get() = false + override val hasDispatchReceiver: Boolean = false } fun isInlineIrExpression(argumentExpression: IrExpression) = diff --git a/compiler/testData/codegen/box/defaultArguments/kt6382.kt b/compiler/testData/codegen/box/defaultArguments/kt6382.kt index d7ce755d751..07910e2cf12 100644 --- a/compiler/testData/codegen/box/defaultArguments/kt6382.kt +++ b/compiler/testData/codegen/box/defaultArguments/kt6382.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // WITH_RUNTIME fun box(): String { diff --git a/compiler/testData/codegen/box/jvmName/classMembers.kt b/compiler/testData/codegen/box/jvmName/classMembers.kt index cfac05164df..9451adf4272 100644 --- a/compiler/testData/codegen/box/jvmName/classMembers.kt +++ b/compiler/testData/codegen/box/jvmName/classMembers.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // TARGET_BACKEND: JVM // WITH_RUNTIME diff --git a/compiler/testData/codegen/box/jvmStatic/convention.kt b/compiler/testData/codegen/box/jvmStatic/convention.kt index 2408a6ae49f..32d196b54cb 100644 --- a/compiler/testData/codegen/box/jvmStatic/convention.kt +++ b/compiler/testData/codegen/box/jvmStatic/convention.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // TARGET_BACKEND: JVM // WITH_RUNTIME diff --git a/compiler/testData/codegen/box/objects/objectVsClassInitialization_kt5291.kt b/compiler/testData/codegen/box/objects/objectVsClassInitialization_kt5291.kt index 86cd1babbab..88ec3c5bb6e 100644 --- a/compiler/testData/codegen/box/objects/objectVsClassInitialization_kt5291.kt +++ b/compiler/testData/codegen/box/objects/objectVsClassInitialization_kt5291.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR public inline fun T.with(f: T.() -> Unit): T { this.f() return this diff --git a/compiler/testData/codegen/box/properties/lateinit/isInitializedAndDeinitialize/sideEffects.kt b/compiler/testData/codegen/box/properties/lateinit/isInitializedAndDeinitialize/sideEffects.kt index 56b494e09e4..c0717a7ae89 100644 --- a/compiler/testData/codegen/box/properties/lateinit/isInitializedAndDeinitialize/sideEffects.kt +++ b/compiler/testData/codegen/box/properties/lateinit/isInitializedAndDeinitialize/sideEffects.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // WITH_RUNTIME class Foo { diff --git a/compiler/testData/codegen/box/ranges/forInStringWithCustomIterator.kt b/compiler/testData/codegen/box/ranges/forInStringWithCustomIterator.kt index 301c0fe4c71..e7853e62294 100644 --- a/compiler/testData/codegen/box/ranges/forInStringWithCustomIterator.kt +++ b/compiler/testData/codegen/box/ranges/forInStringWithCustomIterator.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // TARGET_BACKEND: JVM // WITH_RUNTIME diff --git a/compiler/testData/codegen/boxInline/callableReference/kt15449.kt b/compiler/testData/codegen/boxInline/callableReference/kt15449.kt index a16864330be..7d537bbffa9 100644 --- a/compiler/testData/codegen/boxInline/callableReference/kt15449.kt +++ b/compiler/testData/codegen/boxInline/callableReference/kt15449.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/complex/swapAndWith.kt b/compiler/testData/codegen/boxInline/complex/swapAndWith.kt index 29e08dc3af0..0c1f2a7c6ed 100644 --- a/compiler/testData/codegen/boxInline/complex/swapAndWith.kt +++ b/compiler/testData/codegen/boxInline/complex/swapAndWith.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // NO_CHECK_LAMBDA_INLINING // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/complex/swapAndWith2.kt b/compiler/testData/codegen/boxInline/complex/swapAndWith2.kt index 5e926f04486..bd914e909a6 100644 --- a/compiler/testData/codegen/boxInline/complex/swapAndWith2.kt +++ b/compiler/testData/codegen/boxInline/complex/swapAndWith2.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/localFunInLambda/defaultParam.kt b/compiler/testData/codegen/boxInline/localFunInLambda/defaultParam.kt index 314302a9652..aaa80a83e67 100644 --- a/compiler/testData/codegen/boxInline/localFunInLambda/defaultParam.kt +++ b/compiler/testData/codegen/boxInline/localFunInLambda/defaultParam.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt inline fun run(c: () -> T): T = c() diff --git a/compiler/testData/codegen/boxInline/localFunInLambda/localFunInLambda.kt b/compiler/testData/codegen/boxInline/localFunInLambda/localFunInLambda.kt index ff9022721d3..aef72d1f305 100644 --- a/compiler/testData/codegen/boxInline/localFunInLambda/localFunInLambda.kt +++ b/compiler/testData/codegen/boxInline/localFunInLambda/localFunInLambda.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt // WITH_RUNTIME package test diff --git a/compiler/testData/codegen/boxInline/simple/kt28547.kt b/compiler/testData/codegen/boxInline/simple/kt28547.kt new file mode 100644 index 00000000000..578689dd41c --- /dev/null +++ b/compiler/testData/codegen/boxInline/simple/kt28547.kt @@ -0,0 +1,52 @@ +// FILE: 1.kt +package test + +class C { + var inserting: Boolean = false + fun nextSlot(): Any? = null + fun startNode(key: Any?) {} + fun endNode() {} + fun emitNode(node: Any?) {} + fun useNode(): Any? = null + fun skipValue() {} + fun updateValue(value: Any?) {} +} + +class B(val composer: C, val node: T) { + inline fun bar(value: V, block: T.(V) -> Unit) = with(composer) { + if (inserting || nextSlot() != value) { + updateValue(value) + node.block(value) + } else skipValue() + } +} + +class A(val composer: C) { + inline fun foo(key: Any, ctor: () -> T, update: B.() -> Unit) = with(composer) { + startNode(key) + val node = if (inserting) + ctor().also { emitNode(it) } + else useNode() as T + B(this, node).update() + endNode() + } +} + +// FILE: 2.kt +import test.* + +fun box(): String { + val a = A(C()) + val str = "OK" + var result = "fail" + a.foo( + 123, + { "abc" }, + { + bar(str) { } + result = "OK" + } + ) + + return result +} \ No newline at end of file diff --git a/compiler/testData/codegen/boxInline/simple/kt28547_2.kt b/compiler/testData/codegen/boxInline/simple/kt28547_2.kt new file mode 100644 index 00000000000..93f65f7d5e5 --- /dev/null +++ b/compiler/testData/codegen/boxInline/simple/kt28547_2.kt @@ -0,0 +1,21 @@ +// FILE: 1.kt + +package test + +inline fun Double.run(body : Double.() -> String): String { + return this.body() +} + +// FILE: 2.kt + +import test.* + +fun box(): String { + var captured = "fail" + return 1.0.run { + if (this == 1.0) { + "OK" + } else captured + } + +} diff --git a/compiler/testData/codegen/boxInline/simple/safeCall.kt b/compiler/testData/codegen/boxInline/simple/safeCall.kt index 4bea1f5009a..a89a92c1ad6 100644 --- a/compiler/testData/codegen/boxInline/simple/safeCall.kt +++ b/compiler/testData/codegen/boxInline/simple/safeCall.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java index bd9894ab702..e32768d52bd 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java @@ -2813,6 +2813,16 @@ public class BlackBoxInlineCodegenTestGenerated extends AbstractBlackBoxInlineCo runTest("compiler/testData/codegen/boxInline/simple/kt17431.kt"); } + @TestMetadata("kt28547.kt") + public void testKt28547() throws Exception { + runTest("compiler/testData/codegen/boxInline/simple/kt28547.kt"); + } + + @TestMetadata("kt28547_2.kt") + public void testKt28547_2() throws Exception { + runTest("compiler/testData/codegen/boxInline/simple/kt28547_2.kt"); + } + @TestMetadata("params.kt") public void testParams() throws Exception { runTest("compiler/testData/codegen/boxInline/simple/params.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java index bb3c12e23a3..37fbfa719d0 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java @@ -2813,6 +2813,16 @@ public class CompileKotlinAgainstInlineKotlinTestGenerated extends AbstractCompi runTest("compiler/testData/codegen/boxInline/simple/kt17431.kt"); } + @TestMetadata("kt28547.kt") + public void testKt28547() throws Exception { + runTest("compiler/testData/codegen/boxInline/simple/kt28547.kt"); + } + + @TestMetadata("kt28547_2.kt") + public void testKt28547_2() throws Exception { + runTest("compiler/testData/codegen/boxInline/simple/kt28547_2.kt"); + } + @TestMetadata("params.kt") public void testParams() throws Exception { runTest("compiler/testData/codegen/boxInline/simple/params.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java index 4f26856960b..0d7bb615f4f 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java @@ -2813,6 +2813,16 @@ public class IrBlackBoxInlineCodegenTestGenerated extends AbstractIrBlackBoxInli runTest("compiler/testData/codegen/boxInline/simple/kt17431.kt"); } + @TestMetadata("kt28547.kt") + public void testKt28547() throws Exception { + runTest("compiler/testData/codegen/boxInline/simple/kt28547.kt"); + } + + @TestMetadata("kt28547_2.kt") + public void testKt28547_2() throws Exception { + runTest("compiler/testData/codegen/boxInline/simple/kt28547_2.kt"); + } + @TestMetadata("params.kt") public void testParams() throws Exception { runTest("compiler/testData/codegen/boxInline/simple/params.kt");