diff --git a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/overrides/FakeOverrideBuilderStrategy.kt b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/overrides/FakeOverrideBuilderStrategy.kt index 02271aa1ee5..0efd8bac12f 100644 --- a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/overrides/FakeOverrideBuilderStrategy.kt +++ b/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.IrType -import org.jetbrains.kotlin.ir.types.IrTypeProjection -import org.jetbrains.kotlin.ir.types.extractTypeParameters +import org.jetbrains.kotlin.ir.types.* 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 = - buildFakeOverrideMember(superType, member, clazz, friendModules, unimplementedOverridesStrategy) + fun fakeOverrideMember(superType: IrType, member: IrOverridableMember, clazz: IrClass): IrOverridableMember? { + 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>, -): 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>, -): 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> = 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 { - makeExternal(clazz.isExternal) - - val isInvisible = isPrivateToThisModule(clazz, classifier.owner, friendModules) - if (isInvisible && !member.annotations.hasAnnotation(StandardNames.FqNames.publishedApi)) - visibility = DescriptorVisibilities.INVISIBLE_FAKE - } + .apply { makeExternal(clazz.isExternal) } } // TODO: this is JS-specific functionality which should be moved out of the common ir.tree. diff --git a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/overrides/IrFakeOverrideBuilder.kt b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/overrides/IrFakeOverrideBuilder.kt index e6fac575419..6e255d0aac0 100644 --- a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/overrides/IrFakeOverrideBuilder.kt +++ b/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 fromCurrent = clazz.declarations.filterIsInstance() - - val allFromSuper = superTypes.flatMap { superType -> + val allFromSuper = clazz.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 { !it.isStaticMember }.groupBy { it.name } - allFromSuperByName.forEach { group -> + allFromSuperByName.forEach { (name, superMembers) -> generateOverridesInFunctionGroup( - group.value, - fromCurrent.filter { it.name == group.key && !it.isStaticMember }, + superMembers, + allFromCurrentByName[name] ?: emptyList(), clazz, oldSignatures ) @@ -113,10 +110,10 @@ class IrFakeOverrideBuilder( val superClass = superType.getClass() ?: error("Unexpected super type: $superType") superClass.declarations .filterIsInstanceAnd { - it !in overriddenMembers && it.symbol !in ignoredParentSymbols && !it.isStaticMember && !DescriptorVisibilities.isPrivate(it.visibility) + it !in overriddenMembers && it.symbol !in ignoredParentSymbols && !it.isStaticMember } - .map { overriddenMember -> - val fakeOverride = strategy.fakeOverrideMember(superType, overriddenMember, clazz) + .mapNotNull { overriddenMember -> + 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 - if (isVisibleFake && isVisibleForOverride(fromCurrent, fromSupertype.original)) { - overridden += fromSupertype - } + 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, compatibilityMode: Boolean ) { - val effectiveOverridden = overridables.filter { it.override.isVisibleInClass(currentClass) } - - // The descriptor based algorithm goes further building invisible fakes here, - // 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 modality = determineModalityForFakeOverride(overridables, currentClass) + val visibility = findMemberWithMaxVisibility(overridables).override.visibility + val mostSpecific = selectMostSpecificMember(overridables) 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() diff --git a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/overrides/IrVisibilityUtil.kt b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/overrides/IrVisibilityUtil.kt index 8875a395a2b..2eda43aee03 100644 --- a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/overrides/IrVisibilityUtil.kt +++ b/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 -} +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/bridges/internalMethodOverrideInFriendModule.kt b/compiler/testData/codegen/box/bridges/internalMethodOverrideInFriendModule.kt index c3c3bb18280..04d4c751318 100644 --- a/compiler/testData/codegen/box/bridges/internalMethodOverrideInFriendModule.kt +++ b/compiler/testData/codegen/box/bridges/internalMethodOverrideInFriendModule.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: WASM // WITH_STDLIB // MODULE: lib1 diff --git a/compiler/testData/klib/evolution/widenSuperMemberVisibility.kt b/compiler/testData/klib/evolution/widenSuperMemberVisibility.kt index 547a2488deb..f724a5c25aa 100644 --- a/compiler/testData/klib/evolution/widenSuperMemberVisibility.kt +++ b/compiler/testData/klib/evolution/widenSuperMemberVisibility.kt @@ -3,43 +3,64 @@ // VERSION: 1 open class X { - private fun foo() = "private in super" - fun bar() = foo() + private fun foo1() = "private in super" + fun testX1() = foo1() + private fun foo2() = "private in super" + fun testX2() = foo2() - private val zeg = "private in super" - fun lim() = zeg + private val val1 = "private in super" + 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" - fun bar() = foo() + public fun foo1() = "public in super" + fun testX1() = foo1() + open public fun foo2() = "public in super" + fun testX2() = foo2() - private val zeg = "public in super" - fun lim() = zeg + public val val1 = "public in super" + 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" - fun qux() = foo() + private fun foo1() = "private in derived" + 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" - Y().bar() != "public in super" -> "fail 2" - Y().qux() != "private in derived" -> "fail 3" + X().testX1() != "public in super" -> "fail X().testX1()" + Y().testX1() != "public in super" -> "fail Y().testX1()" + Y().testY1() != "private in derived" -> "fail Y().testY1()" - X().lim() != "public in super" -> "fail 4" - Y().lim() != "public in super" -> "fail 5" - Y().tes() != "private in derived" -> "fail 6" + X().testX2() != "public in super" -> "fail X().testX2()" + Y().testX2() != "public in super" -> "fail Y().testX2()" + 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" }