Surface error for plain return statement in coroutine earlier (#100985)

When a plain return statement was used in a coroutine, the error "return
statement not allowed in coroutine" was surfaced too late (e.g. after
other errors in the return statement). Surfacing it earlier now, to make
the issue more obvious.

Author: ivanaivanovska

clang/lib/Sema/SemaCoroutine.cpp

@@ -684,6 +684,19 @@ bool Sema::checkFinalSuspendNoThrow(const Stmt *FinalSuspend) {
   return ThrowingDecls.empty();
 }
 
+// [stmt.return.coroutine]p1:
+//   A coroutine shall not enclose a return statement ([stmt.return]).
+static void checkReturnStmtInCoroutine(Sema &S, FunctionScopeInfo *FSI) {
+  assert(FSI && "FunctionScopeInfo is null");
+  assert(FSI->FirstCoroutineStmtLoc.isValid() &&
+         "first coroutine location not set");
+  if (FSI->FirstReturnLoc.isInvalid())
+    return;
+  S.Diag(FSI->FirstReturnLoc, diag::err_return_in_coroutine);
+  S.Diag(FSI->FirstCoroutineStmtLoc, diag::note_declared_coroutine_here)
+      << FSI->getFirstCoroutineStmtKeyword();
+}
+
 bool Sema::ActOnCoroutineBodyStart(Scope *SC, SourceLocation KWLoc,
                                    StringRef Keyword) {
   // Ignore previous expr evaluation contexts.
@@ -694,6 +707,10 @@ bool Sema::ActOnCoroutineBodyStart(Scope *SC, SourceLocation KWLoc,
   auto *ScopeInfo = getCurFunction();
   assert(ScopeInfo->CoroutinePromise);
 
+  // Avoid duplicate errors, report only on first keyword.
+  if (ScopeInfo->FirstCoroutineStmtLoc == KWLoc)
+    checkReturnStmtInCoroutine(*this, ScopeInfo);
+
   // If we have existing coroutine statements then we have already built
   // the initial and final suspend points.
   if (!ScopeInfo->NeedsCoroutineSuspends)
@@ -801,6 +818,7 @@ ExprResult Sema::ActOnCoawaitExpr(Scope *S, SourceLocation Loc, Expr *E) {
     if (R.isInvalid()) return ExprError();
     E = R.get();
   }
+
   ExprResult Lookup = BuildOperatorCoawaitLookupExpr(S, Loc);
   if (Lookup.isInvalid())
     return ExprError();
@@ -1118,16 +1136,6 @@ void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) {
   if (Fn->FirstVLALoc.isValid())
     Diag(Fn->FirstVLALoc, diag::err_vla_in_coroutine_unsupported);
 
-  // [stmt.return.coroutine]p1:
-  //   A coroutine shall not enclose a return statement ([stmt.return]).
-  if (Fn->FirstReturnLoc.isValid()) {
-    assert(Fn->FirstCoroutineStmtLoc.isValid() &&
-                   "first coroutine location not set");
-    Diag(Fn->FirstReturnLoc, diag::err_return_in_coroutine);
-    Diag(Fn->FirstCoroutineStmtLoc, diag::note_declared_coroutine_here)
-            << Fn->getFirstCoroutineStmtKeyword();
-  }
-
   // Coroutines will get splitted into pieces. The GNU address of label
   // extension wouldn't be meaningful in coroutines.
   for (AddrLabelExpr *ALE : Fn->AddrLabels)

clang/lib/Sema/SemaStmt.cpp

@@ -3747,6 +3747,16 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
         Diag(ReturnLoc, diag::err_acc_branch_in_out_compute_construct)
         << /*return*/ 1 << /*out of */ 0);
 
+  // using plain return in a coroutine is not allowed.
+  FunctionScopeInfo *FSI = getCurFunction();
+  if (FSI->FirstReturnLoc.isInvalid() && FSI->isCoroutine()) {
+    assert(FSI->FirstCoroutineStmtLoc.isValid() &&
+           "first coroutine location not set");
+    Diag(ReturnLoc, diag::err_return_in_coroutine);
+    Diag(FSI->FirstCoroutineStmtLoc, diag::note_declared_coroutine_here)
+        << FSI->getFirstCoroutineStmtKeyword();
+  }
+
   StmtResult R =
       BuildReturnStmt(ReturnLoc, RetVal.get(), /*AllowRecovery=*/true);
   if (R.isInvalid() || ExprEvalContexts.back().isDiscardedStatementContext())

clang/test/SemaCXX/coroutines.cpp

@@ -4,6 +4,9 @@
 // RUN: %clang_cc1 -std=c++23 -fsyntax-only -verify=expected,cxx20_23,cxx23    %s -fcxx-exceptions -fexceptions -Wunused-result
 // RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,cxx14_20,cxx20_23 %s -fcxx-exceptions -fexceptions -Wunused-result
 
+// Run without -verify to check the order of errors we show.
+// RUN: not %clang_cc1 -std=c++20 -fsyntax-only %s -fcxx-exceptions -fexceptions -Wunused-result 2>&1 | FileCheck %s
+
 void no_coroutine_traits_bad_arg_await() {
   co_await a; // expected-error {{include <coroutine>}}
   // expected-error@-1 {{use of undeclared identifier 'a'}}
@@ -154,12 +157,15 @@ namespace std {
 template <class PromiseType = void>
 struct coroutine_handle {
   static coroutine_handle from_address(void *) noexcept;
+  static coroutine_handle from_promise(PromiseType &promise);
 };
 template <>
 struct coroutine_handle<void> {
   template <class PromiseType>
   coroutine_handle(coroutine_handle<PromiseType>) noexcept;
   static coroutine_handle from_address(void *) noexcept;
+  template <class PromiseType>
+  static coroutine_handle from_promise(PromiseType &promise);
 };
 } // namespace std
 
@@ -206,6 +212,22 @@ void mixed_yield_invalid() {
   return; // expected-error {{return statement not allowed in coroutine}}
 }
 
+void mixed_yield_return_first(bool b) {
+  if (b) {
+    return; // expected-error {{return statement not allowed in coroutine}}
+  }
+  co_yield 0; // expected-note {{function is a coroutine due to use of 'co_yield'}}
+}
+
+template<typename T>
+void mixed_return_for_range(bool b, T t) {
+  if (b) {
+    return; // expected-error {{return statement not allowed in coroutine}}
+  }
+  for co_await (auto i : t){}; // expected-warning {{'for co_await' belongs to CoroutineTS instead of C++20, which is deprecated}}
+  // expected-note@-1 {{function is a coroutine due to use of 'co_await'}}
+}
+
 template <class T>
 void mixed_yield_template(T) {
   co_yield blah; // expected-error {{use of undeclared identifier}}
@@ -264,6 +286,13 @@ void mixed_coreturn(void_tag, bool b) {
     return; // expected-error {{not allowed in coroutine}}
 }
 
+void mixed_coreturn_return_first(void_tag, bool b) {
+  if (b)
+    return; // expected-error {{not allowed in coroutine}}
+  else
+    co_return; // expected-note {{use of 'co_return'}}
+}
+
 void mixed_coreturn_invalid(bool b) {
   if (b)
     co_return; // expected-note {{use of 'co_return'}}
@@ -291,6 +320,53 @@ void mixed_coreturn_template2(bool b, T) {
     return; // expected-error {{not allowed in coroutine}}
 }
 
+struct promise_handle;
+
+struct Handle : std::coroutine_handle<promise_handle> { // expected-note 4{{not viable}}
+    // expected-note@-1 4{{not viable}}
+    using promise_type = promise_handle;
+};
+
+struct promise_handle {
+    Handle get_return_object() noexcept {
+      { return Handle(std::coroutine_handle<Handle::promise_type>::from_promise(*this)); }
+    }
+    suspend_never initial_suspend() const noexcept { return {}; }
+    suspend_never final_suspend() const noexcept { return {}; }
+    void return_void() const noexcept {}
+    void unhandled_exception() const noexcept {}
+};
+
+Handle mixed_return_value() {
+  co_await a; // expected-note {{function is a coroutine due to use of 'co_await' here}}
+  return 0; // expected-error {{return statement not allowed in coroutine}}
+  // expected-error@-1 {{no viable conversion from returned value of type}}
+  // Check that we first show that return is not allowed in coroutine.
+  // The error about bad conversion is most likely spurious so we prefer to have it afterwards.
+  // CHECK-NOT: error: no viable conversion from returned value of type
+  // CHECK: error: return statement not allowed in coroutine
+  // CHECK: error: no viable conversion from returned value of type
+}
+
+Handle mixed_return_value_return_first(bool b) {
+  if (b) {
+    return 0; // expected-error {{no viable conversion from returned value of type}}
+    // expected-error@-1 {{return statement not allowed in coroutine}}
+  }
+  co_await a; // expected-note {{function is a coroutine due to use of 'co_await' here}}
+  co_return 0; // expected-error {{no member named 'return_value' in 'promise_handle'}}
+}
+
+Handle mixed_multiple_returns(bool b) {
+  if (b) {
+    return 0; // expected-error {{no viable conversion from returned value of type}}
+    // expected-error@-1 {{return statement not allowed in coroutine}}
+  }
+  co_await a; // expected-note {{function is a coroutine due to use of 'co_await' here}}
+  // The error 'return statement not allowed in coroutine' should appear only once.
+  return 0; // expected-error {{no viable conversion from returned value of type}}
+}
+
 struct CtorDtor {
   CtorDtor() {
     co_yield 0; // expected-error {{'co_yield' cannot be used in a constructor}}