From bbe499c1b67c586fb6b508ae6d1228f696dfd83a Mon Sep 17 00:00:00 2001 From: "sebastian.sellmair" Date: Tue, 6 Apr 2021 10:32:17 +0200 Subject: [PATCH] [Commonizer] Re-introduce node builders 'parentCommonDeclaration' context --- .../mergers/ClassConstructorMerger.kt | 2 +- .../mergedtree/mergers/ClassMerger.kt | 3 +- .../mergedtree/mergers/FunctionMerger.kt | 6 +- .../mergedtree/mergers/PropertyMerger.kt | 6 +- .../commonizer/mergedtree/nodeBuilders.kt | 25 +++- .../kotlin/commonizer/tree/mergeCirTree.kt | 32 +++-- .../commonizer/core/TypeCommonizerTest.kt | 1 + .../FunctionReturnTypeCommonizationTest.kt | 120 ++++++++++++++++++ 8 files changed, 175 insertions(+), 20 deletions(-) create mode 100644 native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/FunctionReturnTypeCommonizationTest.kt diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/ClassConstructorMerger.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/ClassConstructorMerger.kt index fb010e62f3f..13c82728a4e 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/ClassConstructorMerger.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/ClassConstructorMerger.kt @@ -19,7 +19,7 @@ internal object ClassConstructorMerger { ) = with(context) { val approximationKey = ConstructorApproximationKey(constructor, context.typeResolver) val constructorNode: CirClassConstructorNode = classNode.constructors.getOrPut(approximationKey) { - buildClassConstructorNode(storageManager, targets, classifiers) + buildClassConstructorNode(storageManager, targets, classifiers, classNode.commonDeclaration) } constructorNode.targetDeclarations[context.targetIndex] = CirDeserializers.constructor( source = constructor, diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/ClassMerger.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/ClassMerger.kt index 10e15089a96..5261cb487e0 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/ClassMerger.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/ClassMerger.kt @@ -26,8 +26,9 @@ internal class ClassMerger( val classId = classEntry.classId val className = classId.relativeNameSegments.last() + val maybeClassOwnerNode: CirClassNode? = ownerNode as? CirClassNode val classNode: CirClassNode = ownerNode.classes.getOrPut(className) { - buildClassNode(storageManager, targets, classifiers, classId) + buildClassNode(storageManager, targets, classifiers, maybeClassOwnerNode?.commonDeclaration, classId) } val clazz: KmClass? diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/FunctionMerger.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/FunctionMerger.kt index 3b83943e5a4..730c8825009 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/FunctionMerger.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/FunctionMerger.kt @@ -27,14 +27,16 @@ internal object FunctionMerger { return } + val maybeClassOwnerNode: CirClassNode? = ownerNode as? CirClassNode + val approximationKey = FunctionApproximationKey(function, context.typeResolver) val functionNode: CirFunctionNode = ownerNode.functions.getOrPut(approximationKey) { - buildFunctionNode(storageManager, targets, classifiers) + buildFunctionNode(storageManager, targets, classifiers, maybeClassOwnerNode?.commonDeclaration) } functionNode.targetDeclarations[context.targetIndex] = CirDeserializers.function( name = approximationKey.name, source = function, - containingClass = ownerNode.run { this as? CirClassNode }?.targetDeclarations?.get(context.targetIndex), + containingClass = maybeClassOwnerNode?.targetDeclarations?.get(context.targetIndex), typeResolver = context.typeResolver ) } diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/PropertyMerger.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/PropertyMerger.kt index 563f9fac917..f54f65c3033 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/PropertyMerger.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/mergers/PropertyMerger.kt @@ -21,14 +21,16 @@ internal object PropertyMerger { if (property.isFakeOverride()) return + val maybeClassOwnerNode: CirClassNode? = ownerNode as? CirClassNode + val approximationKey = PropertyApproximationKey(property, context.typeResolver) val propertyNode: CirPropertyNode = ownerNode.properties.getOrPut(approximationKey) { - buildPropertyNode(storageManager, targets, classifiers) + buildPropertyNode(storageManager, targets, classifiers, maybeClassOwnerNode?.commonDeclaration) } propertyNode.targetDeclarations[context.targetIndex] = CirDeserializers.property( name = approximationKey.name, source = property, - containingClass = ownerNode.run { this as? CirClassNode }?.targetDeclarations?.get(context.targetIndex), + containingClass = maybeClassOwnerNode?.targetDeclarations?.get(context.targetIndex), typeResolver = context.typeResolver ) } diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/nodeBuilders.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/nodeBuilders.kt index 00f401e53dd..84da6242c9d 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/nodeBuilders.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/nodeBuilders.kt @@ -48,9 +48,11 @@ internal fun buildPropertyNode( storageManager: StorageManager, size: Int, classifiers: CirKnownClassifiers, + parentCommonDeclaration: NullableLazyValue<*>? ): CirPropertyNode = buildNode( storageManager = storageManager, size = size, + parentCommonDeclaration = parentCommonDeclaration, commonizerProducer = { PropertyCommonizer(classifiers) }, nodeProducer = ::CirPropertyNode ) @@ -59,9 +61,11 @@ internal fun buildFunctionNode( storageManager: StorageManager, size: Int, classifiers: CirKnownClassifiers, + parentCommonDeclaration: NullableLazyValue<*>? ): CirFunctionNode = buildNode( storageManager = storageManager, size = size, + parentCommonDeclaration = parentCommonDeclaration, commonizerProducer = { FunctionCommonizer(classifiers) }, nodeProducer = ::CirFunctionNode ) @@ -70,10 +74,12 @@ internal fun buildClassNode( storageManager: StorageManager, size: Int, classifiers: CirKnownClassifiers, + parentCommonDeclaration: NullableLazyValue<*>?, classId: CirEntityId ): CirClassNode = buildNode( storageManager = storageManager, size = size, + parentCommonDeclaration = parentCommonDeclaration, commonizerProducer = { ClassCommonizer(classifiers) }, recursionMarker = CirClassRecursionMarker, nodeProducer = { targetDeclarations, commonDeclaration -> @@ -87,9 +93,11 @@ internal fun buildClassConstructorNode( storageManager: StorageManager, size: Int, classifiers: CirKnownClassifiers, + parentCommonDeclaration: NullableLazyValue<*>? ): CirClassConstructorNode = buildNode( storageManager = storageManager, size = size, + parentCommonDeclaration = parentCommonDeclaration, commonizerProducer = { ClassConstructorCommonizer(classifiers) }, nodeProducer = ::CirClassConstructorNode ) @@ -114,13 +122,14 @@ internal fun buildTypeAliasNode( private fun > buildNode( storageManager: StorageManager, size: Int, + parentCommonDeclaration: NullableLazyValue<*>? = null, commonizerProducer: () -> Commonizer, recursionMarker: R? = null, nodeProducer: (CommonizedGroup, NullableLazyValue) -> N ): N { val targetDeclarations = CommonizedGroup(size) - val commonComputable = { commonize(targetDeclarations, commonizerProducer()) } + val commonComputable = { commonize(parentCommonDeclaration, targetDeclarations, commonizerProducer()) } val commonLazyValue = if (recursionMarker != null) storageManager.createRecursionTolerantNullableLazyValue(commonComputable, recursionMarker) @@ -141,3 +150,17 @@ internal fun commonize( return commonizer.result } + +@Suppress("NOTHING_TO_INLINE") +private inline fun commonize( + parentCommonDeclaration: NullableLazyValue<*>?, + targetDeclarations: CommonizedGroup, + commonizer: Commonizer +): R? { + if (parentCommonDeclaration != null && parentCommonDeclaration.invoke() == null) { + // don't commonize declaration if it's parent failed to commonize + return null + } + + return commonize(targetDeclarations, commonizer) +} 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 681b83fb508..02936461dc6 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/tree/mergeCirTree.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/tree/mergeCirTree.kt @@ -8,7 +8,6 @@ package org.jetbrains.kotlin.commonizer.tree import org.jetbrains.kotlin.commonizer.TargetDependent import org.jetbrains.kotlin.commonizer.cir.CirRoot import org.jetbrains.kotlin.commonizer.mergedtree.* -import org.jetbrains.kotlin.commonizer.tree.* import org.jetbrains.kotlin.storage.StorageManager internal data class TargetBuildingContext( @@ -49,34 +48,41 @@ internal fun CirModuleNode.buildPackage(context: TargetBuildingContext, treePack treePackage.classes.forEach { clazz -> packageNode.buildClass(context, clazz) } } -internal fun CirNodeWithMembers<*, *>.buildClass(context: TargetBuildingContext, treeClass: CirTreeClass) { +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, treeClass.id) + buildClassNode(context.storageManager, context.targets, context.classifiers, parent?.commonDeclaration, treeClass.id) } classNode.targetDeclarations[context.targetIndex] = treeClass.clazz - treeClass.functions.forEach { function -> classNode.buildFunction(context, function) } - treeClass.properties.forEach { property -> classNode.buildProperty(context, property) } - treeClass.constructors.forEach { constructor -> classNode.buildConstructor(context, constructor) } - treeClass.classes.forEach { clazz -> classNode.buildClass(context, clazz) } + treeClass.functions.forEach { function -> classNode.buildFunction(context, function, classNode) } + treeClass.properties.forEach { property -> classNode.buildProperty(context, property, classNode) } + treeClass.constructors.forEach { constructor -> classNode.buildConstructor(context, constructor, classNode) } + treeClass.classes.forEach { clazz -> classNode.buildClass(context, clazz, classNode) } } -internal fun CirNodeWithMembers<*, *>.buildFunction(context: TargetBuildingContext, treeFunction: CirTreeFunction) { +internal fun CirNodeWithMembers<*, *>.buildFunction( + context: TargetBuildingContext, treeFunction: CirTreeFunction, parent: CirNode<*, *>? = null +) { val functionNode = functions.getOrPut(treeFunction.approximationKey) { - buildFunctionNode(context.storageManager, context.targets, context.classifiers) + buildFunctionNode(context.storageManager, context.targets, context.classifiers, parent?.commonDeclaration) } functionNode.targetDeclarations[context.targetIndex] = treeFunction.function } -internal fun CirNodeWithMembers<*, *>.buildProperty(context: TargetBuildingContext, treeProperty: CirTreeProperty) { +internal fun CirNodeWithMembers<*, *>.buildProperty( + context: TargetBuildingContext, treeProperty: CirTreeProperty, parent: CirNode<*, *>? = null) { val propertyNode = properties.getOrPut(treeProperty.approximationKey) { - buildPropertyNode(context.storageManager, context.targets, context.classifiers) + buildPropertyNode(context.storageManager, context.targets, context.classifiers, parent?.commonDeclaration) } propertyNode.targetDeclarations[context.targetIndex] = treeProperty.property } -internal fun CirClassNode.buildConstructor(context: TargetBuildingContext, treeConstructor: CirTreeClassConstructor) { +internal fun CirClassNode.buildConstructor( + context: TargetBuildingContext, treeConstructor: CirTreeClassConstructor, parent: CirNode<*, *>? +) { val constructorNode = constructors.getOrPut(treeConstructor.approximationKey) { - buildClassConstructorNode(context.storageManager, context.targets, context.classifiers) + buildClassConstructorNode(context.storageManager, context.targets, context.classifiers, parent?.commonDeclaration) } constructorNode.targetDeclarations[context.targetIndex] = treeConstructor.constructor } diff --git a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/TypeCommonizerTest.kt b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/TypeCommonizerTest.kt index 8fbb1e411f5..6ad2a1e4dfb 100644 --- a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/TypeCommonizerTest.kt +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/TypeCommonizerTest.kt @@ -469,6 +469,7 @@ class TypeCommonizerTest : AbstractCommonizerTest() { storageManager = LockBasedStorageManager.NO_LOCKS, size = variants.size, classifiers = classifiers, + parentCommonDeclaration = null, classId = type.classifierId ) } diff --git a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/FunctionReturnTypeCommonizationTest.kt b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/FunctionReturnTypeCommonizationTest.kt new file mode 100644 index 00000000000..69145096c33 --- /dev/null +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/FunctionReturnTypeCommonizationTest.kt @@ -0,0 +1,120 @@ +/* + * 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 FunctionReturnTypeCommonizationTest : AbstractInlineSourcesCommonizationTest() { + + fun `test non-commonized return type of top level function`() { + val result = commonize { + outputTarget("(a,b)") + simpleSingleSourceTarget( + "a", """ + class A { // NOTE: Class + class B + } + fun x(): A.B = TODO() + """ + ) + + simpleSingleSourceTarget( + "b", """ + interface A { // NOTE: Interface + class B + } + fun x(): A.B = TODO() + """ + ) + } + + /** + * -> A is not commonized (interface vs class) + * -> B is not commonized + * -> fun x can't be commonized + * -> Empty commonization + */ + result.assertCommonized( + "(a,b)", "" + ) + + result.assertCommonized( + "a", """ + class A { + class B + } + fun x(): A.B = TODO() + """ + ) + + result.assertCommonized( + "b", """ + interface A { + class B + } + fun x(): A.B = TODO() + """ + ) + } + + + fun `test commonized return type of top level function`() { + val result = commonize { + outputTarget("(a,b)") + simpleSingleSourceTarget( + "a", """ + interface A { + class B + } + fun x(): A.B = TODO() + """ + ) + + simpleSingleSourceTarget( + "b", """ + interface A { + class B + } + fun x(): A.B = TODO() + """ + ) + } + + /** + * -> A is not commonized (interface vs class) + * -> B is not commonized + * -> fun x can't be commonized + * -> Empty commonization + */ + result.assertCommonized( + "(a,b)", """ + expect interface A { + expect class B expect constructor() + } + expect fun x(): A.B + """ + ) + + result.assertCommonized( + "a", """ + interface A { + class B + } + fun x(): A.B = TODO() + """ + ) + + result.assertCommonized( + "b", """ + interface A { + class B + } + fun x(): A.B = TODO() + """ + ) + } +}