From cb505d11011dafaaced064c20d120686e9bc085d Mon Sep 17 00:00:00 2001 From: pyos Date: Tue, 21 Sep 2021 12:16:51 +0200 Subject: [PATCH] IR: keep capture sets the same when copying objects for tailrec funs tailrec f(x: () -> T = { y }, y: T = ...) = f() -- at the call we know that in `x` the observed value of `y` is `null`, but the constructor should still have a single parameter. --- .../FirBlackBoxCodegenTestGenerated.java | 6 ++++ .../backend/common/lower/TailrecLowering.kt | 32 +++++++++---------- .../useNextParamInLambdaTailrec.kt | 9 ++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++++ .../IrBlackBoxCodegenTestGenerated.java | 6 ++++ .../LightAnalysisModeTestGenerated.java | 5 +++ .../IrJsCodegenBoxES6TestGenerated.java | 5 +++ .../IrJsCodegenBoxTestGenerated.java | 5 +++ .../semantics/JsCodegenBoxTestGenerated.java | 5 +++ .../IrCodegenBoxWasmTestGenerated.java | 5 +++ 10 files changed, 67 insertions(+), 17 deletions(-) create mode 100644 compiler/testData/codegen/box/defaultArguments/useNextParamInLambdaTailrec.kt diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java index 89fc66c70f7..0a01a68f379 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java @@ -13090,6 +13090,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambda.kt"); } + @Test + @TestMetadata("useNextParamInLambdaTailrec.kt") + public void testUseNextParamInLambdaTailrec() throws Exception { + runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambdaTailrec.kt"); + } + @Test @TestMetadata("useThisInLambda.kt") public void testUseThisInLambda() throws Exception { diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/TailrecLowering.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/TailrecLowering.kt index 62497d12b63..c9cc42f604d 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/TailrecLowering.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/TailrecLowering.kt @@ -21,6 +21,7 @@ import org.jetbrains.kotlin.backend.common.BodyLoweringPass import org.jetbrains.kotlin.backend.common.collectTailRecursionCalls import org.jetbrains.kotlin.backend.common.deepCopyWithVariables import org.jetbrains.kotlin.ir.IrElement +import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET import org.jetbrains.kotlin.ir.builders.* import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.expressions.* @@ -156,25 +157,22 @@ private class BodyTransformer( // that captures another parameter, it won't capture it as a mutable ref. parameter to irTemporary(argument) } - - // For each unspecified argument set the corresponding variable to default: - parameters - .filter { it !in parameterToArgument } - .let { if (properComputationOrderOfTailrecDefaultParameters) it else it.asReversed() } + // Create new null-initialized variables for all other values in case of forward references: + // fun f(x: () -> T = { y }, y: T = ...) // in `f()`, `x()` returns `null` + val defaultValuedParameters = parameters.filter { it !in parameterToArgument } + defaultValuedParameters.associateWithTo(parameterToArgument) { + // Note that we intentionally keep the original type of the parameter for the variable even though that violates type safety + // if it's non-null. This ensures that capture parameters have the same types for all copies of `x`. + // TODO: on the JVM, this might not be correct for inline classes - copy code from JvmDefaultParameterInjector? + irTemporary(IrConstImpl.defaultValueForType(UNDEFINED_OFFSET, UNDEFINED_OFFSET, it.type.makeNullable()), irType = it.type) + } + // Now replace those variables with ones containing actual default values. Unused null-valued temporaries will hopefully + // be optimized out later. + val remapper = VariableRemapper(parameterToArgument) + defaultValuedParameters.let { if (properComputationOrderOfTailrecDefaultParameters) it else it.asReversed() } .associateWithTo(parameterToArgument) { parameter -> val originalDefaultValue = parameter.defaultValue?.expression ?: throw Error("no argument specified for $parameter") - // Copy default value, mapping parameters to variables containing freshly computed arguments: - val defaultValue = originalDefaultValue - .deepCopyWithVariables().patchDeclarationParents(parent) - .transform(object : VariableRemapper(parameterToArgument) { - override fun visitGetValue(expression: IrGetValue): IrExpression { - // If this parameter references a different parameter declared later, produce null: - if (expression.symbol.owner.let { it is IrValueParameter && it.parent == irFunction && it !in parameterToArgument }) - return IrConstImpl.defaultValueForType(startOffset, endOffset, expression.type.makeNullable()) - return super.visitGetValue(expression) - } - }, null) - irTemporary(defaultValue) + irTemporary(originalDefaultValue.deepCopyWithVariables().patchDeclarationParents(parent).transform(remapper, null)) } // Copy the new `val`s into the `var`s declared outside the loop: diff --git a/compiler/testData/codegen/box/defaultArguments/useNextParamInLambdaTailrec.kt b/compiler/testData/codegen/box/defaultArguments/useNextParamInLambdaTailrec.kt new file mode 100644 index 00000000000..7a33b8fa0dc --- /dev/null +++ b/compiler/testData/codegen/box/defaultArguments/useNextParamInLambdaTailrec.kt @@ -0,0 +1,9 @@ +// IGNORE_BACKEND: JVM +// IGNORE_BACKEND_FIR: JVM_IR +tailrec fun foo(x: () -> String? = { y }, y: String = "fail"): String? { + if (y == "start") + return foo() + return x() +} + +fun box() = foo(y = "start") ?: "OK" diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java index 1fecf8fc3f8..000da7d2e36 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java @@ -13012,6 +13012,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambda.kt"); } + @Test + @TestMetadata("useNextParamInLambdaTailrec.kt") + public void testUseNextParamInLambdaTailrec() throws Exception { + runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambdaTailrec.kt"); + } + @Test @TestMetadata("useThisInLambda.kt") public void testUseThisInLambda() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index afc0f5b5900..759abc5fa59 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -13090,6 +13090,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambda.kt"); } + @Test + @TestMetadata("useNextParamInLambdaTailrec.kt") + public void testUseNextParamInLambdaTailrec() throws Exception { + runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambdaTailrec.kt"); + } + @Test @TestMetadata("useThisInLambda.kt") public void testUseThisInLambda() throws Exception { diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 1304d78f581..1c1f6ed4609 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -10415,6 +10415,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) public static class DefaultArguments extends AbstractLightAnalysisModeTest { + @TestMetadata("useNextParamInLambdaTailrec.kt") + public void ignoreUseNextParamInLambdaTailrec() throws Exception { + runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambdaTailrec.kt"); + } + private void runTest(String testDataFilePath) throws Exception { KotlinTestUtils.runTest(this::doTest, TargetBackend.JVM, testDataFilePath); } diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java index fcc55570328..9986fd489ea 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java @@ -9362,6 +9362,11 @@ public class IrJsCodegenBoxES6TestGenerated extends AbstractIrJsCodegenBoxES6Tes runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambda.kt"); } + @TestMetadata("useNextParamInLambdaTailrec.kt") + public void testUseNextParamInLambdaTailrec() throws Exception { + runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambdaTailrec.kt"); + } + @TestMetadata("useThisInLambda.kt") public void testUseThisInLambda() throws Exception { runTest("compiler/testData/codegen/box/defaultArguments/useThisInLambda.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java index 1f1adfdda53..e94e6835c81 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java @@ -8768,6 +8768,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambda.kt"); } + @TestMetadata("useNextParamInLambdaTailrec.kt") + public void testUseNextParamInLambdaTailrec() throws Exception { + runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambdaTailrec.kt"); + } + @TestMetadata("useThisInLambda.kt") public void testUseThisInLambda() throws Exception { runTest("compiler/testData/codegen/box/defaultArguments/useThisInLambda.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index 559d91a3745..af04a71fb4d 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -8733,6 +8733,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambda.kt"); } + @TestMetadata("useNextParamInLambdaTailrec.kt") + public void testUseNextParamInLambdaTailrec() throws Exception { + runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambdaTailrec.kt"); + } + @TestMetadata("useThisInLambda.kt") public void testUseThisInLambda() throws Exception { runTest("compiler/testData/codegen/box/defaultArguments/useThisInLambda.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java index a4a2744f0ff..5613a5a7ab7 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java @@ -4914,6 +4914,11 @@ public class IrCodegenBoxWasmTestGenerated extends AbstractIrCodegenBoxWasmTest runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambda.kt"); } + @TestMetadata("useNextParamInLambdaTailrec.kt") + public void testUseNextParamInLambdaTailrec() throws Exception { + runTest("compiler/testData/codegen/box/defaultArguments/useNextParamInLambdaTailrec.kt"); + } + @TestMetadata("useThisInLambda.kt") public void testUseThisInLambda() throws Exception { runTest("compiler/testData/codegen/box/defaultArguments/useThisInLambda.kt");