From 4f53204a31f5c71be823712e1a9ce6fa06ee1ae2 Mon Sep 17 00:00:00 2001 From: Sergej Jaskiewicz Date: Wed, 20 Dec 2023 13:07:04 +0100 Subject: [PATCH] [K/N] Set `linkViaSignatures` to `false` This is already the case for Kotlin/JS. This is required for fixing KT-63670: `linkViaSignatures` implies that `SymbolTable` is used in FIR2IR, and in case if two declarations have the same `IdSignature`, we have a crash in FIR2IR: exception: java.lang.IllegalStateException: IR declaration with signature "/foo|foo(){}[0]" found in SymbolTable and not found in declaration storage If we're not using SymbolTable, FIR2IR finishes successfully, and we can safely proceed to the KLIB serialization stage, at which point we can show the user a diagnostic about clashing signatures. Since we've disabled SymbolTable, we also needed to fix the logic that relied on its presence. Fortunately, such logic only includes filtering out unused libraries. Instead of iterating over symbols in SymbolTable, we iterate over cached symbols in Fir2IrDeclarationStorage and Fir2IrClassifierStorage, which seems to do the trick just as well. --- .../backend/DelicateDeclarationStorageApi.kt | 13 ++++++++++ .../fir/backend/Fir2IrClassifierStorage.kt | 16 +++++++++++++ .../fir/backend/Fir2IrDeclarationStorage.kt | 18 ++++++++++++++ .../jetbrains/kotlin/backend/konan/Fir2Ir.kt | 24 +++++++++---------- 4 files changed, 59 insertions(+), 12 deletions(-) create mode 100644 compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/DelicateDeclarationStorageApi.kt diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/DelicateDeclarationStorageApi.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/DelicateDeclarationStorageApi.kt new file mode 100644 index 00000000000..e1fdc6d52da --- /dev/null +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/DelicateDeclarationStorageApi.kt @@ -0,0 +1,13 @@ +/* + * Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.fir.backend + +/** + * This annotation marks that the annotated method of [Fir2IrDeclarationStorage] or [Fir2IrClassifierStorage] is not very safe and should + * be used only if you really know what you are doing. + */ +@RequiresOptIn +annotation class DelicateDeclarationStorageApi diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrClassifierStorage.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrClassifierStorage.kt index fa4f13715eb..b5e3a468fac 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrClassifierStorage.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrClassifierStorage.kt @@ -53,6 +53,22 @@ class Fir2IrClassifierStorage( private val localClassesCreatedOnTheFly: MutableMap = mutableMapOf() + /** + * This function is quite messy and doesn't have a good contract of what exactly is traversed. + * The basic idea is to traverse the symbols which can be reasonably referenced from other modules. + * + * Be careful when using it, and avoid it, except really needed. + */ + @DelicateDeclarationStorageApi + fun forEachCachedDeclarationSymbol(block: (IrSymbol) -> Unit) { + classCache.values.forEach { block(it.symbol) } + typeAliasCache.values.forEach { block(it.symbol) } + enumEntryCache.values.forEach { block(it.symbol) } + fieldsForContextReceivers.values.forEach { fields -> + fields.forEach { block(it.symbol) } + } + } + private var processMembersOfClassesOnTheFlyImmediately = false private fun FirTypeRef.toIrType(typeOrigin: ConversionTypeOrigin = ConversionTypeOrigin.DEFAULT): IrType = 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 b2bbdaaaa95..40044c63b97 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 @@ -93,6 +93,24 @@ class Fir2IrDeclarationStorage( private val delegateVariableForPropertyCache: ConcurrentHashMap = commonMemberStorage.delegateVariableForPropertyCache + /** + * This function is quite messy and doesn't have a good contract of what exactly is traversed. + * The basic idea is to traverse the symbols which can be reasonably referenced from other modules. + * + * Be careful when using it, and avoid it, except really needed. + */ + @DelicateDeclarationStorageApi + fun forEachCachedDeclarationSymbol(block: (IrSymbol) -> Unit) { + functionCache.values.forEach(block) + constructorCache.values.forEach(block) + propertyCache.values.forEach(block) + getterForPropertyCache.values.forEach(block) + setterForPropertyCache.values.forEach(block) + backingFieldForPropertyCache.values.forEach(block) + propertyForBackingFieldCache.values.forEach(block) + delegateVariableForPropertyCache.values.forEach(block) + } + // interface A { /* $1 */ fun foo() } // interface B : A { // /* $2 */ fake_override fun foo() diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/Fir2Ir.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/Fir2Ir.kt index ecea02a4ff1..ee40a46c498 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/Fir2Ir.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/Fir2Ir.kt @@ -1,7 +1,6 @@ package org.jetbrains.kotlin.backend.konan import org.jetbrains.kotlin.backend.common.extensions.IrGenerationExtension -import org.jetbrains.kotlin.backend.common.serialization.mangle.ManglerChecker import org.jetbrains.kotlin.backend.common.serialization.metadata.DynamicTypeDeserializer import org.jetbrains.kotlin.backend.konan.driver.PhaseContext import org.jetbrains.kotlin.backend.konan.driver.phases.Fir2IrOutput @@ -31,19 +30,16 @@ import org.jetbrains.kotlin.fir.declarations.utils.isLateInit import org.jetbrains.kotlin.fir.descriptors.FirModuleDescriptor import org.jetbrains.kotlin.fir.pipeline.convertToIrAndActualize import org.jetbrains.kotlin.fir.references.FirReference -import org.jetbrains.kotlin.fir.signaturer.Ir2FirManglerAdapter import org.jetbrains.kotlin.incremental.components.LookupTracker -import org.jetbrains.kotlin.ir.ObsoleteDescriptorBasedAPI import org.jetbrains.kotlin.ir.declarations.IrClass import org.jetbrains.kotlin.ir.declarations.IrDeclaration import org.jetbrains.kotlin.ir.declarations.IrExternalPackageFragment import org.jetbrains.kotlin.ir.declarations.IrMemberWithContainerSource import org.jetbrains.kotlin.ir.objcinterop.IrObjCOverridabilityCondition +import org.jetbrains.kotlin.ir.symbols.IrSymbol import org.jetbrains.kotlin.ir.types.IrTypeSystemContextImpl -import org.jetbrains.kotlin.ir.util.DelicateSymbolTableApi import org.jetbrains.kotlin.ir.util.SymbolTable import org.jetbrains.kotlin.ir.util.getPackageFragment -import org.jetbrains.kotlin.ir.visitors.acceptVoid import org.jetbrains.kotlin.library.isNativeStdlib import org.jetbrains.kotlin.library.metadata.KlibMetadataFactories import org.jetbrains.kotlin.name.NativeForwardDeclarationKind @@ -100,7 +96,7 @@ internal fun PhaseContext.fir2Ir( val fir2IrConfiguration = Fir2IrConfiguration( languageVersionSettings = configuration.languageVersionSettings, diagnosticReporter = diagnosticsReporter, - linkViaSignatures = true, + linkViaSignatures = false, evaluatedConstTracker = configuration .putIfAbsent(CommonConfigurationKeys.EVALUATED_CONST_TRACKER, EvaluatedConstTracker.create()), inlineConstTracker = null, @@ -125,16 +121,20 @@ internal fun PhaseContext.fir2Ir( "`${irModuleFragment.name}` must be Name.special, since it's required by KlibMetadataModuleDescriptorFactoryImpl.createDescriptorOptionalBuiltIns()" } - @OptIn(ObsoleteDescriptorBasedAPI::class, DelicateSymbolTableApi::class) + @OptIn(DelicateDeclarationStorageApi::class) val usedPackages = buildSet { - components.symbolTable.descriptorExtension.forEachDeclarationSymbol { - val p = it.owner as? IrDeclaration ?: return@forEachDeclarationSymbol - val fragment = (p.getPackageFragment() as? IrExternalPackageFragment) ?: return@forEachDeclarationSymbol + fun addExternalPackage(it: IrSymbol) { + // FIXME(KT-64742): Fir2IrDeclarationStorage caches may contain unbound IR symbols, so we filter them out. + val p = it.takeIf { it.isBound }?.owner as? IrDeclaration ?: return + val fragment = (p.getPackageFragment() as? IrExternalPackageFragment) ?: return add(fragment.packageFqName) } - // This packages exists in all platform libraries, but can contain only synthetic declarations. + components.declarationStorage.forEachCachedDeclarationSymbol(::addExternalPackage) + components.classifierStorage.forEachCachedDeclarationSymbol(::addExternalPackage) + + // These packages exist in all platform libraries, but can contain only synthetic declarations. // These declarations are not really located in klib, so we don't need to depend on klib to use them. - removeAll(NativeForwardDeclarationKind.entries.map { it.packageFqName }) + removeAll(NativeForwardDeclarationKind.entries.map { it.packageFqName }.toSet()) }.toList()