From 7c450f98842eea736616c1fa491c9ba8f86326a2 Mon Sep 17 00:00:00 2001 From: "sebastian.sellmair" Date: Fri, 9 Jul 2021 17:45:44 +0200 Subject: [PATCH] [Commonizer] Implement CirAliasTypeSubstitutor This will substitute non-commonizable classifiers with known type-aliases (which might be commonizable). A simple example depending on this substitution comes from posix: Most function and properties use the `FILE` typealias which is available across all platforms. Some linux platforms use `__IO_FILE` in their signature, which is just linux specific. This type substitution will figure out, that this type can be substituted with `FILE`. ^KT-47433 Verification Pending --- .../kotlin/commonizer/cir/CirType.kt | 30 +++++ .../mergedtree/CirAliasTypeSubstitutor.kt | 100 +++++++++++++++++ .../mergedtree/CirClassifierIndex.kt | 70 ++++++++++++ .../mergedtree/CirTypeSubstitutor.kt | 45 ++++++++ .../kotlin/commonizer/tree/mergeCirTree.kt | 27 ++++- .../commonizer/utils/commonizedGroup.kt | 4 +- ...hicalClassAndTypeAliasCommonizationTest.kt | 63 ----------- .../HierarchicalPropertyCommonizationTest.kt | 2 +- ...chicalTypeSubstitutionCommonizationTest.kt | 106 ++++++++++++++++++ 9 files changed, 375 insertions(+), 72 deletions(-) create mode 100644 native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirAliasTypeSubstitutor.kt create mode 100644 native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirClassifierIndex.kt create mode 100644 native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirTypeSubstitutor.kt create mode 100644 native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalTypeSubstitutionCommonizationTest.kt diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/cir/CirType.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/cir/CirType.kt index 7a5ee28ed84..ca7bac5cddc 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/cir/CirType.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/cir/CirType.kt @@ -127,6 +127,22 @@ abstract class CirClassType : CirClassOrTypeAliasType(), CirHasVisibility { ) ) + fun CirClassType.copyInterned( + classifierId: CirEntityId = this.classifierId, + outerType: CirClassType? = this.outerType, + visibility: Visibility = this.visibility, + arguments: List = this.arguments, + isMarkedNullable: Boolean = this.isMarkedNullable + ): CirClassType { + return createInterned( + classId = classifierId, + outerType = outerType, + visibility = visibility, + arguments = arguments, + isMarkedNullable = isMarkedNullable + ) + } + private val interner = Interner() } } @@ -170,6 +186,20 @@ abstract class CirTypeAliasType : CirClassOrTypeAliasType() { ) ) + fun CirTypeAliasType.copyInterned( + classifierId: CirEntityId = this.classifierId, + underlyingType: CirClassOrTypeAliasType = this.underlyingType, + arguments: List = this.arguments, + isMarkedNullable: Boolean = this.isMarkedNullable + ): CirTypeAliasType { + return createInterned( + typeAliasId = classifierId, + underlyingType = underlyingType, + arguments = arguments, + isMarkedNullable = isMarkedNullable + ) + } + private val interner = Interner() } } diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirAliasTypeSubstitutor.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirAliasTypeSubstitutor.kt new file mode 100644 index 00000000000..45247644644 --- /dev/null +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirAliasTypeSubstitutor.kt @@ -0,0 +1,100 @@ +/* + * 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.mergedtree + +import org.jetbrains.kotlin.commonizer.cir.* +import org.jetbrains.kotlin.commonizer.cir.CirClassType.Companion.copyInterned +import org.jetbrains.kotlin.commonizer.cir.CirTypeAliasType.Companion.copyInterned + +internal class CirAliasTypeSubstitutor( + private val commonDependencies: CirProvidedClassifiers, + private val classifierIndices: List +) : CirTypeSubstitutor { + + override fun substitute(targetIndex: Int, type: CirType): CirType { + return when (type) { + is CirFlexibleType -> type + is CirTypeParameterType -> type + is CirClassOrTypeAliasType -> substituteClassOrTypeAliasType(targetIndex, type) + } + } + + private fun substituteTypeProjection(targetIndex: Int, projection: CirTypeProjection): CirTypeProjection { + return when (projection) { + is CirRegularTypeProjection -> { + val newType = substitute(targetIndex, projection.type) + if (newType != projection.type) projection.copy(type = newType) else projection + } + is CirStarTypeProjection -> projection + } + } + + 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! + */ + if (isCommon(type.classifierId)) { + val newArguments = type.arguments.map { argument -> substituteTypeProjection(targetIndex, argument) } + return if (newArguments != type.arguments) when (type) { + is CirTypeAliasType -> type.copyInterned(arguments = newArguments) + is CirClassType -> type.copyInterned(arguments = newArguments) + } 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. + * + * Without any evidence of libraries that would benefit from this higher effort, we will just skip this case. + */ + if (type.arguments.isNotEmpty()) { + return type + } + + return when (type) { + is CirClassType -> substituteClassType(targetIndex, type) + is CirTypeAliasType -> substituteClassType(targetIndex, type.unabbreviate()) + } + } + + /** + * Tries to find a suitable type alias pointing to the given class [type] which is available in 'common' + * This function does *not* support types with arguments! + */ + private fun substituteClassType(targetIndex: Int, type: CirClassType): CirClassOrTypeAliasType { + if (type.arguments.isNotEmpty()) return type + if (type.outerType != null) return type + val classifierIndex = classifierIndices[targetIndex] + + var typeAliases = classifierIndex.findTypeAliasesWithUnderlyingType(type.classifierId) + + while (typeAliases.isNotEmpty()) { + val commonTypeAlias = typeAliases.firstOrNull { (id, typeAlias) -> typeAlias.typeParameters.isEmpty() && isCommon(id) } + + if (commonTypeAlias != null) { + return CirTypeAliasType.createInterned( + typeAliasId = commonTypeAlias.id, + underlyingType = commonTypeAlias.typeAlias.underlyingType, + arguments = emptyList(), + isMarkedNullable = type.isMarkedNullable + ) + } + + typeAliases = typeAliases.flatMap { (id, _) -> + classifierIndex.findTypeAliasesWithUnderlyingType(id) + } + } + + return type + } + + private fun isCommon(id: CirEntityId): Boolean = + commonDependencies.hasClassifier(id) || classifierIndices.all { index -> id in index.allClassifierIds } +} + diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirClassifierIndex.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirClassifierIndex.kt new file mode 100644 index 00000000000..5400b3363d1 --- /dev/null +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirClassifierIndex.kt @@ -0,0 +1,70 @@ +/* + * 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. + */ + +/* + * 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.mergedtree + +import org.jetbrains.kotlin.commonizer.cir.CirEntityId +import org.jetbrains.kotlin.commonizer.tree.* +import org.jetbrains.kotlin.commonizer.utils.compact +import org.jetbrains.kotlin.commonizer.utils.compactMapValues + +internal fun CirClassifierIndex(tree: CirTreeRoot): CirClassifierIndex { + return CirUnderlyingTypeIndexBuilder().apply { invoke(tree) }.build() +} + +internal interface CirClassifierIndex { + val allClassifierIds: Set + fun findTypeAliasesWithUnderlyingType(underlyingClassifier: CirEntityId): List +} + +private class CirClassifierIndexImpl( + override val allClassifierIds: Set, + private val typeAliasesByUnderlyingType: Map> +) : CirClassifierIndex { + + override fun findTypeAliasesWithUnderlyingType(underlyingClassifier: CirEntityId): List { + return typeAliasesByUnderlyingType[underlyingClassifier].orEmpty() + } +} + +private class CirUnderlyingTypeIndexBuilder { + private val index = mutableMapOf>() + private val classifierIds = mutableSetOf() + + operator fun invoke(tree: CirTreeRoot) { + tree.modules.forEach { module -> this(module) } + } + + operator fun invoke(module: CirTreeModule) { + module.packages.forEach { pkg -> this(pkg) } + } + + operator fun invoke(pkg: CirTreePackage) { + pkg.typeAliases.forEach { typeAlias -> this(typeAlias) } + pkg.classes.forEach { clazz -> this(clazz) } + } + + operator fun invoke(typeAlias: CirTreeTypeAlias) { + classifierIds.add(typeAlias.id) + index.getOrPut(typeAlias.typeAlias.underlyingType.classifierId) { mutableListOf() }.add(typeAlias) + } + + operator fun invoke(clazz: CirTreeClass) { + classifierIds.add(clazz.id) + clazz.classes.forEach { innerClazz -> this(innerClazz) } + } + + fun build(): CirClassifierIndex { + return CirClassifierIndexImpl( + allClassifierIds = classifierIds.toSet(), + typeAliasesByUnderlyingType = index.compactMapValues { (_, list) -> list.compact() } + ) + } +} diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirTypeSubstitutor.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirTypeSubstitutor.kt new file mode 100644 index 00000000000..4ad2f66a45b --- /dev/null +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirTypeSubstitutor.kt @@ -0,0 +1,45 @@ +/* + * 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.mergedtree + +import org.jetbrains.kotlin.commonizer.cir.CirFunction +import org.jetbrains.kotlin.commonizer.cir.CirProperty +import org.jetbrains.kotlin.commonizer.cir.CirType +import org.jetbrains.kotlin.commonizer.cir.CirValueParameter.Companion.copyInterned + +interface CirTypeSubstitutor { + fun substitute(targetIndex: Int, type: CirType): CirType +} + +fun CirTypeSubstitutor.substitute(index: Int, property: CirProperty): CirProperty { + val newReturnType = substitute(index, property.returnType) + return if (newReturnType != property.returnType) property.copy(returnType = newReturnType) else property +} + +fun CirTypeSubstitutor.substitute(index: Int, function: CirFunction): CirFunction { + val newExtensionReceiverType = function.extensionReceiver?.type?.let { substitute(index, it) } + + val newExtensionReceiver = if (newExtensionReceiverType != function.extensionReceiver?.type) + function.extensionReceiver?.copy(type = checkNotNull(newExtensionReceiverType)) + else function.extensionReceiver + + val newValueParameters = function.valueParameters.map { valueParameter -> + val newReturnType = substitute(index, valueParameter.returnType) + if (newReturnType != valueParameter.returnType) valueParameter.copyInterned(returnType = newReturnType) else valueParameter + } + + val newReturnType = substitute(index, function.returnType) + + return if ( + newExtensionReceiver != function.extensionReceiver || + newValueParameters != function.valueParameters || + newReturnType != function.returnType + ) function.copy( + extensionReceiver = newExtensionReceiver, + valueParameters = newValueParameters, + returnType = newReturnType + ) else function +} \ 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 70f55519565..e67c5c4b983 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/tree/mergeCirTree.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/tree/mergeCirTree.kt @@ -11,7 +11,10 @@ import org.jetbrains.kotlin.commonizer.mergedtree.* import org.jetbrains.kotlin.storage.StorageManager internal data class TargetBuildingContext( - val storageManager: StorageManager, val classifiers: CirKnownClassifiers, val memberContext: CirMemberContext = CirMemberContext.empty, + 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)) @@ -24,7 +27,17 @@ internal fun mergeCirTree( roots.targets.withIndex().forEach { (targetIndex, target) -> node.targetDeclarations[targetIndex] = CirRoot.create(target) node.buildModules( - TargetBuildingContext(storageManager, classifiers, CirMemberContext.empty, roots.size, targetIndex), roots[target].modules + TargetBuildingContext( + storageManager = storageManager, + classifiers = classifiers, + memberContext = CirMemberContext.empty, + typeSubstitutor = CirAliasTypeSubstitutor( + commonDependencies = classifiers.commonDependencies, + classifierIndices = roots.map(::CirClassifierIndex) + ), + targets = roots.size, + targetIndex = targetIndex + ), roots[target].modules ) } return node @@ -70,19 +83,21 @@ internal fun CirNodeWithMembers<*, *>.buildClass( internal fun CirNodeWithMembers<*, *>.buildFunction( context: TargetBuildingContext, function: CirFunction, parent: CirNode<*, *>? = null ) { - val functionNode = functions.getOrPut(FunctionApproximationKey.create(context.memberContext, function)) { + 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)) } - functionNode.targetDeclarations[context.targetIndex] = function + functionNode.targetDeclarations[context.targetIndex] = newFunction } internal fun CirNodeWithMembers<*, *>.buildProperty( context: TargetBuildingContext, property: CirProperty, parent: CirNode<*, *>? = null ) { - val propertyNode = properties.getOrPut(PropertyApproximationKey.create(context.memberContext, property)) { + 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)) } - propertyNode.targetDeclarations[context.targetIndex] = property + propertyNode.targetDeclarations[context.targetIndex] = newProperty } internal fun CirClassNode.buildConstructor( 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 8f73367f11d..3d5d48fa9ca 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/utils/commonizedGroup.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/utils/commonizedGroup.kt @@ -23,9 +23,9 @@ class CommonizedGroup( return elements[index] as T? } - operator fun set(index: Int, value: T) { + operator fun set(index: Int, value: T?) { val oldValue = this[index] - check(oldValue == null) { + check(oldValue == null || value == null) { "$oldValue can not be overwritten with $value at index $index" } diff --git a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalClassAndTypeAliasCommonizationTest.kt b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalClassAndTypeAliasCommonizationTest.kt index a38d02bfbee..2a1f5eb1edd 100644 --- a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalClassAndTypeAliasCommonizationTest.kt +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalClassAndTypeAliasCommonizationTest.kt @@ -7,7 +7,6 @@ package org.jetbrains.kotlin.commonizer.hierarchical import org.jetbrains.kotlin.commonizer.AbstractInlineSourcesCommonizationTest import org.jetbrains.kotlin.commonizer.assertCommonized -import org.junit.Test class HierarchicalClassAndTypeAliasCommonizationTest : AbstractInlineSourcesCommonizationTest() { @@ -573,66 +572,4 @@ class HierarchicalClassAndTypeAliasCommonizationTest : AbstractInlineSourcesComm """.trimIndent() ) } - - @Suppress("unused") - fun `ignored KT-47433 - test boxed function using TA and expanded type`() { - val result = commonize { - outputTarget("(a, b)") - - simpleSingleSourceTarget( - "a", """ - class Box - class A - typealias X = A - fun x(x: Box) - """.trimIndent() - ) - - simpleSingleSourceTarget( - "b", """ - class Box - class B - typealias X = B - fun x(x: Box) - """.trimIndent() - ) - } - - result.assertCommonized( - "(a, b)", """ - expect class Box expect constructor() - expect class X expect constructor() - expect fun x(x: Box) - """.trimIndent() - ) - } - - @Suppress("unused") - fun `ignored KT-47433 - test parameters with non-commonized TA expanding to a commonized type`() { - val result = commonize { - outputTarget("(a, b)") - - simpleSingleSourceTarget( - "a", """ - class X - fun useX(x: X) = Unit - """.trimIndent() - ) - - simpleSingleSourceTarget( - "b", """ - class X - typealias B = X - fun useX(x: B) = Unit - """.trimIndent() - ) - } - - result.assertCommonized( - "(a, b)", """ - expect class X expect constructor() - expect fun useX(x: X) - """.trimIndent() - ) - } } 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 6cfa5127479..dd67849c0d2 100644 --- a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalPropertyCommonizationTest.kt +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalPropertyCommonizationTest.kt @@ -184,7 +184,7 @@ class HierarchicalPropertyCommonizationTest : AbstractInlineSourcesCommonization expect class AB expect constructor() expect typealias TA_AB = AB // https://youtrack.jetbrains.com/issue/KT-47100 - expect val x: AB + expect val x: TA_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 new file mode 100644 index 00000000000..27acf9c5f29 --- /dev/null +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalTypeSubstitutionCommonizationTest.kt @@ -0,0 +1,106 @@ +/* + * 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.hierarchical + +import org.jetbrains.kotlin.commonizer.AbstractInlineSourcesCommonizationTest +import org.jetbrains.kotlin.commonizer.assertCommonized + +class HierarchicalTypeSubstitutionCommonizationTest : AbstractInlineSourcesCommonizationTest() { + + fun `test boxed function using TA and expanded type`() { + val result = commonize { + outputTarget("(a, b)") + + simpleSingleSourceTarget( + "a", """ + class Box + class A + typealias X = A + fun x(x: Box) + """.trimIndent() + ) + + simpleSingleSourceTarget( + "b", """ + class Box + class B + typealias X = B + fun x(x: Box) + """.trimIndent() + ) + } + + result.assertCommonized( + "(a, b)", """ + expect class Box expect constructor() + expect class X expect constructor() + expect fun x(x: Box) + """.trimIndent() + ) + } + + + fun `test boxed function using TA and expanded type - with box from dependencies`() { + val result = commonize { + outputTarget("(a, b)") + + registerDependency("a", "b", "(a, b)") { + source("""class Box""") + } + + simpleSingleSourceTarget( + "a", """ + class A + typealias X = A + fun x(x: Box) + """.trimIndent() + ) + + simpleSingleSourceTarget( + "b", """ + class B + typealias X = B + fun x(x: Box) + """.trimIndent() + ) + } + + result.assertCommonized( + "(a, b)", """ + expect class X expect constructor() + expect fun x(x: Box) + """.trimIndent() + ) + } + + fun `test parameters with non-commonized TA expanding to a commonized type`() { + val result = commonize { + outputTarget("(a, b)") + + simpleSingleSourceTarget( + "a", """ + class X + fun useX(x: X) = Unit + """.trimIndent() + ) + + simpleSingleSourceTarget( + "b", """ + class X + typealias B = X + fun useX(x: B) = Unit + """.trimIndent() + ) + } + + result.assertCommonized( + "(a, b)", """ + expect class X expect constructor() + expect fun useX(x: X) + """.trimIndent() + ) + } +} \ No newline at end of file