From bcd7a531b9d0bcb2f49f8044166a5239a7138877 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Mon, 19 Sep 2022 17:53:29 +0200 Subject: [PATCH] K2: filter invisible overridden in status resolver & FIR2IR Related to KT-53197 --- .../kotlin/fir/backend/ConversionUtils.kt | 36 ++++++++----- .../generators/FakeOverrideGenerator.kt | 4 +- .../kotlin/fir/FirVisibilityChecker.kt | 13 +++-- .../resolve/transformers/FirStatusResolver.kt | 30 +++++++---- .../notOverridingPackagePrivate.fir.kt | 23 -------- .../visibility/notOverridingPackagePrivate.kt | 1 + .../internalPotentialOverride.fir.ir.txt | 54 ------------------- .../internalPotentialOverride.fir.kt.txt | 31 ----------- .../firProblems/internalPotentialOverride.kt | 1 + 9 files changed, 57 insertions(+), 136 deletions(-) delete mode 100644 compiler/testData/diagnostics/tests/visibility/notOverridingPackagePrivate.fir.kt delete mode 100644 compiler/testData/ir/irText/firProblems/internalPotentialOverride.fir.ir.txt delete mode 100644 compiler/testData/ir/irText/firProblems/internalPotentialOverride.fir.kt.txt diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/ConversionUtils.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/ConversionUtils.kt index a553a78ec7e..f6a2dbd6678 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/ConversionUtils.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/ConversionUtils.kt @@ -17,7 +17,6 @@ import org.jetbrains.kotlin.fir.declarations.builder.buildFile import org.jetbrains.kotlin.fir.declarations.synthetic.FirSyntheticProperty import org.jetbrains.kotlin.fir.declarations.utils.isInline import org.jetbrains.kotlin.fir.declarations.utils.isJava -import org.jetbrains.kotlin.fir.declarations.utils.visibility import org.jetbrains.kotlin.fir.expressions.* import org.jetbrains.kotlin.fir.expressions.impl.FirNoReceiverExpression import org.jetbrains.kotlin.fir.extensions.FirExtensionApiInternals @@ -370,11 +369,14 @@ internal fun FirSimpleFunction.processOverriddenFunctionSymbols( ) { val scope = containingClass.unsubstitutedScope(session, scopeSession, withForcedTypeCalculator = true) scope.processFunctionsByName(name) {} - scope.processOverriddenFunctionsFromSuperClasses(symbol, containingClass) { - if (it.fir.visibility == Visibilities.Private) { + scope.processOverriddenFunctionsFromSuperClasses(symbol, containingClass) { overriddenSymbol -> + if (!session.visibilityChecker.isVisibleForOverriding( + session, candidateInDerivedClass = symbol.fir, candidateInBaseClass = overriddenSymbol.fir + ) + ) { return@processOverriddenFunctionsFromSuperClasses ProcessorAction.NEXT } - processor(it) + processor(overriddenSymbol) ProcessorAction.NEXT } @@ -442,11 +444,14 @@ internal fun FirProperty.processOverriddenPropertySymbols( val scope = containingClass.unsubstitutedScope(session, scopeSession, withForcedTypeCalculator = true) scope.processPropertiesByName(name) {} val overriddenSet = mutableSetOf() - scope.processOverriddenPropertiesFromSuperClasses(symbol, containingClass) { - if (it.fir.visibility == Visibilities.Private) { + scope.processOverriddenPropertiesFromSuperClasses(symbol, containingClass) { overriddenSymbol -> + if (!session.visibilityChecker.isVisibleForOverriding( + session, candidateInDerivedClass = symbol.fir, candidateInBaseClass = overriddenSymbol.fir + ) + ) { return@processOverriddenPropertiesFromSuperClasses ProcessorAction.NEXT } - processor(it) + processor(overriddenSymbol) ProcessorAction.NEXT } @@ -475,15 +480,20 @@ internal fun FirProperty.generateOverriddenAccessorSymbols(containingClass: FirC val overriddenSet = mutableSetOf() val superClasses = containingClass.getSuperTypesAsIrClasses() ?: return emptyList() - scope.processOverriddenPropertiesFromSuperClasses(symbol, containingClass) { - if (it.fir.visibility == Visibilities.Private) { + scope.processOverriddenPropertiesFromSuperClasses(symbol, containingClass) { overriddenSymbol -> + if (!session.visibilityChecker.isVisibleForOverriding( + session, candidateInDerivedClass = symbol.fir, candidateInBaseClass = overriddenSymbol.fir + ) + ) { return@processOverriddenPropertiesFromSuperClasses ProcessorAction.NEXT } - for (overriddenProperty in fakeOverrideGenerator.getOverriddenSymbolsInSupertypes(it, superClasses)) { - val overriddenAccessor = if (isGetter) overriddenProperty.owner.getter?.symbol else overriddenProperty.owner.setter?.symbol - if (overriddenAccessor != null) { - overriddenSet += overriddenAccessor + for (overriddenIrPropertySymbol in fakeOverrideGenerator.getOverriddenSymbolsInSupertypes(overriddenSymbol, superClasses)) { + val overriddenIrAccessorSymbol = + if (isGetter) overriddenIrPropertySymbol.owner.getter?.symbol + else overriddenIrPropertySymbol.owner.setter?.symbol + if (overriddenIrAccessorSymbol != null) { + overriddenSet += overriddenIrAccessorSymbol } } ProcessorAction.NEXT diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/FakeOverrideGenerator.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/FakeOverrideGenerator.kt index f46af86ee2b..848eaded656 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/FakeOverrideGenerator.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/FakeOverrideGenerator.kt @@ -5,7 +5,6 @@ package org.jetbrains.kotlin.fir.backend.generators -import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.fir.* import org.jetbrains.kotlin.fir.backend.Fir2IrComponents import org.jetbrains.kotlin.fir.backend.Fir2IrConversionScope @@ -88,7 +87,6 @@ class FakeOverrideGenerator( return result } - @OptIn(ExperimentalStdlibApi::class) fun generateFakeOverridesForName( irClass: IrClass, name: Name, @@ -205,7 +203,7 @@ class FakeOverrideGenerator( // But they are treated differently in IR (real declarations have already been declared before) and such methods are present among realDeclarationSymbols if (originalSymbol in realDeclarationSymbols) return - if (originalDeclaration.visibility == Visibilities.Private) return + if (!session.visibilityChecker.isVisibleForOverriding(session, klass.moduleData, klass.symbol, originalDeclaration)) return val origin = IrDeclarationOrigin.FAKE_OVERRIDE val baseSymbol = originalSymbol.unwrapSubstitutionAndIntersectionOverrides() as S diff --git a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt index 9ff09d5382d..1641856dd1c 100644 --- a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt +++ b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt @@ -120,17 +120,24 @@ abstract class FirVisibilityChecker : FirSessionComponent { session: FirSession, candidateInDerivedClass: FirMemberDeclaration, candidateInBaseClass: FirMemberDeclaration + ): Boolean = isVisibleForOverriding(session, candidateInDerivedClass.moduleData, candidateInDerivedClass.symbol, candidateInBaseClass) + + fun isVisibleForOverriding( + session: FirSession, + derivedClassModuleData: FirModuleData, + symbolFromDerivedClass: FirBasedSymbol<*>, + candidateInBaseClass: FirMemberDeclaration, ): Boolean = when (candidateInBaseClass.visibility) { Visibilities.Internal -> { - candidateInBaseClass.moduleData == candidateInDerivedClass.moduleData || - (candidateInDerivedClass.moduleData == session.moduleData && + candidateInBaseClass.moduleData == derivedClassModuleData || + (derivedClassModuleData == session.moduleData && session.moduleVisibilityChecker?.isInFriendModule(candidateInBaseClass) == true) } Visibilities.Private, Visibilities.PrivateToThis -> false Visibilities.Protected -> true else -> platformOverrideVisibilityCheck( - session, candidateInDerivedClass.symbol, candidateInBaseClass.symbol, candidateInBaseClass.visibility + session, symbolFromDerivedClass, candidateInBaseClass.symbol, candidateInBaseClass.visibility ) } diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirStatusResolver.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirStatusResolver.kt index d08b0206ab5..833ec641fc8 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirStatusResolver.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirStatusResolver.kt @@ -93,8 +93,13 @@ class FirStatusResolver( return buildList { scope.processPropertiesByName(property.name) {} - scope.processDirectOverriddenPropertiesWithBaseScope(property.symbol) { symbol, _ -> - this += symbol.fir + scope.processDirectOverriddenPropertiesWithBaseScope(property.symbol) { overriddenSymbol, _ -> + if (session.visibilityChecker.isVisibleForOverriding( + session, candidateInDerivedClass = property, candidateInBaseClass = overriddenSymbol.fir + ) + ) { + this += overriddenSymbol.fir + } ProcessorAction.NEXT } } @@ -129,7 +134,12 @@ class FirStatusResolver( val symbol = function.symbol scope.processFunctionsByName(function.name) {} scope.processDirectOverriddenFunctionsWithBaseScope(symbol) { overriddenSymbol, _ -> - this += overriddenSymbol.fir + if (session.visibilityChecker.isVisibleForOverriding( + session, candidateInDerivedClass = function, candidateInBaseClass = overriddenSymbol.fir + ) + ) { + this += overriddenSymbol.fir + } ProcessorAction.NEXT } }.mapNotNull { @@ -215,15 +225,18 @@ class FirStatusResolver( isLocal -> Visibilities.Local else -> resolveVisibility(declaration, containingClass, containingProperty, overriddenStatuses) } + Visibilities.Private -> when { declaration is FirPropertyAccessor -> if (containingProperty?.visibility == Visibilities.PrivateToThis) { Visibilities.PrivateToThis } else { Visibilities.Private } + isPrivateToThis(declaration, containingClass) -> Visibilities.PrivateToThis else -> Visibilities.Private } + else -> status.visibility } @@ -375,14 +388,12 @@ class FirStatusResolver( containingClass == null -> Modality.FINAL containingClass.classKind == ClassKind.INTERFACE -> { when { - declaration.visibility == Visibilities.Private -> - Modality.FINAL - !declaration.hasOwnBodyOrAccessorBody() -> - Modality.ABSTRACT - else -> - Modality.OPEN + declaration.visibility == Visibilities.Private -> Modality.FINAL + !declaration.hasOwnBodyOrAccessorBody() -> Modality.ABSTRACT + else -> Modality.OPEN } } + else -> { if (declaration.isOverride && (containingClass.modality != Modality.FINAL || containingClass.classKind == ClassKind.ENUM_CLASS) @@ -394,6 +405,7 @@ class FirStatusResolver( } } } + else -> Modality.FINAL } diff --git a/compiler/testData/diagnostics/tests/visibility/notOverridingPackagePrivate.fir.kt b/compiler/testData/diagnostics/tests/visibility/notOverridingPackagePrivate.fir.kt deleted file mode 100644 index 3a38fb1c46f..00000000000 --- a/compiler/testData/diagnostics/tests/visibility/notOverridingPackagePrivate.fir.kt +++ /dev/null @@ -1,23 +0,0 @@ -// FILE: base/Base.java -package base; - -public abstract class Base { - public void foo() { - packagePrivateFoo(); - } - - /* package-private */ void packagePrivateFoo() {}; -} - -// FILE: Impl.kt -package impl -import base.* - -class Impl : Base() { - fun packagePrivateFoo() { /*not an override*/ } -} - -fun foo() { - Impl().foo() - Impl().packagePrivateFoo() -} diff --git a/compiler/testData/diagnostics/tests/visibility/notOverridingPackagePrivate.kt b/compiler/testData/diagnostics/tests/visibility/notOverridingPackagePrivate.kt index 94373ef1edd..8ebdad29422 100644 --- a/compiler/testData/diagnostics/tests/visibility/notOverridingPackagePrivate.kt +++ b/compiler/testData/diagnostics/tests/visibility/notOverridingPackagePrivate.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // FILE: base/Base.java package base; diff --git a/compiler/testData/ir/irText/firProblems/internalPotentialOverride.fir.ir.txt b/compiler/testData/ir/irText/firProblems/internalPotentialOverride.fir.ir.txt deleted file mode 100644 index 96c5e3985f8..00000000000 --- a/compiler/testData/ir/irText/firProblems/internalPotentialOverride.fir.ir.txt +++ /dev/null @@ -1,54 +0,0 @@ -Module: m1 -FILE fqName: fileName:/A.kt - CLASS CLASS name:A modality:OPEN visibility:public superTypes:[kotlin.Any] - $this: VALUE_PARAMETER INSTANCE_RECEIVER name: type:.A - CONSTRUCTOR visibility:public <> () returnType:.A [primary] - BLOCK_BODY - DELEGATING_CONSTRUCTOR_CALL 'public constructor () [primary] declared in kotlin.Any' - INSTANCE_INITIALIZER_CALL classDescriptor='CLASS CLASS name:A modality:OPEN visibility:public superTypes:[kotlin.Any]' - FUN name:foo visibility:internal modality:OPEN <> ($this:.A) returnType:kotlin.Int - $this: VALUE_PARAMETER name: type:.A - BLOCK_BODY - RETURN type=kotlin.Nothing from='internal open fun foo (): kotlin.Int declared in .A' - CONST Int type=kotlin.Int value=1 - FUN FAKE_OVERRIDE name:equals visibility:public modality:OPEN <> ($this:kotlin.Any, other:kotlin.Any?) returnType:kotlin.Boolean [fake_override,operator] - overridden: - public open fun equals (other: kotlin.Any?): kotlin.Boolean [operator] declared in kotlin.Any - $this: VALUE_PARAMETER name: type:kotlin.Any - VALUE_PARAMETER name:other index:0 type:kotlin.Any? - FUN FAKE_OVERRIDE name:hashCode visibility:public modality:OPEN <> ($this:kotlin.Any) returnType:kotlin.Int [fake_override] - overridden: - public open fun hashCode (): kotlin.Int declared in kotlin.Any - $this: VALUE_PARAMETER name: type:kotlin.Any - FUN FAKE_OVERRIDE name:toString visibility:public modality:OPEN <> ($this:kotlin.Any) returnType:kotlin.String [fake_override] - overridden: - public open fun toString (): kotlin.String declared in kotlin.Any - $this: VALUE_PARAMETER name: type:kotlin.Any -Module: m2 -FILE fqName: fileName:/B.kt - CLASS CLASS name:B modality:FINAL visibility:public superTypes:[.A] - $this: VALUE_PARAMETER INSTANCE_RECEIVER name: type:.B - CONSTRUCTOR visibility:public <> () returnType:.B [primary] - BLOCK_BODY - DELEGATING_CONSTRUCTOR_CALL 'public constructor () [primary] declared in .A' - INSTANCE_INITIALIZER_CALL classDescriptor='CLASS CLASS name:B modality:FINAL visibility:public superTypes:[.A]' - FUN name:foo visibility:internal modality:FINAL <> ($this:.B) returnType:kotlin.String - overridden: - internal open fun foo (): kotlin.Int declared in .A - $this: VALUE_PARAMETER name: type:.B - BLOCK_BODY - RETURN type=kotlin.Nothing from='internal final fun foo (): kotlin.String declared in .B' - CONST String type=kotlin.String value="OK" - FUN FAKE_OVERRIDE name:equals visibility:public modality:OPEN <> ($this:kotlin.Any, other:kotlin.Any?) returnType:kotlin.Boolean [fake_override,operator] - overridden: - public open fun equals (other: kotlin.Any?): kotlin.Boolean [fake_override,operator] declared in .A - $this: VALUE_PARAMETER name: type:kotlin.Any - VALUE_PARAMETER name:other index:0 type:kotlin.Any? - FUN FAKE_OVERRIDE name:hashCode visibility:public modality:OPEN <> ($this:kotlin.Any) returnType:kotlin.Int [fake_override] - overridden: - public open fun hashCode (): kotlin.Int [fake_override] declared in .A - $this: VALUE_PARAMETER name: type:kotlin.Any - FUN FAKE_OVERRIDE name:toString visibility:public modality:OPEN <> ($this:kotlin.Any) returnType:kotlin.String [fake_override] - overridden: - public open fun toString (): kotlin.String [fake_override] declared in .A - $this: VALUE_PARAMETER name: type:kotlin.Any diff --git a/compiler/testData/ir/irText/firProblems/internalPotentialOverride.fir.kt.txt b/compiler/testData/ir/irText/firProblems/internalPotentialOverride.fir.kt.txt deleted file mode 100644 index 36be6f8e194..00000000000 --- a/compiler/testData/ir/irText/firProblems/internalPotentialOverride.fir.kt.txt +++ /dev/null @@ -1,31 +0,0 @@ -// MODULE: m1 -// FILE: A.kt - -open class A { - constructor() /* primary */ { - super/*Any*/() - /* () */ - - } - - internal open fun foo(): Int { - return 1 - } - -} - -// MODULE: m2 -// FILE: B.kt - -class B : A { - constructor() /* primary */ { - super/*A*/() - /* () */ - - } - - internal override fun foo(): String { - return "OK" - } - -} diff --git a/compiler/testData/ir/irText/firProblems/internalPotentialOverride.kt b/compiler/testData/ir/irText/firProblems/internalPotentialOverride.kt index fb045ece013..b14b06830fb 100644 --- a/compiler/testData/ir/irText/firProblems/internalPotentialOverride.kt +++ b/compiler/testData/ir/irText/firProblems/internalPotentialOverride.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // TARGET_BACKEND: JVM_IR // MODULE: m1 // FILE: A.kt