From c25cde88712965e2835953f708e8a970a020a0cb Mon Sep 17 00:00:00 2001 From: Dmitriy Novozhilov Date: Wed, 11 Oct 2023 12:52:04 +0300 Subject: [PATCH] [FIR2IR] Provide correct module data to FirModuleDescriptor Properly implement friend modules visibility check ^KT-61384 Fixed ^KT-62475 Fixed --- .../kotlin/fir/backend/Fir2IrConverter.kt | 2 +- .../fir/backend/Fir2IrDeclarationStorage.kt | 39 +++++++++++------- .../kotlin/fir/backend/FirIrProvider.kt | 28 ++++++------- .../kotlin/fir/backend/IrBuiltInsOverFir.kt | 2 +- .../generators/Fir2IrClassifiersGenerator.kt | 5 ++- .../fir/descriptors/FirModuleDescriptor.kt | 40 ++++++++++++++----- .../internalMethodOverrideInFriendModule.kt | 1 - ...ternalMethodOverrideMultipleInheritance.kt | 1 - .../compileKotlinAgainstKotlin/jvmField.kt | 1 - .../jvmFieldInConstructor.kt | 1 - .../fakeOverride/internalFromFriendModule.kt | 1 - 11 files changed, 73 insertions(+), 48 deletions(-) diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrConverter.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrConverter.kt index 4d6bad134cd..1c8c605aa6d 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrConverter.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrConverter.kt @@ -554,7 +554,7 @@ class Fir2IrConverter( typeContextProvider: (IrBuiltIns) -> IrTypeSystemContext ): Fir2IrResult { session.lazyDeclarationResolver.disableLazyResolveContractChecks() - val moduleDescriptor = FirModuleDescriptor(session, kotlinBuiltIns) + val moduleDescriptor = FirModuleDescriptor.createSourceModuleDescriptor(session, kotlinBuiltIns) val components = Fir2IrComponentsStorage( session, scopeSession, irFactory, fir2IrExtensions, fir2IrConfiguration, visibilityConverter, { irBuiltins -> diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrDeclarationStorage.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrDeclarationStorage.kt index 3451c5e0c07..d83abb85ad6 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrDeclarationStorage.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrDeclarationStorage.kt @@ -38,7 +38,6 @@ import org.jetbrains.kotlin.ir.declarations.lazy.IrLazyClass import org.jetbrains.kotlin.ir.expressions.IrSyntheticBodyKind import org.jetbrains.kotlin.ir.symbols.* import org.jetbrains.kotlin.ir.symbols.impl.IrClassSymbolImpl -import org.jetbrains.kotlin.ir.types.IrSimpleType import org.jetbrains.kotlin.ir.util.* import org.jetbrains.kotlin.load.kotlin.FacadeClassSource import org.jetbrains.kotlin.name.* @@ -48,19 +47,18 @@ import org.jetbrains.kotlin.serialization.deserialization.descriptors.Deserializ import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedContainerSource import org.jetbrains.kotlin.utils.threadLocal import java.util.concurrent.ConcurrentHashMap -import org.jetbrains.kotlin.fir.java.hasJvmFieldAnnotation @OptIn(LeakedDeclarationCaches::class) class Fir2IrDeclarationStorage( private val components: Fir2IrComponents, - private val moduleDescriptor: FirModuleDescriptor, + private val sourceModuleDescriptor: FirModuleDescriptor, commonMemberStorage: Fir2IrCommonMemberStorage ) : Fir2IrComponents by components { private val fragmentCache: ConcurrentHashMap = ConcurrentHashMap() private class ExternalPackageFragments( - val fragmentForDependencies: IrExternalPackageFragment, + val fragmentsForDependencies: ConcurrentHashMap, val fragmentForPrecompiledBinaries: IrExternalPackageFragment ) @@ -156,32 +154,43 @@ class Fir2IrDeclarationStorage( fun getIrExternalPackageFragment( fqName: FqName, + moduleData: FirModuleData, firOrigin: FirDeclarationOrigin = FirDeclarationOrigin.Library ): IrExternalPackageFragment { val fragments = fragmentCache.getOrPut(fqName) { - val fragmentForDependencies = callablesGenerator.createExternalPackageFragment( - fqName, FirModuleDescriptor(session, moduleDescriptor.builtIns) - ) - val fragmentForPrecompiledBinaries = callablesGenerator.createExternalPackageFragment(fqName, moduleDescriptor) - ExternalPackageFragments(fragmentForDependencies, fragmentForPrecompiledBinaries) + val fragmentForPrecompiledBinaries = callablesGenerator.createExternalPackageFragment(fqName, sourceModuleDescriptor) + ExternalPackageFragments(ConcurrentHashMap(), fragmentForPrecompiledBinaries) } // Make sure that external package fragments have a different module descriptor. The module descriptors are compared // to determine if objects need regeneration because they are from different modules. // But keep original module descriptor for the fragments coming from parts compiled on the previous incremental step return when (firOrigin) { FirDeclarationOrigin.Precompiled -> fragments.fragmentForPrecompiledBinaries - else -> fragments.fragmentForDependencies + else -> fragments.fragmentsForDependencies.getOrPut(moduleData) { + val moduleDescriptor = FirModuleDescriptor.createDependencyModuleDescriptor( + moduleData, + sourceModuleDescriptor.builtIns + ) + callablesGenerator.createExternalPackageFragment(fqName, moduleDescriptor) + } } } - private fun getIrExternalOrBuiltInsPackageFragment(fqName: FqName, firOrigin: FirDeclarationOrigin): IrExternalPackageFragment { + private fun getIrExternalOrBuiltInsPackageFragment( + fqName: FqName, + moduleData: FirModuleData, + firOrigin: FirDeclarationOrigin, + ): IrExternalPackageFragment { val isBuiltIn = fqName in BUILT_INS_PACKAGE_FQ_NAMES - return if (isBuiltIn) getIrBuiltInsPackageFragment(fqName) else getIrExternalPackageFragment(fqName, firOrigin) + return when(isBuiltIn) { + true -> getIrBuiltInsPackageFragment(fqName) + false -> getIrExternalPackageFragment(fqName, moduleData, firOrigin) + } } private fun getIrBuiltInsPackageFragment(fqName: FqName): IrExternalPackageFragment { return builtInsFragmentCache.getOrPut(fqName) { - callablesGenerator.createExternalPackageFragment(FirBuiltInsPackageFragment(fqName, moduleDescriptor)) + callablesGenerator.createExternalPackageFragment(FirBuiltInsPackageFragment(fqName, sourceModuleDescriptor)) } } @@ -1164,12 +1173,12 @@ class Fir2IrDeclarationStorage( val parentPackage = when (firBasedSymbol) { is FirCallableSymbol<*> -> { - getIrExternalPackageFragment(packageFqName, firOrigin) + getIrExternalPackageFragment(packageFqName, firBasedSymbol.moduleData, firOrigin) } else -> { // TODO: All classes from BUILT_INS_PACKAGE_FQ_NAMES are considered built-ins now, // which is not exact and can lead to some problems - getIrExternalOrBuiltInsPackageFragment(packageFqName, firOrigin) + getIrExternalOrBuiltInsPackageFragment(packageFqName, firBasedSymbol.moduleData, firOrigin) } } diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/FirIrProvider.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/FirIrProvider.kt index db69208e790..98fc1fbfa15 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/FirIrProvider.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/FirIrProvider.kt @@ -66,13 +66,11 @@ class FirIrProvider(val components: Fir2IrComponents) : IrProvider { val nameSegments = signature.nameSegments val topName = Name.identifier(nameSegments[0]) - val packageFragment = declarationStorage.getIrExternalPackageFragment(packageFqName) - val firCandidates: List - val parent: IrDeclarationParent + val parentClass: IrClass? if (nameSegments.size == 1 && kind != SymbolKind.CLASS_SYMBOL) { firCandidates = symbolProvider.getTopLevelCallableSymbols(packageFqName, topName).map { it.fir } - parent = packageFragment // TODO: need to insert file facade class on JVM + parentClass = null // TODO: need to insert file facade class on JVM } else { val topLevelClass = symbolProvider.getClassLikeSymbolByClassId(ClassId(packageFqName, topName))?.fir as? FirRegularClass ?: return null @@ -95,28 +93,28 @@ class FirIrProvider(val components: Fir2IrComponents) : IrProvider { when (kind) { SymbolKind.CLASS_SYMBOL -> { firCandidates = listOf(firClass) - parent = firParentClass?.let { findIrClass(it) } ?: packageFragment + parentClass = firParentClass?.let { findIrClass(it) } } SymbolKind.ENUM_ENTRY_SYMBOL -> { val lastName = Name.guessByFirstCharacter(nameSegments.last()) val firCandidate = firClass.declarations.singleOrNull { (it as? FirEnumEntry)?.name == lastName } firCandidates = firCandidate?.let { listOf(it) } ?: return null - parent = findIrClass(firClass) + parentClass = findIrClass(firClass) } SymbolKind.CONSTRUCTOR_SYMBOL -> { val constructors = mutableListOf() scope.processDeclaredConstructors { constructors.add(it.fir) } firCandidates = constructors - parent = findIrClass(firClass) + parentClass = findIrClass(firClass) } SymbolKind.FUNCTION_SYMBOL -> { - parent = findIrClass(firClass) + parentClass = findIrClass(firClass) val lastName = Name.guessByFirstCharacter(nameSegments.last()) val functions = mutableListOf() scope.processFunctionsByName(lastName) { functionSymbol -> val dispatchReceiverClassId = (functionSymbol.fir.dispatchReceiverType as? ConeClassLikeType)?.lookupTag?.classId val function = if (dispatchReceiverClassId != null && dispatchReceiverClassId != classId) { - fakeOverrideGenerator.createFirFunctionFakeOverride(firClass, parent, functionSymbol, scope)!!.first + fakeOverrideGenerator.createFirFunctionFakeOverride(firClass, parentClass, functionSymbol, scope)!!.first } else functionSymbol.fir functions.add(function) } @@ -126,21 +124,21 @@ class FirIrProvider(val components: Fir2IrComponents) : IrProvider { firCandidates = functions } SymbolKind.PROPERTY_SYMBOL -> { - parent = findIrClass(firClass) + parentClass = findIrClass(firClass) val lastName = Name.guessByFirstCharacter(nameSegments.last()) val properties = mutableListOf() scope.processPropertiesByName(lastName) { propertySymbol -> propertySymbol as FirPropertySymbol val dispatchReceiverClassId = (propertySymbol.fir.dispatchReceiverType as? ConeClassLikeType)?.lookupTag?.classId val property = if (dispatchReceiverClassId != null && dispatchReceiverClassId != classId) { - fakeOverrideGenerator.createFirPropertyFakeOverride(firClass, parent, propertySymbol, scope)!!.first + fakeOverrideGenerator.createFirPropertyFakeOverride(firClass, parentClass, propertySymbol, scope)!!.first } else propertySymbol.fir properties.add(property) } firCandidates = properties } SymbolKind.FIELD_SYMBOL -> { - parent = findIrClass(firClass) + parentClass = findIrClass(firClass) val lastName = Name.guessByFirstCharacter(nameSegments.last()) val fields = mutableListOf() scope.processPropertiesByName(lastName) { propertySymbol -> @@ -154,13 +152,13 @@ class FirIrProvider(val components: Fir2IrComponents) : IrProvider { else -> { val lastName = nameSegments.last() firCandidates = firClass.declarations.filter { it is FirCallableDeclaration && it.symbol.name.asString() == lastName } - parent = findIrClass(firClass) + parentClass = findIrClass(firClass) } } } - val firDeclaration = findDeclarationByHash(firCandidates, signature.id) - ?: return null + val firDeclaration = findDeclarationByHash(firCandidates, signature.id) ?: return null + val parent = parentClass ?: declarationStorage.getIrExternalPackageFragment(packageFqName, firDeclaration.moduleData) return when (kind) { SymbolKind.CLASS_SYMBOL -> { diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/IrBuiltInsOverFir.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/IrBuiltInsOverFir.kt index 37363dca416..dcf0d173586 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/IrBuiltInsOverFir.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/IrBuiltInsOverFir.kt @@ -699,7 +699,7 @@ class IrBuiltInsOverFir( private fun findIrParent(firSymbol: FirCallableSymbol<*>): IrDeclarationParent? { return when (val containingClassLookupTag = firSymbol.containingClassLookupTag()) { - null -> components.declarationStorage.getIrExternalPackageFragment(firSymbol.callableId.packageName) + null -> components.declarationStorage.getIrExternalPackageFragment(firSymbol.callableId.packageName, firSymbol.moduleData) else -> components.classifierStorage.findIrClass(containingClassLookupTag) } } diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/Fir2IrClassifiersGenerator.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/Fir2IrClassifiersGenerator.kt index 111e54dd9b7..e0df0ba6c84 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/Fir2IrClassifiersGenerator.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/Fir2IrClassifiersGenerator.kt @@ -12,6 +12,7 @@ import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.utils.* import org.jetbrains.kotlin.fir.expressions.FirAnonymousObjectExpression import org.jetbrains.kotlin.fir.lazy.Fir2IrLazyClass +import org.jetbrains.kotlin.fir.moduleData import org.jetbrains.kotlin.fir.resolve.getSymbolByLookupTag import org.jetbrains.kotlin.fir.resolve.providers.firProvider import org.jetbrains.kotlin.fir.symbols.ConeClassLikeLookupTag @@ -468,7 +469,9 @@ class Fir2IrClassifiersGenerator(val components: Fir2IrComponents) : Fir2IrCompo val parentId = classId.outerClassId val parentClass = parentId?.let { createIrClassForNotFoundClass(it.toLookupTag()) } - val irParent = parentClass ?: declarationStorage.getIrExternalPackageFragment(classId.packageFqName) + val irParent = parentClass ?: declarationStorage.getIrExternalPackageFragment( + classId.packageFqName, session.moduleData.dependencies.first() + ) return symbolTable.declareClassIfNotExists(signature, { IrClassPublicSymbolImpl(signature) }) { irFactory.createClass( diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/descriptors/FirModuleDescriptor.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/descriptors/FirModuleDescriptor.kt index 02d4737c827..0d5cbd4b9b5 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/descriptors/FirModuleDescriptor.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/descriptors/FirModuleDescriptor.kt @@ -5,46 +5,66 @@ package org.jetbrains.kotlin.fir.descriptors -import org.jetbrains.kotlin.builtins.DefaultBuiltIns import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.descriptors.annotations.Annotations +import org.jetbrains.kotlin.fir.FirModuleData import org.jetbrains.kotlin.fir.FirSession import org.jetbrains.kotlin.fir.moduleData import org.jetbrains.kotlin.fir.resolve.providers.symbolProvider import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.platform.TargetPlatform +import org.jetbrains.kotlin.utils.addToStdlib.shouldNotBeCalled -class FirModuleDescriptor( +class FirModuleDescriptor private constructor( val session: FirSession, + val moduleData: FirModuleData, override val builtIns: KotlinBuiltIns ) : ModuleDescriptor { + companion object { + fun createSourceModuleDescriptor(session: FirSession, builtIns: KotlinBuiltIns): FirModuleDescriptor { + require(session.kind == FirSession.Kind.Source) + return FirModuleDescriptor(session, session.moduleData, builtIns) + } + + fun createDependencyModuleDescriptor(moduleData: FirModuleData, builtIns: KotlinBuiltIns): FirModuleDescriptor { + // We may create dependency module descriptor for java sources, which have source session and source module data + return FirModuleDescriptor(moduleData.session, moduleData, builtIns) + } + } + override fun shouldSeeInternalsOf(targetModule: ModuleDescriptor): Boolean { - return false + if (targetModule !is FirModuleDescriptor) return false + return when (targetModule.moduleData) { + this.moduleData, + in moduleData.friendDependencies, + in moduleData.dependsOnDependencies -> true + else -> false + } } override val platform: TargetPlatform - get() = session.moduleData.platform + get() = moduleData.platform override fun getPackage(fqName: FqName): PackageViewDescriptor { val symbolProvider = session.symbolProvider if (symbolProvider.getPackage(fqName) != null) { return FirPackageViewDescriptor(fqName, this) } - TODO("Missing package reporting") + error("Module $moduleData doesn't contain package $fqName") } override fun getSubPackagesOf(fqName: FqName, nameFilter: (Name) -> Boolean): Collection { - TODO("not implemented") + shouldNotBeCalled() } override var allDependencyModules: List = emptyList() override val expectedByModules: List - get() = TODO("not implemented") + get() = shouldNotBeCalled() override val allExpectedByModules: Set - get() = TODO("not implemented") + get() = shouldNotBeCalled() override fun getCapability(capability: ModuleCapability): T? { return null @@ -62,14 +82,14 @@ class FirModuleDescriptor( } override fun getName(): Name { - return session.moduleData.name + return moduleData.name } override val stableName: Name get() = name override fun acceptVoid(visitor: DeclarationDescriptorVisitor?) { - TODO("not implemented") + shouldNotBeCalled() } override val annotations: Annotations diff --git a/compiler/testData/codegen/box/bridges/internalMethodOverrideInFriendModule.kt b/compiler/testData/codegen/box/bridges/internalMethodOverrideInFriendModule.kt index 78ec6d95f84..c3c3bb18280 100644 --- a/compiler/testData/codegen/box/bridges/internalMethodOverrideInFriendModule.kt +++ b/compiler/testData/codegen/box/bridges/internalMethodOverrideInFriendModule.kt @@ -1,5 +1,4 @@ // IGNORE_BACKEND: WASM -// IGNORE_CODEGEN_WITH_IR_FAKE_OVERRIDE_GENERATION: KT-61384 // WITH_STDLIB // MODULE: lib1 diff --git a/compiler/testData/codegen/box/bridges/internalMethodOverrideMultipleInheritance.kt b/compiler/testData/codegen/box/bridges/internalMethodOverrideMultipleInheritance.kt index 600f1290ac5..d80e9805f55 100644 --- a/compiler/testData/codegen/box/bridges/internalMethodOverrideMultipleInheritance.kt +++ b/compiler/testData/codegen/box/bridges/internalMethodOverrideMultipleInheritance.kt @@ -1,6 +1,5 @@ // IGNORE_BACKEND: WASM // WITH_STDLIB -// IGNORE_CODEGEN_WITH_IR_FAKE_OVERRIDE_GENERATION: KT-61384 // MODULE: lib1 diff --git a/compiler/testData/codegen/box/compileKotlinAgainstKotlin/jvmField.kt b/compiler/testData/codegen/box/compileKotlinAgainstKotlin/jvmField.kt index b984f96961e..e18363e9b37 100644 --- a/compiler/testData/codegen/box/compileKotlinAgainstKotlin/jvmField.kt +++ b/compiler/testData/codegen/box/compileKotlinAgainstKotlin/jvmField.kt @@ -1,5 +1,4 @@ // TARGET_BACKEND: JVM -// IGNORE_CODEGEN_WITH_IR_FAKE_OVERRIDE_GENERATION: KT-61384 // WITH_STDLIB // MODULE: lib diff --git a/compiler/testData/codegen/box/compileKotlinAgainstKotlin/jvmFieldInConstructor.kt b/compiler/testData/codegen/box/compileKotlinAgainstKotlin/jvmFieldInConstructor.kt index fcc5d70d810..cde88e56413 100644 --- a/compiler/testData/codegen/box/compileKotlinAgainstKotlin/jvmFieldInConstructor.kt +++ b/compiler/testData/codegen/box/compileKotlinAgainstKotlin/jvmFieldInConstructor.kt @@ -1,5 +1,4 @@ // TARGET_BACKEND: JVM -// IGNORE_CODEGEN_WITH_IR_FAKE_OVERRIDE_GENERATION: KT-61384 // WITH_STDLIB // MODULE: lib diff --git a/compiler/testData/codegen/box/fakeOverride/internalFromFriendModule.kt b/compiler/testData/codegen/box/fakeOverride/internalFromFriendModule.kt index d1a3680f109..373f2a5ab5a 100644 --- a/compiler/testData/codegen/box/fakeOverride/internalFromFriendModule.kt +++ b/compiler/testData/codegen/box/fakeOverride/internalFromFriendModule.kt @@ -1,5 +1,4 @@ // IGNORE_BACKEND: WASM -// IGNORE_CODEGEN_WITH_IR_FAKE_OVERRIDE_GENERATION: KT-61384 // MODULE: lib // FILE: l.kt