Report error on non-tail suspend-calls

It only concerns calls inside another suspend function

 #KT-14924 In Progress

compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt

@@ -113,7 +113,7 @@ class ControlFlowInformationProvider private constructor(
 
         checkDefiniteReturn(expectedReturnType ?: NO_EXPECTED_TYPE, unreachableCode)
 
-        markTailCalls()
+        markAndCheckTailCalls()
     }
 
     private fun collectReturnExpressions(returnedExpressions: MutableCollection<KtElement>) {
@@ -798,19 +798,45 @@ class ControlFlowInformationProvider private constructor(
     ////////////////////////////////////////////////////////////////////////////////
     // Tail calls
 
-    private fun markTailCalls() {
+    private fun markAndCheckTailCalls() {
         val subroutineDescriptor = trace.get(BindingContext.DECLARATION_TO_DESCRIPTOR, subroutine) as? FunctionDescriptor ?: return
+
+        markAndCheckRecursiveTailCalls(subroutineDescriptor)
+        checkSuspendCalls(subroutineDescriptor)
+    }
+
+    private fun checkSuspendCalls(currentFunction: FunctionDescriptor) {
+
+        if (!currentFunction.isSuspend) return
+
+        traverseCalls { instruction, resolvedCall ->
+            val calleeDescriptor = resolvedCall.resultingDescriptor as? FunctionDescriptor ?: return@traverseCalls
+            if (!calleeDescriptor.isSuspend) return@traverseCalls
+
+            // Suspend functions are allowed to be called only within coroutines (may be non-tail calls of course)
+            // or another suspend function (here they must be called only in tail position)
+            val enclosingSuspendFunction =
+                    trace.get(BindingContext.ENCLOSING_SUSPEND_FUNCTION_FOR_SUSPEND_FUNCTION_CALL, resolvedCall.call)
+                    ?.let(DescriptorToSourceUtils::descriptorToDeclaration) as? KtElement
+                    ?: return@traverseCalls
+
+            val element = instruction.element
+            val isUsedAsExpression = instruction.owner.getUsages(instruction.outputValue).isNotEmpty()
+
+            if (!isUsedAsExpression || !instruction.isTailCall(enclosingSuspendFunction) || isInsideTry(element)) {
+                trace.report(Errors.SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE.on(element))
+            }
+        }
+    }
+
+    private fun markAndCheckRecursiveTailCalls(subroutineDescriptor: FunctionDescriptor) {
         if (!subroutineDescriptor.isTailrec) return
 
         // finally blocks are copied which leads to multiple diagnostics reported on one instruction
         class KindAndCall(var kind: TailRecursionKind, internal val call: ResolvedCall<*>)
 
         val calls = HashMap<KtElement, KindAndCall>()
-        pseudocode.traverse(TraversalOrder.FORWARD) { instruction ->
+        traverseCalls traverse@{ instruction, resolvedCall ->
-
-            if (instruction !is CallInstruction) return@traverse
-            val resolvedCall = instruction.element.getResolvedCall(trace.bindingContext) ?: return@traverse
-
             // is this a recursive call?
             val functionDescriptor = resolvedCall.resultingDescriptor
             if (functionDescriptor.original != subroutineDescriptor) return@traverse
@@ -821,25 +847,14 @@ class ControlFlowInformationProvider private constructor(
 
             val element = instruction.element
             //noinspection unchecked
-            val parent = PsiTreeUtil.getParentOfType(
-                    element,
-                    KtTryExpression::class.java, KtFunction::class.java, KtAnonymousInitializer::class.java
-            )
 
-            if (parent is KtTryExpression) {
+            if (isInsideTry(element)) {
                 // We do not support tail calls Collections.singletonMap() try-catch-finally, for simplicity of the mental model
                 // very few cases there would be real tail-calls, and it's often not so easy for the user to see why
                 calls.put(element, KindAndCall(IN_TRY, resolvedCall))
                 return@traverse
             }
 
-            val isTail = traverseFollowingInstructions(
-                    instruction,
-                    HashSet<Instruction>(),
-                    TraversalOrder.FORWARD,
-                    TailRecursionDetector(subroutine, instruction)
-            )
-
             // A tail call is not allowed to change dispatch receiver
             //   class C {
             //       fun foo(other: C) {
@@ -848,7 +863,7 @@ class ControlFlowInformationProvider private constructor(
             //   }
             val sameDispatchReceiver = resolvedCall.hasThisOrNoDispatchReceiver(trace.bindingContext)
 
-            val kind = if (isTail && sameDispatchReceiver) TAIL_CALL else NON_TAIL
+            val kind = if (sameDispatchReceiver && instruction.isTailCall()) TAIL_CALL else NON_TAIL
 
             val kindAndCall = calls[element]
             calls.put(element, KindAndCall(combineKinds(kind, kindAndCall?.kind), resolvedCall))
@@ -871,6 +886,29 @@ class ControlFlowInformationProvider private constructor(
         }
     }
 
+    private fun isInsideTry(element: KtElement) =
+            PsiTreeUtil.getParentOfType(
+                    element,
+                    KtTryExpression::class.java, KtFunction::class.java, KtAnonymousInitializer::class.java
+            ) is KtTryExpression
+
+    private fun CallInstruction.isTailCall(subroutine: KtElement = this@ControlFlowInformationProvider.subroutine) =
+        traverseFollowingInstructions(
+            this,
+            HashSet<Instruction>(),
+            TraversalOrder.FORWARD,
+            TailCallDetector(subroutine, this)
+        )
+
+    private inline fun traverseCalls(crossinline onCall: (instruction: CallInstruction, resolvedCall: ResolvedCall<*>) -> Unit) {
+        pseudocode.traverse(TraversalOrder.FORWARD) { instruction ->
+            if (instruction !is CallInstruction) return@traverse
+            val resolvedCall = instruction.element.getResolvedCall(trace.bindingContext) ?: return@traverse
+
+            onCall(instruction, resolvedCall)
+        }
+    }
+
     ////////////////////////////////////////////////////////////////////////////////
     // Utility classes and methods
 

compiler/frontend/src/org/jetbrains/kotlin/cfg/TailCallDetector.java

@@ -30,11 +30,11 @@ import org.jetbrains.kotlin.cfg.pseudocode.instructions.special.SubroutineSinkIn
 import org.jetbrains.kotlin.cfg.pseudocodeTraverser.TraverseInstructionResult;
 import org.jetbrains.kotlin.psi.KtElement;
 
-public class TailRecursionDetector extends InstructionVisitorWithResult<Boolean> implements Function1<Instruction, TraverseInstructionResult> {
+public class TailCallDetector extends InstructionVisitorWithResult<Boolean> implements Function1<Instruction, TraverseInstructionResult> {
     private final KtElement subroutine;
     private final Instruction start;
 
-    public TailRecursionDetector(@NotNull KtElement subroutine, @NotNull Instruction start) {
+    public TailCallDetector(@NotNull KtElement subroutine, @NotNull Instruction start) {
         this.subroutine = subroutine;
         this.start = start;
     }

compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java

@@ -863,6 +863,7 @@ public interface Errors {
 
     DiagnosticFactory0<PsiElement> NON_LOCAL_SUSPENSION_POINT = DiagnosticFactory0.create(ERROR);
     DiagnosticFactory0<PsiElement> ILLEGAL_SUSPEND_FUNCTION_CALL = DiagnosticFactory0.create(ERROR);
+    DiagnosticFactory0<PsiElement> SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE = DiagnosticFactory0.create(ERROR);
 
 
     // Error sets

compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java

@@ -821,6 +821,7 @@ public class DefaultErrorMessages {
         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.put(ILLEGAL_SUSPEND_FUNCTION_CALL, "Suspend functions are only allowed to be called from a coroutine or another suspend function");
+        MAP.put(SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE, "Only tail calls are allowed to another suspension function, and these calls must be used as return values");
 
         MAP.setImmutable();
 

compiler/testData/codegen/box/coroutines/controlFlow/throwFromCatch.kt

@@ -11,7 +11,8 @@ class Controller {
         Suspend
     }
 
-    suspend fun suspendLogAndThrow(exception: Throwable): Nothing = suspendWithCurrentContinuation { c ->
+    // Tail calls are not allowed to be Nothing typed. See KT-15051
+    suspend fun suspendLogAndThrow(exception: Throwable): Any? = suspendWithCurrentContinuation { c ->
         result += "throw(${exception.message});"
         c.resumeWithException(exception)
         Suspend

compiler/testData/diagnostics/tests/coroutines/tailCalls/forbidden.kt

@@ -0,0 +1,17 @@
+// !DIAGNOSTICS: -UNUSED_PARAMETER
+
+fun nonSuspend() {}
+
+suspend fun foo() {
+    <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>suspendWithCurrentContinuation { x: Continuation<Int> -> }<!>
+
+    nonSuspend()
+}
+
+suspend fun unitSuspend() {
+    <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>suspendWithCurrentContinuation { x: Continuation<Int> -> }<!>
+}
+
+suspend fun baz(): Int = run {
+    <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>suspendWithCurrentContinuation { x: Continuation<Int> -> }<!>
+}

compiler/testData/diagnostics/tests/coroutines/tailCalls/forbidden.txt

@@ -0,0 +1,6 @@
+package
+
+public suspend fun baz(): kotlin.Int
+public suspend fun foo(): kotlin.Unit
+public fun nonSuspend(): kotlin.Unit
+public suspend fun unitSuspend(): kotlin.Unit

compiler/testData/diagnostics/tests/coroutines/tailCalls/localFunctions.kt

@@ -0,0 +1,17 @@
+suspend fun baz() = 1
+suspend fun unit() {}
+
+suspend fun foo() {
+    suspend fun bar() {
+        <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE, SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>baz()<!>
+        return unit()
+    }
+
+    suspend fun foobar1(): Int {
+        return baz()
+    }
+
+    suspend fun foobar2() {
+        return unit()
+    }
+}

compiler/testData/diagnostics/tests/coroutines/tailCalls/localFunctions.txt

@@ -0,0 +1,5 @@
+package
+
+public suspend fun baz(): kotlin.Int
+public suspend fun foo(): kotlin.Unit
+public suspend fun unit(): kotlin.Unit

compiler/testData/diagnostics/tests/coroutines/tailCalls/nothingTypedSuspendFunction.kt

@@ -0,0 +1,5 @@
+// Tail calls are not allowed to be Nothing typed. See KT-15051
+suspend fun suspendLogAndThrow(exception: Throwable): Nothing = <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>suspendWithCurrentContinuation { c ->
+    c.resumeWithException(exception)
+    Suspend
+}<!>

compiler/testData/diagnostics/tests/coroutines/tailCalls/nothingTypedSuspendFunction.txt

@@ -0,0 +1,3 @@
+package
+
+public suspend fun suspendLogAndThrow(/*0*/ exception: kotlin.Throwable): kotlin.Nothing

compiler/testData/diagnostics/tests/coroutines/tailCalls/recursive.kt

@@ -0,0 +1,18 @@
+suspend fun unit1() {
+    <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>unit1()<!>
+}
+
+suspend fun unit2() {
+    return unit2()
+}
+
+suspend fun int1(): Int {
+    return int1()
+}
+
+suspend fun int2(): Int = int2()
+
+suspend fun int3(): Int {
+    <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>int3()<!>
+    return int3()
+}

compiler/testData/diagnostics/tests/coroutines/tailCalls/recursive.txt

@@ -0,0 +1,7 @@
+package
+
+public suspend fun int1(): kotlin.Int
+public suspend fun int2(): kotlin.Int
+public suspend fun int3(): kotlin.Int
+public suspend fun unit1(): kotlin.Unit
+public suspend fun unit2(): kotlin.Unit

compiler/testData/diagnostics/tests/coroutines/tailCalls/tryCatch.kt

@@ -0,0 +1,39 @@
+// !DIAGNOSTICS: -UNUSED_PARAMETER
+
+fun nonSuspend() {}
+
+suspend fun baz(): Int = 1
+
+suspend fun tryCatch(): Int {
+    return try {
+        <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>suspendWithCurrentContinuation { x: Continuation<Int> -> }<!>
+    } catch (e: Exception) {
+        <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>baz()<!> // another suspend function
+    }
+}
+
+suspend fun tryFinally(): Int {
+    return try {
+        <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>suspendWithCurrentContinuation { x: Continuation<Int> -> }<!>
+    } finally {
+        nonSuspend()
+    }
+}
+
+suspend fun returnInFinally(): Int {
+    try {
+    } finally {
+        // Probably this is too restrictive, but it does not matter much
+        return <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE, SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>baz()<!>
+    }
+}
+
+suspend fun tryCatchFinally(): Int {
+    return try {
+        <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>suspendWithCurrentContinuation { x: Continuation<Int> -> }<!>
+    } catch (e: Exception) {
+        <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>baz()<!> // another suspend function
+    } finally {
+        <!SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE, SUSPENSION_CALL_MUST_BE_USED_AS_RETURN_VALUE!>baz()<!>
+    }
+}

compiler/testData/diagnostics/tests/coroutines/tailCalls/tryCatch.txt

@@ -0,0 +1,8 @@
+package
+
+public suspend fun baz(): kotlin.Int
+public fun nonSuspend(): kotlin.Unit
+public suspend fun returnInFinally(): kotlin.Int
+public suspend fun tryCatch(): kotlin.Int
+public suspend fun tryCatchFinally(): kotlin.Int
+public suspend fun tryFinally(): kotlin.Int

compiler/testData/diagnostics/tests/coroutines/tailCalls/valid.kt

@@ -0,0 +1,29 @@
+// !DIAGNOSTICS: -UNUSED_PARAMETER
+
+suspend fun baz() = 1
+
+suspend fun foo() {}
+
+suspend fun bar0() = baz()
+suspend fun bar01(): Int {
+    return baz()
+}
+
+suspend fun bar1() {
+    return if (1.hashCode() > 0) {
+        foo()
+    }
+    else suspendWithCurrentContinuation { x: Continuation<Unit> -> }
+}
+
+suspend fun bar2() =
+        if (1.hashCode() > 0) {
+            foo()
+        }
+        else suspendWithCurrentContinuation { x: Continuation<Unit> -> }
+
+suspend fun bar3() =
+        when {
+            true -> { foo() }
+            else -> suspendWithCurrentContinuation { x: Continuation<Unit> -> }
+        }

compiler/testData/diagnostics/tests/coroutines/tailCalls/valid.txt

@@ -0,0 +1,9 @@
+package
+
+public suspend fun bar0(): kotlin.Int
+public suspend fun bar01(): kotlin.Int
+public suspend fun bar1(): kotlin.Unit
+public suspend fun bar2(): kotlin.Unit
+public suspend fun bar3(): kotlin.Unit
+public suspend fun baz(): kotlin.Int
+public suspend fun foo(): kotlin.Unit

compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java

@@ -4335,6 +4335,51 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest {
                 String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/wrongHandleResult.kt");
                 doTest(fileName);
             }
+
+            @TestMetadata("compiler/testData/diagnostics/tests/coroutines/tailCalls")
+            @TestDataPath("$PROJECT_ROOT")
+            @RunWith(JUnit3RunnerWithInners.class)
+            public static class TailCalls extends AbstractDiagnosticsTest {
+                public void testAllFilesPresentInTailCalls() throws Exception {
+                    KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/tests/coroutines/tailCalls"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
+                }
+
+                @TestMetadata("forbidden.kt")
+                public void testForbidden() throws Exception {
+                    String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/tailCalls/forbidden.kt");
+                    doTest(fileName);
+                }
+
+                @TestMetadata("localFunctions.kt")
+                public void testLocalFunctions() throws Exception {
+                    String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/tailCalls/localFunctions.kt");
+                    doTest(fileName);
+                }
+
+                @TestMetadata("nothingTypedSuspendFunction.kt")
+                public void testNothingTypedSuspendFunction() throws Exception {
+                    String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/tailCalls/nothingTypedSuspendFunction.kt");
+                    doTest(fileName);
+                }
+
+                @TestMetadata("recursive.kt")
+                public void testRecursive() throws Exception {
+                    String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/tailCalls/recursive.kt");
+                    doTest(fileName);
+                }
+
+                @TestMetadata("tryCatch.kt")
+                public void testTryCatch() throws Exception {
+                    String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/tailCalls/tryCatch.kt");
+                    doTest(fileName);
+                }
+
+                @TestMetadata("valid.kt")
+                public void testValid() throws Exception {
+                    String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/tailCalls/valid.kt");
+                    doTest(fileName);
+                }
+            }
         }
 
         @TestMetadata("compiler/testData/diagnostics/tests/cyclicHierarchy")