From 06684f207a4c2c25f89792910410efda93d452f3 Mon Sep 17 00:00:00 2001 From: Sergej Jaskiewicz Date: Tue, 30 Jan 2024 15:50:12 +0100 Subject: [PATCH] [klib] Don't rely on Lazy IR for signature clash checking We only want to report signature clashes for declarations that come from the module currently being serialized. In GlobalDeclarationTable, declarations from other modules could also be stored. Checking whether a declaration is a Lazy IR declaration to determine if it comes from an external module works okay, but it is a hack which relies on an implementation detail of IR, which may or may not work in the future. Use a more robust logic here, since IrFileSerializer is always aware which declarations are declared in the current module and which are just referenced from it. --- .../common/serialization/DeclarationTable.kt | 42 ++++++++----------- .../common/serialization/IrFileSerializer.kt | 12 +++--- .../signature/IdSignatureFactory.kt | 2 +- .../signatureClash.diag.txt | 8 ++++ .../klibSerializationTests/signatureClash.kt | 8 ++++ 5 files changed, 40 insertions(+), 32 deletions(-) diff --git a/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/serialization/DeclarationTable.kt b/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/serialization/DeclarationTable.kt index d0360af1bfc..e591faa5737 100644 --- a/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/serialization/DeclarationTable.kt +++ b/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/serialization/DeclarationTable.kt @@ -12,7 +12,6 @@ import org.jetbrains.kotlin.ir.IrBuiltIns import org.jetbrains.kotlin.ir.declarations.IrDeclaration import org.jetbrains.kotlin.ir.declarations.IrFile import org.jetbrains.kotlin.ir.declarations.IrSymbolOwner -import org.jetbrains.kotlin.ir.declarations.lazy.IrLazyDeclarationBase import org.jetbrains.kotlin.ir.util.IdSignature import org.jetbrains.kotlin.ir.util.KotlinMangler import org.jetbrains.kotlin.ir.util.render @@ -26,14 +25,20 @@ abstract class GlobalDeclarationTable(private val mangler: KotlinMangler.IrMangl protected fun loadKnownBuiltins(builtIns: IrBuiltIns) { builtIns.knownBuiltins.forEach { val symbol = (it as IrSymbolOwner).symbol - table[it] = symbol.signature!!.also { id -> clashDetector.trackDeclarationIfInCurrentModule(it, id) } + table[it] = symbol.signature!!.also { id -> clashDetector.trackDeclaration(it, id) } } } - open fun computeSignatureByDeclaration(declaration: IrDeclaration, compatibleMode: Boolean): IdSignature { + fun computeSignatureByDeclaration( + declaration: IrDeclaration, + compatibleMode: Boolean, + recordInSignatureClashDetector: Boolean, + ): IdSignature { return table.getOrPut(declaration) { - publicIdSignatureComputer.composePublicIdSignature(declaration, compatibleMode).also { - clashDetector.trackDeclarationIfInCurrentModule(declaration, it) + publicIdSignatureComputer.composePublicIdSignature(declaration, compatibleMode) + }.also { + if (recordInSignatureClashDetector) { + clashDetector.trackDeclaration(declaration, it) } } } @@ -41,17 +46,6 @@ abstract class GlobalDeclarationTable(private val mangler: KotlinMangler.IrMangl fun isExportedDeclaration(declaration: IrDeclaration, compatibleMode: Boolean): Boolean = with(mangler) { declaration.isExported(compatibleMode) } } -private fun IdSignatureClashDetector.trackDeclarationIfInCurrentModule(declaration: IrDeclaration, signature: IdSignature) { - // Only count signature clashes on the declarations declared in the module being serialized. - // If there is a declaration in an external module with the same signature as some declaration in the current module, - // so be it (for now). - // Note that when we are serializing a module into a KLIB, the declarations from other modules always come in the form of Lazy IR, - // so this check is enough to ensure that this declaration is declared in the module currently being serialized. - if (declaration !is IrLazyDeclarationBase) { - trackDeclaration(declaration, signature) - } -} - open class DeclarationTable(globalTable: GlobalDeclarationTable) { protected val table = hashMapOf() protected open val globalDeclarationTable: GlobalDeclarationTable = globalTable @@ -74,20 +68,18 @@ open class DeclarationTable(globalTable: GlobalDeclarationTable) { return table.getOrPut(declaration) { signaturer.composeFileLocalIdSignature(declaration, compatibleMode) } } - private fun computeSignatureByDeclaration(declaration: IrDeclaration, compatibleMode: Boolean): IdSignature { - tryComputeBackendSpecificSignature(declaration)?.let { return it } - return if (declaration.isLocalDeclaration(compatibleMode)) { - allocateIndexedSignature(declaration, compatibleMode) - } else globalDeclarationTable.computeSignatureByDeclaration(declaration, compatibleMode) - } - fun privateDeclarationSignature(declaration: IrDeclaration, compatibleMode: Boolean, builder: () -> IdSignature): IdSignature { assert(declaration.isLocalDeclaration(compatibleMode)) return table.getOrPut(declaration) { builder() } } - open fun signatureByDeclaration(declaration: IrDeclaration, compatibleMode: Boolean): IdSignature { - return computeSignatureByDeclaration(declaration, compatibleMode) + fun signatureByDeclaration(declaration: IrDeclaration, compatibleMode: Boolean, recordInSignatureClashDetector: Boolean): IdSignature { + tryComputeBackendSpecificSignature(declaration)?.let { return it } + return if (declaration.isLocalDeclaration(compatibleMode)) { + allocateIndexedSignature(declaration, compatibleMode) + } else { + globalDeclarationTable.computeSignatureByDeclaration(declaration, compatibleMode, recordInSignatureClashDetector) + } } fun assumeDeclarationSignature(declaration: IrDeclaration, signature: IdSignature) { diff --git a/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/serialization/IrFileSerializer.kt b/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/serialization/IrFileSerializer.kt index 49f93aec432..36a1ca221fc 100644 --- a/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/serialization/IrFileSerializer.kt +++ b/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/serialization/IrFileSerializer.kt @@ -206,8 +206,8 @@ open class IrFileSerializer( /* ------- IdSignature ------------------------------------------------------ */ - private fun protoIdSignature(declaration: IrDeclaration): Int { - val idSig = declarationTable.signatureByDeclaration(declaration, compatibilityMode.oldSignatures) + private fun protoIdSignature(declaration: IrDeclaration, recordInSignatureClashDetector: Boolean): Int { + val idSig = declarationTable.signatureByDeclaration(declaration, compatibilityMode.oldSignatures, recordInSignatureClashDetector) return idSignatureSerializer.protoIdSignature(idSig) } @@ -256,14 +256,14 @@ open class IrFileSerializer( } } - private fun serializeIrSymbol(symbol: IrSymbol): Long { + private fun serializeIrSymbol(symbol: IrSymbol, isDeclared: Boolean = false): Long { val symbolKind = protoSymbolKind(symbol) val signatureId = when { symbol is IrFileSymbol -> idSignatureSerializer.protoIdSignature(IdSignature.FileSignature(symbol)) // TODO: special signature for files? else -> { val declaration = symbol.owner as? IrDeclaration ?: error("Expected IrDeclaration: ${symbol.owner.render()}") - protoIdSignature(declaration) + protoIdSignature(declaration, recordInSignatureClashDetector = isDeclared) } } @@ -1003,7 +1003,7 @@ open class IrFileSerializer( private fun serializeIrDeclarationBase(declaration: IrDeclaration, flags: Long?): ProtoDeclarationBase { return with(ProtoDeclarationBase.newBuilder()) { - symbol = serializeIrSymbol((declaration as IrSymbolOwner).symbol) + symbol = serializeIrSymbol((declaration as IrSymbolOwner).symbol, isDeclared = true) coordinates = serializeCoordinates(declaration.startOffset, declaration.endOffset) addAllAnnotation(serializeAnnotations(declaration.annotations)) flags?.let { setFlags(it) } @@ -1345,7 +1345,7 @@ open class IrFileSerializer( } val byteArray = serializeDeclaration(it).toByteArray() - val idSig = declarationTable.signatureByDeclaration(it, compatibilityMode.oldSignatures) + val idSig = declarationTable.signatureByDeclaration(it, compatibilityMode.oldSignatures, recordInSignatureClashDetector = false) require(idSig == idSig.topLevelSignature()) { "IdSig: $idSig\ntopLevel: ${idSig.topLevelSignature()}" } require(!idSig.isPackageSignature()) { "IsSig: $idSig\nDeclaration: ${it.render()}" } diff --git a/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/serialization/signature/IdSignatureFactory.kt b/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/serialization/signature/IdSignatureFactory.kt index e7a28371257..a5b2997ab77 100644 --- a/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/serialization/signature/IdSignatureFactory.kt +++ b/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/serialization/signature/IdSignatureFactory.kt @@ -224,7 +224,7 @@ class IdSignatureFactory( mask = 0, description = null, ) - is IrDeclaration -> table.signatureByDeclaration(container, compatibleMode) + is IrDeclaration -> table.signatureByDeclaration(container, compatibleMode, recordInSignatureClashDetector = false) else -> error("Unexpected container ${container.render()}") } diff --git a/compiler/testData/diagnostics/klibSerializationTests/signatureClash.diag.txt b/compiler/testData/diagnostics/klibSerializationTests/signatureClash.diag.txt index fc99ca58a4f..e99ba770625 100644 --- a/compiler/testData/diagnostics/klibSerializationTests/signatureClash.diag.txt +++ b/compiler/testData/diagnostics/klibSerializationTests/signatureClash.diag.txt @@ -12,3 +12,11 @@ fun foo(): kotlin.Long defined in com.example.klib.serialization.diagnostics fun foo(): kotlin.String defined in com.example.klib.serialization.diagnostics fun foo(): kotlin.Int defined in com.example.klib.serialization.diagnostics + +/main.kt:35:1: error: Platform declaration clash: The following declarations have the same IR signature (com.example.klib.serialization.diagnostics/bar|bar(){}[0]): + fun bar(): kotlin.Long defined in com.example.klib.serialization.diagnostics + fun bar(): kotlin.Long defined in com.example.klib.serialization.diagnostics + +/main.kt:38:1: error: Platform declaration clash: The following declarations have the same IR signature (com.example.klib.serialization.diagnostics/bar|bar(){}[0]): + fun bar(): kotlin.Long defined in com.example.klib.serialization.diagnostics + fun bar(): kotlin.Long defined in com.example.klib.serialization.diagnostics diff --git a/compiler/testData/diagnostics/klibSerializationTests/signatureClash.kt b/compiler/testData/diagnostics/klibSerializationTests/signatureClash.kt index 5eaf4a816d2..88d9e0041a2 100644 --- a/compiler/testData/diagnostics/klibSerializationTests/signatureClash.kt +++ b/compiler/testData/diagnostics/klibSerializationTests/signatureClash.kt @@ -27,4 +27,12 @@ fun movedToLib() {} fun main() { foo() movedToLib() + + // Test that the diagnostic is reported for declarations that are referenced before they are declared + bar() } + +@Deprecated("", level = DeprecationLevel.HIDDEN) +fun bar(): Long = 0L + +fun bar(): Long = 1L