From d760a2ec3b8bf8e2b6a6533f586c3f7986b23aa0 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 26 May 2016 16:45:24 +0300 Subject: [PATCH] Add CallChecker prohibiting non-local suspension calls Effectively suspension point is local <=> return to relevant lambda is allowed in place --- .../jetbrains/kotlin/diagnostics/Errors.java | 3 + .../rendering/DefaultErrorMessages.java | 1 + .../kotlin/resolve/TargetPlatform.kt | 2 +- .../checkers/CoroutineSuspendCallChecker.kt | 39 +++++++++++ .../irrelevantSuspendDeclarations.kt | 26 +++++++ .../irrelevantSuspendDeclarations.txt | 20 ++++++ .../tests/coroutines/nonLocalSuspension.kt | 68 +++++++++++++++++++ .../tests/coroutines/nonLocalSuspension.txt | 16 +++++ .../checkers/DiagnosticsTestGenerated.java | 12 ++++ 9 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/CoroutineSuspendCallChecker.kt create mode 100644 compiler/testData/diagnostics/tests/coroutines/irrelevantSuspendDeclarations.kt create mode 100644 compiler/testData/diagnostics/tests/coroutines/irrelevantSuspendDeclarations.txt create mode 100644 compiler/testData/diagnostics/tests/coroutines/nonLocalSuspension.kt create mode 100644 compiler/testData/diagnostics/tests/coroutines/nonLocalSuspension.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index 7d76fa3eefa..82e2a33c92b 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -801,6 +801,9 @@ public interface Errors { DiagnosticFactory0 NON_LOCAL_RETURN_IN_DISABLED_INLINE = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 NON_LOCAL_SUSPENSION_POINT = DiagnosticFactory0.create(ERROR); + + // Error sets ImmutableSet> UNRESOLVED_REFERENCE_DIAGNOSTICS = ImmutableSet.of( UNRESOLVED_REFERENCE, NAMED_PARAMETER_NOT_FOUND, UNRESOLVED_REFERENCE_WRONG_RECEIVER); 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 6a4bdec2da9..1b0faf91557 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -753,6 +753,7 @@ public class DefaultErrorMessages { MAP.put(NON_LOCAL_RETURN_NOT_ALLOWED, "Can''t inline ''{0}'' here: it may contain non-local returns. Add ''crossinline'' modifier to parameter declaration ''{0}''", ELEMENT_TEXT); MAP.put(INLINE_CALL_CYCLE, "The ''{0}'' invocation is a part of inline cycle", NAME); MAP.put(NON_LOCAL_RETURN_IN_DISABLED_INLINE, "Non-local returns are not allowed with inlining disabled"); + MAP.put(NON_LOCAL_SUSPENSION_POINT, "Suspension functions can be called only within coroutine body"); MAP.setImmutable(); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/TargetPlatform.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/TargetPlatform.kt index aee0714ae10..3db024e8645 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/TargetPlatform.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/TargetPlatform.kt @@ -67,7 +67,7 @@ private val DEFAULT_DECLARATION_CHECKERS = listOf( private val DEFAULT_CALL_CHECKERS = listOf(CapturingInClosureChecker(), InlineCheckerWrapper(), ReifiedTypeParameterSubstitutionChecker(), SafeCallChecker(), InvokeConventionChecker(), CallReturnsArrayOfNothingChecker(), - ConstructorHeaderCallChecker, ProtectedConstructorCallChecker) + ConstructorHeaderCallChecker, ProtectedConstructorCallChecker, CoroutineSuspendCallChecker) private val DEFAULT_TYPE_CHECKERS = emptyList() private val DEFAULT_VALIDATORS = listOf(DeprecatedSymbolValidator(), OperatorValidator(), InfixValidator()) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/CoroutineSuspendCallChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/CoroutineSuspendCallChecker.kt new file mode 100644 index 00000000000..330caa9b368 --- /dev/null +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/CoroutineSuspendCallChecker.kt @@ -0,0 +1,39 @@ +/* + * Copyright 2010-2016 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.resolve.calls.checkers + +import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.diagnostics.Errors +import org.jetbrains.kotlin.psi.KtExpression +import org.jetbrains.kotlin.resolve.calls.context.BasicCallResolutionContext +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall +import org.jetbrains.kotlin.resolve.coroutine.CoroutineReceiverValue +import org.jetbrains.kotlin.resolve.inline.InlineUtil + +object CoroutineSuspendCallChecker : CallChecker { + override fun check(resolvedCall: ResolvedCall<*>, context: BasicCallResolutionContext) { + val descriptor = resolvedCall.candidateDescriptor as? FunctionDescriptor ?: return + if (!descriptor.isSuspend || descriptor.initialSignatureDescriptor == null) return + + val dispatchReceiverOwner = (resolvedCall.dispatchReceiver as? CoroutineReceiverValue)?.declarationDescriptor ?: return + val callElement = resolvedCall.call.callElement as KtExpression + + if (!InlineUtil.checkNonLocalReturnUsage(dispatchReceiverOwner, callElement, context.trace)) { + context.trace.report(Errors.NON_LOCAL_SUSPENSION_POINT.on(resolvedCall.call.calleeExpression ?: callElement)) + } + } +} diff --git a/compiler/testData/diagnostics/tests/coroutines/irrelevantSuspendDeclarations.kt b/compiler/testData/diagnostics/tests/coroutines/irrelevantSuspendDeclarations.kt new file mode 100644 index 00000000000..4e4a8c83d0b --- /dev/null +++ b/compiler/testData/diagnostics/tests/coroutines/irrelevantSuspendDeclarations.kt @@ -0,0 +1,26 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER +// !CHECK_TYPE +class Controller { + suspend fun suspendHere(a: String, x: Continuation) { + } +} + +class A { + suspend fun suspendHere(a: Int, x: Continuation) { + } +} + +fun builder(coroutine c: Controller.() -> Continuation) {} + +fun test() { + builder { + suspendHere("") + + with(A()) { + suspendHere("") + // This test checks that suspending functions + // that are not from coroutine controller can't be called by suspending convention + suspendHere(1) + } + } +} diff --git a/compiler/testData/diagnostics/tests/coroutines/irrelevantSuspendDeclarations.txt b/compiler/testData/diagnostics/tests/coroutines/irrelevantSuspendDeclarations.txt new file mode 100644 index 00000000000..1aee26178e3 --- /dev/null +++ b/compiler/testData/diagnostics/tests/coroutines/irrelevantSuspendDeclarations.txt @@ -0,0 +1,20 @@ +package + +public fun builder(/*0*/ coroutine c: Controller.() -> kotlin.coroutines.Continuation): kotlin.Unit +public fun test(): kotlin.Unit + +public final class A { + public constructor A() + 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 final suspend fun suspendHere(/*0*/ a: kotlin.Int, /*1*/ x: kotlin.coroutines.Continuation): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class Controller { + public constructor Controller() + 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 final suspend fun suspendHere(/*0*/ a: kotlin.String, /*1*/ x: kotlin.coroutines.Continuation): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/coroutines/nonLocalSuspension.kt b/compiler/testData/diagnostics/tests/coroutines/nonLocalSuspension.kt new file mode 100644 index 00000000000..2a9bf2e2f64 --- /dev/null +++ b/compiler/testData/diagnostics/tests/coroutines/nonLocalSuspension.kt @@ -0,0 +1,68 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +class Controller { + suspend fun suspendHere(x: Continuation) { + } + + suspend fun another(a: T, x: Continuation) { + } +} + +fun builder(coroutine c: Controller.() -> Continuation) { } + +inline fun run(x: () -> Unit) {} + +inline fun runCross(crossinline x: () -> Unit) {} + +fun noinline(x: () -> Unit) {} + +fun foo() { + var result = 1 + builder { + suspendHere() + another("") + another(1) + + result += suspendHere() + + run { + result += suspendHere() + run { + suspendHere() + } + } + + runCross { + result += suspendHere() + runCross { + suspendHere() + } + } + + noinline { + result += suspendHere() + noinline { + suspendHere() + } + } + + class A { + fun bar() { + suspendHere() + } + } + + object : Any() { + fun baz() { + suspendHere() + } + } + + builder { + suspendHere() + + another(1) + another("") + } + } +} diff --git a/compiler/testData/diagnostics/tests/coroutines/nonLocalSuspension.txt b/compiler/testData/diagnostics/tests/coroutines/nonLocalSuspension.txt new file mode 100644 index 00000000000..b75e8cc9a68 --- /dev/null +++ b/compiler/testData/diagnostics/tests/coroutines/nonLocalSuspension.txt @@ -0,0 +1,16 @@ +package + +public fun builder(/*0*/ coroutine c: Controller.() -> kotlin.coroutines.Continuation): kotlin.Unit +public fun foo(): kotlin.Unit +public fun noinline(/*0*/ x: () -> kotlin.Unit): kotlin.Unit +public inline fun run(/*0*/ x: () -> kotlin.Unit): kotlin.Unit +public inline fun runCross(/*0*/ crossinline x: () -> kotlin.Unit): kotlin.Unit + +public final class Controller { + public constructor Controller() + public final suspend fun another(/*0*/ a: T, /*1*/ x: kotlin.coroutines.Continuation): kotlin.Unit + 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 final suspend fun suspendHere(/*0*/ x: kotlin.coroutines.Continuation): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 32e95d1f884..5b378e4c60f 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -3867,12 +3867,24 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/tests/coroutines"), Pattern.compile("^(.+)\\.kt$"), true); } + @TestMetadata("irrelevantSuspendDeclarations.kt") + public void testIrrelevantSuspendDeclarations() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/irrelevantSuspendDeclarations.kt"); + doTest(fileName); + } + @TestMetadata("lambdaExpectedType.kt") public void testLambdaExpectedType() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/lambdaExpectedType.kt"); doTest(fileName); } + @TestMetadata("nonLocalSuspension.kt") + public void testNonLocalSuspension() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/nonLocalSuspension.kt"); + doTest(fileName); + } + @TestMetadata("suspendFunctions.kt") public void testSuspendFunctions() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/suspendFunctions.kt");