[IrFakeOverrideBuilder] Consolidate visibility handling

Now fake overrides declarations invisible for override
are totally ignored by FakeOverrideBuilder, instead of
creating fake overrides for them, and than filtering them
out later.

As side-effect it fixes KT-64972.

^KT-64974

compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/overrides/FakeOverrideBuilderStrategy.kt

@@ -11,13 +11,11 @@ import org.jetbrains.kotlin.descriptors.ModuleDescriptor
 import org.jetbrains.kotlin.ir.declarations.*
 import org.jetbrains.kotlin.ir.symbols.IrClassSymbol
 import org.jetbrains.kotlin.ir.symbols.IrTypeParameterSymbol
-import org.jetbrains.kotlin.ir.types.IrSimpleType
+import org.jetbrains.kotlin.ir.types.*
-import org.jetbrains.kotlin.ir.types.IrType
-import org.jetbrains.kotlin.ir.types.IrTypeProjection
-import org.jetbrains.kotlin.ir.types.extractTypeParameters
 import org.jetbrains.kotlin.ir.util.getPackageFragment
 import org.jetbrains.kotlin.ir.util.hasAnnotation
 import org.jetbrains.kotlin.ir.util.render
+import org.jetbrains.kotlin.name.StandardClassIds
 import org.jetbrains.kotlin.types.Variance
 
 /**
@@ -37,10 +35,15 @@ abstract class FakeOverrideBuilderStrategy(
     private val unimplementedOverridesStrategy: IrUnimplementedOverridesStrategy
 ) {
     /**
-     * Creates a fake override for [member] from [superType] to be added to the class [clazz]
+     * Creates a fake override for [member] from [superType] to be added to the class [clazz] or returns null,
+     * if no fake override should be created for this member
      */
-    fun fakeOverrideMember(superType: IrType, member: IrOverridableMember, clazz: IrClass): IrOverridableMember =
+    fun fakeOverrideMember(superType: IrType, member: IrOverridableMember, clazz: IrClass): IrOverridableMember? {
-        buildFakeOverrideMember(superType, member, clazz, friendModules, unimplementedOverridesStrategy)
+        return if (isVisibleForOverrideInClass(member, clazz))
+            buildFakeOverrideMember(superType, member, clazz, unimplementedOverridesStrategy)
+        else
+            null
+    }
 
     /**
      * This function is a callback for fake override creation finish.
@@ -62,6 +65,41 @@ abstract class FakeOverrideBuilderStrategy(
         }
     }
 
+    private fun isInFriendModules(
+        fromModule: ModuleDescriptor,
+        toModule: ModuleDescriptor,
+    ): Boolean {
+        val fromModuleName = fromModule.name.asStringStripSpecialMarkers()
+        val toModuleName = toModule.name.asStringStripSpecialMarkers()
+
+        return fromModuleName == toModuleName || friendModules[fromModuleName]?.contains(toModuleName) == true
+    }
+
+    private fun isVisibleForOverrideInClass(original: IrOverridableMember, clazz: IrClass) : Boolean {
+        return when {
+            DescriptorVisibilities.isPrivate(original.visibility) -> false
+            original.visibility == DescriptorVisibilities.INVISIBLE_FAKE -> false
+            original.visibility == DescriptorVisibilities.INTERNAL -> {
+                val thisModule = clazz.getPackageFragment().moduleDescriptor
+                val memberModule = original.getPackageFragment().moduleDescriptor
+
+                when {
+                    thisModule == memberModule -> true
+                    isInFriendModules(thisModule, memberModule) -> true
+                    // TODO: this is very questionable - KT-63381
+                    original.hasAnnotation(StandardClassIds.Annotations.PublishedApi) -> true
+                    else -> false
+                }
+            }
+            else -> {
+                original.visibility.visibleFromPackage(
+                    clazz.getPackageFragment().packageFqName,
+                    original.getPackageFragment().packageFqName
+                )
+            }
+        }
+    }
+
     /**
      * Most implementations need [file] in which they are working now.
      *
@@ -93,33 +131,10 @@ abstract class FakeOverrideBuilderStrategy(
     protected abstract fun linkPropertyFakeOverride(property: IrPropertyWithLateBinding, manglerCompatibleMode: Boolean)
 }
 
-private fun IrOverridableMember.isPrivateToThisModule(
-    thisClass: IrClass, memberClass: IrClass, friendModules: Map<String, Collection<String>>,
-): Boolean {
-    if (visibility != DescriptorVisibilities.INTERNAL) return false
-
-    val thisModule = thisClass.getPackageFragment().moduleDescriptor
-    val memberModule = memberClass.getPackageFragment().moduleDescriptor
-
-    return thisModule != memberModule && !isInFriendModules(thisModule, memberModule, friendModules)
-}
-
-private fun isInFriendModules(
-    fromModule: ModuleDescriptor,
-    toModule: ModuleDescriptor,
-    friendModules: Map<String, Collection<String>>,
-): Boolean {
-    val fromModuleName = fromModule.name.asStringStripSpecialMarkers()
-    val toModuleName = toModule.name.asStringStripSpecialMarkers()
-
-    return fromModuleName == toModuleName || friendModules[fromModuleName]?.contains(toModuleName) == true
-}
-
 fun buildFakeOverrideMember(
     superType: IrType,
     member: IrOverridableMember,
     clazz: IrClass,
-    friendModules: Map<String, Collection<String>> = emptyMap(),
     unimplementedOverridesStrategy: IrUnimplementedOverridesStrategy = IrUnimplementedOverridesStrategy.ProcessAsFakeOverrides,
 ): IrOverridableMember {
     require(superType is IrSimpleType) { "superType is $superType, expected IrSimpleType" }
@@ -144,13 +159,7 @@ fun buildFakeOverrideMember(
 
     return CopyIrTreeWithSymbolsForFakeOverrides(member, substitutionMap, clazz, unimplementedOverridesStrategy)
         .copy()
-        .apply {
+        .apply { makeExternal(clazz.isExternal) }
-            makeExternal(clazz.isExternal)
-
-            val isInvisible = isPrivateToThisModule(clazz, classifier.owner, friendModules)
-            if (isInvisible && !member.annotations.hasAnnotation(StandardNames.FqNames.publishedApi))
-                visibility = DescriptorVisibilities.INVISIBLE_FAKE
-        }
 }
 
 // TODO: this is JS-specific functionality which should be moved out of the common ir.tree.

compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/overrides/IrFakeOverrideBuilder.kt

@@ -65,27 +65,24 @@ class IrFakeOverrideBuilder(
      */
     fun buildFakeOverridesForClass(clazz: IrClass, oldSignatures: Boolean) {
         strategy.inFile(clazz.fileOrNull) {
-            val superTypes = clazz.superTypes
+            val allFromSuper = clazz.superTypes.flatMap { superType ->
-
-            val fromCurrent = clazz.declarations.filterIsInstance<IrOverridableMember>()
-
-            val allFromSuper = superTypes.flatMap { superType ->
                 val superClass = superType.getClass() ?: error("Unexpected super type: $superType")
                 superClass.declarations
                     .filter { it.isOverridableMemberOrAccessor() }
-                    .map {
+                    .mapNotNull {
                         val overriddenMember = it as IrOverridableMember
-                        val fakeOverride = strategy.fakeOverrideMember(superType, overriddenMember, clazz)
+                        val fakeOverride = strategy.fakeOverrideMember(superType, overriddenMember, clazz) ?: return@mapNotNull null
                         FakeOverride(fakeOverride, overriddenMember)
                     }
             }
 
             val allFromSuperByName = allFromSuper.groupBy { it.override.name }
+            val allFromCurrentByName = clazz.declarations.filterIsInstanceAnd<IrOverridableMember> { !it.isStaticMember }.groupBy { it.name }
 
-            allFromSuperByName.forEach { group ->
+            allFromSuperByName.forEach { (name, superMembers) ->
                 generateOverridesInFunctionGroup(
-                    group.value,
+                    superMembers,
-                    fromCurrent.filter { it.name == group.key && !it.isStaticMember },
+                    allFromCurrentByName[name] ?: emptyList(),
                     clazz,
                     oldSignatures
                 )
@@ -113,10 +110,10 @@ class IrFakeOverrideBuilder(
             val superClass = superType.getClass() ?: error("Unexpected super type: $superType")
             superClass.declarations
                 .filterIsInstanceAnd<IrOverridableMember> {
-                    it !in overriddenMembers && it.symbol !in ignoredParentSymbols && !it.isStaticMember && !DescriptorVisibilities.isPrivate(it.visibility)
+                    it !in overriddenMembers && it.symbol !in ignoredParentSymbols && !it.isStaticMember
                 }
-                .map { overriddenMember ->
+                .mapNotNull { overriddenMember ->
-                    val fakeOverride = strategy.fakeOverrideMember(superType, overriddenMember, clazz)
+                    val fakeOverride = strategy.fakeOverrideMember(superType, overriddenMember, clazz) ?: return@mapNotNull null
                     FakeOverride(fakeOverride, overriddenMember)
                 }
         }
@@ -168,10 +165,7 @@ class IrFakeOverrideBuilder(
             // Note: We do allow overriding multiple FOs at once one of which is `isInline=true`.
             when (overrideChecker.isOverridableBy(fromSupertype.override, fromCurrent, checkIsInlineFlag = true).result) {
                 OverrideCompatibilityInfo.Result.OVERRIDABLE -> {
-                    val isVisibleFake = fromSupertype.override.visibility != DescriptorVisibilities.INVISIBLE_FAKE
+                    overridden += fromSupertype
-                    if (isVisibleFake && isVisibleForOverride(fromCurrent, fromSupertype.original)) {
-                        overridden += fromSupertype
-                    }
                     bound += fromSupertype
                 }
                 OverrideCompatibilityInfo.Result.CONFLICT -> {
@@ -181,7 +175,11 @@ class IrFakeOverrideBuilder(
             }
         }
 
-        fromCurrent.overriddenSymbols = overridden.memoryOptimizedMap { it.original.symbol }
+        // because of binary incompatible changes, it's possible to have private member colliding with fake override
+        // In that case we shouldn't generate fake override, but also shouldn't mark them as overridden
+        if (!DescriptorVisibilities.isPrivate(fromCurrent.visibility)) {
+            fromCurrent.overriddenSymbols = overridden.memoryOptimizedMap { it.original.symbol }
+        }
 
         return bound
     }
@@ -295,7 +293,7 @@ class IrFakeOverrideBuilder(
             "Unexpected fake override accessor kind: $this"
         }
         // For descriptors it gets INVISIBLE_FAKE.
-        if (this.visibility == DescriptorVisibilities.PRIVATE) return null
+        if (DescriptorVisibilities.isPrivate(this.visibility)) return null
 
         this.visibility = newVisibility
         this.modality = newModality
@@ -308,15 +306,9 @@ class IrFakeOverrideBuilder(
         addedFakeOverrides: MutableList<IrOverridableMember>,
         compatibilityMode: Boolean
     ) {
-        val effectiveOverridden = overridables.filter { it.override.isVisibleInClass(currentClass) }
+        val modality = determineModalityForFakeOverride(overridables, currentClass)
-
+        val visibility = findMemberWithMaxVisibility(overridables).override.visibility
-        // The descriptor based algorithm goes further building invisible fakes here,
+        val mostSpecific = selectMostSpecificMember(overridables)
-        // but we don't use invisible fakes in IR
-        if (effectiveOverridden.isEmpty()) return
-
-        val modality = determineModalityForFakeOverride(effectiveOverridden, currentClass)
-        val visibility = findMemberWithMaxVisibility(effectiveOverridden).override.visibility
-        val mostSpecific = selectMostSpecificMember(effectiveOverridden)
 
         val fakeOverride = mostSpecific.override.apply {
             when (this) {
@@ -334,7 +326,7 @@ class IrFakeOverrideBuilder(
             }
         }
 
-        fakeOverride.overriddenSymbols = effectiveOverridden.memoryOptimizedMap { it.original.symbol }
+        fakeOverride.overriddenSymbols = overridables.memoryOptimizedMap { it.original.symbol }
 
         require(
             fakeOverride.overriddenSymbols.isNotEmpty()
@@ -510,10 +502,10 @@ private val IrOverridableMember.returnType: IrType
     }
 
 fun IrSimpleFunction.isOverridableFunction(): Boolean =
-    visibility != DescriptorVisibilities.PRIVATE && hasDispatchReceiver
+    !DescriptorVisibilities.isPrivate(visibility) && hasDispatchReceiver
 
 fun IrProperty.isOverridableProperty(): Boolean =
-    visibility != DescriptorVisibilities.PRIVATE && (getter.hasDispatchReceiver || setter.hasDispatchReceiver)
+    !DescriptorVisibilities.isPrivate(visibility) && (getter.hasDispatchReceiver || setter.hasDispatchReceiver)
 
 fun IrDeclaration.isOverridableMemberOrAccessor(): Boolean = when (this) {
     is IrSimpleFunction -> isOverridableFunction()

compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/overrides/IrVisibilityUtil.kt

@@ -11,10 +11,6 @@ import org.jetbrains.kotlin.ir.declarations.*
 import org.jetbrains.kotlin.ir.types.getClass
 import org.jetbrains.kotlin.ir.util.*
 
-fun isVisibleForOverride(overriding: IrOverridableMember, fromSuper: IrOverridableMember): Boolean {
-    return fromSuper.isVisibleInClass(overriding.parentAsClass)
-}
-
 fun IrDeclarationWithVisibility.isEffectivelyPrivate(): Boolean {
     fun DescriptorVisibility.isNonPrivate(): Boolean =
         this == DescriptorVisibilities.PUBLIC
@@ -35,26 +31,4 @@ fun IrDeclarationWithVisibility.isEffectivelyPrivate(): Boolean {
 
         else -> true
     }
-}
+}
-
-internal fun IrOverridableMember.isVisibleInClass(klass: IrClass): Boolean {
-    if (DescriptorVisibilities.isPrivate(visibility) || visibility == DescriptorVisibilities.INVISIBLE_FAKE) return false
-
-    // OverridingUtil.isVisibleForOverride just calls DescriptorVisibilities.isVisible here. However, we can't use descriptors. Moreover,
-    // the current member's parent has already been reassigned to the current class (in OverridingUtil, it is still the original class where
-    // the member was declared at this point). So we load the original class from the dispatch receiver.
-    val dispatchReceiver = when (this) {
-        is IrSimpleFunction -> dispatchReceiverParameter
-        is IrProperty -> getter?.dispatchReceiverParameter
-        else -> error("Unsupported member: ${render()}")
-    } ?: error("Members without dispatch receiver are not possible here: ${render()} (klass=${klass.fqNameWhenAvailable}")
-
-    // Package-private Java members are only overridable within the same package.
-    val originalClass = dispatchReceiver.type.getClass()!!
-    if (!visibility.visibleFromPackage(klass.getPackageFragment().packageFqName, originalClass.getPackageFragment().packageFqName))
-        return false
-
-    // TODO (KT-61384): also check internal.
-
-    return true
-}

compiler/testData/codegen/box/bridges/internalMethodOverrideInFriendModule.kt

@@ -1,4 +1,3 @@
-// IGNORE_BACKEND: WASM
 // WITH_STDLIB
 
 // MODULE: lib1

compiler/testData/klib/evolution/widenSuperMemberVisibility.kt

@@ -3,43 +3,64 @@
 // VERSION: 1
 
 open class X {
-    private fun foo() = "private in super"
+    private fun foo1() = "private in super"
-    fun bar() = foo()
+    fun testX1() = foo1()
+    private fun foo2() = "private in super"
+    fun testX2() = foo2()
 
-    private val zeg = "private in super"
+    private val val1 = "private in super"
-    fun lim() = zeg
+    fun testX3() = val1
+    private val val2 = "private in super"
+    fun testX4() = val2
 }
 
 // FILE: B.kt
 // VERSION: 2
 
 open class X {
-    public fun foo() = "public in super"
+    public fun foo1() = "public in super"
-    fun bar() = foo()
+    fun testX1() = foo1()
+    open public fun foo2() = "public in super"
+    fun testX2() = foo2()
 
-    private val zeg = "public in super"
+    public val val1 = "public in super"
-    fun lim() = zeg
+    fun testX3() = val1
+    open public val val2 = "public in super"
+    fun testX4() = val2
 }
 
 // MODULE: mainLib(lib)
 // FILE: mainLib.kt
 
 class Y: X() {
-    private fun foo() = "private in derived"
+    private fun foo1() = "private in derived"
-    fun qux() = foo()
+    fun testY1() = foo1()
+    private fun foo2() = "private in derived"
+    fun testY2() = foo2()
+
+    private val val1 = "private in derived"
+    fun testY3() = val1
+    private val val2 = "private in derived"
+    fun testY4() = val2
 
-    private val zeg = "private in derived"
-    fun tes() = zeg
 }
 
 fun lib(): String = when {
-    X().bar() != "public in super" -> "fail 1"
+    X().testX1() != "public in super" -> "fail X().testX1()"
-    Y().bar() != "public in super" -> "fail 2"
+    Y().testX1() != "public in super" -> "fail Y().testX1()"
-    Y().qux() != "private in derived" -> "fail 3"
+    Y().testY1() != "private in derived" -> "fail Y().testY1()"
 
-    X().lim() != "public in super" -> "fail 4"
+    X().testX2() != "public in super" -> "fail X().testX2()"
-    Y().lim() != "public in super" -> "fail 5"
+    Y().testX2() != "public in super" -> "fail Y().testX2()"
-    Y().tes() != "private in derived" -> "fail 6"
+    Y().testY2() != "private in derived" -> "fail Y().testY2()"
+
+    X().testX3() != "public in super" -> "fail X().testX3()"
+    Y().testX3() != "public in super" -> "fail Y().testX3()"
+    Y().testY3() != "private in derived" -> "fail Y().testY3()"
+
+    X().testX4() != "public in super" -> "fail X().testX4()"
+    Y().testX4() != "public in super" -> "fail Y().testX4()"
+    Y().testY4() != "private in derived" -> "fail Y().testY4()"
 
     else -> "OK"
 }