From 241f457943e1f643a90b10de736802fbbabb2833 Mon Sep 17 00:00:00 2001 From: Ivan Kochurkin Date: Wed, 1 Feb 2023 21:46:37 +0100 Subject: [PATCH] [IR] Use full name for expect/actual functions linking, simplify code Don't put type parameters to expect-actual map since it's useless ^KT-56329 Fixed --- .../FirBlackBoxCodegenTestGenerated.java | 6 ++ .../actualizer/ExpectActualCollector.kt | 90 +++++-------------- .../backend/common/actualizer/IrActualizer.kt | 13 ++- .../common/actualizer/IrActualizerUtils.kt | 84 ++++++++++------- .../actualizer/MissingFakeOverridesAdder.kt | 67 ++++++-------- .../box/multiplatform/multiModule/kt-56329.kt | 27 ++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++ .../IrBlackBoxCodegenTestGenerated.java | 6 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ 9 files changed, 160 insertions(+), 144 deletions(-) create mode 100644 compiler/testData/codegen/box/multiplatform/multiModule/kt-56329.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 b40b2928780..c4592fff7d5 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 @@ -33123,6 +33123,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT public void testKt_51753_2() throws Exception { runTest("compiler/testData/codegen/box/multiplatform/multiModule/kt-51753-2.kt"); } + + @Test + @TestMetadata("kt-56329.kt") + public void testKt_56329() throws Exception { + runTest("compiler/testData/codegen/box/multiplatform/multiModule/kt-56329.kt"); + } } } diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/ExpectActualCollector.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/ExpectActualCollector.kt index 9d9b4e80646..fc3c578d919 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/ExpectActualCollector.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/ExpectActualCollector.kt @@ -16,21 +16,20 @@ import org.jetbrains.kotlin.ir.util.kotlinFqName import org.jetbrains.kotlin.ir.visitors.IrElementVisitor import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid -import org.jetbrains.kotlin.name.CallableId import org.jetbrains.kotlin.name.FqName internal class ExpectActualCollector(private val mainFragment: IrModuleFragment, private val dependentFragments: List) { - fun collect(): Map { + fun collect(): Pair, Map> { val result = mutableMapOf() // Collect and link classifiers at first to make it possible to expand type aliases on the callables linking val (allActualDeclarations, typeAliasMap) = result.appendExpectActualClassifiersMap() result.appendExpectActualCallablesMap(allActualDeclarations, typeAliasMap, dependentFragments) - return result + return result to typeAliasMap } private fun MutableMap.appendExpectActualClassifiersMap(): Pair, Map> { val actualClassifiers = mutableMapOf() - // There is no list for builtins declarations, that's why they are being collected from typealiases + // There is no list for builtins declarations; that's why they are being collected from typealiases val allActualDeclarations = mutableSetOf() val typeAliasMap = mutableMapOf() // It's used to link members from expect class that have typealias actual @@ -78,15 +77,6 @@ internal class ExpectActualCollector(private val mainFragment: IrModuleFragment, visitDeclaration(declaration, data) } - override fun visitTypeParameter(declaration: IrTypeParameter, data: Boolean) { - if (!data && !declaration.isExpect) { - actualClassifiers[FqName.fromSegments( - listOf(declaration.parent.kotlinFqName.asString(), declaration.name.asString()) - )] = declaration.symbol - } - visitDeclaration(declaration, data) - } - override fun visitDeclaration(declaration: IrDeclarationBase, data: Boolean) { if (!data && !declaration.isExpect) { allActualClassifiers.add(declaration) @@ -128,16 +118,6 @@ internal class ExpectActualCollector(private val mainFragment: IrModuleFragment, visitElement(declaration) } - override fun visitTypeParameter(declaration: IrTypeParameter) { - if (declaration.isProperExpect) { - addLinkOrReportMissing( - declaration, - FqName.fromSegments(listOf(declaration.parent.kotlinFqName.asString(), declaration.name.asString())) - ) - } - visitElement(declaration) - } - override fun visitElement(element: IrElement) { element.acceptChildrenVoid(this) } @@ -148,30 +128,23 @@ internal class ExpectActualCollector(private val mainFragment: IrModuleFragment, typeAliasMap: Map, dependentFragments: List ) { - val actualFunctions = mutableMapOf>() - val actualProperties = mutableMapOf() + val actualMembers = mutableMapOf() - collectActualCallables(actualFunctions, actualProperties, allActualDeclarations) - val collector = CallablesLinkCollector(this, actualFunctions, actualProperties, typeAliasMap) + collectActualCallables(this, actualMembers, allActualDeclarations) + val collector = CallablesLinkCollector(this, actualMembers, typeAliasMap) dependentFragments.forEach { collector.visitModuleFragment(it) } } private fun collectActualCallables( - actualFunctions: MutableMap>, - actualProperties: MutableMap, - allActualDeclarations: Set + expectActualMap: MutableMap, + actualMembers: MutableMap, + allActualDeclarations: Set, ) { fun collectActualsCallables(declaration: IrDeclaration) { when (declaration) { - is IrFunction -> { - actualFunctions.getOrPut(CallableId(declaration.parent.kotlinFqName, declaration.name)) { - mutableListOf() - }.add(declaration) - } + is IrFunction, is IrProperty -> { - actualProperties.getOrPut(CallableId(declaration.parent.kotlinFqName, declaration.name)) { - declaration - } + actualMembers[generateIrElementFullName(declaration, expectActualMap)] = declaration as IrDeclarationBase } is IrClass -> { for (member in declaration.declarations) { @@ -188,40 +161,23 @@ internal class ExpectActualCollector(private val mainFragment: IrModuleFragment, class CallablesLinkCollector( private val expectActualMap: MutableMap, - private val actualFunctions: MutableMap>, - private val actualProperties: MutableMap, + private val actualMembers: Map, private val typeAliasMap: Map ) : IrElementVisitorVoid { - private fun actualizeCallable(declaration: IrDeclarationWithName): CallableId { - val fullName = declaration.parent.kotlinFqName - return CallableId(typeAliasMap[fullName] ?: fullName, declaration.name) - } + override fun visitFunction(declaration: IrFunction) = addLink(declaration) - override fun visitFunction(declaration: IrFunction) { + override fun visitProperty(declaration: IrProperty) = addLink(declaration) + + private fun addLink(declaration: IrDeclarationBase) { if (!declaration.isExpect) return - val functions = actualFunctions[actualizeCallable(declaration)] - var isActualFunctionFound = false - if (functions != null) { - for (actualFunction in functions) { - if (checkParameters(declaration, actualFunction, expectActualMap)) { - expectActualMap[declaration.symbol] = actualFunction.symbol - isActualFunctionFound = true - break - } + val member = actualMembers[generateIrElementFullName(declaration, expectActualMap, typeAliasMap)] + if (member != null) { + expectActualMap[declaration.symbol] = member.symbol + if (declaration is IrProperty) { + member as IrProperty + declaration.getter?.symbol?.let { expectActualMap[it] = member.getter!!.symbol } + declaration.setter?.symbol?.let { expectActualMap[it] = member.setter!!.symbol } } - } - if (!isActualFunctionFound) { - reportMissingActual(declaration) - } - } - - override fun visitProperty(declaration: IrProperty) { - if (!declaration.isExpect) return - val properties = actualProperties[actualizeCallable(declaration)] - if (properties != null) { - expectActualMap[declaration.symbol] = properties.symbol - declaration.getter?.symbol?.let { expectActualMap[it] = properties.getter!!.symbol } - declaration.setter?.symbol?.let { expectActualMap[it] = properties.setter!!.symbol } } else { reportMissingActual(declaration) } diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrActualizer.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrActualizer.kt index 5032d711820..e591ef89b58 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrActualizer.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrActualizer.kt @@ -8,12 +8,13 @@ package org.jetbrains.kotlin.backend.common.actualizer import org.jetbrains.kotlin.backend.common.ir.isProperExpect import org.jetbrains.kotlin.ir.declarations.IrModuleFragment import org.jetbrains.kotlin.ir.symbols.IrSymbol +import org.jetbrains.kotlin.name.FqName object IrActualizer { fun actualize(mainFragment: IrModuleFragment, dependentFragments: List) { - val expectActualMap = ExpectActualCollector(mainFragment, dependentFragments).collect() + val (expectActualMap, typeAliasMap) = ExpectActualCollector(mainFragment, dependentFragments).collect() removeExpectDeclaration(dependentFragments) // TODO: consider removing this call. See ExpectDeclarationRemover.kt - addMissingFakeOverrides(expectActualMap, dependentFragments) + addMissingFakeOverrides(expectActualMap, dependentFragments, typeAliasMap) linkExpectToActual(expectActualMap, dependentFragments) mergeIrFragments(mainFragment, dependentFragments) } @@ -26,8 +27,12 @@ object IrActualizer { } } - private fun addMissingFakeOverrides(expectActualMap: Map, dependentFragments: List) { - MissingFakeOverridesAdder(expectActualMap).apply { dependentFragments.forEach { visitModuleFragment(it) } } + private fun addMissingFakeOverrides( + expectActualMap: Map, + dependentFragments: List, + typeAliasMap: Map + ) { + MissingFakeOverridesAdder(expectActualMap, typeAliasMap).apply { dependentFragments.forEach { visitModuleFragment(it) } } } private fun linkExpectToActual(expectActualMap: Map, dependentFragments: List) { diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrActualizerUtils.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrActualizerUtils.kt index ec0a99c75c5..f05547e3099 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrActualizerUtils.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrActualizerUtils.kt @@ -6,55 +6,71 @@ package org.jetbrains.kotlin.backend.common.actualizer import org.jetbrains.kotlin.ir.IrElement -import org.jetbrains.kotlin.ir.declarations.IrClass -import org.jetbrains.kotlin.ir.declarations.IrDeclarationWithName -import org.jetbrains.kotlin.ir.declarations.IrFunction -import org.jetbrains.kotlin.ir.declarations.IrValueParameter +import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.symbols.IrSymbol +import org.jetbrains.kotlin.ir.types.IrType import org.jetbrains.kotlin.ir.types.classifierOrFail +import org.jetbrains.kotlin.ir.util.kotlinFqName import org.jetbrains.kotlin.ir.util.render +import org.jetbrains.kotlin.name.FqName -fun checkParameters( - expectFunction: IrFunction, - actualFunction: IrFunction, - expectActualTypesMap: Map -): Boolean { - fun checkParameter(expectParameter: IrValueParameter?, actualParameter: IrValueParameter?): Boolean { - if (expectParameter == null) { - return actualParameter == null - } - if (actualParameter == null) { - return false - } +fun generateIrElementFullName( + declaration: IrElement, + expectActualTypesMap: Map, + typeAliasMap: Map? = null +): String { + return StringBuilder().apply { appendElementFullName(declaration, expectActualTypesMap, this, typeAliasMap) }.toString() +} - val expectParameterTypeSymbol = expectParameter.type.classifierOrFail - val actualizedParameterTypeSymbol = expectActualTypesMap[expectParameterTypeSymbol] ?: expectParameterTypeSymbol - if (actualizedParameterTypeSymbol != actualParameter.type.classifierOrFail) { - return false - } - return true +private fun appendElementFullName( + declaration: IrElement, + expectActualTypesMap: Map, + result: StringBuilder, + typeAliasMap: Map? = null +) { + if (declaration !is IrDeclarationBase) return + + val parentName = declaration.parent.kotlinFqName + if (parentName.asString().isNotEmpty()) { + result.append(typeAliasMap?.get(parentName) ?: parentName.asString()) + result.append('.') } - if (expectFunction.valueParameters.size != actualFunction.valueParameters.size || - !checkParameter(expectFunction.extensionReceiverParameter, actualFunction.extensionReceiverParameter) - ) { - return false - } - for ((expectParameter, actualParameter) in expectFunction.valueParameters.zip(actualFunction.valueParameters)) { - if (!checkParameter(expectParameter, actualParameter)) { - return false - } + if (declaration is IrDeclarationWithName) { + result.append(declaration.name) } - return true + if (declaration is IrFunction) { + fun appendType(type: IrType) { + val typeClassifier = type.classifierOrFail + val actualizedTypeSymbol = expectActualTypesMap[typeClassifier] ?: typeClassifier + appendElementFullName(actualizedTypeSymbol.owner, expectActualTypesMap, result) + } + + val extensionReceiverType = declaration.extensionReceiverParameter?.type + if (extensionReceiverType != null) { + result.append('[') + appendType(extensionReceiverType) + result.append(']') + } + + result.append('(') + for ((index, parameter) in declaration.valueParameters.withIndex()) { + appendType(parameter.type) + if (index < declaration.valueParameters.size - 1) { + result.append(',') + } + } + result.append(')') + } } fun reportMissingActual(irElement: IrElement) { - // TODO: set up diagnostics reporting + // TODO: setup diagnostics reporting throw AssertionError("Missing actual for ${irElement.render()}") } fun reportManyInterfacesMembersNotImplemented(declaration: IrClass, actualMember: IrDeclarationWithName) { - // TODO: set up diagnostics reporting + // TODO: setup diagnostics reporting throw AssertionError("${declaration.name} must override ${actualMember.name} because it inherits multiple interface methods of it") } \ No newline at end of file diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/MissingFakeOverridesAdder.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/MissingFakeOverridesAdder.kt index 18831f30a97..412e3754225 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/MissingFakeOverridesAdder.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/MissingFakeOverridesAdder.kt @@ -18,11 +18,15 @@ import org.jetbrains.kotlin.ir.types.isAny import org.jetbrains.kotlin.ir.util.deepCopyWithSymbols import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid +import org.jetbrains.kotlin.name.FqName -class MissingFakeOverridesAdder(private val expectActualMap: Map) : IrElementVisitorVoid { +class MissingFakeOverridesAdder( + private val expectActualMap: Map, + private val typeAliasMap: Map +) : IrElementVisitorVoid { override fun visitClass(declaration: IrClass) { if (!declaration.isExpect) { - processSupertypes(declaration, expectActualMap) + processSupertypes(declaration) } visitElement(declaration) } @@ -30,48 +34,33 @@ class MissingFakeOverridesAdder(private val expectActualMap: Map) { - val members by lazy(LazyThreadSafetyMode.NONE) { - declaration.declarations.filter { !it.isBuiltinMember() }.filterIsInstance() - .groupBy { it.name } - } + private fun processSupertypes(declaration: IrClass) { + val members by lazy(LazyThreadSafetyMode.NONE) { + declaration.declarations.filter { !it.isBuiltinMember() }.associateBy { + generateIrElementFullName(it, expectActualMap, typeAliasMap) + } + } - for (superType in declaration.superTypes) { - val actualClass = expectActualMap[superType.classifierOrFail]?.owner as? IrClass ?: continue - for (actualMember in actualClass.declarations) { - if (actualMember.isBuiltinMember()) continue - when (actualMember) { - is IrFunctionImpl -> { - val existingMembers = members[actualMember.name] - - var isActualFunctionFound = false - if (existingMembers != null) { - for (existingMember in existingMembers) { - if (existingMember is IrFunction) { - if (checkParameters(existingMember, actualMember, expectActualMap)) { - isActualFunctionFound = true - break - } - } + for (superType in declaration.superTypes) { + val actualClass = expectActualMap[superType.classifierOrFail]?.owner as? IrClass ?: continue + for (actualMember in actualClass.declarations) { + if (actualMember.isBuiltinMember()) continue + when (actualMember) { + is IrFunctionImpl, + is IrPropertyImpl -> { + if (members[generateIrElementFullName(actualMember, expectActualMap, typeAliasMap)] != null) { + reportManyInterfacesMembersNotImplemented(declaration, actualMember as IrDeclarationWithName) + continue } - } - if (isActualFunctionFound) { - reportManyInterfacesMembersNotImplemented(declaration, actualMember) - continue + val newMember = if (actualMember is IrFunctionImpl) { + createFakeOverrideFunction(actualMember, declaration) + } else { + createFakeOverrideProperty(actualMember as IrPropertyImpl, declaration) + } + declaration.declarations.add(newMember) } - - declaration.declarations.add(createFakeOverrideFunction(actualMember, declaration)) - } - is IrPropertyImpl -> { - if (members[actualMember.name] != null) { - reportManyInterfacesMembersNotImplemented(declaration, actualMember) - continue - } - - declaration.declarations.add(createFakeOverrideProperty(actualMember, declaration)) } } } diff --git a/compiler/testData/codegen/box/multiplatform/multiModule/kt-56329.kt b/compiler/testData/codegen/box/multiplatform/multiModule/kt-56329.kt new file mode 100644 index 00000000000..d7dd689311f --- /dev/null +++ b/compiler/testData/codegen/box/multiplatform/multiModule/kt-56329.kt @@ -0,0 +1,27 @@ +// TARGET_BACKEND: JVM +// !LANGUAGE: +MultiPlatformProjects +// ISSUE: KT-56329 + +// MODULE: common +// TARGET_PLATFORM: Common +// FILE: common.kt + +expect class S + +expect fun foo(y: T): String + +expect fun foo(y: T, x: S): String + +fun ok() = foo(1) + foo(2, "K" as S) + +// MODULE: jvm()()(common) +// TARGET_PLATFORM: JVM +// FILE: main.kt + +actual typealias S = String + +actual fun foo(y: T) = "O" + +actual fun foo(y: T, x: S) = x + +fun box() = ok() \ No newline at end of file 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 1c5787609b1..ba5c3451401 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 @@ -31809,6 +31809,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { public void testGetRidOfDoubleBindingInFir2IrLazyProperty() throws Exception { runTest("compiler/testData/codegen/box/multiplatform/multiModule/getRidOfDoubleBindingInFir2IrLazyProperty.kt"); } + + @Test + @TestMetadata("kt-56329.kt") + public void testKt_56329() throws Exception { + runTest("compiler/testData/codegen/box/multiplatform/multiModule/kt-56329.kt"); + } } } 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 f7b6b526b5a..a009bcbb795 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 @@ -33123,6 +33123,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes public void testKt_51753_2() throws Exception { runTest("compiler/testData/codegen/box/multiplatform/multiModule/kt-51753-2.kt"); } + + @Test + @TestMetadata("kt-56329.kt") + public void testKt_56329() throws Exception { + runTest("compiler/testData/codegen/box/multiplatform/multiModule/kt-56329.kt"); + } } } diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 17cb6d1c123..5fa138ba5ba 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -27078,6 +27078,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes public void testGetRidOfDoubleBindingInFir2IrLazyProperty() throws Exception { runTest("compiler/testData/codegen/box/multiplatform/multiModule/getRidOfDoubleBindingInFir2IrLazyProperty.kt"); } + + @TestMetadata("kt-56329.kt") + public void testKt_56329() throws Exception { + runTest("compiler/testData/codegen/box/multiplatform/multiModule/kt-56329.kt"); + } } }