[clang] Surface error for plain return statement in coroutine earlier (PR #100985)

[via cfe-commits]

https://github.com/ivanaivanovska created 
https://github.com/llvm/llvm-project/pull/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.

>From 8ee4d7bcdbc186e86d4ff2374b12a54f05d1fc38 Mon Sep 17 00:00:00 2001
From: Ivana Ivanovska <iivanov...@google.com>
Date: Mon, 29 Jul 2024 08:08:00 +0000
Subject: [PATCH] Surface error for plain return statement in coroutine earlier

---
 clang/lib/Sema/SemaCoroutine.cpp  |  2 +-
 clang/lib/Sema/SemaStmt.cpp       | 10 +++++++++
 clang/test/SemaCXX/coroutines.cpp | 35 +++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 81334c817b2af..87d0d44c5af66 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1120,7 +1120,7 @@ void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, 
Stmt *&Body) {
 
   // [stmt.return.coroutine]p1:
   //   A coroutine shall not enclose a return statement ([stmt.return]).
-  if (Fn->FirstReturnLoc.isValid()) {
+  if (Fn->FirstReturnLoc.isValid() && Fn->FirstReturnLoc < 
Fn->FirstCoroutineStmtLoc) {
     assert(Fn->FirstCoroutineStmtLoc.isValid() &&
                    "first coroutine location not set");
     Diag(Fn->FirstReturnLoc, diag::err_return_in_coroutine);
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 34d2d398f244d..3909892ef0a6f 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/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 (getLangOpts().Coroutines && 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())
diff --git a/clang/test/SemaCXX/coroutines.cpp 
b/clang/test/SemaCXX/coroutines.cpp
index 2292932583fff..b4f362c621929 100644
--- a/clang/test/SemaCXX/coroutines.cpp
+++ b/clang/test/SemaCXX/coroutines.cpp
@@ -154,12 +154,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
 
@@ -291,6 +294,38 @@ 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 
2{{candidate constructor (the implicit copy constructor) not viable}}
+    // expected-note@-1 2{{candidate constructor (the implicit move 
constructor) 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}}
+}
+
+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'}}
+}
+
 struct CtorDtor {
   CtorDtor() {
     co_yield 0; // expected-error {{'co_yield' cannot be used in a 
constructor}}

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits