From b1d109e7a327e17275b4ddc1a8101889ce480d32 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 27 Feb 2023 13:56:12 +0100 Subject: [PATCH] JVM IR: do not use JvmDeclarationOrigin in duplicate signatures Apparently the client code which reports errors only meaningfully uses the `descriptor` field of `JvmDeclarationOrigin` in case of JVM IR. --- .../SignatureCollectingClassBuilderFactory.kt | 3 +- ...FactoryForDuplicateSignatureDiagnostics.kt | 12 +++--- .../jvm/compiler/duplicateJvmSignatureUtil.kt | 6 +-- .../ConflictingJvmDeclarationsData.kt | 11 +++-- .../diagnostics/DefaultErrorMessagesJvm.java | 9 ++-- .../jvm/diagnostics/JvmBackendErrors.kt | 14 ++---- .../backend/jvm/codegen/ClassCodegen.kt | 6 +-- .../jvm/codegen/JvmSignatureClashDetector.kt | 43 +++++++------------ 8 files changed, 44 insertions(+), 60 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/SignatureCollectingClassBuilderFactory.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/SignatureCollectingClassBuilderFactory.kt index 57d7571fc63..b5de2c9c566 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/SignatureCollectingClassBuilderFactory.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/SignatureCollectingClassBuilderFactory.kt @@ -78,7 +78,8 @@ abstract class SignatureCollectingClassBuilderFactory( classInternalName, classCreatedFor, signature, - elementsAndDescriptors + elementsAndDescriptors, + elementsAndDescriptors.mapNotNull(JvmDeclarationOrigin::descriptor), )) } onClassDone(classCreatedFor, classInternalName, signatures) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/BuilderFactoryForDuplicateSignatureDiagnostics.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/BuilderFactoryForDuplicateSignatureDiagnostics.kt index 5ddee9ac14b..99f1146c663 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/BuilderFactoryForDuplicateSignatureDiagnostics.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/BuilderFactoryForDuplicateSignatureDiagnostics.kt @@ -71,17 +71,17 @@ class BuilderFactoryForDuplicateSignatureDiagnostics( } private fun reportConflictingJvmSignatures(data: ConflictingJvmDeclarationsData) { - val noOwnImplementations = data.signatureOrigins.all { it.originKind in EXTERNAL_SOURCES_KINDS } + val noOwnImplementations = data.signatureOrigins!!.all { it.originKind in EXTERNAL_SOURCES_KINDS } val elements = LinkedHashSet() if (noOwnImplementations) { - elements.addIfNotNull(data.classOrigin.element) + elements.addIfNotNull(data.classOrigin!!.element) } else { - for (origin in data.signatureOrigins) { + for (origin in data.signatureOrigins!!) { var element = origin.element if (element == null || origin.originKind in EXTERNAL_SOURCES_KINDS) { - element = data.classOrigin.element + element = data.classOrigin!!.element } elements.addIfNotNull(element) @@ -186,7 +186,9 @@ class BuilderFactoryForDuplicateSignatureDiagnostics( } } - val data = ConflictingJvmDeclarationsData(classInternalName, classOrigin, rawSignature, origins) + val data = ConflictingJvmDeclarationsData( + classInternalName, classOrigin, rawSignature, origins, origins.mapNotNull(JvmDeclarationOrigin::descriptor), + ) if (memberElement != null) { return ConflictingDeclarationError.AccidentalOverride(memberElement, data) } diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/duplicateJvmSignatureUtil.kt b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/duplicateJvmSignatureUtil.kt index 04aef58f2a3..78cf78f8d8f 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/duplicateJvmSignatureUtil.kt +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/duplicateJvmSignatureUtil.kt @@ -120,9 +120,9 @@ class FilteredJvmDiagnostics(val jvmDiagnostics: Diagnostics, val otherDiagnosti } private infix fun ConflictingJvmDeclarationsData.higherThan(other: ConflictingJvmDeclarationsData): Boolean { - return when (other.classOrigin.originKind) { - INTERFACE_DEFAULT_IMPL -> this.classOrigin.originKind != INTERFACE_DEFAULT_IMPL - MULTIFILE_CLASS_PART -> this.classOrigin.originKind == MULTIFILE_CLASS + return when (other.classOrigin?.originKind) { + INTERFACE_DEFAULT_IMPL -> classOrigin?.originKind != INTERFACE_DEFAULT_IMPL + MULTIFILE_CLASS_PART -> classOrigin?.originKind == MULTIFILE_CLASS else -> false } } diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ConflictingJvmDeclarationsData.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ConflictingJvmDeclarationsData.kt index d6835dceefb..c915fcef4fe 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ConflictingJvmDeclarationsData.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ConflictingJvmDeclarationsData.kt @@ -16,9 +16,12 @@ package org.jetbrains.kotlin.resolve.jvm.diagnostics +import org.jetbrains.kotlin.descriptors.DeclarationDescriptor + class ConflictingJvmDeclarationsData( - val classInternalName: String, - val classOrigin: JvmDeclarationOrigin, - val signature: RawSignature, - val signatureOrigins: Collection + val classInternalName: String, + val classOrigin: JvmDeclarationOrigin?, + val signature: RawSignature, + val signatureOrigins: Collection?, + val signatureDescriptors: Collection, ) diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java index 73a5c6c770e..0244f20dc41 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java @@ -12,8 +12,9 @@ import org.jetbrains.kotlin.resolve.MemberComparator; import org.jetbrains.kotlin.utils.StringsKt; import java.util.List; +import java.util.stream.Collectors; -import static kotlin.collections.CollectionsKt.*; +import static kotlin.collections.CollectionsKt.map; import static org.jetbrains.kotlin.diagnostics.rendering.CommonRenderers.STRING; import static org.jetbrains.kotlin.diagnostics.rendering.Renderers.*; import static org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm.*; @@ -22,10 +23,8 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension { private static final DiagnosticParameterRenderer CONFLICTING_JVM_DECLARATIONS_DATA = (data, context) -> { - List renderedDescriptors = sortedWith( - mapNotNull(data.getSignatureOrigins(), JvmDeclarationOrigin::getDescriptor), - MemberComparator.INSTANCE - ); + List renderedDescriptors = + data.getSignatureDescriptors().stream().sorted(MemberComparator.INSTANCE).collect(Collectors.toList()); RenderingContext renderingContext = new RenderingContext.Impl(renderedDescriptors); return "The following declarations have the same JVM signature " + "(" + data.getSignature().getName() + data.getSignature().getDesc() + "):\n" + diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/JvmBackendErrors.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/JvmBackendErrors.kt index 2e4af0d0e0a..51c9a1c6017 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/JvmBackendErrors.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/JvmBackendErrors.kt @@ -43,20 +43,14 @@ object KtDefaultJvmErrorMessages : BaseDiagnosticRendererFactory() { @JvmField val CONFLICTING_JVM_DECLARATIONS_DATA = Renderer { - val renderedDescriptors: List = - it.signatureOrigins.mapNotNull( - JvmDeclarationOrigin::descriptor - ).sortedWith(MemberComparator.INSTANCE) - val renderingContext: RenderingContext = - RenderingContext.Impl(renderedDescriptors) + val renderedDescriptors = it.signatureDescriptors.sortedWith(MemberComparator.INSTANCE) + val renderingContext = RenderingContext.Impl(renderedDescriptors) """ The following declarations have the same JVM signature (${it.signature.name}${it.signature.desc}): """.trimIndent() + - join(renderedDescriptors.map { descriptor: DeclarationDescriptor? -> - " " + Renderers.WITHOUT_MODIFIERS.render( - descriptor!!, renderingContext - ) + join(renderedDescriptors.map { descriptor -> + " " + Renderers.WITHOUT_MODIFIERS.render(descriptor, renderingContext) }, "\n") } 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 e0967418a60..dbc0fdb1846 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 @@ -111,9 +111,7 @@ class ClassCodegen private constructor( private val jvmSignatureClashDetector = JvmSignatureClashDetector(this) - private val classOrigin = irClass.descriptorOrigin - - private val visitor = state.factory.newVisitor(classOrigin, type, irClass.fileParent.loadSourceFilesInfo()).apply { + private val visitor = state.factory.newVisitor(irClass.descriptorOrigin, type, irClass.fileParent.loadSourceFilesInfo()).apply { val signature = typeMapper.mapClassSignature(irClass, type, context.state.classBuilderMode.generateBodies) // Ensure that the backend only produces class names that would be valid in the frontend for JVM. if (context.state.classBuilderMode.generateBodies && signature.hasInvalidName()) { @@ -218,7 +216,7 @@ class ClassCodegen private constructor( generateInnerAndOuterClasses() visitor.done(state.generateSmapCopyToAnnotation) - jvmSignatureClashDetector.reportErrors(classOrigin) + jvmSignatureClashDetector.reportErrors() } private fun shouldSkipCodeGenerationAccordingToGenerationFilter(): Boolean { diff --git a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmSignatureClashDetector.kt b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmSignatureClashDetector.kt index ad9e35ec93a..123629f68a8 100644 --- a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmSignatureClashDetector.kt +++ b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmSignatureClashDetector.kt @@ -6,7 +6,6 @@ package org.jetbrains.kotlin.backend.jvm.codegen import org.jetbrains.kotlin.backend.common.lower.ANNOTATION_IMPLEMENTATION -import org.jetbrains.kotlin.backend.common.psi.PsiSourceManager import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin import org.jetbrains.kotlin.descriptors.DescriptorVisibilities import org.jetbrains.kotlin.diagnostics.KtDiagnosticFactory1 @@ -14,7 +13,10 @@ import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.descriptors.toIrBasedDescriptor import org.jetbrains.kotlin.ir.util.file import org.jetbrains.kotlin.ir.util.isFakeOverride -import org.jetbrains.kotlin.resolve.jvm.diagnostics.* +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 JvmSignatureClashDetector( @@ -66,13 +68,13 @@ class JvmSignatureClashDetector( private fun IrFunction.isSpecialOverride(): Boolean = origin in SPECIAL_BRIDGES_AND_OVERRIDES - fun reportErrors(classOrigin: JvmDeclarationOrigin) { - reportMethodSignatureConflicts(classOrigin) - reportPredefinedMethodSignatureConflicts(classOrigin) - reportFieldSignatureConflicts(classOrigin) + fun reportErrors() { + reportMethodSignatureConflicts() + reportPredefinedMethodSignatureConflicts() + reportFieldSignatureConflicts() } - private fun reportMethodSignatureConflicts(classOrigin: JvmDeclarationOrigin) { + private fun reportMethodSignatureConflicts() { for ((rawSignature, methods) in methodsBySignature) { if (methods.size <= 1) continue @@ -80,7 +82,7 @@ class JvmSignatureClashDetector( val specialOverridesCount = methods.count { it.isSpecialOverride() } val realMethodsCount = methods.size - fakeOverridesCount - specialOverridesCount - val conflictingJvmDeclarationsData = getConflictingJvmDeclarationsData(classOrigin, rawSignature, methods) + val conflictingJvmDeclarationsData = getConflictingJvmDeclarationsData(rawSignature, methods) when { realMethodsCount == 0 && (fakeOverridesCount > 1 || specialOverridesCount > 1) -> @@ -118,23 +120,22 @@ class JvmSignatureClashDetector( } } - private fun reportPredefinedMethodSignatureConflicts(classOrigin: JvmDeclarationOrigin) { + private fun reportPredefinedMethodSignatureConflicts() { for (predefinedSignature in PREDEFINED_SIGNATURES) { val knownMethods = methodsBySignature[predefinedSignature] ?: continue val methods = knownMethods.filter { !it.isFakeOverride && !it.isSpecialOverride() } if (methods.isEmpty()) continue val conflictingJvmDeclarationsData = ConflictingJvmDeclarationsData( - classCodegen.type.internalName, classOrigin, predefinedSignature, - methods.map { it.getJvmDeclarationOrigin() } + JvmDeclarationOrigin(JvmDeclarationOriginKind.OTHER, null, null) + classCodegen.type.internalName, null, predefinedSignature, null, methods.map(IrFunction::toIrBasedDescriptor), ) reportJvmSignatureClash(JvmBackendErrors.ACCIDENTAL_OVERRIDE, methods, conflictingJvmDeclarationsData) } } - private fun reportFieldSignatureConflicts(classOrigin: JvmDeclarationOrigin) { + private fun reportFieldSignatureConflicts() { for ((rawSignature, fields) in fieldsBySignature) { if (fields.size <= 1) continue - val conflictingJvmDeclarationsData = getConflictingJvmDeclarationsData(classOrigin, rawSignature, fields) + val conflictingJvmDeclarationsData = getConflictingJvmDeclarationsData(rawSignature, fields) reportJvmSignatureClash(JvmBackendErrors.CONFLICTING_JVM_DECLARATIONS, fields, conflictingJvmDeclarationsData) } } @@ -152,27 +153,13 @@ class JvmSignatureClashDetector( } private fun getConflictingJvmDeclarationsData( - classOrigin: JvmDeclarationOrigin, rawSignature: RawSignature, methods: Collection ): ConflictingJvmDeclarationsData = ConflictingJvmDeclarationsData( - classCodegen.type.internalName, - classOrigin, - rawSignature, - methods.map { it.getJvmDeclarationOrigin() } + classCodegen.type.internalName, null, rawSignature, null, methods.map(IrDeclaration::toIrBasedDescriptor), ) - private fun IrDeclaration.getJvmDeclarationOrigin(): JvmDeclarationOrigin { - // It looks like 'JvmDeclarationOriginKind' is not really used in error reporting. - // However, if needed, we can provide more meaningful information regarding function origin. - return JvmDeclarationOrigin( - JvmDeclarationOriginKind.OTHER, - PsiSourceManager.findPsiElement(this), - toIrBasedDescriptor() - ) - } - companion object { val SPECIAL_BRIDGES_AND_OVERRIDES = setOf( IrDeclarationOrigin.BRIDGE,