Skip to content

Commit

Permalink
[FIR] Report the uses of compareAnd* methods, not all atomics to pr…
Browse files Browse the repository at this point in the history
…imitives

Some operations on such atomics are still valid, so this change prevents
`@Suppress` that one would consider adding if they want to use those
operations.

^KT-73508
  • Loading branch information
lunakoly authored and Space Team committed Feb 13, 2025
1 parent b58a9a5 commit e011994
Show file tree
Hide file tree
Showing 30 changed files with 212 additions and 313 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import kotlin.concurrent.atomics.AtomicReference

open class KotlinClass {
open fun foo(a: <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>AtomicReference<Int><!>) { }
open fun foo(a: AtomicReference<Int>) { }
}

// FILE: JavaClass.java
Expand All @@ -28,5 +28,5 @@ import kotlin.concurrent.atomics.AtomicReference

fun usage(a: JavaClass) {
a.foo(java.util.concurrent.atomic.AtomicReference(""))
a.foo(<!ARGUMENT_TYPE_MISMATCH, ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>AtomicReference(1)<!>)
a.foo(<!ARGUMENT_TYPE_MISMATCH!>AtomicReference(1)<!>)
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import org.jetbrains.kotlin.fir.analysis.checkers.type.FirResolvedTypeRefChecker
import org.jetbrains.kotlin.fir.analysis.checkers.type.TypeCheckers
import org.jetbrains.kotlin.fir.analysis.jvm.checkers.type.FirArrayOfNullableNothingTypeChecker
import org.jetbrains.kotlin.fir.analysis.jvm.checkers.type.FirFunctionalTypeParameterNameChecker
import org.jetbrains.kotlin.fir.analysis.jvm.checkers.type.FirJvmAtomicReferenceArrayToPrimitiveTypeChecker
import org.jetbrains.kotlin.fir.analysis.jvm.checkers.type.FirJvmAtomicReferenceToPrimitiveTypeChecker
import org.jetbrains.kotlin.fir.analysis.jvm.checkers.type.FirJvmModuleAccessibilityTypeChecker

object JvmTypeCheckers : TypeCheckers() {
Expand All @@ -23,8 +21,6 @@ object JvmTypeCheckers : TypeCheckers() {
override val resolvedTypeRefCheckers: Set<FirResolvedTypeRefChecker> = setOf(
FirDynamicUnsupportedChecker,
FirJvmModuleAccessibilityTypeChecker,
FirJvmAtomicReferenceToPrimitiveTypeChecker,
FirJvmAtomicReferenceArrayToPrimitiveTypeChecker,
FirArrayOfNullableNothingTypeChecker,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@ import org.jetbrains.kotlin.name.JvmStandardClassIds

object FirJvmAtomicReferenceToPrimitiveCallChecker :
AbstractAtomicReferenceToPrimitiveCallChecker(
JvmStandardClassIds.ATOMIC_REFERENCE_CLASS_ID,
JvmStandardClassIds.atomicByPrimitive,
MppCheckerKind.Platform,
JvmStandardClassIds.Callables.atomicReferenceCompareAndSet,
JvmStandardClassIds.Callables.atomicReferenceCompareAndExchange,
)

object FirJvmAtomicReferenceArrayToPrimitiveCallChecker :
AbstractAtomicReferenceToPrimitiveCallChecker(
JvmStandardClassIds.ATOMIC_REFERENCE_ARRAY_CLASS_ID,
JvmStandardClassIds.atomicArrayByPrimitive,
MppCheckerKind.Platform,
JvmStandardClassIds.Callables.atomicReferenceArrayCompareAndSet,
JvmStandardClassIds.Callables.atomicReferenceArrayCompareAndExchange,
)

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,18 @@ package org.jetbrains.kotlin.fir.analysis.native.checkers

import org.jetbrains.kotlin.fir.analysis.checkers.MppCheckerKind
import org.jetbrains.kotlin.fir.analysis.checkers.expression.AbstractAtomicReferenceToPrimitiveCallChecker
import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol
import org.jetbrains.kotlin.fir.symbols.impl.FirFunctionSymbol
import org.jetbrains.kotlin.name.NativeRuntimeNames

object FirNativeAtomicReferenceToPrimitiveCallChecker : AbstractAtomicReferenceToPrimitiveCallChecker(
NativeRuntimeNames.AtomicReference,
NativeRuntimeNames.atomicByPrimitive,
MppCheckerKind.Platform,
NativeRuntimeNames.Callables.atomicReferenceCompareAndSet,
NativeRuntimeNames.Callables.atomicReferenceCompareAndExchange,
)

object FirNativeAtomicArrayToPrimitiveCallChecker : AbstractAtomicReferenceToPrimitiveCallChecker(
NativeRuntimeNames.AtomicArray,
NativeRuntimeNames.atomicArrayByPrimitive,
MppCheckerKind.Platform,
) {
override val FirBasedSymbol<*>.canInstantiateProblematicAtomicReference: Boolean
get() = this is FirFunctionSymbol<*> && callableId == NativeRuntimeNames.Callables.AtomicArray
}
NativeRuntimeNames.Callables.atomicArrayCompareAndSet,
NativeRuntimeNames.Callables.atomicArrayCompareAndExchange,
)

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import org.jetbrains.kotlin.fir.analysis.checkers.type.*
object NativeTypeCheckers : TypeCheckers() {
override val resolvedTypeRefCheckers: Set<FirResolvedTypeRefChecker>
get() = setOf(
FirNativeAtomicReferenceToPrimitiveTypeChecker,
FirNativeAtomicArrayToPrimitiveTypeChecker,
FirDynamicUnsupportedChecker,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ object CommonTypeCheckers : TypeCheckers() {
FirContextualFunctionTypeChecker,
FirKotlinActualAnnotationHasNoEffectInKotlinTypeChecker,
FirProjectionRelationChecker,
FirCommonAtomicReferenceToPrimitiveTypeChecker,
FirCommonAtomicReferenceArrayToPrimitiveTypeChecker,
FirArrayOfNothingTypeChecker,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ import org.jetbrains.kotlin.types.model.TypeCheckerProviderContext
import org.jetbrains.kotlin.util.ImplementationStatus
import org.jetbrains.kotlin.util.OperatorNameConventions
import org.jetbrains.kotlin.util.getChildren
import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull
import kotlin.contracts.ExperimentalContracts
import kotlin.contracts.contract

Expand Down Expand Up @@ -986,7 +985,7 @@ private fun ConeKotlinType.containsMalformedArgument(context: CheckerContext, al
it.type?.fullyExpandedType(context.session)?.isMalformedExpandedType(context, allowNullableNothing) == true
}

fun checkAtomicReferenceAccess(
fun reportAtomicToPrimitiveProblematicAccess(
type: ConeKotlinType,
source: KtSourceElement?,
atomicReferenceClassId: ClassId,
Expand All @@ -997,7 +996,7 @@ fun checkAtomicReferenceAccess(
val expanded = type.fullyExpandedType(context.session)
val argument = expanded.typeArguments.firstOrNull()?.type ?: return

if (expanded.classId == atomicReferenceClassId && (argument.isPrimitive || argument.isValueClass(context.session))) {
if (argument.isPrimitive || argument.isValueClass(context.session)) {
val candidate = appropriateCandidatesForArgument[argument.classId]
reporter.reportOn(source, FirErrors.ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY, atomicReferenceClassId, argument, candidate, context)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,47 +7,55 @@ package org.jetbrains.kotlin.fir.analysis.checkers.expression

import org.jetbrains.kotlin.diagnostics.DiagnosticReporter
import org.jetbrains.kotlin.fir.analysis.checkers.MppCheckerKind
import org.jetbrains.kotlin.fir.analysis.checkers.checkAtomicReferenceAccess
import org.jetbrains.kotlin.fir.analysis.checkers.reportAtomicToPrimitiveProblematicAccess
import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.expressions.FirFunctionCall
import org.jetbrains.kotlin.fir.references.resolved
import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol
import org.jetbrains.kotlin.fir.symbols.impl.FirConstructorSymbol
import org.jetbrains.kotlin.fir.resolve.fullyExpandedType
import org.jetbrains.kotlin.fir.symbols.impl.FirFunctionSymbol
import org.jetbrains.kotlin.fir.types.classId
import org.jetbrains.kotlin.fir.types.resolvedType
import org.jetbrains.kotlin.name.CallableId
import org.jetbrains.kotlin.name.ClassId
import org.jetbrains.kotlin.name.StandardClassIds
import org.jetbrains.kotlin.name.withClassId

abstract class AbstractAtomicReferenceToPrimitiveCallChecker(
val atomicReferenceClassId: ClassId,
val appropriateCandidatesForArgument: Map<ClassId, ClassId>,
mppKind: MppCheckerKind,
firstProblematicCallableId: CallableId,
vararg remainingProblematicCallableIds: CallableId,
) : FirFunctionCallChecker(mppKind) {
val problematicCallableIds: Set<CallableId> = setOf(firstProblematicCallableId, *remainingProblematicCallableIds)

override fun check(expression: FirFunctionCall, context: CheckerContext, reporter: DiagnosticReporter) {
val callable = expression.calleeReference.resolved?.resolvedSymbol ?: return
val callable = expression.calleeReference.resolved?.resolvedSymbol as? FirFunctionSymbol<*> ?: return
val receiverType = expression.dispatchReceiver?.resolvedType?.fullyExpandedType(context.session) ?: return
val atomicReferenceClassId = receiverType.classId ?: return
val fullyExpandedCallableId = callable.callableId.withClassId(atomicReferenceClassId)

if (callable.canInstantiateProblematicAtomicReference) {
checkAtomicReferenceAccess(
expression.resolvedType, expression.source,
if (fullyExpandedCallableId in problematicCallableIds) {
reportAtomicToPrimitiveProblematicAccess(
receiverType, expression.source,
atomicReferenceClassId, appropriateCandidatesForArgument,
context, reporter
)
}
}

open val FirBasedSymbol<*>.canInstantiateProblematicAtomicReference: Boolean
get() = this is FirConstructorSymbol
}

object FirCommonAtomicReferenceToPrimitiveCallChecker :
AbstractAtomicReferenceToPrimitiveCallChecker(
StandardClassIds.AtomicReference,
StandardClassIds.atomicByPrimitive,
MppCheckerKind.Platform,
StandardClassIds.Callables.atomicReferenceCompareAndSet,
StandardClassIds.Callables.atomicReferenceCompareAndExchange,
)

object FirCommonAtomicArrayToPrimitiveCallChecker :
AbstractAtomicReferenceToPrimitiveCallChecker(
StandardClassIds.AtomicArray,
StandardClassIds.atomicArrayByPrimitive,
MppCheckerKind.Platform,
StandardClassIds.Callables.atomicArrayCompareAndSetAt,
StandardClassIds.Callables.atomicArrayCompareAndExchangeAt,
)

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -1494,7 +1494,7 @@ object FirErrorsDefaultMessages : BaseDiagnosticRendererFactory() {
)
map.put(
ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY,
"''{0}'' uses identity equality, but ''{1}'' does not have a consistent identity.{2}",
"This call may have inconsistent results because ''{0}'' uses identity equality and ''{1}'' does not have a consistent identity.{2}",
CLASS_ID_RELATIVE_NAME_ONLY,
RENDER_TYPE,
suggestIfNotNull(" Consider using ''{0}'' instead.", CLASS_ID_RELATIVE_NAME_ONLY),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@ import kotlin.concurrent.AtomicReference
import kotlin.concurrent.AtomicArray

fun testKotlin() {
val k = <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>AtomicArray(1) { 127 }<!>
k.compareAndSet(0, 127, 128) // true
k.compareAndSet(0, 128, 7777) // false
val k = AtomicArray(1) { 127 }
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>k.compareAndSet(0, 127, 128)<!> // true
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>k.compareAndSet(0, 128, 7777)<!> // false

val kk: <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>AtomicArray<Int><!>
val kk: AtomicArray<Int>
kk = k

val l = <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>AtomicReference(127)<!>
l.compareAndSet(127, 128) // true
l.compareAndSet(128, 7777) // false
val l = AtomicReference(127)
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>l.compareAndSet(127, 128)<!> // true
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>l.compareAndSet(128, 7777)<!> // false

val ll: <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>AtomicReference<Int><!>
val ll: AtomicReference<Int>
ll = l
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ value class Box(val name: String)
fun main() {
val test = Box("Test")
val rest = Box("Rest")
val box = <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>AtomicReference(test)<!>
val box = AtomicReference(test)

box.compareAndSet(test, rest)
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>box.compareAndSet(test, rest)<!>
println(box.get())
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,22 @@
import java.util.concurrent.atomic.AtomicReferenceArray

fun testJava() {
val j = <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>AtomicReferenceArray<Int>(127)<!>
j.compareAndSet(0, 127, 128) // true
j.compareAndSet(0, 128, 7777) // false
val j = AtomicReferenceArray<Int>(127)
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>j.compareAndSet(0, 127, 128)<!> // true
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>j.compareAndSet(0, 128, 7777)<!> // false

val jj: <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>AtomicReferenceArray<Int><!>
val jj: AtomicReferenceArray<Int>
jj = j
}

typealias JavaAtomicReferenceArray<T> = AtomicReferenceArray<T>

fun testTypealiasedJava() {
val j = <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>JavaAtomicReferenceArray<Int>(127)<!>
j.compareAndSet(0, 127, 128) // true
j.compareAndSet(0, 128, 7777) // false
val j = JavaAtomicReferenceArray<Int>(127)
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>j.compareAndSet(0, 127, 128)<!> // true
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>j.compareAndSet(0, 128, 7777)<!> // false

val jj: <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>JavaAtomicReferenceArray<Int><!>
val jj: JavaAtomicReferenceArray<Int>
jj = j
}

Expand All @@ -34,21 +34,21 @@ fun testTypealiasedJava() {
import kotlin.concurrent.atomics.AtomicArray

fun testKotlin() {
val k = <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>AtomicArray(arrayOf(127))<!>
k.compareAndSetAt(0, 127, 128) // true
k.compareAndSetAt(0, 128, 7777) // false
val k = AtomicArray(arrayOf(127))
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>k.compareAndSetAt(0, 127, 128)<!> // true
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>k.compareAndSetAt(0, 128, 7777)<!> // false

val kk: <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>AtomicArray<Int><!>
val kk: AtomicArray<Int>
kk = k
}

typealias KotlinAtomicArray<T> = AtomicArray<T>

fun testTypealiasedKotlin() {
val k = <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>KotlinAtomicArray(arrayOf(127))<!>
k.compareAndSetAt(0, 127, 128) // true
k.compareAndSetAt(0, 128, 7777) // false
val k = KotlinAtomicArray(arrayOf(127))
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>k.compareAndSetAt(0, 127, 128)<!> // true
<!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>k.compareAndSetAt(0, 128, 7777)<!> // false

val kk: <!ATOMIC_REF_WITHOUT_CONSISTENT_IDENTITY!>KotlinAtomicArray<Int><!>
val kk: KotlinAtomicArray<Int>
kk = k
}
Loading

0 comments on commit e011994

Please # to comment.