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.
This commit is contained in:
Alexander Udalov
2023-02-27 13:56:12 +01:00
parent 00fff3de72
commit b1d109e7a3
8 changed files with 44 additions and 60 deletions
@@ -78,7 +78,8 @@ abstract class SignatureCollectingClassBuilderFactory(
classInternalName, classInternalName,
classCreatedFor, classCreatedFor,
signature, signature,
elementsAndDescriptors elementsAndDescriptors,
elementsAndDescriptors.mapNotNull(JvmDeclarationOrigin::descriptor),
)) ))
} }
onClassDone(classCreatedFor, classInternalName, signatures) onClassDone(classCreatedFor, classInternalName, signatures)
@@ -71,17 +71,17 @@ class BuilderFactoryForDuplicateSignatureDiagnostics(
} }
private fun reportConflictingJvmSignatures(data: ConflictingJvmDeclarationsData) { 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<PsiElement>() val elements = LinkedHashSet<PsiElement>()
if (noOwnImplementations) { if (noOwnImplementations) {
elements.addIfNotNull(data.classOrigin.element) elements.addIfNotNull(data.classOrigin!!.element)
} else { } else {
for (origin in data.signatureOrigins) { for (origin in data.signatureOrigins!!) {
var element = origin.element var element = origin.element
if (element == null || origin.originKind in EXTERNAL_SOURCES_KINDS) { if (element == null || origin.originKind in EXTERNAL_SOURCES_KINDS) {
element = data.classOrigin.element element = data.classOrigin!!.element
} }
elements.addIfNotNull(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) { if (memberElement != null) {
return ConflictingDeclarationError.AccidentalOverride(memberElement, data) return ConflictingDeclarationError.AccidentalOverride(memberElement, data)
} }
@@ -120,9 +120,9 @@ class FilteredJvmDiagnostics(val jvmDiagnostics: Diagnostics, val otherDiagnosti
} }
private infix fun ConflictingJvmDeclarationsData.higherThan(other: ConflictingJvmDeclarationsData): Boolean { private infix fun ConflictingJvmDeclarationsData.higherThan(other: ConflictingJvmDeclarationsData): Boolean {
return when (other.classOrigin.originKind) { return when (other.classOrigin?.originKind) {
INTERFACE_DEFAULT_IMPL -> this.classOrigin.originKind != INTERFACE_DEFAULT_IMPL INTERFACE_DEFAULT_IMPL -> classOrigin?.originKind != INTERFACE_DEFAULT_IMPL
MULTIFILE_CLASS_PART -> this.classOrigin.originKind == MULTIFILE_CLASS MULTIFILE_CLASS_PART -> classOrigin?.originKind == MULTIFILE_CLASS
else -> false else -> false
} }
} }
@@ -16,9 +16,12 @@
package org.jetbrains.kotlin.resolve.jvm.diagnostics package org.jetbrains.kotlin.resolve.jvm.diagnostics
import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
class ConflictingJvmDeclarationsData( class ConflictingJvmDeclarationsData(
val classInternalName: String, val classInternalName: String,
val classOrigin: JvmDeclarationOrigin, val classOrigin: JvmDeclarationOrigin?,
val signature: RawSignature, val signature: RawSignature,
val signatureOrigins: Collection<JvmDeclarationOrigin> val signatureOrigins: Collection<JvmDeclarationOrigin>?,
val signatureDescriptors: Collection<DeclarationDescriptor>,
) )
@@ -12,8 +12,9 @@ import org.jetbrains.kotlin.resolve.MemberComparator;
import org.jetbrains.kotlin.utils.StringsKt; import org.jetbrains.kotlin.utils.StringsKt;
import java.util.List; 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.CommonRenderers.STRING;
import static org.jetbrains.kotlin.diagnostics.rendering.Renderers.*; import static org.jetbrains.kotlin.diagnostics.rendering.Renderers.*;
import static org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm.*; import static org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm.*;
@@ -22,10 +23,8 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension {
private static final DiagnosticParameterRenderer<ConflictingJvmDeclarationsData> CONFLICTING_JVM_DECLARATIONS_DATA = private static final DiagnosticParameterRenderer<ConflictingJvmDeclarationsData> CONFLICTING_JVM_DECLARATIONS_DATA =
(data, context) -> { (data, context) -> {
List<DeclarationDescriptor> renderedDescriptors = sortedWith( List<DeclarationDescriptor> renderedDescriptors =
mapNotNull(data.getSignatureOrigins(), JvmDeclarationOrigin::getDescriptor), data.getSignatureDescriptors().stream().sorted(MemberComparator.INSTANCE).collect(Collectors.toList());
MemberComparator.INSTANCE
);
RenderingContext renderingContext = new RenderingContext.Impl(renderedDescriptors); RenderingContext renderingContext = new RenderingContext.Impl(renderedDescriptors);
return "The following declarations have the same JVM signature " + return "The following declarations have the same JVM signature " +
"(" + data.getSignature().getName() + data.getSignature().getDesc() + "):\n" + "(" + data.getSignature().getName() + data.getSignature().getDesc() + "):\n" +
@@ -43,20 +43,14 @@ object KtDefaultJvmErrorMessages : BaseDiagnosticRendererFactory() {
@JvmField @JvmField
val CONFLICTING_JVM_DECLARATIONS_DATA = Renderer<ConflictingJvmDeclarationsData> { val CONFLICTING_JVM_DECLARATIONS_DATA = Renderer<ConflictingJvmDeclarationsData> {
val renderedDescriptors: List<DeclarationDescriptor?> = val renderedDescriptors = it.signatureDescriptors.sortedWith(MemberComparator.INSTANCE)
it.signatureOrigins.mapNotNull( val renderingContext = RenderingContext.Impl(renderedDescriptors)
JvmDeclarationOrigin::descriptor
).sortedWith(MemberComparator.INSTANCE)
val renderingContext: RenderingContext =
RenderingContext.Impl(renderedDescriptors)
""" """
The following declarations have the same JVM signature (${it.signature.name}${it.signature.desc}): The following declarations have the same JVM signature (${it.signature.name}${it.signature.desc}):
""".trimIndent() + """.trimIndent() +
join(renderedDescriptors.map { descriptor: DeclarationDescriptor? -> join(renderedDescriptors.map { descriptor ->
" " + Renderers.WITHOUT_MODIFIERS.render( " " + Renderers.WITHOUT_MODIFIERS.render(descriptor, renderingContext)
descriptor!!, renderingContext
)
}, "\n") }, "\n")
} }
@@ -111,9 +111,7 @@ class ClassCodegen private constructor(
private val jvmSignatureClashDetector = JvmSignatureClashDetector(this) private val jvmSignatureClashDetector = JvmSignatureClashDetector(this)
private val classOrigin = irClass.descriptorOrigin private val visitor = state.factory.newVisitor(irClass.descriptorOrigin, type, irClass.fileParent.loadSourceFilesInfo()).apply {
private val visitor = state.factory.newVisitor(classOrigin, type, irClass.fileParent.loadSourceFilesInfo()).apply {
val signature = typeMapper.mapClassSignature(irClass, type, context.state.classBuilderMode.generateBodies) 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. // Ensure that the backend only produces class names that would be valid in the frontend for JVM.
if (context.state.classBuilderMode.generateBodies && signature.hasInvalidName()) { if (context.state.classBuilderMode.generateBodies && signature.hasInvalidName()) {
@@ -218,7 +216,7 @@ class ClassCodegen private constructor(
generateInnerAndOuterClasses() generateInnerAndOuterClasses()
visitor.done(state.generateSmapCopyToAnnotation) visitor.done(state.generateSmapCopyToAnnotation)
jvmSignatureClashDetector.reportErrors(classOrigin) jvmSignatureClashDetector.reportErrors()
} }
private fun shouldSkipCodeGenerationAccordingToGenerationFilter(): Boolean { private fun shouldSkipCodeGenerationAccordingToGenerationFilter(): Boolean {
@@ -6,7 +6,6 @@
package org.jetbrains.kotlin.backend.jvm.codegen package org.jetbrains.kotlin.backend.jvm.codegen
import org.jetbrains.kotlin.backend.common.lower.ANNOTATION_IMPLEMENTATION 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.backend.jvm.JvmLoweredDeclarationOrigin
import org.jetbrains.kotlin.descriptors.DescriptorVisibilities import org.jetbrains.kotlin.descriptors.DescriptorVisibilities
import org.jetbrains.kotlin.diagnostics.KtDiagnosticFactory1 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.descriptors.toIrBasedDescriptor
import org.jetbrains.kotlin.ir.util.file import org.jetbrains.kotlin.ir.util.file
import org.jetbrains.kotlin.ir.util.isFakeOverride 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 import org.jetbrains.kotlin.utils.SmartSet
class JvmSignatureClashDetector( class JvmSignatureClashDetector(
@@ -66,13 +68,13 @@ class JvmSignatureClashDetector(
private fun IrFunction.isSpecialOverride(): Boolean = private fun IrFunction.isSpecialOverride(): Boolean =
origin in SPECIAL_BRIDGES_AND_OVERRIDES origin in SPECIAL_BRIDGES_AND_OVERRIDES
fun reportErrors(classOrigin: JvmDeclarationOrigin) { fun reportErrors() {
reportMethodSignatureConflicts(classOrigin) reportMethodSignatureConflicts()
reportPredefinedMethodSignatureConflicts(classOrigin) reportPredefinedMethodSignatureConflicts()
reportFieldSignatureConflicts(classOrigin) reportFieldSignatureConflicts()
} }
private fun reportMethodSignatureConflicts(classOrigin: JvmDeclarationOrigin) { private fun reportMethodSignatureConflicts() {
for ((rawSignature, methods) in methodsBySignature) { for ((rawSignature, methods) in methodsBySignature) {
if (methods.size <= 1) continue if (methods.size <= 1) continue
@@ -80,7 +82,7 @@ class JvmSignatureClashDetector(
val specialOverridesCount = methods.count { it.isSpecialOverride() } val specialOverridesCount = methods.count { it.isSpecialOverride() }
val realMethodsCount = methods.size - fakeOverridesCount - specialOverridesCount val realMethodsCount = methods.size - fakeOverridesCount - specialOverridesCount
val conflictingJvmDeclarationsData = getConflictingJvmDeclarationsData(classOrigin, rawSignature, methods) val conflictingJvmDeclarationsData = getConflictingJvmDeclarationsData(rawSignature, methods)
when { when {
realMethodsCount == 0 && (fakeOverridesCount > 1 || specialOverridesCount > 1) -> 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) { for (predefinedSignature in PREDEFINED_SIGNATURES) {
val knownMethods = methodsBySignature[predefinedSignature] ?: continue val knownMethods = methodsBySignature[predefinedSignature] ?: continue
val methods = knownMethods.filter { !it.isFakeOverride && !it.isSpecialOverride() } val methods = knownMethods.filter { !it.isFakeOverride && !it.isSpecialOverride() }
if (methods.isEmpty()) continue if (methods.isEmpty()) continue
val conflictingJvmDeclarationsData = ConflictingJvmDeclarationsData( val conflictingJvmDeclarationsData = ConflictingJvmDeclarationsData(
classCodegen.type.internalName, classOrigin, predefinedSignature, classCodegen.type.internalName, null, predefinedSignature, null, methods.map(IrFunction::toIrBasedDescriptor),
methods.map { it.getJvmDeclarationOrigin() } + JvmDeclarationOrigin(JvmDeclarationOriginKind.OTHER, null, null)
) )
reportJvmSignatureClash(JvmBackendErrors.ACCIDENTAL_OVERRIDE, methods, conflictingJvmDeclarationsData) reportJvmSignatureClash(JvmBackendErrors.ACCIDENTAL_OVERRIDE, methods, conflictingJvmDeclarationsData)
} }
} }
private fun reportFieldSignatureConflicts(classOrigin: JvmDeclarationOrigin) { private fun reportFieldSignatureConflicts() {
for ((rawSignature, fields) in fieldsBySignature) { for ((rawSignature, fields) in fieldsBySignature) {
if (fields.size <= 1) continue if (fields.size <= 1) continue
val conflictingJvmDeclarationsData = getConflictingJvmDeclarationsData(classOrigin, rawSignature, fields) val conflictingJvmDeclarationsData = getConflictingJvmDeclarationsData(rawSignature, fields)
reportJvmSignatureClash(JvmBackendErrors.CONFLICTING_JVM_DECLARATIONS, fields, conflictingJvmDeclarationsData) reportJvmSignatureClash(JvmBackendErrors.CONFLICTING_JVM_DECLARATIONS, fields, conflictingJvmDeclarationsData)
} }
} }
@@ -152,27 +153,13 @@ class JvmSignatureClashDetector(
} }
private fun getConflictingJvmDeclarationsData( private fun getConflictingJvmDeclarationsData(
classOrigin: JvmDeclarationOrigin,
rawSignature: RawSignature, rawSignature: RawSignature,
methods: Collection<IrDeclaration> methods: Collection<IrDeclaration>
): ConflictingJvmDeclarationsData = ): ConflictingJvmDeclarationsData =
ConflictingJvmDeclarationsData( ConflictingJvmDeclarationsData(
classCodegen.type.internalName, classCodegen.type.internalName, null, rawSignature, null, methods.map(IrDeclaration::toIrBasedDescriptor),
classOrigin,
rawSignature,
methods.map { it.getJvmDeclarationOrigin() }
) )
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 { companion object {
val SPECIAL_BRIDGES_AND_OVERRIDES = setOf( val SPECIAL_BRIDGES_AND_OVERRIDES = setOf(
IrDeclarationOrigin.BRIDGE, IrDeclarationOrigin.BRIDGE,