[clang] db1375f - Surface error for plain return statement in coroutine earlier (#100985)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 2 04:49:13 PDT 2024
Author: ivanaivanovska
Date: 2024-08-02T13:49:11+02:00
New Revision: db1375f6d907f1af34c03b5174b7e0432f615d21
URL: https://github.com/llvm/llvm-project/commit/db1375f6d907f1af34c03b5174b7e0432f615d21
DIFF: https://github.com/llvm/llvm-project/commit/db1375f6d907f1af34c03b5174b7e0432f615d21.diff
LOG: 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.
Added:
Modified:
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/SemaCXX/coroutines.cpp
Removed:
################################################################################
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 81334c817b2af..68ad6e3fd6414 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/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)
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 34d2d398f244d..d283eaa511011 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 (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())
diff --git a/clang/test/SemaCXX/coroutines.cpp b/clang/test/SemaCXX/coroutines.cpp
index 2292932583fff..068fdab4bfe38 100644
--- a/clang/test/SemaCXX/coroutines.cpp
+++ b/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 at -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 at -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 at -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 at -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 at -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 at -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}}
More information about the cfe-commits
mailing list