From e0cb145c6b3b08595fccc4a251d3f160920c7d88 Mon Sep 17 00:00:00 2001 From: Sergej Jaskiewicz Date: Tue, 9 Jan 2024 19:42:45 +0100 Subject: [PATCH] [JVM] Split `JvmSignatureClashDetector` into two classes One for methods, one for fields. Later we are going to use the new `SignatureClashDetector` class for detecting clashes of KLIB signatures. --- .../backend/jvm/codegen/ClassCodegen.kt | 8 +- .../codegen/JvmFieldSignatureClashDetector.kt | 40 +++++ .../JvmMethodSignatureClashDetector.kt | 141 ++++++++---------- .../ir/linkage/SignatureClashDetector.kt | 70 +++++++++ 4 files changed, 179 insertions(+), 80 deletions(-) create mode 100644 compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmFieldSignatureClashDetector.kt create mode 100644 compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/linkage/SignatureClashDetector.kt diff --git a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt index c66311becf2..6b796bfd762 100644 --- a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt +++ b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt @@ -112,6 +112,7 @@ class ClassCodegen private constructor( val reifiedTypeParametersUsages = ReifiedTypeParametersUsages() private val jvmMethodSignatureClashDetector = JvmMethodSignatureClashDetector(this) + private val jvmFieldSignatureClashDetector = JvmFieldSignatureClashDetector(this) private val visitor = state.factory.newVisitor(irClass.descriptorOrigin, type, irClass.fileParent.loadSourceFilesInfo()).apply { val signature = typeMapper.mapClassSignature(irClass, type, state.classBuilderMode.generateBodies) @@ -217,7 +218,8 @@ class ClassCodegen private constructor( generateInnerAndOuterClasses() visitor.done(config.generateSmapCopyToAnnotation) - jvmMethodSignatureClashDetector.reportErrors() + jvmMethodSignatureClashDetector.reportErrorsTo(context.ktDiagnosticReporter) + jvmFieldSignatureClashDetector.reportErrorsTo(context.ktDiagnosticReporter) } private fun shouldSkipCodeGenerationAccordingToGenerationFilter(): Boolean { @@ -354,7 +356,7 @@ class ClassCodegen private constructor( fieldSignature, (field.initializer?.expression as? IrConst<*>)?.value ) - jvmMethodSignatureClashDetector.trackField(field, RawSignature(fieldName, fieldType.descriptor, MemberKind.FIELD)) + jvmFieldSignatureClashDetector.trackDeclaration(field, RawSignature(fieldName, fieldType.descriptor, MemberKind.FIELD)) if (field.origin != JvmLoweredDeclarationOrigin.CONTINUATION_CLASS_RESULT_FIELD) { val skipNullabilityAnnotations = @@ -442,7 +444,7 @@ class ClassCodegen private constructor( } else { node.accept(smapCopyingVisitor) } - jvmMethodSignatureClashDetector.trackMethod(method, RawSignature(node.name, node.desc, MemberKind.METHOD)) + jvmMethodSignatureClashDetector.trackDeclaration(method, RawSignature(node.name, node.desc, MemberKind.METHOD)) when (val metadata = method.metadata) { is MetadataSource.Property -> metadataSerializer.bindPropertyMetadata(metadata, Method(node.name, node.desc), method.origin) diff --git a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmFieldSignatureClashDetector.kt b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmFieldSignatureClashDetector.kt new file mode 100644 index 00000000000..766b3197bad --- /dev/null +++ b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmFieldSignatureClashDetector.kt @@ -0,0 +1,40 @@ +/* + * Copyright 2010-2023 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.backend.jvm.codegen + +import org.jetbrains.kotlin.ir.IrDiagnosticReporter +import org.jetbrains.kotlin.ir.declarations.IrDeclaration +import org.jetbrains.kotlin.ir.declarations.IrField +import org.jetbrains.kotlin.ir.descriptors.toIrBasedDescriptor +import org.jetbrains.kotlin.ir.linkage.SignatureClashDetector +import org.jetbrains.kotlin.resolve.jvm.diagnostics.ConflictingJvmDeclarationsData +import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmBackendErrors +import org.jetbrains.kotlin.resolve.jvm.diagnostics.RawSignature + +internal class JvmFieldSignatureClashDetector( + private val classCodegen: ClassCodegen, +) : SignatureClashDetector() { + + override fun reportSignatureConflict( + signature: RawSignature, + declarations: Collection, + diagnosticReporter: IrDiagnosticReporter + ) { + reportSignatureClashTo( + diagnosticReporter, + JvmBackendErrors.CONFLICTING_JVM_DECLARATIONS, + declarations, + ConflictingJvmDeclarationsData( + classInternalName = classCodegen.type.internalName, + classOrigin = null, + signature = signature, + signatureOrigins = null, + signatureDescriptors = declarations.map(IrDeclaration::toIrBasedDescriptor), + ), + reportOnIfSynthetic = { classCodegen.irClass }, + ) + } +} \ No newline at end of file diff --git a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmMethodSignatureClashDetector.kt b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmMethodSignatureClashDetector.kt index 00af44ea6b1..3ebca3a657b 100644 --- a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmMethodSignatureClashDetector.kt +++ b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmMethodSignatureClashDetector.kt @@ -9,38 +9,30 @@ import org.jetbrains.kotlin.backend.common.lower.ANNOTATION_IMPLEMENTATION import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin import org.jetbrains.kotlin.descriptors.DescriptorVisibilities import org.jetbrains.kotlin.diagnostics.KtDiagnosticFactory1 -import org.jetbrains.kotlin.ir.declarations.* +import org.jetbrains.kotlin.ir.IrDiagnosticReporter +import org.jetbrains.kotlin.ir.declarations.IrDeclaration +import org.jetbrains.kotlin.ir.declarations.IrDeclarationOrigin +import org.jetbrains.kotlin.ir.declarations.IrFunction +import org.jetbrains.kotlin.ir.declarations.IrSimpleFunction import org.jetbrains.kotlin.ir.descriptors.toIrBasedDescriptor -import org.jetbrains.kotlin.ir.util.file +import org.jetbrains.kotlin.ir.linkage.SignatureClashDetector import org.jetbrains.kotlin.ir.util.isFakeOverride -import org.jetbrains.kotlin.ir.util.sourceElement import org.jetbrains.kotlin.resolve.jvm.diagnostics.ConflictingJvmDeclarationsData import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmBackendErrors import org.jetbrains.kotlin.resolve.jvm.diagnostics.MemberKind import org.jetbrains.kotlin.resolve.jvm.diagnostics.RawSignature -import org.jetbrains.kotlin.utils.SmartSet class JvmMethodSignatureClashDetector( private val classCodegen: ClassCodegen -) { - private val methodsBySignature = LinkedHashMap>() - private val fieldsBySignature = LinkedHashMap>() - - fun trackField(irField: IrField, rawSignature: RawSignature) { - fieldsBySignature.getOrPut(rawSignature) { SmartSet.create() }.add(irField) - } - - fun trackMethod(irFunction: IrFunction, rawSignature: RawSignature) { - methodsBySignature.getOrPut(rawSignature) { SmartSet.create() }.add(irFunction) - } +) : SignatureClashDetector() { fun trackFakeOverrideMethod(irFunction: IrFunction) { if (irFunction.dispatchReceiverParameter != null) { for (overriddenFunction in getOverriddenFunctions(irFunction as IrSimpleFunction)) { - if (!overriddenFunction.isFakeOverride) trackMethod(irFunction, mapRawSignature(overriddenFunction)) + if (!overriddenFunction.isFakeOverride) trackDeclaration(irFunction, mapRawSignature(overriddenFunction)) } } else { - trackMethod(irFunction, mapRawSignature(irFunction)) + trackDeclaration(irFunction, mapRawSignature(irFunction)) } } @@ -69,90 +61,85 @@ class JvmMethodSignatureClashDetector( private fun IrFunction.isSpecialOverride(): Boolean = origin in SPECIAL_BRIDGES_AND_OVERRIDES - fun reportErrors() { - reportMethodSignatureConflicts() - reportPredefinedMethodSignatureConflicts() - reportFieldSignatureConflicts() + override fun reportErrorsTo(diagnosticReporter: IrDiagnosticReporter) { + super.reportErrorsTo(diagnosticReporter) + reportPredefinedMethodSignatureConflicts(diagnosticReporter) } - private fun reportMethodSignatureConflicts() { - for ((rawSignature, methods) in methodsBySignature) { - if (methods.size <= 1) continue + override fun reportSignatureConflict( + signature: RawSignature, + declarations: Collection, + diagnosticReporter: IrDiagnosticReporter + ) { + val fakeOverridesCount = declarations.count { it.isFakeOverride } + val specialOverridesCount = declarations.count { it.isSpecialOverride() } + val realMethodsCount = declarations.size - fakeOverridesCount - specialOverridesCount - val fakeOverridesCount = methods.count { it.isFakeOverride } - val specialOverridesCount = methods.count { it.isSpecialOverride() } - val realMethodsCount = methods.size - fakeOverridesCount - specialOverridesCount + val conflictingJvmDeclarationsData = getConflictingJvmDeclarationsData(signature, declarations) - val conflictingJvmDeclarationsData = getConflictingJvmDeclarationsData(rawSignature, methods) - - when { - realMethodsCount == 0 && (fakeOverridesCount > 1 || specialOverridesCount > 1) -> - if (classCodegen.irClass.origin != JvmLoweredDeclarationOrigin.DEFAULT_IMPLS) { - reportJvmSignatureClash( - JvmBackendErrors.CONFLICTING_INHERITED_JVM_DECLARATIONS, - listOf(classCodegen.irClass), - conflictingJvmDeclarationsData - ) - } - - fakeOverridesCount == 0 && specialOverridesCount == 0 -> { - // In IFoo$DefaultImpls we should report errors only if there are private methods among conflicting ones - // (otherwise such errors would be reported twice: once for IFoo and once for IFoo$DefaultImpls). - if (classCodegen.irClass.origin != JvmLoweredDeclarationOrigin.DEFAULT_IMPLS || - methods.any { DescriptorVisibilities.isPrivate(it.visibility) } - ) { - reportJvmSignatureClash( - JvmBackendErrors.CONFLICTING_JVM_DECLARATIONS, - methods, - conflictingJvmDeclarationsData - ) - } + when { + realMethodsCount == 0 && (fakeOverridesCount > 1 || specialOverridesCount > 1) -> + if (classCodegen.irClass.origin != JvmLoweredDeclarationOrigin.DEFAULT_IMPLS) { + reportJvmSignatureClash( + diagnosticReporter, + JvmBackendErrors.CONFLICTING_INHERITED_JVM_DECLARATIONS, + listOf(classCodegen.irClass), + conflictingJvmDeclarationsData + ) } - else -> - if (classCodegen.irClass.origin != JvmLoweredDeclarationOrigin.DEFAULT_IMPLS) { - reportJvmSignatureClash( - JvmBackendErrors.ACCIDENTAL_OVERRIDE, - methods.filter { !it.isFakeOverride && !it.isSpecialOverride() }, - conflictingJvmDeclarationsData - ) - } + fakeOverridesCount == 0 && specialOverridesCount == 0 -> { + // In IFoo$DefaultImpls we should report errors only if there are private methods among conflicting ones + // (otherwise such errors would be reported twice: once for IFoo and once for IFoo$DefaultImpls). + if (classCodegen.irClass.origin != JvmLoweredDeclarationOrigin.DEFAULT_IMPLS || + declarations.any { DescriptorVisibilities.isPrivate(it.visibility) } + ) { + reportJvmSignatureClash( + diagnosticReporter, + JvmBackendErrors.CONFLICTING_JVM_DECLARATIONS, + declarations, + conflictingJvmDeclarationsData + ) + } } + + else -> + if (classCodegen.irClass.origin != JvmLoweredDeclarationOrigin.DEFAULT_IMPLS) { + reportJvmSignatureClash( + diagnosticReporter, + JvmBackendErrors.ACCIDENTAL_OVERRIDE, + declarations.filter { !it.isFakeOverride && !it.isSpecialOverride() }, + conflictingJvmDeclarationsData + ) + } } } - private fun reportPredefinedMethodSignatureConflicts() { + private fun reportPredefinedMethodSignatureConflicts(diagnosticReporter: IrDiagnosticReporter) { for (predefinedSignature in PREDEFINED_SIGNATURES) { - val knownMethods = methodsBySignature[predefinedSignature] ?: continue - val methods = knownMethods.filter { !it.isFakeOverride && !it.isSpecialOverride() } + val methods = declarationsWithSignature(predefinedSignature).filter { !it.isFakeOverride && !it.isSpecialOverride() } if (methods.isEmpty()) continue val conflictingJvmDeclarationsData = ConflictingJvmDeclarationsData( classCodegen.type.internalName, null, predefinedSignature, null, methods.map(IrFunction::toIrBasedDescriptor), ) - reportJvmSignatureClash(JvmBackendErrors.ACCIDENTAL_OVERRIDE, methods, conflictingJvmDeclarationsData) - } - } - - private fun reportFieldSignatureConflicts() { - for ((rawSignature, fields) in fieldsBySignature) { - if (fields.size <= 1) continue - val conflictingJvmDeclarationsData = getConflictingJvmDeclarationsData(rawSignature, fields) - reportJvmSignatureClash(JvmBackendErrors.CONFLICTING_JVM_DECLARATIONS, fields, conflictingJvmDeclarationsData) + reportJvmSignatureClash(diagnosticReporter, JvmBackendErrors.ACCIDENTAL_OVERRIDE, methods, conflictingJvmDeclarationsData) } } private fun reportJvmSignatureClash( + diagnosticReporter: IrDiagnosticReporter, diagnosticFactory1: KtDiagnosticFactory1, irDeclarations: Collection, conflictingJvmDeclarationsData: ConflictingJvmDeclarationsData ) { - irDeclarations.mapTo(LinkedHashSet()) { irDeclaration -> + reportSignatureClashTo( + diagnosticReporter, + diagnosticFactory1, + irDeclarations, + conflictingJvmDeclarationsData, // Offset can be negative (SYNTHETIC_OFFSET) for delegated members; report an error on the class in that case. - val reportOn = irDeclaration.takeUnless { it.startOffset < 0 } ?: classCodegen.irClass - classCodegen.context.ktDiagnosticReporter.at(reportOn.sourceElement(), reportOn, irDeclaration.file) - }.forEach { - it.report(diagnosticFactory1, conflictingJvmDeclarationsData) - } + reportOnIfSynthetic = { classCodegen.irClass }, + ) } private fun getConflictingJvmDeclarationsData( 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 new file mode 100644 index 00000000000..2944026e25f --- /dev/null +++ b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/linkage/SignatureClashDetector.kt @@ -0,0 +1,70 @@ +/* + * Copyright 2010-2023 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.ir.linkage + +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.sourceElement +import org.jetbrains.kotlin.utils.SmartSet + +/** + * Collects the information about declarations and their signatures and reports cases when multiple declarations have the same signature. + */ +abstract class SignatureClashDetector { + private val declarationsBySignature: HashMap> = LinkedHashMap() + + /** + * Returns all the declarations that have [signature] previously recorded by [trackDeclaration]. + */ + protected fun declarationsWithSignature(signature: Signature): Set = + declarationsBySignature[signature] ?: emptySet() + + /** + * Records the declaration, so it could later participate in signature clash detection. + */ + fun trackDeclaration(declaration: Declaration, rawSignature: Signature) { + declarationsBySignature.computeIfAbsent(rawSignature) { SmartSet.create() }.add(declaration) + } + + /** + * Invoked by [reportErrorsTo] whenever at least two declarations with the same [signature] are detected. + * + * Use [reportSignatureClashTo] in the implementation to report a diagnostic. + */ + protected abstract fun reportSignatureConflict( + signature: Signature, + declarations: Collection, + diagnosticReporter: IrDiagnosticReporter, + ) + + /** + * Reports all detected signature clashes. + */ + open fun reportErrorsTo(diagnosticReporter: IrDiagnosticReporter) { + for ((signature, declarations) in declarationsBySignature) { + if (declarations.size <= 1) continue + reportSignatureConflict(signature, declarations, diagnosticReporter) + } + } + + protected inline fun reportSignatureClashTo( + diagnosticReporter: IrDiagnosticReporter, + diagnosticFactory: KtDiagnosticFactory1, + declarations: Collection, + data: Data, + reportOnIfSynthetic: (ConflictingDeclaration) -> IrElement, + ) { + declarations.mapTo(LinkedHashSet()) { declaration -> + val reportOn = declaration.takeUnless { it.startOffset < 0 } ?: reportOnIfSynthetic(declaration) + diagnosticReporter.at(reportOn.sourceElement(), reportOn, declaration.file) + }.forEach { + it.report(diagnosticFactory, data) + } + } +} \ No newline at end of file