From c4d90dc7440584ad972276d7147c573b8b15e1e4 Mon Sep 17 00:00:00 2001 From: "sebastian.sellmair" Date: Mon, 12 Jul 2021 15:15:43 +0200 Subject: [PATCH] [Commonizer] Implement alis type substitution as CirNodeTransformer This has the advantage, that the substitution only has to run on functions or properties that are 'incomplete' (missing at least one other target declaration) ^KT-47433 Verification Pending --- .../org/jetbrains/kotlin/commonizer/facade.kt | 4 + .../commonizer/mergedtree/CirMemberContext.kt | 2 +- .../CirAliasTypeSubstitutor.kt | 44 ++++++----- .../TypeSubstitutionCirNodeTransformer.kt | 76 +++++++++++++++++++ .../kotlin/commonizer/tree/mergeCirTree.kt | 25 +++--- .../commonizer/utils/commonizedGroup.kt | 6 +- .../HierarchicalPropertyCommonizationTest.kt | 35 ++++++++- ...chicalTypeSubstitutionCommonizationTest.kt | 38 +++++++++- 8 files changed, 189 insertions(+), 41 deletions(-) rename native/commonizer/src/org/jetbrains/kotlin/commonizer/{mergedtree => transformer}/CirAliasTypeSubstitutor.kt (71%) create mode 100644 native/commonizer/src/org/jetbrains/kotlin/commonizer/transformer/TypeSubstitutionCirNodeTransformer.kt diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/facade.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/facade.kt index b3e67fe604f..ad392f8a0dc 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/facade.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/facade.kt @@ -14,6 +14,7 @@ import org.jetbrains.kotlin.commonizer.mergedtree.CirNode.Companion.indexOfCommo import org.jetbrains.kotlin.commonizer.mergedtree.CirRootNode import org.jetbrains.kotlin.commonizer.metadata.CirTreeSerializer import org.jetbrains.kotlin.commonizer.transformer.InlineTypeAliasCirNodeTransformer +import org.jetbrains.kotlin.commonizer.transformer.TypeSubstitutionCirNodeTransformer import org.jetbrains.kotlin.commonizer.tree.CirTreeRoot import org.jetbrains.kotlin.commonizer.tree.defaultCirTreeRootDeserializer import org.jetbrains.kotlin.commonizer.tree.mergeCirTree @@ -56,7 +57,10 @@ internal fun commonizeTarget( ) val mergedTree = mergeCirTree(parameters.storageManager, classifiers, availableTrees) + InlineTypeAliasCirNodeTransformer(parameters.storageManager, classifiers).invoke(mergedTree) + TypeSubstitutionCirNodeTransformer(parameters.storageManager, classifiers, availableTrees).invoke(mergedTree) + mergedTree.accept(CommonizationVisitor(classifiers, mergedTree), Unit) return mergedTree diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirMemberContext.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirMemberContext.kt index 5fae986c377..c3beed9e7fb 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirMemberContext.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirMemberContext.kt @@ -7,7 +7,7 @@ package org.jetbrains.kotlin.commonizer.mergedtree import org.jetbrains.kotlin.commonizer.cir.CirClass -internal class CirMemberContext private constructor(val classes: List) { +class CirMemberContext private constructor(internal val classes: List) { companion object { val empty = CirMemberContext(emptyList()) diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirAliasTypeSubstitutor.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/transformer/CirAliasTypeSubstitutor.kt similarity index 71% rename from native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirAliasTypeSubstitutor.kt rename to native/commonizer/src/org/jetbrains/kotlin/commonizer/transformer/CirAliasTypeSubstitutor.kt index 45247644644..b8580bb1a2a 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirAliasTypeSubstitutor.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/transformer/CirAliasTypeSubstitutor.kt @@ -3,11 +3,15 @@ * 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.commonizer.mergedtree +package org.jetbrains.kotlin.commonizer.transformer import org.jetbrains.kotlin.commonizer.cir.* import org.jetbrains.kotlin.commonizer.cir.CirClassType.Companion.copyInterned import org.jetbrains.kotlin.commonizer.cir.CirTypeAliasType.Companion.copyInterned +import org.jetbrains.kotlin.commonizer.mergedtree.CirClassifierIndex +import org.jetbrains.kotlin.commonizer.mergedtree.CirProvidedClassifiers +import org.jetbrains.kotlin.commonizer.mergedtree.CirTypeSubstitutor +import org.jetbrains.kotlin.commonizer.tree.CirTreeTypeAlias internal class CirAliasTypeSubstitutor( private val commonDependencies: CirProvidedClassifiers, @@ -33,7 +37,6 @@ internal class CirAliasTypeSubstitutor( } private fun substituteClassOrTypeAliasType(targetIndex: Int, type: CirClassOrTypeAliasType): CirClassOrTypeAliasType { - /* Classifier id is available in all platforms or is provided by common dependencies. The classifier itself does not require substitution, but type arguments potentially do! @@ -46,7 +49,6 @@ internal class CirAliasTypeSubstitutor( } else type } - /** * Yet, we do not have any evidence that it is worth trying to substitute types with arguments when the type itself is * not common at all. This substitutor could try to substitute the type & it's arguments. @@ -70,28 +72,34 @@ internal class CirAliasTypeSubstitutor( private fun substituteClassType(targetIndex: Int, type: CirClassType): CirClassOrTypeAliasType { if (type.arguments.isNotEmpty()) return type if (type.outerType != null) return type + if (isCommon(type.classifierId)) return type val classifierIndex = classifierIndices[targetIndex] - var typeAliases = classifierIndex.findTypeAliasesWithUnderlyingType(type.classifierId) + val typeAliases = classifierIndex.findTypeAliasesWithUnderlyingType(type.classifierId) + val commonTypeAlias = findSuitableCommonTypeAlias(classifierIndex, typeAliases) ?: return type - while (typeAliases.isNotEmpty()) { - val commonTypeAlias = typeAliases.firstOrNull { (id, typeAlias) -> typeAlias.typeParameters.isEmpty() && isCommon(id) } + return CirTypeAliasType.createInterned( + typeAliasId = commonTypeAlias.id, + underlyingType = commonTypeAlias.typeAlias.underlyingType, + arguments = emptyList(), + isMarkedNullable = type.isMarkedNullable + ) + } - if (commonTypeAlias != null) { - return CirTypeAliasType.createInterned( - typeAliasId = commonTypeAlias.id, - underlyingType = commonTypeAlias.typeAlias.underlyingType, - arguments = emptyList(), - isMarkedNullable = type.isMarkedNullable - ) - } + private tailrec fun findSuitableCommonTypeAlias( + index: CirClassifierIndex, + typeAliases: List + ): CirTreeTypeAlias? { + if (typeAliases.isEmpty()) return null - typeAliases = typeAliases.flatMap { (id, _) -> - classifierIndex.findTypeAliasesWithUnderlyingType(id) - } + val commonTypeAlias = typeAliases.firstOrNull { (id, typeAlias) -> typeAlias.typeParameters.isEmpty() && isCommon(id) } + if (commonTypeAlias != null) { + return commonTypeAlias } - return type + return findSuitableCommonTypeAlias( + index, typeAliases.flatMap { (id, _) -> index.findTypeAliasesWithUnderlyingType(id) } + ) } private fun isCommon(id: CirEntityId): Boolean = diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/transformer/TypeSubstitutionCirNodeTransformer.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/transformer/TypeSubstitutionCirNodeTransformer.kt new file mode 100644 index 00000000000..cb4ab57b969 --- /dev/null +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/transformer/TypeSubstitutionCirNodeTransformer.kt @@ -0,0 +1,76 @@ +/* + * Copyright 2010-2021 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.commonizer.transformer + +import org.jetbrains.kotlin.commonizer.mergedtree.* +import org.jetbrains.kotlin.commonizer.mergedtree.CirNodeRelationship.ParentNode +import org.jetbrains.kotlin.commonizer.tree.CirTreeRoot +import org.jetbrains.kotlin.storage.StorageManager + +internal fun TypeSubstitutionCirNodeTransformer( + storageManager: StorageManager, classifiers: CirKnownClassifiers, roots: Iterable +): TypeSubstitutionCirNodeTransformer { + val typeSubstitutor = CirAliasTypeSubstitutor(classifiers.commonDependencies, roots.map(::CirClassifierIndex)) + return TypeSubstitutionCirNodeTransformer(storageManager, classifiers, typeSubstitutor) +} + +internal class TypeSubstitutionCirNodeTransformer( + private val storageManager: StorageManager, + private val classifiers: CirKnownClassifiers, + private val typeSubstitutor: CirTypeSubstitutor +) : CirNodeTransformer { + + override fun invoke(root: CirRootNode) { + for (index in 0 until root.targetDeclarations.size) { + root.modules.forEach { (_, module) -> this(module, index) } + } + } + + private operator fun invoke(module: CirModuleNode, index: Int) { + module.packages.forEach { (_, pkg) -> this(pkg, index) } + } + + private operator fun invoke(pkg: CirPackageNode, index: Int) { + pkg.functions.values.toList().forEach { function -> this(pkg, function, index, CirMemberContext.empty) } + pkg.properties.values.toList().forEach { property -> this(pkg, property, index, CirMemberContext.empty) } + pkg.classes.values.toList().forEach { clazz -> this(clazz, index, CirMemberContext.empty) } + } + + private operator fun invoke(clazz: CirClassNode, index: Int, context: CirMemberContext) { + val contextWithClass = context.withContextOf(clazz.targetDeclarations[index] ?: return) + clazz.functions.values.toList().forEach { function -> this(clazz, function, index, contextWithClass) } + clazz.properties.values.toList().forEach { property -> this(clazz, property, index, contextWithClass) } + clazz.classes.values.toList().forEach { innerClass -> this(innerClass, index, contextWithClass) } + } + + private operator fun invoke(parent: CirNodeWithMembers<*, *>, function: CirFunctionNode, index: Int, context: CirMemberContext) { + /* Only perform type substitution to nodes that are not 'complete' */ + if (function.targetDeclarations.none { it == null }) return + val originalFunction = function.targetDeclarations[index] ?: return + val newFunction = typeSubstitutor.substitute(index, originalFunction) + if (originalFunction == newFunction) return + + val approximationKey = FunctionApproximationKey.create(context, newFunction) + val newNode = parent.functions.getOrPut(approximationKey) { + buildFunctionNode(storageManager, parent.targetDeclarations.size, classifiers, ParentNode(parent)) + } + + newNode.targetDeclarations.setAllowingOverride(index, newFunction) + } + + private operator fun invoke(parent: CirNodeWithMembers<*, *>, property: CirPropertyNode, index: Int, context: CirMemberContext) { + val originalProperty = property.targetDeclarations[index] ?: return + val newProperty = typeSubstitutor.substitute(index, originalProperty) + if (originalProperty == newProperty) return + + val approximationKey = PropertyApproximationKey.create(context, newProperty) + val newNode = parent.properties.getOrPut(approximationKey) { + buildPropertyNode(storageManager, parent.targetDeclarations.size, classifiers, ParentNode(parent)) + } + + newNode.targetDeclarations.setAllowingOverride(index, newProperty) + } +} \ No newline at end of file diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/tree/mergeCirTree.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/tree/mergeCirTree.kt index e67c5c4b983..6eb5ce57975 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/tree/mergeCirTree.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/tree/mergeCirTree.kt @@ -8,13 +8,14 @@ package org.jetbrains.kotlin.commonizer.tree import org.jetbrains.kotlin.commonizer.TargetDependent import org.jetbrains.kotlin.commonizer.cir.* import org.jetbrains.kotlin.commonizer.mergedtree.* +import org.jetbrains.kotlin.commonizer.mergedtree.CirNodeRelationship.Companion.ParentNode +import org.jetbrains.kotlin.commonizer.mergedtree.CirNodeRelationship.ParentNode import org.jetbrains.kotlin.storage.StorageManager internal data class TargetBuildingContext( val storageManager: StorageManager, val classifiers: CirKnownClassifiers, val memberContext: CirMemberContext = CirMemberContext.empty, - val typeSubstitutor: CirTypeSubstitutor, val targets: Int, val targetIndex: Int ) { fun withMemberContextOf(clazz: CirClass) = copy(memberContext = memberContext.withContextOf(clazz)) @@ -31,10 +32,6 @@ internal fun mergeCirTree( storageManager = storageManager, classifiers = classifiers, memberContext = CirMemberContext.empty, - typeSubstitutor = CirAliasTypeSubstitutor( - commonDependencies = classifiers.commonDependencies, - classifierIndices = roots.map(::CirClassifierIndex) - ), targets = roots.size, targetIndex = targetIndex ), roots[target].modules @@ -70,7 +67,7 @@ internal fun CirNodeWithMembers<*, *>.buildClass( context: TargetBuildingContext, treeClass: CirTreeClass, parent: CirNode<*, *>? = null ) { val classNode = classes.getOrPut(treeClass.clazz.name) { - buildClassNode(context.storageManager, context.targets, context.classifiers, CirNodeRelationship.ParentNode(parent), treeClass.id) + buildClassNode(context.storageManager, context.targets, context.classifiers, ParentNode(parent), treeClass.id) } classNode.targetDeclarations[context.targetIndex] = treeClass.clazz val contextWithClass = context.withMemberContextOf(treeClass.clazz) @@ -83,28 +80,26 @@ internal fun CirNodeWithMembers<*, *>.buildClass( internal fun CirNodeWithMembers<*, *>.buildFunction( context: TargetBuildingContext, function: CirFunction, parent: CirNode<*, *>? = null ) { - val newFunction = context.typeSubstitutor.substitute(context.targetIndex, function) - val functionNode = functions.getOrPut(FunctionApproximationKey.create(context.memberContext, newFunction)) { - buildFunctionNode(context.storageManager, context.targets, context.classifiers, CirNodeRelationship.ParentNode(parent)) + val functionNode = functions.getOrPut(FunctionApproximationKey.create(context.memberContext, function)) { + buildFunctionNode(context.storageManager, context.targets, context.classifiers, ParentNode(parent)) } - functionNode.targetDeclarations[context.targetIndex] = newFunction + functionNode.targetDeclarations[context.targetIndex] = function } internal fun CirNodeWithMembers<*, *>.buildProperty( context: TargetBuildingContext, property: CirProperty, parent: CirNode<*, *>? = null ) { - val newProperty = context.typeSubstitutor.substitute(context.targetIndex, property) - val propertyNode = properties.getOrPut(PropertyApproximationKey.create(context.memberContext, newProperty)) { - buildPropertyNode(context.storageManager, context.targets, context.classifiers, CirNodeRelationship.ParentNode(parent)) + val propertyNode = properties.getOrPut(PropertyApproximationKey.create(context.memberContext, property)) { + buildPropertyNode(context.storageManager, context.targets, context.classifiers, ParentNode(parent)) } - propertyNode.targetDeclarations[context.targetIndex] = newProperty + propertyNode.targetDeclarations[context.targetIndex] = property } internal fun CirClassNode.buildConstructor( context: TargetBuildingContext, constructor: CirClassConstructor, parent: CirNode<*, *> ) { val constructorNode = constructors.getOrPut(ConstructorApproximationKey.create(context.memberContext, constructor)) { - buildClassConstructorNode(context.storageManager, context.targets, context.classifiers, CirNodeRelationship.ParentNode(parent)) + buildClassConstructorNode(context.storageManager, context.targets, context.classifiers, ParentNode(parent)) } constructorNode.targetDeclarations[context.targetIndex] = constructor } diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/utils/commonizedGroup.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/utils/commonizedGroup.kt index 3d5d48fa9ca..1bb612d8129 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/utils/commonizedGroup.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/utils/commonizedGroup.kt @@ -31,7 +31,9 @@ class CommonizedGroup( elements[index] = value } + + internal fun setAllowingOverride(index: Int, value: T?) { + elements[index] = value + } } -interface I - diff --git a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalPropertyCommonizationTest.kt b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalPropertyCommonizationTest.kt index dd67849c0d2..6e39a895c15 100644 --- a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalPropertyCommonizationTest.kt +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalPropertyCommonizationTest.kt @@ -182,9 +182,40 @@ class HierarchicalPropertyCommonizationTest : AbstractInlineSourcesCommonization result.assertCommonized( "(a, b)", """ expect class AB expect constructor() - expect typealias TA_AB = AB + typealias TA_AB = AB // https://youtrack.jetbrains.com/issue/KT-47100 - expect val x: TA_AB + expect val x: AB + """.trimIndent() + ) + } + + fun `test single typeAliased property and double typeAliased property - with reversed order`() { + val result = commonize { + outputTarget("(a, b)") + simpleSingleSourceTarget( + "a", """ + class AB + typealias TA_AB = AB + typealias TA_B = TA_AB + val x: TA_B = TA_B() + """.trimIndent() + ) + + simpleSingleSourceTarget( + "b", """ + class AB + typealias TA_AB = AB + val x: TA_AB = TA_AB() + """.trimIndent() + ) + } + + result.assertCommonized( + "(a, b)", """ + expect class AB expect constructor() + typealias TA_AB = AB + // https://youtrack.jetbrains.com/issue/KT-47100 + expect val x: AB """.trimIndent() ) } diff --git a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalTypeSubstitutionCommonizationTest.kt b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalTypeSubstitutionCommonizationTest.kt index 27acf9c5f29..1943e240fc5 100644 --- a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalTypeSubstitutionCommonizationTest.kt +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalTypeSubstitutionCommonizationTest.kt @@ -10,7 +10,7 @@ import org.jetbrains.kotlin.commonizer.assertCommonized class HierarchicalTypeSubstitutionCommonizationTest : AbstractInlineSourcesCommonizationTest() { - fun `test boxed function using TA and expanded type`() { + fun `test function with boxed parameter`() { val result = commonize { outputTarget("(a, b)") @@ -43,7 +43,7 @@ class HierarchicalTypeSubstitutionCommonizationTest : AbstractInlineSourcesCommo } - fun `test boxed function using TA and expanded type - with box from dependencies`() { + fun `test function with boxed parameter - with box from dependencies`() { val result = commonize { outputTarget("(a, b)") @@ -76,7 +76,7 @@ class HierarchicalTypeSubstitutionCommonizationTest : AbstractInlineSourcesCommo ) } - fun `test parameters with non-commonized TA expanding to a commonized type`() { + fun `test function parameter with suitable typealias`() { val result = commonize { outputTarget("(a, b)") @@ -103,4 +103,36 @@ class HierarchicalTypeSubstitutionCommonizationTest : AbstractInlineSourcesCommo """.trimIndent() ) } + + fun `test boxed property return type`() { + val result = commonize { + outputTarget("(a, b)") + + simpleSingleSourceTarget( + "a", """ + class Box + class A + typealias X = A + val x: Box + """.trimIndent() + ) + + simpleSingleSourceTarget( + "b", """ + class Box + class B + typealias X = B + val x: Box + """.trimIndent() + ) + } + + result.assertCommonized( + "(a, b)", """ + expect class Box expect constructor() + expect class X expect constructor() + expect val x: Box + """.trimIndent() + ) + } } \ No newline at end of file