Skip to content

Commit f0f6a1a

Browse files
Applied fixes related to the comments in the PR.
1 parent d35544d commit f0f6a1a

File tree

3 files changed

+56
-13
lines changed

3 files changed

+56
-13
lines changed

clang/lib/Sema/SemaCoroutine.cpp

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,18 @@ bool Sema::checkFinalSuspendNoThrow(const Stmt *FinalSuspend) {
684684
return ThrowingDecls.empty();
685685
}
686686

687+
// [stmt.return.coroutine]p1:
688+
// A coroutine shall not enclose a return statement ([stmt.return]).
689+
static void checkReturnStmtInCoroutine(Sema &S, FunctionScopeInfo *FSI) {
690+
if (FSI && FSI->FirstReturnLoc.isValid()) {
691+
assert(FSI->FirstCoroutineStmtLoc.isValid() &&
692+
"first coroutine location not set");
693+
S.Diag(FSI->FirstReturnLoc, diag::err_return_in_coroutine);
694+
S.Diag(FSI->FirstCoroutineStmtLoc, diag::note_declared_coroutine_here)
695+
<< FSI->getFirstCoroutineStmtKeyword();
696+
}
697+
}
698+
687699
bool Sema::ActOnCoroutineBodyStart(Scope *SC, SourceLocation KWLoc,
688700
StringRef Keyword) {
689701
// Ignore previous expr evaluation contexts.
@@ -694,6 +706,10 @@ bool Sema::ActOnCoroutineBodyStart(Scope *SC, SourceLocation KWLoc,
694706
auto *ScopeInfo = getCurFunction();
695707
assert(ScopeInfo->CoroutinePromise);
696708

709+
if (ScopeInfo->FirstCoroutineStmtLoc == KWLoc) {
710+
checkReturnStmtInCoroutine(*this, ScopeInfo);
711+
}
712+
697713
// If we have existing coroutine statements then we have already built
698714
// the initial and final suspend points.
699715
if (!ScopeInfo->NeedsCoroutineSuspends)
@@ -801,6 +817,7 @@ ExprResult Sema::ActOnCoawaitExpr(Scope *S, SourceLocation Loc, Expr *E) {
801817
if (R.isInvalid()) return ExprError();
802818
E = R.get();
803819
}
820+
804821
ExprResult Lookup = BuildOperatorCoawaitLookupExpr(S, Loc);
805822
if (Lookup.isInvalid())
806823
return ExprError();
@@ -1118,16 +1135,6 @@ void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) {
11181135
if (Fn->FirstVLALoc.isValid())
11191136
Diag(Fn->FirstVLALoc, diag::err_vla_in_coroutine_unsupported);
11201137

1121-
// [stmt.return.coroutine]p1:
1122-
// A coroutine shall not enclose a return statement ([stmt.return]).
1123-
if (Fn->FirstReturnLoc.isValid() && Fn->FirstReturnLoc < Fn->FirstCoroutineStmtLoc) {
1124-
assert(Fn->FirstCoroutineStmtLoc.isValid() &&
1125-
"first coroutine location not set");
1126-
Diag(Fn->FirstReturnLoc, diag::err_return_in_coroutine);
1127-
Diag(Fn->FirstCoroutineStmtLoc, diag::note_declared_coroutine_here)
1128-
<< Fn->getFirstCoroutineStmtKeyword();
1129-
}
1130-
11311138
// Coroutines will get splitted into pieces. The GNU address of label
11321139
// extension wouldn't be meaningful in coroutines.
11331140
for (AddrLabelExpr *ALE : Fn->AddrLabels)

clang/lib/Sema/SemaStmt.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3749,7 +3749,7 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
37493749

37503750
// using plain return in a coroutine is not allowed.
37513751
FunctionScopeInfo *FSI = getCurFunction();
3752-
if (getLangOpts().Coroutines && FSI->isCoroutine()) {
3752+
if (FSI->FirstReturnLoc.isInvalid() && FSI->isCoroutine()) {
37533753
assert(FSI->FirstCoroutineStmtLoc.isValid() &&
37543754
"first coroutine location not set");
37553755
Diag(ReturnLoc, diag::err_return_in_coroutine);

clang/test/SemaCXX/coroutines.cpp

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
// RUN: %clang_cc1 -std=c++23 -fsyntax-only -verify=expected,cxx20_23,cxx23 %s -fcxx-exceptions -fexceptions -Wunused-result
55
// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,cxx14_20,cxx20_23 %s -fcxx-exceptions -fexceptions -Wunused-result
6+
// RUN: not %clang_cc1 -std=c++20 -fsyntax-only %s -fcxx-exceptions -fexceptions -Wunused-result 2>&1 | FileCheck %s
67

78
void no_coroutine_traits_bad_arg_await() {
89
co_await a; // expected-error {{include <coroutine>}}
@@ -209,6 +210,22 @@ void mixed_yield_invalid() {
209210
return; // expected-error {{return statement not allowed in coroutine}}
210211
}
211212

213+
void mixed_yield_return_first(bool b) {
214+
if (b) {
215+
return; // expected-error {{return statement not allowed in coroutine}}
216+
}
217+
co_yield 0; // expected-note {{function is a coroutine due to use of 'co_yield'}}
218+
}
219+
220+
template<typename T>
221+
void mixed_return_for_range(bool b, T t) {
222+
if (b) {
223+
return; // expected-error {{return statement not allowed in coroutine}}
224+
}
225+
for co_await (auto i : t){}; // expected-warning {{'for co_await' belongs to CoroutineTS instead of C++20, which is deprecated}}
226+
// expected-note@-1 {{function is a coroutine due to use of 'co_await'}}
227+
}
228+
212229
template <class T>
213230
void mixed_yield_template(T) {
214231
co_yield blah; // expected-error {{use of undeclared identifier}}
@@ -267,6 +284,13 @@ void mixed_coreturn(void_tag, bool b) {
267284
return; // expected-error {{not allowed in coroutine}}
268285
}
269286

287+
void mixed_coreturn_return_first(void_tag, bool b) {
288+
if (b)
289+
return; // expected-error {{not allowed in coroutine}}
290+
else
291+
co_return; // expected-note {{use of 'co_return'}}
292+
}
293+
270294
void mixed_coreturn_invalid(bool b) {
271295
if (b)
272296
co_return; // expected-note {{use of 'co_return'}}
@@ -296,8 +320,8 @@ void mixed_coreturn_template2(bool b, T) {
296320

297321
struct promise_handle;
298322

299-
struct Handle : std::coroutine_handle<promise_handle> { // expected-note 2{{candidate constructor (the implicit copy constructor) not viable}}
300-
// expected-note@-1 2{{candidate constructor (the implicit move constructor) not viable}}
323+
struct Handle : std::coroutine_handle<promise_handle> { // expected-note 4{{not viable}}
324+
// expected-note@-1 4{{not viable}}
301325
using promise_type = promise_handle;
302326
};
303327

@@ -315,6 +339,9 @@ Handle mixed_return_value() {
315339
co_await a; // expected-note {{function is a coroutine due to use of 'co_await' here}}
316340
return 0; // expected-error {{return statement not allowed in coroutine}}
317341
// expected-error@-1 {{no viable conversion from returned value of type}}
342+
// CHECK-NOT: error: no viable conversion from returned value of type
343+
// CHECK: error: return statement not allowed in coroutine
344+
// CHECK: error: no viable conversion from returned value of type
318345
}
319346

320347
Handle mixed_return_value_return_first(bool b) {
@@ -326,6 +353,15 @@ Handle mixed_return_value_return_first(bool b) {
326353
co_return 0; // expected-error {{no member named 'return_value' in 'promise_handle'}}
327354
}
328355

356+
Handle mixed_multiple_returns(bool b) {
357+
if (b) {
358+
return 0; // expected-error {{no viable conversion from returned value of type}}
359+
// expected-error@-1 {{return statement not allowed in coroutine}}
360+
}
361+
co_await a; // expected-note {{function is a coroutine due to use of 'co_await' here}}
362+
return 0; // expected-error {{no viable conversion from returned value of type}}
363+
}
364+
329365
struct CtorDtor {
330366
CtorDtor() {
331367
co_yield 0; // expected-error {{'co_yield' cannot be used in a constructor}}

0 commit comments

Comments
 (0)