[Commonizer] Stricter processing of forward declarations

native/commonizer/src/org/jetbrains/kotlin/descriptors/commonizer/TargetProvider.kt

@@ -28,7 +28,16 @@ interface BuiltInsProvider {
 }
 
 interface ModulesProvider {
-    class ModuleInfo(val name: String, val originalLocation: File)
+    class ModuleInfo(
+        val name: String,
+        val originalLocation: File,
+        val cInteropAttributes: CInteropModuleAttributes?
+    )
+
+    class CInteropModuleAttributes(
+        val mainPackageFqName: String,
+        val exportForwardDeclarations: Collection<String>
+    )
 
     fun loadModuleInfos(): Map<String, ModuleInfo>
     fun loadModules(): Map<String, ModuleDescriptor>

native/commonizer/src/org/jetbrains/kotlin/descriptors/commonizer/core/TypeCommonizer.kt

@@ -8,8 +8,10 @@ package org.jetbrains.kotlin.descriptors.commonizer.core
 import org.jetbrains.kotlin.descriptors.DescriptorVisibility
 import org.jetbrains.kotlin.descriptors.commonizer.cir.*
 import org.jetbrains.kotlin.descriptors.commonizer.cir.factory.CirTypeFactory
+import org.jetbrains.kotlin.descriptors.commonizer.core.CommonizedTypeAliasAnswer.Companion.FAILURE_MISSING_IN_SOME_TARGET
+import org.jetbrains.kotlin.descriptors.commonizer.core.CommonizedTypeAliasAnswer.Companion.SUCCESS_FROM_DEPENDEE_LIBRARY
 import org.jetbrains.kotlin.descriptors.commonizer.mergedtree.CirClassifiersCache
-import org.jetbrains.kotlin.descriptors.commonizer.mergedtree.CirNodeWithClassId
+import org.jetbrains.kotlin.descriptors.commonizer.utils.isUnderKotlinNativeSyntheticPackages
 import org.jetbrains.kotlin.descriptors.commonizer.utils.isUnderStandardKotlinPackages
 import org.jetbrains.kotlin.name.ClassId
 import org.jetbrains.kotlin.types.Variance
@@ -66,7 +68,7 @@ private class ClassTypeCommonizer(private val cache: CirClassifiersCache) : Abst
         isMarkedNullable == next.isMarkedNullable
                 && classId == next.classifierId
                 && outerType.commonizeWith(next.outerType)
-                && commonizeClassifier(classId) { cache.classNode(classId) }.first
+                && commonizeClass(classId, cache)
                 && arguments.commonizeWith(next.arguments)
 }
 
@@ -102,11 +104,11 @@ private class TypeAliasTypeCommonizer(private val cache: CirClassifiersCache) :
             return false
 
         if (commonizedTypeBuilder == null) {
-            val (commonized, commonClassifier) = commonizeClassifier(typeAliasId) { cache.typeAliasNode(typeAliasId) }
-            if (!commonized)
+            val answer = commonizeTypeAlias(typeAliasId, cache)
+            if (!answer.commonized)
                 return false
 
-            commonizedTypeBuilder = when (commonClassifier) {
+            commonizedTypeBuilder = when (val commonClassifier = answer.commonClassifier) {
                 is CirClass -> CommonizedTypeAliasTypeBuilder.forClass(commonClassifier)
                 is CirTypeAlias -> CommonizedTypeAliasTypeBuilder.forTypeAlias(commonClassifier)
                 null -> CommonizedTypeAliasTypeBuilder.forKnownUnderlyingType(next.underlyingType)
@@ -216,26 +218,59 @@ private class TypeArgumentListCommonizer(cache: CirClassifiersCache) : AbstractL
     singleElementCommonizerFactory = { TypeArgumentCommonizer(cache) }
 )
 
-private inline fun <reified T : CirClassifier> commonizeClassifier(
-    classifierId: ClassId,
-    classifierNode: (classifierId: ClassId) -> CirNodeWithClassId<*, T>?
-): Pair<Boolean, T?> {
-    if (classifierId.packageFqName.isUnderStandardKotlinPackages) {
-        /* either class or type alias from Kotlin stdlib */
-        return true to null
+private fun commonizeClass(classId: ClassId, cache: CirClassifiersCache): Boolean {
+    val packageFqName = classId.packageFqName
+    if (packageFqName.isUnderStandardKotlinPackages) {
+        // The class is from Kotlin stdlib. Already commonized.
+        return true
+    } else if (packageFqName.isUnderKotlinNativeSyntheticPackages) {
+        // C/Obj-C forward declarations are:
+        // - Either resolved to real classes/interfaces from other interop libraries (which are generated by C-interop tool and
+        //   are known to have modality/visibility/other attributes to successfully pass commonization).
+        // - Or resolved to the same synthetic classes/interfaces.
+        // ... and therefore are considered as successfully commonized.
+        return true
     }
 
-    /* or descriptors themselves can be commonized */
-    return when (val node = classifierNode(classifierId)) {
+    return when (val node = cache.classNode(classId)) {
         null -> {
-            // No node means that the class or type alias was not subject for commonization at all, probably it lays
-            // not in commonized module descriptors but somewhere in their dependencies.
-            true to null
+            // No node means that the class was not subject for commonization.
+            // - Either it is missing in certain targets at all => not commonized.
+            // - Or it is a known forward declaration => consider it as commonized.
+            cache.isExportedForwardDeclaration(classId)
         }
         else -> {
-            // If entry is present, then contents (common declaration) should not be null.
-            val commonClassifier = node.commonDeclaration()
-            (commonClassifier != null) to commonClassifier
+            // Common declaration in node is not null -> successfully commonized.
+            (node.commonDeclaration() != null)
         }
     }
 }
+
+private fun commonizeTypeAlias(typeAliasId: ClassId, cache: CirClassifiersCache): CommonizedTypeAliasAnswer {
+    val packageFqName = typeAliasId.packageFqName
+    if (packageFqName.isUnderStandardKotlinPackages) {
+        // The type alias is from Kotlin stdlib. Already commonized.
+        return SUCCESS_FROM_DEPENDEE_LIBRARY
+    }
+
+    return when (val node = cache.typeAliasNode(typeAliasId)) {
+        null -> {
+            // No node means that the type alias was not subject for commonization. It is missing in some target(s) => not commonized.
+            FAILURE_MISSING_IN_SOME_TARGET
+        }
+        else -> {
+            // Common declaration in node is not null -> successfully commonized.
+            CommonizedTypeAliasAnswer.create(node.commonDeclaration())
+        }
+    }
+}
+
+private class CommonizedTypeAliasAnswer(val commonized: Boolean, val commonClassifier: CirClassifier?) {
+    companion object {
+        val SUCCESS_FROM_DEPENDEE_LIBRARY = CommonizedTypeAliasAnswer(true, null)
+        val FAILURE_MISSING_IN_SOME_TARGET = CommonizedTypeAliasAnswer(false, null)
+
+        fun create(commonClassifier: CirClassifier?) =
+            if (commonClassifier != null) CommonizedTypeAliasAnswer(true, commonClassifier) else FAILURE_MISSING_IN_SOME_TARGET
+    }
+}

native/commonizer/src/org/jetbrains/kotlin/descriptors/commonizer/konan/NativeDistributionModulesProvider.kt

@@ -10,6 +10,7 @@ import org.jetbrains.kotlin.config.LanguageVersionSettingsImpl
 import org.jetbrains.kotlin.descriptors.ModuleDescriptor
 import org.jetbrains.kotlin.descriptors.commonizer.BuiltInsProvider
 import org.jetbrains.kotlin.descriptors.commonizer.ModulesProvider
+import org.jetbrains.kotlin.descriptors.commonizer.ModulesProvider.CInteropModuleAttributes
 import org.jetbrains.kotlin.descriptors.commonizer.ModulesProvider.ModuleInfo
 import org.jetbrains.kotlin.descriptors.commonizer.utils.NativeFactories
 import org.jetbrains.kotlin.descriptors.commonizer.utils.createKotlinNativeForwardDeclarationsModule
@@ -36,10 +37,20 @@ internal class NativeDistributionModulesProvider(
 
     override fun loadModuleInfos(): Map<String, ModuleInfo> {
         return libraries.platformLibs.associate { library ->
-            val name = library.manifestData.uniqueName
+            val manifestData = library.manifestData
+
+            val name = manifestData.uniqueName
             val location = File(library.library.libraryFile.path)
 
-            name to ModuleInfo(name, location)
+            val cInteropAttributes = if (manifestData.isInterop) {
+                val packageFqName = manifestData.packageFqName
+                    ?: manifestData.shortName?.let { "platform.$it" }
+                    ?: manifestData.uniqueName.substringAfter("platform.").let { "platform.$it" }
+
+                CInteropModuleAttributes(packageFqName, manifestData.exportForwardDeclarations)
+            } else null
+
+            name to ModuleInfo(name, location, cInteropAttributes)
         }
     }
 

native/commonizer/src/org/jetbrains/kotlin/descriptors/commonizer/konan/NativeSensitiveManifestData.kt

@@ -37,7 +37,7 @@ internal data class NativeSensitiveManifestData(
         addOptionalProperty(KLIB_PROPERTY_DEPENDS, dependencies.isNotEmpty()) { dependencies.joinToString(separator = " ") }
         addOptionalProperty(KLIB_PROPERTY_INTEROP, isInterop) { "true" }
         addOptionalProperty(KLIB_PROPERTY_PACKAGE, packageFqName != null) { packageFqName!! }
-        addOptionalProperty(KLIB_PROPERTY_EXPORT_FORWARD_DECLARATIONS, exportForwardDeclarations.isNotEmpty()) {
+        addOptionalProperty(KLIB_PROPERTY_EXPORT_FORWARD_DECLARATIONS, exportForwardDeclarations.isNotEmpty() || isInterop) {
             exportForwardDeclarations.joinToString(" ")
         }
         addOptionalProperty(KLIB_PROPERTY_NATIVE_TARGETS, nativeTargets.isNotEmpty()) {
@@ -51,15 +51,22 @@ internal data class NativeSensitiveManifestData(
 
         check(uniqueName == other.uniqueName)
 
-        // Assumption: It's enough to merge native targets list, other properties can be taken from 'this' manifest.
+        // Merge algorithm:
+        // - Unite native target lists.
+        // - Intersect dependency lists.
+        // - Boolean and 'isInterop'.
+        // - If both libs are 'isInterop' then intersect exported forward declaration lists.
+        // - Other properties can be taken from 'this' manifest.
+
+        val bothAreInterop = isInterop && other.isInterop
 
         return NativeSensitiveManifestData(
             uniqueName = uniqueName,
             versions = versions,
             dependencies = (dependencies intersect other.dependencies).toList(),
-            isInterop = isInterop,
+            isInterop = bothAreInterop,
             packageFqName = packageFqName,
-            exportForwardDeclarations = exportForwardDeclarations,
+            exportForwardDeclarations = if (bothAreInterop) (exportForwardDeclarations intersect other.exportForwardDeclarations).toList() else emptyList(),
             nativeTargets = HashSet<String>().apply {
                 addAll(nativeTargets)
                 addAll(other.nativeTargets)

native/commonizer/src/org/jetbrains/kotlin/descriptors/commonizer/mergedtree/CirClassifiersCache.kt

@@ -6,22 +6,35 @@
 package org.jetbrains.kotlin.descriptors.commonizer.mergedtree
 
 import gnu.trove.THashMap
+import gnu.trove.THashSet
+import org.jetbrains.kotlin.descriptors.commonizer.utils.isUnderKotlinNativeSyntheticPackages
 import org.jetbrains.kotlin.name.ClassId
 
 interface CirClassifiersCache {
+    fun isExportedForwardDeclaration(classId: ClassId): Boolean
+
     fun classNode(classId: ClassId): CirClassNode?
     fun typeAliasNode(typeAliasId: ClassId): CirTypeAliasNode?
 
+    fun addExportedForwardDeclaration(classId: ClassId)
+
     fun addClassNode(classId: ClassId, node: CirClassNode)
     fun addTypeAliasNode(typeAliasId: ClassId, node: CirTypeAliasNode)
 }
 
 class DefaultCirClassifiersCache : CirClassifiersCache {
+    private val exportedForwardDeclarations = THashSet<ClassId>()
     private val classNodes = THashMap<ClassId, CirClassNode>()
     private val typeAliases = THashMap<ClassId, CirTypeAliasNode>()
 
-    override fun classNode(classId: ClassId): CirClassNode? = classNodes[classId]
-    override fun typeAliasNode(typeAliasId: ClassId): CirTypeAliasNode? = typeAliases[typeAliasId]
+    override fun isExportedForwardDeclaration(classId: ClassId) = classId in exportedForwardDeclarations
+    override fun classNode(classId: ClassId) = classNodes[classId]
+    override fun typeAliasNode(typeAliasId: ClassId) = typeAliases[typeAliasId]
+
+    override fun addExportedForwardDeclaration(classId: ClassId) {
+        check(!classId.packageFqName.isUnderKotlinNativeSyntheticPackages)
+        exportedForwardDeclarations += classId
+    }
 
     override fun addClassNode(classId: ClassId, node: CirClassNode) {
         val oldNode = classNodes.put(classId, node)

native/commonizer/src/org/jetbrains/kotlin/descriptors/commonizer/mergedtree/CirTreeMerger.kt

@@ -21,6 +21,31 @@ import org.jetbrains.kotlin.resolve.scopes.MemberScope
 import org.jetbrains.kotlin.storage.NullableLazyValue
 import org.jetbrains.kotlin.storage.StorageManager
 
+/**
+ * N.B. Limitations on C/Obj-C interop.
+ *
+ * [Case 1]: An interop library with two fragments for two targets. The first fragment has a forward declaration of classifier A.
+ * The second one has a definition of class A. Both fragments have a top-level callable (ex: function)
+ * with the same signature that refers to type "A" as its return type.
+ *
+ * What will happen: Forward declarations will be ignored during building CIR merged tree. So the node for class A
+ * will contain CirClass "A" for the second target only. This node will not succeed in commonization, and no common class
+ * declaration will be produced. As a result the top-level callable will not be commonized, as it refers to the type "A"
+ * that is not formally commonized.
+ *
+ * This is not strictly correct: The classifier "A" exists in both targets though in different form. So if the user
+ * would write shared source code that uses "A" and the callable, then this code would successfully compile against both targets.
+ *
+ * The reason why commonization of such classifiers is not supported yet is that this is quite a rare case that requires
+ * a complex implementation with potential performance penalty.
+ *
+ * [Case 2]: A library with two fragments for two targets. The first fragment is interop. The second one is not.
+ * Similarly to case 1, the 1st fragment has a forward declaration of a classifier, and the 2nd has a real classifier.
+ *
+ * At the moment, this is an exotic case. It could happen if someone tries to commonize an MPP library for Native and non-Native
+ * targets (which is not supported yet), or a Native library where one fragment is produced via C-interop tool and the other one
+ * is compiled from Kotlin/Native source code (not sure this should be supported at all).
+ */
 class CirTreeMerger(
     private val storageManager: StorageManager,
     private val cache: CirClassifiersCache,
@@ -40,7 +65,8 @@ class CirTreeMerger(
         val commonModuleNames = allModuleInfos.map { it.keys }.reduce { a, b -> a intersect b }
 
         parameters.targetProviders.forEachIndexed { targetIndex, targetProvider ->
-            processTarget(rootNode, targetIndex, targetProvider, commonModuleNames)
+            val commonModuleInfos = allModuleInfos[targetIndex].filterKeys { it in commonModuleNames }
+            processTarget(rootNode, targetIndex, targetProvider, commonModuleInfos)
             parameters.progressLogger?.invoke("Loaded declarations for [${targetProvider.target.name}]")
             System.gc()
         }
@@ -61,7 +87,7 @@ class CirTreeMerger(
         rootNode: CirRootNode,
         targetIndex: Int,
         targetProvider: TargetProvider,
-        commonModuleNames: Set<String>
+        commonModuleInfos: Map<String, ModuleInfo>
     ) {
         rootNode.targetDeclarations[targetIndex] = CirRootFactory.create(
             targetProvider.target,
@@ -73,16 +99,28 @@ class CirTreeMerger(
         val modules: MutableMap<Name, CirModuleNode> = rootNode.modules
 
         moduleDescriptors.forEach { (name, moduleDescriptor) ->
-            if (name in commonModuleNames)
-                processModule(modules, targetIndex, moduleDescriptor)
+            val moduleInfo = commonModuleInfos[name] ?: return@forEach
+            processModule(modules, targetIndex, moduleInfo, moduleDescriptor)
         }
     }
 
     private fun processModule(
         modules: MutableMap<Name, CirModuleNode>,
         targetIndex: Int,
+        moduleInfo: ModuleInfo,
         moduleDescriptor: ModuleDescriptor
     ) {
+        moduleInfo.cInteropAttributes?.let { cInteropAttributes ->
+            val exportForwardDeclarations = cInteropAttributes.exportForwardDeclarations.takeIf { it.isNotEmpty() } ?: return@let
+            val mainPackageFqName = FqName(cInteropAttributes.mainPackageFqName).intern()
+
+            exportForwardDeclarations.forEach { classFqName ->
+                // Class has synthetic package FQ name (cnames/objcnames). Need to transfer it to the main package.
+                val className = Name.identifier(classFqName.substringAfterLast('.')).intern()
+                cache.addExportedForwardDeclaration(internedClassId(mainPackageFqName, className))
+            }
+        }
+
         val moduleName: Name = moduleDescriptor.name.intern()
         val moduleNode: CirModuleNode = modules.getOrPut(moduleName) {
             buildModuleNode(storageManager, size)

native/commonizer/src/org/jetbrains/kotlin/descriptors/commonizer/utils/fqName.kt

@@ -46,7 +46,8 @@ internal val FqName.isUnderKotlinNativeSyntheticPackages: Boolean
 internal val FqName.isUnderDarwinPackage: Boolean
     get() = asString().hasPrefix(DARWIN_PACKAGE)
 
-private fun FqName.hasAnyPrefix(prefixes: List<String>): Boolean =
+@Suppress("NOTHING_TO_INLINE")
+private inline fun FqName.hasAnyPrefix(prefixes: List<String>): Boolean =
     asString().let { fqName -> prefixes.any(fqName::hasPrefix) }
 
 private fun String.hasPrefix(prefix: String): Boolean {

native/commonizer/tests/org/jetbrains/kotlin/descriptors/commonizer/core/ExtensionReceiverCommonizerTest.kt

@@ -8,7 +8,7 @@ package org.jetbrains.kotlin.descriptors.commonizer.core
 import org.jetbrains.kotlin.descriptors.commonizer.cir.CirExtensionReceiver
 import org.jetbrains.kotlin.descriptors.commonizer.cir.factory.CirExtensionReceiverFactory
 import org.jetbrains.kotlin.descriptors.commonizer.cir.factory.CirTypeFactory
-import org.jetbrains.kotlin.descriptors.commonizer.utils.EMPTY_CLASSIFIERS_CACHE
+import org.jetbrains.kotlin.descriptors.commonizer.utils.MOCK_CLASSIFIERS_CACHE
 import org.jetbrains.kotlin.descriptors.commonizer.utils.mockClassType
 import org.junit.Test
 
@@ -49,7 +49,7 @@ class ExtensionReceiverCommonizerTest : AbstractCommonizerTest<CirExtensionRecei
         mockExtensionReceiver("kotlin.String")
     )
 
-    override fun createCommonizer() = ExtensionReceiverCommonizer(EMPTY_CLASSIFIERS_CACHE)
+    override fun createCommonizer() = ExtensionReceiverCommonizer(MOCK_CLASSIFIERS_CACHE)
 }
 
 private fun mockExtensionReceiver(typeFqName: String) = CirExtensionReceiverFactory.create(

native/commonizer/tests/org/jetbrains/kotlin/descriptors/commonizer/core/TypeParameterCommonizerTest.kt

@@ -8,14 +8,14 @@ package org.jetbrains.kotlin.descriptors.commonizer.core
 import org.jetbrains.kotlin.descriptors.commonizer.cir.CirTypeParameter
 import org.jetbrains.kotlin.descriptors.commonizer.cir.factory.CirTypeFactory
 import org.jetbrains.kotlin.descriptors.commonizer.cir.factory.CirTypeParameterFactory
-import org.jetbrains.kotlin.descriptors.commonizer.utils.EMPTY_CLASSIFIERS_CACHE
+import org.jetbrains.kotlin.descriptors.commonizer.utils.MOCK_CLASSIFIERS_CACHE
 import org.jetbrains.kotlin.descriptors.commonizer.utils.mockClassType
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.types.Variance
 import org.junit.Test
 
 class TypeParameterCommonizerTest : AbstractCommonizerTest<CirTypeParameter, CirTypeParameter>() {
-    override fun createCommonizer() = TypeParameterCommonizer(EMPTY_CLASSIFIERS_CACHE)
+    override fun createCommonizer() = TypeParameterCommonizer(MOCK_CLASSIFIERS_CACHE)
 
     @Test
     fun allAreReified() = doTestSuccess(

native/commonizer/tests/org/jetbrains/kotlin/descriptors/commonizer/core/TypeParameterListCommonizerTest.kt

@@ -6,7 +6,7 @@
 package org.jetbrains.kotlin.descriptors.commonizer.core
 
 import org.jetbrains.kotlin.descriptors.commonizer.cir.CirTypeParameter
-import org.jetbrains.kotlin.descriptors.commonizer.utils.EMPTY_CLASSIFIERS_CACHE
+import org.jetbrains.kotlin.descriptors.commonizer.utils.MOCK_CLASSIFIERS_CACHE
 import org.junit.Test
 
 class TypeParameterListCommonizerTest : AbstractCommonizerTest<List<CirTypeParameter>, List<CirTypeParameter>>() {
@@ -108,7 +108,7 @@ class TypeParameterListCommonizerTest : AbstractCommonizerTest<List<CirTypeParam
         )
     )
 
-    override fun createCommonizer() = TypeParameterListCommonizer(EMPTY_CLASSIFIERS_CACHE)
+    override fun createCommonizer() = TypeParameterListCommonizer(MOCK_CLASSIFIERS_CACHE)
 
     private companion object {
         fun mockTypeParams(vararg params: Pair<String, String>): List<CirTypeParameter> {