diff --git a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirJvmDiagnosticsList.kt b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirJvmDiagnosticsList.kt index 26115ceeec8..a5839a52d51 100644 --- a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirJvmDiagnosticsList.kt +++ b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirJvmDiagnosticsList.kt @@ -10,6 +10,7 @@ import org.jetbrains.kotlin.config.LanguageFeature.* import org.jetbrains.kotlin.fir.PrivateForInline import org.jetbrains.kotlin.fir.checkers.generator.diagnostics.model.DiagnosticList import org.jetbrains.kotlin.fir.checkers.generator.diagnostics.model.PositioningStrategy +import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol import org.jetbrains.kotlin.fir.types.ConeKotlinType import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName @@ -149,6 +150,12 @@ object JVM_DIAGNOSTICS_LIST : DiagnosticList("FirJvmErrors") { ) } + val SUSPENSION_POINT by object : DiagnosticGroup("Suspension Point") { + val SUSPENSION_POINT_INSIDE_CRITICAL_SECTION by error(PositioningStrategy.REFERENCE_BY_QUALIFIED) { + parameter>("function") + } + } + val MISC by object : DiagnosticGroup("Misc") { val INAPPLICABLE_JVM_FIELD by error { parameter("message") diff --git a/compiler/fir/checkers/checkers.jvm/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/jvm/FirJvmErrors.kt b/compiler/fir/checkers/checkers.jvm/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/jvm/FirJvmErrors.kt index 30fa5b9be37..5a0517fa0cb 100644 --- a/compiler/fir/checkers/checkers.jvm/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/jvm/FirJvmErrors.kt +++ b/compiler/fir/checkers/checkers.jvm/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/jvm/FirJvmErrors.kt @@ -11,6 +11,7 @@ import org.jetbrains.kotlin.config.LanguageFeature.ProhibitJvmOverloadsOnConstru import org.jetbrains.kotlin.config.LanguageFeature.RepeatableAnnotationContainerConstraints import org.jetbrains.kotlin.fir.analysis.diagnostics.* import org.jetbrains.kotlin.fir.analysis.diagnostics.SourceElementPositioningStrategies +import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol import org.jetbrains.kotlin.fir.types.ConeKotlinType import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName @@ -105,6 +106,9 @@ object FirJvmErrors { val REPEATABLE_CONTAINER_TARGET_SET_NOT_A_SUBSET by deprecationError2(RepeatableAnnotationContainerConstraints) val REPEATABLE_ANNOTATION_HAS_NESTED_CLASS_NAMED_CONTAINER by deprecationError0(RepeatableAnnotationContainerConstraints) + // Suspension Point + val SUSPENSION_POINT_INSIDE_CRITICAL_SECTION by error1>(SourceElementPositioningStrategies.REFERENCE_BY_QUALIFIED) + // Misc val INAPPLICABLE_JVM_FIELD by error1() val INAPPLICABLE_JVM_FIELD_WARNING by warning1() diff --git a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/JvmExpressionCheckers.kt b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/JvmExpressionCheckers.kt index e62aa638d83..c0d3607f63f 100644 --- a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/JvmExpressionCheckers.kt +++ b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/JvmExpressionCheckers.kt @@ -23,6 +23,7 @@ object JvmExpressionCheckers : ExpressionCheckers() { get() = setOf( FirJavaGenericVarianceViolationTypeChecker, FirSuperCallWithDefaultsChecker, + FirJvmSuspensionPointInsideMutexLockChecker ) override val annotationCheckers: Set diff --git a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/expression/FirJvmSuspensionPointInsideMutexLockChecker.kt b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/expression/FirJvmSuspensionPointInsideMutexLockChecker.kt new file mode 100644 index 00000000000..4b709b1fa7e --- /dev/null +++ b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/expression/FirJvmSuspensionPointInsideMutexLockChecker.kt @@ -0,0 +1,49 @@ +/* + * Copyright 2010-2021 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.fir.analysis.jvm.checkers.expression + +import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.checkers.context.findClosest +import org.jetbrains.kotlin.fir.analysis.checkers.expression.FirFunctionCallChecker +import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter +import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors +import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn +import org.jetbrains.kotlin.fir.declarations.FirAnonymousFunction +import org.jetbrains.kotlin.fir.declarations.utils.isSuspend +import org.jetbrains.kotlin.fir.expressions.* +import org.jetbrains.kotlin.name.CallableId +import org.jetbrains.kotlin.name.FqName +import org.jetbrains.kotlin.name.Name + +object FirJvmSuspensionPointInsideMutexLockChecker : FirFunctionCallChecker() { + private val synchronizedCallableId = CallableId(FqName("kotlin"), Name.identifier("synchronized")) + private val withLockCallableId = CallableId(FqName("kotlin.concurrent"), Name.identifier("withLock")) + + override fun check(expression: FirFunctionCall, context: CheckerContext, reporter: DiagnosticReporter) { + val symbol = expression.calleeReference.toResolvedCallableSymbol() ?: return + if (!symbol.isSuspend) return + val closestAnonymousFunction = context.findClosest() ?: return + + for (call in context.qualifiedAccessOrAnnotationCalls.asReversed()) { + if (call is FirFunctionCall) { + val callableSymbol = call.calleeReference.toResolvedCallableSymbol() ?: continue + if (callableSymbol.callableId == synchronizedCallableId) { + val unwrappedFirstArgument = call.arguments.elementAtOrNull(1)?.unwrapArgument() ?: return + val firstArgumentAnonymousFunction = + (unwrappedFirstArgument as? FirAnonymousFunctionExpression)?.anonymousFunction ?: return + + if (closestAnonymousFunction == firstArgumentAnonymousFunction) { + reporter.reportOn(expression.source, FirJvmErrors.SUSPENSION_POINT_INSIDE_CRITICAL_SECTION, symbol, context) + } + return + } else if (callableSymbol.callableId == withLockCallableId) { + reporter.reportOn(expression.source, FirJvmErrors.SUSPENSION_POINT_INSIDE_CRITICAL_SECTION, symbol, context) + return + } + } + } + } +} \ No newline at end of file diff --git a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/diagnostics/FirJvmDefaultErrorMessages.kt b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/diagnostics/FirJvmDefaultErrorMessages.kt index b749e604a9c..2ffa1e53409 100644 --- a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/diagnostics/FirJvmDefaultErrorMessages.kt +++ b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/diagnostics/FirJvmDefaultErrorMessages.kt @@ -9,6 +9,7 @@ import org.jetbrains.kotlin.diagnostics.rendering.Renderers.NAME import org.jetbrains.kotlin.diagnostics.rendering.CommonRenderers.STRING import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDefaultErrorMessages import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.RENDER_TYPE +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.SYMBOL import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.TO_STRING import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.CONFLICTING_JVM_DECLARATIONS import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.DEFAULT_METHOD_CALL_FROM_JAVA6_TARGET @@ -69,6 +70,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.REPEATED_A import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.REPEATED_ANNOTATION_WITH_CONTAINER import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.STRICTFP_ON_CLASS import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.SUPER_CALL_WITH_DEFAULT_PARAMETERS +import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.SUSPENSION_POINT_INSIDE_CRITICAL_SECTION import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.SYNCHRONIZED_IN_INTERFACE import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.SYNCHRONIZED_ON_ABSTRACT import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.SYNCHRONIZED_ON_INLINE @@ -257,6 +259,11 @@ object FirJvmDefaultErrorMessages { REPEATABLE_ANNOTATION_HAS_NESTED_CLASS_NAMED_CONTAINER, "Repeatable annotation cannot have a nested class named 'Container'. This name is reserved for auto-generated container class" ) + map.put( + SUSPENSION_POINT_INSIDE_CRITICAL_SECTION, + "The ''{0}'' suspension point is inside a critical section", + SYMBOL + ) } } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.fir.kt deleted file mode 100644 index 4a5225ecd55..00000000000 --- a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.fir.kt +++ /dev/null @@ -1,59 +0,0 @@ -// !DIAGNOSTICS: -UNUSED_PARAMETER, -USELESS_IS_CHECK -// SKIP_TXT -import kotlin.concurrent.withLock - -val lock = java.util.concurrent.locks.ReentrantLock() - -fun builder(c: suspend () -> Unit) {} - -suspend fun getLock() = lock - -suspend fun suspensionPoint() {} - -fun test() { - builder { - synchronized(lock) { - suspensionPoint() - } - - synchronized(lock) label@{ - suspensionPoint() - } - - synchronized(lock, { suspensionPoint() }) - - synchronized(getLock()) { - println("") - } - synchronized(suspend { getLock() } ()) { - println("") - } - synchronized(run { getLock() }) { - println("") - } - lock.withLock { - suspensionPoint() - } - } -} - -suspend fun run() { - synchronized(lock) { - suspensionPoint() - } -} - -suspend fun ifWhenAndOtherNonsence() { - synchronized(lock) { - if (lock == Any()) { - when (1) { - is Int -> { - return@synchronized 1 + returnsInt() - } - else -> {} - } - } else {} - } -} - -suspend fun returnsInt(): Int = 0 \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.kt index 350d269d6e0..c47f69d1461 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // !DIAGNOSTICS: -UNUSED_PARAMETER, -USELESS_IS_CHECK // SKIP_TXT import kotlin.concurrent.withLock diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.fir.kt deleted file mode 100644 index 3c620bda83e..00000000000 --- a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.fir.kt +++ /dev/null @@ -1,61 +0,0 @@ -// !DIAGNOSTICS: -UNUSED_PARAMETER, -USELESS_IS_CHECK -// !LANGUAGE: +NewInference -// SKIP_TXT - -import kotlin.concurrent.withLock - -val lock = java.util.concurrent.locks.ReentrantLock() - -fun builder(c: suspend () -> Unit) {} - -suspend fun getLock() = lock - -suspend fun suspensionPoint() {} - -fun test() { - builder { - synchronized(lock) { - suspensionPoint() - } - - synchronized(lock) label@{ - suspensionPoint() - } - - synchronized(lock, { suspensionPoint() }) - - synchronized(getLock()) { - println("") - } - synchronized(suspend { getLock() } ()) { - println("") - } - synchronized(run { getLock() }) { - println("") - } - lock.withLock { - suspensionPoint() - } - } -} - -suspend fun run() { - synchronized(lock) { - suspensionPoint() - } -} - -suspend fun ifWhenAndOtherNonsence() { - synchronized(lock) { - if (lock == Any()) { - when (1) { - is Int -> { - return@synchronized 1 + returnsInt() - } - else -> {} - } - } else {} - } -} - -suspend fun returnsInt(): Int = 0 \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.kt index f3ee4b1754a..5f91bdbd2fb 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // !DIAGNOSTICS: -UNUSED_PARAMETER, -USELESS_IS_CHECK // !LANGUAGE: +NewInference // SKIP_TXT diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt index 5fe85791d42..51c521b57c8 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt @@ -3990,6 +3990,13 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert token, ) } + add(FirJvmErrors.SUSPENSION_POINT_INSIDE_CRITICAL_SECTION) { firDiagnostic -> + SuspensionPointInsideCriticalSectionImpl( + firSymbolBuilder.callableBuilder.buildCallableSymbol(firDiagnostic.a.fir), + firDiagnostic as FirPsiDiagnostic, + token, + ) + } add(FirJvmErrors.INAPPLICABLE_JVM_FIELD) { firDiagnostic -> InapplicableJvmFieldImpl( firDiagnostic.a, diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt index 3c4773ef29e..03026f4a3d9 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt @@ -2777,6 +2777,11 @@ sealed class KtFirDiagnostic : KtDiagnosticWithPsi { override val diagnosticClass get() = RepeatableAnnotationHasNestedClassNamedContainerWarning::class } + abstract class SuspensionPointInsideCriticalSection : KtFirDiagnostic() { + override val diagnosticClass get() = SuspensionPointInsideCriticalSection::class + abstract val function: KtCallableSymbol + } + abstract class InapplicableJvmField : KtFirDiagnostic() { override val diagnosticClass get() = InapplicableJvmField::class abstract val message: String diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt index 9b280f95928..7ff7aa3efb9 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt @@ -4515,6 +4515,14 @@ internal class RepeatableAnnotationHasNestedClassNamedContainerWarningImpl( override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) } +internal class SuspensionPointInsideCriticalSectionImpl( + override val function: KtCallableSymbol, + firDiagnostic: FirPsiDiagnostic, + override val token: ValidityToken, +) : KtFirDiagnostic.SuspensionPointInsideCriticalSection(), KtAbstractFirDiagnostic { + override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) +} + internal class InapplicableJvmFieldImpl( override val message: String, firDiagnostic: FirPsiDiagnostic,