From a5c8e30bb15f121c0f5b9279e3b5c7496b3a1bbc Mon Sep 17 00:00:00 2001 From: Alexander Korepanov Date: Fri, 13 Jan 2023 16:07:00 +0100 Subject: [PATCH] [JS IR] Use a copy of an original inline function for inlining MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If inline function A calls another inline function B, we must use the original version of inline function A for inlining, which doesn’t have inlined function B. Because during the inlining process, we remap all occurrences of inline function A to a temporary copy of function A, and if the function B somehow uses function A (e.g. callable reference), the built IR will have a reference to the temporary function, not the original one. All these things lead to broken cross-module references. This patch saves the original versions of all inline functions before inlining and provides them during the inline process. ^KT-55930 Fixed --- .../common/lower/inline/FunctionInlining.kt | 7 ++- .../kotlin/ir/backend/js/JsLoweringPhases.kt | 16 +++++-- .../kotlin/ir/backend/js/JsMapping.kt | 4 +- .../SaveInlineFunctionsBeforeInlining.kt | 45 +++++++++++++++++++ .../irToJs/JsIrProgramFragment.kt | 3 +- .../kotlin/js/test/BoxJsTestGenerated.java | 6 +++ .../js/test/fir/FirJsBoxTestGenerated.java | 6 +++ .../js/test/ir/IrBoxJsTestGenerated.java | 6 +++ .../box/inline/recursiveDependency.kt | 24 ++++++++++ 9 files changed, 110 insertions(+), 7 deletions(-) create mode 100644 compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/inline/SaveInlineFunctionsBeforeInlining.kt create mode 100644 js/js.translator/testData/box/inline/recursiveDependency.kt diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/inline/FunctionInlining.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/inline/FunctionInlining.kt index 316d1886863..98f0807db1b 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/inline/FunctionInlining.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/inline/FunctionInlining.kt @@ -42,6 +42,7 @@ fun IrExpression.isAdaptedFunctionReference() = interface InlineFunctionResolver { fun getFunctionDeclaration(symbol: IrFunctionSymbol): IrFunction + fun getFunctionSymbol(irFunction: IrFunction): IrFunctionSymbol } fun IrFunction.isTopLevelInPackage(name: String, packageName: String): Boolean { @@ -72,6 +73,10 @@ open class DefaultInlineFunctionResolver(open val context: CommonBackendContext) else -> (symbol.owner as? IrSimpleFunction)?.resolveFakeOverride() ?: symbol.owner } } + + override fun getFunctionSymbol(irFunction: IrFunction): IrFunctionSymbol { + return irFunction.symbol + } } class FunctionInlining( @@ -206,7 +211,7 @@ class FunctionInlining( symbol = irReturnableBlockSymbol, origin = null, statements = newStatements, - inlineFunctionSymbol = callee.symbol + inlineFunctionSymbol = inlineFunctionResolver.getFunctionSymbol(callee) ).apply { transformChildrenVoid(object : IrElementTransformerVoid() { override fun visitReturn(expression: IrReturn): IrExpression { diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsLoweringPhases.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsLoweringPhases.kt index 8e2502a9ee6..d3c299dc1e6 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsLoweringPhases.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsLoweringPhases.kt @@ -254,10 +254,10 @@ private val wrapInlineDeclarationsWithReifiedTypeParametersLowering = makeBodyLo description = "Wrap inline declarations with reified type parameters" ) -private val functionInliningPhase = makeBodyLoweringPhase( - { FunctionInlining(it, it.innerClassesSupport) }, - name = "FunctionInliningPhase", - description = "Perform function inlining", +private val saveInlineFunctionsBeforeInlining = makeDeclarationTransformerPhase( + ::SaveInlineFunctionsBeforeInlining, + name = "SaveInlineFunctionsBeforeInlining", + description = "Save inline function before inlining", prerequisite = setOf( expectDeclarationsRemovingPhase, sharedVariablesLoweringPhase, localClassesInInlineLambdasPhase, localClassesExtractionFromInlineFunctionsPhase, @@ -265,6 +265,13 @@ private val functionInliningPhase = makeBodyLoweringPhase( ) ) +private val functionInliningPhase = makeBodyLoweringPhase( + { FunctionInlining(it, JsInlineFunctionResolver(it), it.innerClassesSupport) }, + name = "FunctionInliningPhase", + description = "Perform function inlining", + prerequisite = setOf(saveInlineFunctionsBeforeInlining) +) + private val copyInlineFunctionBodyLoweringPhase = makeDeclarationTransformerPhase( ::CopyInlineFunctionBodyLowering, name = "CopyInlineFunctionBody", @@ -843,6 +850,7 @@ val loweringList = listOf( localClassesExtractionFromInlineFunctionsPhase, syntheticAccessorLoweringPhase, wrapInlineDeclarationsWithReifiedTypeParametersLowering, + saveInlineFunctionsBeforeInlining, functionInliningPhase, copyInlineFunctionBodyLoweringPhase, removeInlineDeclarationsWithReifiedTypeParametersLoweringPhase, diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsMapping.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsMapping.kt index 2587d1ef160..bb8f9438bfc 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsMapping.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsMapping.kt @@ -33,6 +33,8 @@ class JsMapping : DefaultMapping() { val suspendArityStore = DefaultDelegateFactory.newDeclarationToDeclarationCollectionMapping>() + val inlineFunctionsBeforeInlining = DefaultDelegateFactory.newDeclarationToDeclarationMapping() + // Wasm mappings val wasmJsInteropFunctionToWrapper = DefaultDelegateFactory.newDeclarationToDeclarationMapping() @@ -45,4 +47,4 @@ class JsMapping : DefaultMapping() { val wasmExternalClassToInstanceCheck = DefaultDelegateFactory.newDeclarationToDeclarationMapping() -} \ No newline at end of file +} diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/inline/SaveInlineFunctionsBeforeInlining.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/inline/SaveInlineFunctionsBeforeInlining.kt new file mode 100644 index 00000000000..a8e9189d743 --- /dev/null +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/inline/SaveInlineFunctionsBeforeInlining.kt @@ -0,0 +1,45 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.ir.backend.js.lower.inline + +import org.jetbrains.kotlin.backend.common.DeclarationTransformer +import org.jetbrains.kotlin.backend.common.lower.inline.DefaultInlineFunctionResolver +import org.jetbrains.kotlin.ir.backend.js.JsIrBackendContext +import org.jetbrains.kotlin.ir.declarations.IrDeclaration +import org.jetbrains.kotlin.ir.declarations.IrFunction +import org.jetbrains.kotlin.ir.deepCopyWithVariables +import org.jetbrains.kotlin.ir.symbols.IrFunctionSymbol +import org.jetbrains.kotlin.ir.util.patchDeclarationParents + +internal class SaveInlineFunctionsBeforeInlining(context: JsIrBackendContext) : DeclarationTransformer { + private val inlineFunctionsBeforeInlining = context.mapping.inlineFunctionsBeforeInlining + + override fun transformFlat(declaration: IrDeclaration): List? { + if (declaration is IrFunction && declaration.isInline) { + inlineFunctionsBeforeInlining[declaration] = declaration.deepCopyWithVariables().also { + it.patchDeclarationParents(declaration.parent) + } + } + + return null + } +} + +internal class JsInlineFunctionResolver(context: JsIrBackendContext) : DefaultInlineFunctionResolver(context) { + private val inlineFunctionsBeforeInlining = context.mapping.inlineFunctionsBeforeInlining + private val inlineFunctionsBeforeInliningSymbols = hashMapOf() + + override fun getFunctionDeclaration(symbol: IrFunctionSymbol): IrFunction { + val function = super.getFunctionDeclaration(symbol) + val functionBeforeInlining = inlineFunctionsBeforeInlining[function] ?: return function + inlineFunctionsBeforeInliningSymbols[functionBeforeInlining] = function.symbol + return functionBeforeInlining + } + + override fun getFunctionSymbol(irFunction: IrFunction): IrFunctionSymbol { + return inlineFunctionsBeforeInliningSymbols[irFunction] ?: super.getFunctionSymbol(irFunction) + } +} diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/transformers/irToJs/JsIrProgramFragment.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/transformers/irToJs/JsIrProgramFragment.kt index ec65d909e64..dc8d5eb1076 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/transformers/irToJs/JsIrProgramFragment.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/transformers/irToJs/JsIrProgramFragment.kt @@ -117,7 +117,8 @@ class CrossModuleDependenciesResolver( if (tag in header.optionalCrossModuleImports) { continue } - error("Internal error: cannot find external signature '$tag' for module ${header.moduleName}") + val name = header.nameBindings[tag] ?: "" + error("Internal error: cannot find external signature '$tag' for name '$name' in module ${header.moduleName}") } builder.imports += CrossModuleRef(fromModuleBuilder, tag) diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/BoxJsTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/BoxJsTestGenerated.java index b85b8d5eea4..1dc6d54bd2c 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/BoxJsTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/BoxJsTestGenerated.java @@ -5162,6 +5162,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest { runTest("js/js.translator/testData/box/inline/privateProperty.kt"); } + @Test + @TestMetadata("recursiveDependency.kt") + public void testRecursiveDependency() throws Exception { + runTest("js/js.translator/testData/box/inline/recursiveDependency.kt"); + } + @Test @TestMetadata("rootConstructor.kt") public void testRootConstructor() throws Exception { diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsBoxTestGenerated.java index f0bb07335b4..480ace16cff 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsBoxTestGenerated.java @@ -5784,6 +5784,12 @@ public class FirJsBoxTestGenerated extends AbstractFirJsBoxTest { runTest("js/js.translator/testData/box/inline/privateProperty.kt"); } + @Test + @TestMetadata("recursiveDependency.kt") + public void testRecursiveDependency() throws Exception { + runTest("js/js.translator/testData/box/inline/recursiveDependency.kt"); + } + @Test @TestMetadata("rootConstructor.kt") public void testRootConstructor() throws Exception { diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java index e3fa55218e7..a02926b23f5 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java @@ -5784,6 +5784,12 @@ public class IrBoxJsTestGenerated extends AbstractIrBoxJsTest { runTest("js/js.translator/testData/box/inline/privateProperty.kt"); } + @Test + @TestMetadata("recursiveDependency.kt") + public void testRecursiveDependency() throws Exception { + runTest("js/js.translator/testData/box/inline/recursiveDependency.kt"); + } + @Test @TestMetadata("rootConstructor.kt") public void testRootConstructor() throws Exception { diff --git a/js/js.translator/testData/box/inline/recursiveDependency.kt b/js/js.translator/testData/box/inline/recursiveDependency.kt new file mode 100644 index 00000000000..2e19d71d21b --- /dev/null +++ b/js/js.translator/testData/box/inline/recursiveDependency.kt @@ -0,0 +1,24 @@ +// IGNORE_BACKEND: JS +// MODULE: lib +// FILE: A.kt +inline fun funA(flag: Boolean): String { + if (flag) { + return funB(flag) + } + return "OK" +} + +// FILE: B.kt +inline fun funB(flag: Boolean): String { + val f = ::funA + if (flag) { + return f(false) + } + return "NOT OK" +} + +// MODULE: main(lib) +// FILE: main.kt +fun box(): String { + return funA(true) +}