From 91adb88eff24d9e0968742dc3d511e132d19dfa9 Mon Sep 17 00:00:00 2001 From: Kirill Rakhman Date: Mon, 3 Apr 2023 13:15:59 +0200 Subject: [PATCH] [K1] Make Volatile diagnostics applicable to kotlin.concurrent.Volatile #KT-55628 --- .../jvm/checkers/declarationCheckers.kt | 18 ------ .../diagnostics/DefaultErrorMessagesJvm.java | 2 - .../resolve/jvm/diagnostics/ErrorsJvm.java | 2 - .../jvm/platform/JvmPlatformConfigurator.kt | 1 - .../jetbrains/kotlin/diagnostics/Errors.java | 3 + .../rendering/DefaultErrorMessages.java | 3 + .../resolve/PlatformConfiguratorBase.kt | 1 + .../checkers/VolatileAnnotationChecker.kt | 40 ++++++++++++ .../annotations/Volatile.fir.kt | 63 +++++++++++++++++++ .../testsWithStdLib/annotations/Volatile.kt | 30 ++++++++- .../testsWithStdLib/annotations/Volatile.txt | 28 +++++++++ 11 files changed, 166 insertions(+), 25 deletions(-) create mode 100644 compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/VolatileAnnotationChecker.kt create mode 100644 compiler/testData/diagnostics/testsWithStdLib/annotations/Volatile.fir.kt diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/declarationCheckers.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/declarationCheckers.kt index bbbce61aa60..eb6efff87a8 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/declarationCheckers.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/declarationCheckers.kt @@ -168,24 +168,6 @@ class JvmNameAnnotationChecker : DeclarationChecker { } } -class VolatileAnnotationChecker : DeclarationChecker { - override fun check(declaration: KtDeclaration, descriptor: DeclarationDescriptor, context: DeclarationCheckerContext) { - if (descriptor !is PropertyDescriptor) return - - val fieldAnnotation = descriptor.backingField?.annotations?.findAnnotation(VOLATILE_ANNOTATION_FQ_NAME) - if (fieldAnnotation != null && !descriptor.isVar) { - val annotationEntry = DescriptorToSourceUtils.getSourceFromAnnotation(fieldAnnotation) ?: return - context.trace.report(ErrorsJvm.VOLATILE_ON_VALUE.on(annotationEntry)) - } - - val delegateAnnotation = descriptor.delegateField?.annotations?.findAnnotation(VOLATILE_ANNOTATION_FQ_NAME) - if (delegateAnnotation != null) { - val annotationEntry = DescriptorToSourceUtils.getSourceFromAnnotation(delegateAnnotation) ?: return - context.trace.report(ErrorsJvm.VOLATILE_ON_DELEGATE.on(annotationEntry)) - } - } -} - class SynchronizedAnnotationChecker : DeclarationChecker { override fun check(declaration: KtDeclaration, descriptor: DeclarationDescriptor, context: DeclarationCheckerContext) { val synchronizedAnnotation = descriptor.findSynchronizedAnnotation() 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 0244f20dc41..2e198d3c6d7 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 @@ -57,8 +57,6 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension { MAP.put(INAPPLICABLE_JVM_NAME, "'@JvmName' annotation is not applicable to this declaration"); MAP.put(ILLEGAL_JVM_NAME, "Illegal JVM name"); - MAP.put(VOLATILE_ON_VALUE, "'@Volatile' annotation cannot be used on immutable properties"); - MAP.put(VOLATILE_ON_DELEGATE, "'@Volatile' annotation cannot be used on delegated properties"); MAP.put(SYNCHRONIZED_ON_ABSTRACT, "'@Synchronized' annotation cannot be used on abstract functions"); MAP.put(SYNCHRONIZED_ON_INLINE, "'@Synchronized' annotation has no effect on inline functions"); MAP.put(SYNCHRONIZED_ON_VALUE_CLASS, "'@Synchronized' annotation has no effect on value classes"); diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java index f5493b0f158..e0440bbc28f 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java @@ -46,8 +46,6 @@ public interface ErrorsJvm { DiagnosticFactory0 STRICTFP_ON_CLASS = DiagnosticFactory0.create(WARNING); - DiagnosticFactory0 VOLATILE_ON_VALUE = DiagnosticFactory0.create(ERROR); - DiagnosticFactory0 VOLATILE_ON_DELEGATE = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 SYNCHRONIZED_ON_ABSTRACT = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 SYNCHRONIZED_ON_INLINE = DiagnosticFactory0.create(WARNING); DiagnosticFactory0 SYNCHRONIZED_ON_VALUE_CLASS = DiagnosticFactory0.create(WARNING); diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt index 39b5418ddab..27cac1f6eb1 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt @@ -27,7 +27,6 @@ import org.jetbrains.kotlin.types.expressions.GenericArrayClassLiteralSupport object JvmPlatformConfigurator : PlatformConfiguratorBase( additionalDeclarationCheckers = listOf( JvmNameAnnotationChecker(), - VolatileAnnotationChecker(), SynchronizedAnnotationChecker(), LocalFunInlineChecker(), ExternalFunChecker(), diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index f4af2e262df..0ea6229dedd 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -367,6 +367,9 @@ public interface Errors { DiagnosticFactory1 REDUNDANT_ANNOTATION_TARGET = DiagnosticFactory1.create(WARNING); DiagnosticFactory0 INAPPLICABLE_FILE_TARGET = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 VOLATILE_ON_VALUE = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 VOLATILE_ON_DELEGATE = DiagnosticFactory0.create(ERROR); + // Classes and interfaces DiagnosticFactory0 PROJECTION_IN_IMMEDIATE_ARGUMENT_TO_SUPERTYPE = diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index a7c5770ff4c..782b708cdf5 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -1208,6 +1208,9 @@ public class DefaultErrorMessages { MAP.put(UNSUPPORTED_CONTEXTUAL_DECLARATION_CALL, "To use contextual declarations, specify the `-Xcontext-receivers` compiler option"); MAP.put(SUBTYPING_BETWEEN_CONTEXT_RECEIVERS, "Subtyping relation between context receivers is prohibited"); + MAP.put(VOLATILE_ON_VALUE, "'@Volatile' annotation cannot be used on immutable properties"); + MAP.put(VOLATILE_ON_DELEGATE, "'@Volatile' annotation cannot be used on delegated properties"); + MAP.setImmutable(); for (Field field : Errors.class.getFields()) { diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt index 421afef17cc..7eb6b439289 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt @@ -57,6 +57,7 @@ private val DEFAULT_DECLARATION_CHECKERS = listOf( UnsupportedUntilRangeDeclarationChecker, DataObjectContentChecker, EnumEntriesRedeclarationChecker, + VolatileAnnotationChecker, ) private val DEFAULT_CALL_CHECKERS = listOf( diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/VolatileAnnotationChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/VolatileAnnotationChecker.kt new file mode 100644 index 00000000000..49fc3f4c7c0 --- /dev/null +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/VolatileAnnotationChecker.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.resolve.checkers + +import org.jetbrains.kotlin.descriptors.DeclarationDescriptor +import org.jetbrains.kotlin.descriptors.FieldDescriptor +import org.jetbrains.kotlin.descriptors.PropertyDescriptor +import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor +import org.jetbrains.kotlin.diagnostics.Errors +import org.jetbrains.kotlin.name.FqName +import org.jetbrains.kotlin.psi.KtDeclaration +import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils + +object VolatileAnnotationChecker : DeclarationChecker { + private val JVM_VOLATILE_ANNOTATION_FQ_NAME = FqName("kotlin.jvm.Volatile") + private val CONCURRENT_VOLATILE_ANNOTATION_FQ_NAME = FqName("kotlin.concurrent.Volatile") + + override fun check(declaration: KtDeclaration, descriptor: DeclarationDescriptor, context: DeclarationCheckerContext) { + if (descriptor !is PropertyDescriptor) return + + val fieldAnnotation = descriptor.backingField?.findVolatileAnnotation() + if (fieldAnnotation != null && !descriptor.isVar) { + val annotationEntry = DescriptorToSourceUtils.getSourceFromAnnotation(fieldAnnotation) ?: return + context.trace.report(Errors.VOLATILE_ON_VALUE.on(annotationEntry)) + } + + val delegateAnnotation = descriptor.delegateField?.findVolatileAnnotation() + if (delegateAnnotation != null) { + val annotationEntry = DescriptorToSourceUtils.getSourceFromAnnotation(delegateAnnotation) ?: return + context.trace.report(Errors.VOLATILE_ON_DELEGATE.on(annotationEntry)) + } + } + + private fun FieldDescriptor.findVolatileAnnotation(): AnnotationDescriptor? { + return annotations.firstOrNull { it.fqName == JVM_VOLATILE_ANNOTATION_FQ_NAME || it.fqName == CONCURRENT_VOLATILE_ANNOTATION_FQ_NAME } + } +} diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/Volatile.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/Volatile.fir.kt new file mode 100644 index 00000000000..afa8ac4c5bc --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/Volatile.fir.kt @@ -0,0 +1,63 @@ +// !DIAGNOSTICS: -UNUSED_ANONYMOUS_PARAMETER +// !API_VERSION: 1.9 +// !LANGUAGE: +MultiPlatformProjects + +// MODULE: common +// FILE: common.kt +// TARGET_PLATFORM: Common +import kotlin.concurrent.Volatile +import kotlin.properties.Delegates + +@OptIn(ExperimentalStdlibApi::class) +class ConcurrentVolatile { + @Volatile val x = 0 + // ok + @Volatile var y = 1 + + @delegate:Volatile var z: String by Delegates.observable("?") { prop, old, new -> old.hashCode() } + + @field:Volatile val w = 2 + + @Volatile + var noBacking: String + get() = "" + set(value) {} +} + +// MODULE: jvm +// FILE: jvm.kt +// TARGET_PLATFORM: JVM +import kotlin.jvm.Volatile as JvmVolatile +import kotlin.concurrent.Volatile +import kotlin.properties.Delegates + +@OptIn(ExperimentalStdlibApi::class) +class ConcurrentVolatileOnJvm { + @Volatile val x = 0 + // ok + @Volatile var y = 1 + + @delegate:Volatile var z: String by Delegates.observable("?") { prop, old, new -> old.hashCode() } + + @field:Volatile val w = 2 + + @Volatile + var noBacking: String + get() = "" + set(value) {} +} + +class JvmVolatile { + @JvmVolatile val x = 0 + // ok + @JvmVolatile var y = 1 + + @delegate:JvmVolatile var z: String by Delegates.observable("?") { prop, old, new -> old.hashCode() } + + @field:JvmVolatile val w = 2 + + @JvmVolatile + var noBacking: String + get() = "" + set(value) {} +} diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/Volatile.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/Volatile.kt index b8e92fe9988..a20d23592b0 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/Volatile.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/Volatile.kt @@ -1,12 +1,38 @@ -// FIR_IDENTICAL // !DIAGNOSTICS: -UNUSED_ANONYMOUS_PARAMETER // !API_VERSION: 1.9 -import kotlin.jvm.Volatile as JvmVolatile +// !LANGUAGE: +MultiPlatformProjects + +// MODULE: common +// FILE: common.kt +// TARGET_PLATFORM: Common import kotlin.concurrent.Volatile import kotlin.properties.Delegates @OptIn(ExperimentalStdlibApi::class) class ConcurrentVolatile { + @Volatile val x = 0 + // ok + @Volatile var y = 1 + + @delegate:Volatile var z: String by Delegates.observable("?") { prop, old, new -> old.hashCode() } + + @field:Volatile val w = 2 + + @Volatile + var noBacking: String + get() = "" + set(value) {} +} + +// MODULE: jvm +// FILE: jvm.kt +// TARGET_PLATFORM: JVM +import kotlin.jvm.Volatile as JvmVolatile +import kotlin.concurrent.Volatile +import kotlin.properties.Delegates + +@OptIn(ExperimentalStdlibApi::class) +class ConcurrentVolatileOnJvm { @Volatile val x = 0 // ok @Volatile var y = 1 diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/Volatile.txt b/compiler/testData/diagnostics/testsWithStdLib/annotations/Volatile.txt index efcddff08d5..a95d7e0b3d8 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/Volatile.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/Volatile.txt @@ -1,7 +1,35 @@ +// -- Module: -- package @kotlin.OptIn(markerClass = {kotlin.ExperimentalStdlibApi::class}) public final class ConcurrentVolatile { public constructor ConcurrentVolatile() + @field:kotlin.concurrent.Volatile public final var noBacking: kotlin.String + @field:kotlin.concurrent.Volatile public final val w: kotlin.Int = 2 + @field:kotlin.concurrent.Volatile public final val x: kotlin.Int = 0 + @field:kotlin.concurrent.Volatile public final var y: kotlin.Int + @delegate:kotlin.concurrent.Volatile public final var z: kotlin.String + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +// -- Module: -- +package + +@kotlin.OptIn(markerClass = {kotlin.ExperimentalStdlibApi::class}) public final class ConcurrentVolatile { + public constructor ConcurrentVolatile() + @field:kotlin.concurrent.Volatile /* = kotlin.jvm.Volatile */ public final var noBacking: kotlin.String + @field:kotlin.concurrent.Volatile /* = kotlin.jvm.Volatile */ public final val w: kotlin.Int = 2 + @field:kotlin.concurrent.Volatile /* = kotlin.jvm.Volatile */ public final val x: kotlin.Int = 0 + @field:kotlin.concurrent.Volatile /* = kotlin.jvm.Volatile */ public final var y: kotlin.Int + @delegate:kotlin.concurrent.Volatile /* = kotlin.jvm.Volatile */ public final var z: kotlin.String + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +@kotlin.OptIn(markerClass = {kotlin.ExperimentalStdlibApi::class}) public final class ConcurrentVolatileOnJvm { + public constructor ConcurrentVolatileOnJvm() @field:kotlin.concurrent.Volatile /* = kotlin.jvm.Volatile */ public final var noBacking: kotlin.String @field:kotlin.concurrent.Volatile /* = kotlin.jvm.Volatile */ public final val w: kotlin.Int = 2 @field:kotlin.concurrent.Volatile /* = kotlin.jvm.Volatile */ public final val x: kotlin.Int = 0