From 91ef053fbce19db22c346526a875e4ed492bf292 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 4 May 2020 17:39:26 +0200 Subject: [PATCH] IR: keep local scope with counter maps across LDL invocations Since LocalDeclarationsLowering is a BodyLoweringPass, local functions inside one declaration are handled independently of local functions in the other declaration. This can lead to name clashes, in case a local function with the same name and signature is declared in overloads in the same container, which results in a signature clash error in JVM IR. The issue became more common with the introduction of adapted function references, where psi2ir generates a local adapter-function with a predefined name, which can easily clash with another reference to the same target in an overload. This led to a compilation error when bootstrapping Kotlin with JVM IR, for example in GradleIRBuilder.kt where there are a lot of references to the same function. --- .../ir/FirBlackBoxCodegenTestGenerated.java | 10 ++++ .../jetbrains/kotlin/backend/common/ir/Ir.kt | 3 ++ .../common/lower/LocalDeclarationsLowering.kt | 47 ++++++++-------- .../noNameClashForReferencesToSameFunction.kt | 26 +++++++++ .../nameClashAcrossDifferentContainers.kt | 53 +++++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 10 ++++ .../LightAnalysisModeTestGenerated.java | 10 ++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 10 ++++ .../IrJsCodegenBoxTestGenerated.java | 10 ++++ .../semantics/JsCodegenBoxTestGenerated.java | 10 ++++ 10 files changed, 166 insertions(+), 23 deletions(-) create mode 100644 compiler/testData/codegen/box/callableReference/adaptedReferences/noNameClashForReferencesToSameFunction.kt create mode 100644 compiler/testData/codegen/box/functions/localFunctions/nameClashAcrossDifferentContainers.kt diff --git a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java index 4993cc70c26..f9af889adb7 100644 --- a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java @@ -2075,6 +2075,11 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noAdaptedReferencesIfNoOptimizedReferencesEnabled.kt"); } + @TestMetadata("noNameClashForReferencesToSameFunction.kt") + public void testNoNameClashForReferencesToSameFunction() throws Exception { + runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noNameClashForReferencesToSameFunction.kt"); + } + @TestMetadata("noReflectionForAdaptedCallableReferences.kt") public void testNoReflectionForAdaptedCallableReferences() throws Exception { runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noReflectionForAdaptedCallableReferences.kt"); @@ -11747,6 +11752,11 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/functions/localFunctions/nameClash.kt"); } + @TestMetadata("nameClashAcrossDifferentContainers.kt") + public void testNameClashAcrossDifferentContainers() throws Exception { + runTest("compiler/testData/codegen/box/functions/localFunctions/nameClashAcrossDifferentContainers.kt"); + } + @TestMetadata("overloadedLocalFunWithoutClosure.kt") public void testOverloadedLocalFunWithoutClosure() throws Exception { runTest("compiler/testData/codegen/box/functions/localFunctions/overloadedLocalFunWithoutClosure.kt"); diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/Ir.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/Ir.kt index b4b181900ea..f01bdc2c205 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/Ir.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/Ir.kt @@ -6,6 +6,7 @@ package org.jetbrains.kotlin.backend.common.ir import org.jetbrains.kotlin.backend.common.CommonBackendContext +import org.jetbrains.kotlin.backend.common.lower.LocalDeclarationsLowering import org.jetbrains.kotlin.builtins.* import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.SimpleFunctionDescriptor @@ -34,6 +35,8 @@ abstract class Ir(val context: T, val irModule: Ir val defaultParameterDeclarationsCache = mutableMapOf() + internal val localScopeWithCounterMap = LocalDeclarationsLowering.LocalScopeWithCounterMap() + // If irType is an inline class type, return the underlying type according to the // unfolding rules of the current backend. Otherwise, returns null. open fun unfoldInlineClassType(irType: IrType): IrType? = null diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/LocalDeclarationsLowering.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/LocalDeclarationsLowering.kt index 8a3df356063..7ed35fd32d2 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/LocalDeclarationsLowering.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/LocalDeclarationsLowering.kt @@ -39,12 +39,15 @@ import org.jetbrains.kotlin.ir.types.impl.IrSimpleTypeImpl import org.jetbrains.kotlin.ir.types.impl.IrTypeAbbreviationImpl import org.jetbrains.kotlin.ir.types.impl.IrUninitializedType import org.jetbrains.kotlin.ir.types.impl.makeTypeProjection -import org.jetbrains.kotlin.ir.util.* +import org.jetbrains.kotlin.ir.util.constructedClass +import org.jetbrains.kotlin.ir.util.file +import org.jetbrains.kotlin.ir.util.parentAsClass import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid import org.jetbrains.kotlin.ir.visitors.acceptVoid import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.utils.addToStdlib.safeAs import java.util.* interface LocalNameProvider { @@ -117,19 +120,22 @@ class LocalDeclarationsLowering( LocalDeclarationsTransformer(irElement, container, classesToLower).lowerLocalDeclarations() } - private class ScopeWithCounter(scope: Scope, irElement: IrElement) : ScopeWithIr(scope, irElement) { + internal class ScopeWithCounter(scope: Scope, irElement: IrElement) : ScopeWithIr(scope, irElement) { // Continuous numbering across all declarations in the container. var counter: Int = 0 + val usedLocalFunctionNames: MutableSet = hashSetOf() + } + + internal class LocalScopeWithCounterMap { + val scopeMap: MutableMap = hashMapOf() } - private val scopeMap: MutableMap = mutableMapOf() // Need to keep LocalFunctionContext.index private val IrSymbolOwner.scopeWithCounter: ScopeWithCounter - get() = scopeMap.getOrPut(this) { + get() = context.ir.localScopeWithCounterMap.scopeMap.getOrPut(this) { ScopeWithCounter(Scope(symbol), this) } - private abstract class LocalContext { val capturedTypeParameterToTypeParameter: MutableMap = mutableMapOf() @@ -229,7 +235,6 @@ class LocalDeclarationsLowering( val localFunctions: MutableMap = LinkedHashMap() val localClasses: MutableMap = LinkedHashMap() val localClassConstructors: MutableMap = LinkedHashMap() - val usedLocalFunctionNames: MutableSet = mutableSetOf() val transformedDeclarations = mutableMapOf() @@ -832,19 +837,17 @@ class LocalDeclarationsLowering( } private fun collectLocalDeclarations() { - val enclosingFileScope = container.file.scopeWithCounter - - val enclosingClassScope = run { + val enclosingFile = container.file + val enclosingClass = run { var currentParent = container as? IrClass ?: container.parent while (currentParent is IrDeclaration && currentParent !is IrClass) { currentParent = currentParent.parent } currentParent as? IrClass - }?.scopeWithCounter + } irElement.acceptVoid(object : IrElementVisitorVoidWithContext() { - override fun visitElement(element: IrElement) { element.acceptChildrenVoid(this) } @@ -866,18 +869,16 @@ class LocalDeclarationsLowering( super.visitSimpleFunction(declaration) if (declaration.visibility == Visibilities.LOCAL) { - val scopeWithIr = - (currentClass ?: enclosingClassScope ?: enclosingFileScope /*file is required for K/N cause file declarations are not split by classes*/ - ?: error("No scope for ${declaration.dump()}")) + val enclosingScope = + // File is required for K/N because file declarations are not split by classes. + currentClass ?: enclosingClass?.scopeWithCounter ?: enclosingFile.scopeWithCounter + val index = + if (enclosingScope is ScopeWithCounter && + (declaration.name.isSpecial || declaration.name in enclosingScope.usedLocalFunctionNames) + ) enclosingScope.counter++ else -1 localFunctions[declaration] = - LocalFunctionContext( - declaration, - if (declaration.name.isSpecial || declaration.name in usedLocalFunctionNames) - (scopeWithIr as ScopeWithCounter).counter++ - else -1, - scopeWithIr.irElement as IrDeclarationContainer - ) - usedLocalFunctionNames.add(declaration.name) + LocalFunctionContext(declaration, index, enclosingScope.irElement as IrDeclarationContainer) + (enclosingScope as? ScopeWithCounter)?.usedLocalFunctionNames?.add(declaration.name) } } @@ -909,4 +910,4 @@ class LocalDeclarationsLowering( } // Local inner classes capture anything through outer -internal fun IrClass.isLocalNotInner(): Boolean = visibility == Visibilities.LOCAL && !isInner \ No newline at end of file +internal fun IrClass.isLocalNotInner(): Boolean = visibility == Visibilities.LOCAL && !isInner diff --git a/compiler/testData/codegen/box/callableReference/adaptedReferences/noNameClashForReferencesToSameFunction.kt b/compiler/testData/codegen/box/callableReference/adaptedReferences/noNameClashForReferencesToSameFunction.kt new file mode 100644 index 00000000000..2a0088ba780 --- /dev/null +++ b/compiler/testData/codegen/box/callableReference/adaptedReferences/noNameClashForReferencesToSameFunction.kt @@ -0,0 +1,26 @@ +// IGNORE_BACKEND_FIR: JVM_IR + +var result = "" + +class C(val token: String) { + fun target(): Int { + result += token + return 42 + } +} + +fun adapt(f: () -> Unit): Unit = f() + +fun overload() { + adapt(C("O")::target) +} + +fun overload(unused: String) { + adapt(C("K")::target) +} + +fun box(): String { + overload() + overload("") + return result +} diff --git a/compiler/testData/codegen/box/functions/localFunctions/nameClashAcrossDifferentContainers.kt b/compiler/testData/codegen/box/functions/localFunctions/nameClashAcrossDifferentContainers.kt new file mode 100644 index 00000000000..9033dbb2c9e --- /dev/null +++ b/compiler/testData/codegen/box/functions/localFunctions/nameClashAcrossDifferentContainers.kt @@ -0,0 +1,53 @@ +// IGNORE_BACKEND_FIR: JVM_IR + +var result = "" + +fun overload() { + fun local() { + result += "1" + } + local() +} + +fun overload(unused: String) { + fun local() { + result += "2" + } + local() + if ("".length < 1) { + fun local() { + result += "3" + } + local() + } +} + +class C { + fun overload() { + fun local() { + result += "4" + } + local() + } + + fun overload(unused: String) { + fun local() { + result += "5" + } + local() + if ("".length < 1) { + fun local() { + result += "6" + } + local() + } + } +} + +fun box(): String { + overload() + overload("") + C().overload() + C().overload("") + return if (result == "123456") "OK" else "Fail: $result" +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 2a4362b0574..ea50ad73169 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -2095,6 +2095,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noAdaptedReferencesIfNoOptimizedReferencesEnabled.kt"); } + @TestMetadata("noNameClashForReferencesToSameFunction.kt") + public void testNoNameClashForReferencesToSameFunction() throws Exception { + runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noNameClashForReferencesToSameFunction.kt"); + } + @TestMetadata("noReflectionForAdaptedCallableReferences.kt") public void testNoReflectionForAdaptedCallableReferences() throws Exception { runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noReflectionForAdaptedCallableReferences.kt"); @@ -12962,6 +12967,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/functions/localFunctions/nameClash.kt"); } + @TestMetadata("nameClashAcrossDifferentContainers.kt") + public void testNameClashAcrossDifferentContainers() throws Exception { + runTest("compiler/testData/codegen/box/functions/localFunctions/nameClashAcrossDifferentContainers.kt"); + } + @TestMetadata("overloadedLocalFunWithoutClosure.kt") public void testOverloadedLocalFunWithoutClosure() throws Exception { runTest("compiler/testData/codegen/box/functions/localFunctions/overloadedLocalFunWithoutClosure.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 93adcac5829..137497de04c 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -2095,6 +2095,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noAdaptedReferencesIfNoOptimizedReferencesEnabled.kt"); } + @TestMetadata("noNameClashForReferencesToSameFunction.kt") + public void testNoNameClashForReferencesToSameFunction() throws Exception { + runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noNameClashForReferencesToSameFunction.kt"); + } + @TestMetadata("noReflectionForAdaptedCallableReferences.kt") public void testNoReflectionForAdaptedCallableReferences() throws Exception { runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noReflectionForAdaptedCallableReferences.kt"); @@ -12962,6 +12967,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/functions/localFunctions/nameClash.kt"); } + @TestMetadata("nameClashAcrossDifferentContainers.kt") + public void testNameClashAcrossDifferentContainers() throws Exception { + runTest("compiler/testData/codegen/box/functions/localFunctions/nameClashAcrossDifferentContainers.kt"); + } + @TestMetadata("overloadedLocalFunWithoutClosure.kt") public void testOverloadedLocalFunWithoutClosure() throws Exception { runTest("compiler/testData/codegen/box/functions/localFunctions/overloadedLocalFunWithoutClosure.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 6554955a6b0..7e16d706efb 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -2075,6 +2075,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noAdaptedReferencesIfNoOptimizedReferencesEnabled.kt"); } + @TestMetadata("noNameClashForReferencesToSameFunction.kt") + public void testNoNameClashForReferencesToSameFunction() throws Exception { + runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noNameClashForReferencesToSameFunction.kt"); + } + @TestMetadata("noReflectionForAdaptedCallableReferences.kt") public void testNoReflectionForAdaptedCallableReferences() throws Exception { runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noReflectionForAdaptedCallableReferences.kt"); @@ -11747,6 +11752,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/functions/localFunctions/nameClash.kt"); } + @TestMetadata("nameClashAcrossDifferentContainers.kt") + public void testNameClashAcrossDifferentContainers() throws Exception { + runTest("compiler/testData/codegen/box/functions/localFunctions/nameClashAcrossDifferentContainers.kt"); + } + @TestMetadata("overloadedLocalFunWithoutClosure.kt") public void testOverloadedLocalFunWithoutClosure() throws Exception { runTest("compiler/testData/codegen/box/functions/localFunctions/overloadedLocalFunWithoutClosure.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java index a116dd67e90..ab1ddf09c44 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java @@ -1515,6 +1515,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/manyDefaultsAndVararg.kt"); } + @TestMetadata("noNameClashForReferencesToSameFunction.kt") + public void testNoNameClashForReferencesToSameFunction() throws Exception { + runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noNameClashForReferencesToSameFunction.kt"); + } + @TestMetadata("simpleDefaultArgument.kt") public void testSimpleDefaultArgument() throws Exception { runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/simpleDefaultArgument.kt"); @@ -10122,6 +10127,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/functions/localFunctions/nameClash.kt"); } + @TestMetadata("nameClashAcrossDifferentContainers.kt") + public void testNameClashAcrossDifferentContainers() throws Exception { + runTest("compiler/testData/codegen/box/functions/localFunctions/nameClashAcrossDifferentContainers.kt"); + } + @TestMetadata("overloadedLocalFunWithoutClosure.kt") public void testOverloadedLocalFunWithoutClosure() throws Exception { runTest("compiler/testData/codegen/box/functions/localFunctions/overloadedLocalFunWithoutClosure.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index e15118c5701..67c128a06c2 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -1515,6 +1515,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/manyDefaultsAndVararg.kt"); } + @TestMetadata("noNameClashForReferencesToSameFunction.kt") + public void testNoNameClashForReferencesToSameFunction() throws Exception { + runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/noNameClashForReferencesToSameFunction.kt"); + } + @TestMetadata("simpleDefaultArgument.kt") public void testSimpleDefaultArgument() throws Exception { runTest("compiler/testData/codegen/box/callableReference/adaptedReferences/simpleDefaultArgument.kt"); @@ -10122,6 +10127,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/functions/localFunctions/nameClash.kt"); } + @TestMetadata("nameClashAcrossDifferentContainers.kt") + public void testNameClashAcrossDifferentContainers() throws Exception { + runTest("compiler/testData/codegen/box/functions/localFunctions/nameClashAcrossDifferentContainers.kt"); + } + @TestMetadata("overloadedLocalFunWithoutClosure.kt") public void testOverloadedLocalFunWithoutClosure() throws Exception { runTest("compiler/testData/codegen/box/functions/localFunctions/overloadedLocalFunWithoutClosure.kt");