From 6900e20096bc14117fd19a01b462e1d62a93fd71 Mon Sep 17 00:00:00 2001 From: Sergej Jaskiewicz Date: Thu, 18 Jan 2024 15:58:50 +0100 Subject: [PATCH] [klib] Fix exception for clashing signatures from different modules If we encounter a declaration in the current module whose signature is the same as that of a declaration in another module which we happen to also reference from the current module, don't report any errors, just like we don't do it in Kotlin/JVM. This leaves the user in the KLIB hell situation, but this is intentional, because otherwise a legitimate change like moving a declaration to another module and marking the original one as `@Deprecated("", level = DeprecationLevel.HIDDEN)` would lead to a error, and we don't want that. Also, don't try to show the diagnostics on a declaration that doesn't have an IrFile. ^KT-65063 Fixed --- .../ir/linkage/SignatureClashDetector.kt | 32 +++++++++++++++---- .../diagnostics/IdSignatureClashDetector.kt | 2 +- .../common/serialization/DeclarationTable.kt | 16 ++++++++-- .../signatureClash.diag.txt | 6 ++-- .../klibSerializationTests/signatureClash.kt | 10 ++++++ .../SignatureClashDiagnostics/lib.kt | 3 ++ .../SignatureClashDiagnostics/main.kt | 4 +++ .../konan/test/blackbox/CompilerOutputTest.kt | 3 +- 8 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 native/native.tests/testData/compilerOutput/SignatureClashDiagnostics/lib.kt diff --git a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/linkage/SignatureClashDetector.kt b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/linkage/SignatureClashDetector.kt index 2944026e25f..d7523683ab0 100644 --- a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/linkage/SignatureClashDetector.kt +++ b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/linkage/SignatureClashDetector.kt @@ -9,7 +9,8 @@ import org.jetbrains.kotlin.diagnostics.KtDiagnosticFactory1 import org.jetbrains.kotlin.ir.IrDiagnosticReporter import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.declarations.IrDeclaration -import org.jetbrains.kotlin.ir.util.file +import org.jetbrains.kotlin.ir.util.fileOrNull +import org.jetbrains.kotlin.ir.util.render import org.jetbrains.kotlin.ir.util.sourceElement import org.jetbrains.kotlin.utils.SmartSet @@ -53,17 +54,34 @@ abstract class SignatureClashDetector reportSignatureClashTo( + protected fun reportSignatureClashTo( diagnosticReporter: IrDiagnosticReporter, diagnosticFactory: KtDiagnosticFactory1, declarations: Collection, data: Data, - reportOnIfSynthetic: (ConflictingDeclaration) -> IrElement, + reportOnIfSynthetic: (ConflictingDeclaration) -> IrElement?, ) { - declarations.mapTo(LinkedHashSet()) { declaration -> - val reportOn = declaration.takeUnless { it.startOffset < 0 } ?: reportOnIfSynthetic(declaration) - diagnosticReporter.at(reportOn.sourceElement(), reportOn, declaration.file) - }.forEach { + val diagnostics = declarations.mapNotNullTo(LinkedHashSet()) { declaration -> + // Declarations that come from other modules may not have a file, so we don't show diagnostics on them. + val containingFile = declaration.fileOrNull ?: return@mapNotNullTo null + val reportOn = declaration.takeUnless { it.startOffset < 0 } ?: reportOnIfSynthetic(declaration) ?: return@mapNotNullTo null + diagnosticReporter.at(reportOn.sourceElement(), reportOn, containingFile) + } + + assert(diagnostics.isEmpty() == declarations.isEmpty()) { + buildString { + append("Different declarations with the same signatures were detected, but no diagnostics will be reported ") + append("because none of those declarations have any source location info associated with them. ") + append("This could happen because the declarations came from an external module, or were generated by a compiler plugin. ") + appendLine("The following declarations have conflicting signatures:") + for (declaration in declarations) { + append(" - ") + appendLine(declaration.render()) + } + } + } + + diagnostics.forEach { it.report(diagnosticFactory, data) } } diff --git a/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/diagnostics/IdSignatureClashDetector.kt b/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/diagnostics/IdSignatureClashDetector.kt index e940dcbb1af..0a8991cf9f4 100644 --- a/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/diagnostics/IdSignatureClashDetector.kt +++ b/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/diagnostics/IdSignatureClashDetector.kt @@ -23,7 +23,7 @@ class IdSignatureClashDetector : SignatureClashDetector clashDetector.trackDeclaration(it, id) } + table[it] = symbol.signature!!.also { id -> clashDetector.trackDeclarationIfInCurrentModule(it, id) } } } open fun computeSignatureByDeclaration(declaration: IrDeclaration, compatibleMode: Boolean): IdSignature { return table.getOrPut(declaration) { publicIdSignatureComputer.composePublicIdSignature(declaration, compatibleMode).also { - clashDetector.trackDeclaration(declaration, it) + clashDetector.trackDeclarationIfInCurrentModule(declaration, it) } } } @@ -40,6 +41,17 @@ 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 diff --git a/compiler/testData/diagnostics/klibSerializationTests/signatureClash.diag.txt b/compiler/testData/diagnostics/klibSerializationTests/signatureClash.diag.txt index 95fa04108f6..3cd11200b93 100644 --- a/compiler/testData/diagnostics/klibSerializationTests/signatureClash.diag.txt +++ b/compiler/testData/diagnostics/klibSerializationTests/signatureClash.diag.txt @@ -1,14 +1,14 @@ -/foo.kt:7:1: error: Platform declaration clash: The following declarations have the same KLIB signature (com.example.klib.serialization.diagnostics/foo|foo(){}[0]): +/foo.kt:13:1: error: Platform declaration clash: The following declarations have the same KLIB signature (com.example.klib.serialization.diagnostics/foo|foo(){}[0]): 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:13:1: error: Platform declaration clash: The following declarations have the same KLIB signature (com.example.klib.serialization.diagnostics/foo|foo(){}[0]): +/main.kt:19:1: error: Platform declaration clash: The following declarations have the same KLIB signature (com.example.klib.serialization.diagnostics/foo|foo(){}[0]): 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:16:1: error: Platform declaration clash: The following declarations have the same KLIB signature (com.example.klib.serialization.diagnostics/foo|foo(){}[0]): +/main.kt:22:1: error: Platform declaration clash: The following declarations have the same KLIB signature (com.example.klib.serialization.diagnostics/foo|foo(){}[0]): 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 diff --git a/compiler/testData/diagnostics/klibSerializationTests/signatureClash.kt b/compiler/testData/diagnostics/klibSerializationTests/signatureClash.kt index ee535c623f7..5eaf4a816d2 100644 --- a/compiler/testData/diagnostics/klibSerializationTests/signatureClash.kt +++ b/compiler/testData/diagnostics/klibSerializationTests/signatureClash.kt @@ -1,6 +1,12 @@ // FIR_IDENTICAL // RENDER_ALL_DIAGNOSTICS_FULL_TEXT +// MODULE: lib +package com.example.klib.serialization.diagnostics + +fun movedToLib() {} + +// MODULE: main(lib) // FILE: foo.kt package com.example.klib.serialization.diagnostics @@ -15,6 +21,10 @@ fun foo(): String = "" fun foo(): Int = 0 +@Deprecated("This function moved to the 'lib' module", level = DeprecationLevel.HIDDEN) +fun movedToLib() {} + fun main() { foo() + movedToLib() } diff --git a/native/native.tests/testData/compilerOutput/SignatureClashDiagnostics/lib.kt b/native/native.tests/testData/compilerOutput/SignatureClashDiagnostics/lib.kt new file mode 100644 index 00000000000..988868572bb --- /dev/null +++ b/native/native.tests/testData/compilerOutput/SignatureClashDiagnostics/lib.kt @@ -0,0 +1,3 @@ +package com.example.klib.serialization.diagnostics + +fun movedToLib() {} diff --git a/native/native.tests/testData/compilerOutput/SignatureClashDiagnostics/main.kt b/native/native.tests/testData/compilerOutput/SignatureClashDiagnostics/main.kt index b985ee8ea4c..66535ff82be 100644 --- a/native/native.tests/testData/compilerOutput/SignatureClashDiagnostics/main.kt +++ b/native/native.tests/testData/compilerOutput/SignatureClashDiagnostics/main.kt @@ -7,6 +7,10 @@ class A { fun foo(): Int = 0 } +@Deprecated("This function moved to the 'lib' module", level = DeprecationLevel.HIDDEN) +fun movedToLib() {} + fun main() { println(A().foo()) + movedToLib() } diff --git a/native/native.tests/tests/org/jetbrains/kotlin/konan/test/blackbox/CompilerOutputTest.kt b/native/native.tests/tests/org/jetbrains/kotlin/konan/test/blackbox/CompilerOutputTest.kt index 2e2645c70d8..b3e6d4e9ee0 100644 --- a/native/native.tests/tests/org/jetbrains/kotlin/konan/test/blackbox/CompilerOutputTest.kt +++ b/native/native.tests/tests/org/jetbrains/kotlin/konan/test/blackbox/CompilerOutputTest.kt @@ -163,7 +163,8 @@ class FirCompilerOutputTest : CompilerOutputTestBase() { // TODO: use the Compiler Core test infrastructure for testing these diagnostics (KT-64393) val rootDir = File("native/native.tests/testData/compilerOutput/SignatureClashDiagnostics") val settings = testRunSettings - val compilationResult = compileLibrary(settings, rootDir.resolve("main.kt")) + val lib = compileLibrary(settings, rootDir.resolve("lib.kt")).assertSuccess().resultingArtifact + val compilationResult = compileLibrary(settings, rootDir.resolve("main.kt"), dependencies = listOf(lib)) val goldenData = rootDir.resolve("output.txt") KotlinTestUtils.assertEqualsToFile(goldenData, compilationResult.toOutput())