[clang] 097208d - [C++20] [Coroutines] Allow promise_type to not define return_void or return_value
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 23 21:39:37 PST 2021
Author: Chuanqi Xu
Date: 2021-12-24T13:37:51+08:00
New Revision: 097208dbf07786f3da84aec5b5f571c6e95a10e2
URL: https://github.com/llvm/llvm-project/commit/097208dbf07786f3da84aec5b5f571c6e95a10e2
DIFF: https://github.com/llvm/llvm-project/commit/097208dbf07786f3da84aec5b5f571c6e95a10e2.diff
LOG: [C++20] [Coroutines] Allow promise_type to not define return_void or return_value
According to [dcl.fct.def.coroutine]p6, the promise_type is allowed to
not define return_void nor return_value:
> If searches for the names return_void and return_value in the scope
> of the promise type each find any declarations, the program is
> ill-formed.
> [Note 1: If return_void is found, flowing off the end of a coroutine is
> equivalent to a co_return with no operand. Otherwise, flowing off the
> end of a coroutine results in
> undefined behavior ([stmt.return.coroutine]). — end note]
So the program isn't ill-formed if the promise_type doesn't define
return_void nor return_value. It is just a potential UB. So the program
should be allowed to compile.
Reviewed By: urnathan
Differential Revision: https://reviews.llvm.org/D116204
Added:
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCoroutine.cpp
clang/test/SemaCXX/coroutines-exp-namespace.cpp
clang/test/SemaCXX/coroutines.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f50c5a0c711ad..3b6341d2232d8 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11071,8 +11071,6 @@ def err_coroutine_type_missing_specialization : Error<
"specialization %0">;
def err_coroutine_promise_incompatible_return_functions : Error<
"the coroutine promise type %0 declares both 'return_value' and 'return_void'">;
-def err_coroutine_promise_requires_return_function : Error<
- "the coroutine promise type %0 must declare either 'return_value' or 'return_void'">;
def note_coroutine_promise_implicit_await_transform_required_here : Note<
"call to 'await_transform' implicitly required by 'co_await' here">;
def note_coroutine_promise_suspend_implicitly_required : Note<
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 336e0c5f1d465..e89cecd08ccae 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1432,9 +1432,13 @@ bool CoroutineStmtBuilder::makeOnFallthrough() {
assert(!IsPromiseDependentType &&
"cannot make statement while the promise type is dependent");
- // [dcl.fct.def.coroutine]/4
- // The unqualified-ids 'return_void' and 'return_value' are looked up in
- // the scope of class P. If both are found, the program is ill-formed.
+ // [dcl.fct.def.coroutine]/p6
+ // If searches for the names return_void and return_value in the scope of
+ // the promise type each find any declarations, the program is ill-formed.
+ // [Note 1: If return_void is found, flowing off the end of a coroutine is
+ // equivalent to a co_return with no operand. Otherwise, flowing off the end
+ // of a coroutine results in undefined behavior ([stmt.return.coroutine]). —
+ // end note]
bool HasRVoid, HasRValue;
LookupResult LRVoid =
lookupMember(S, "return_void", PromiseRecordDecl, Loc, HasRVoid);
@@ -1455,18 +1459,20 @@ bool CoroutineStmtBuilder::makeOnFallthrough() {
<< LRValue.getLookupName();
return false;
} else if (!HasRVoid && !HasRValue) {
- // FIXME: The PDTS currently specifies this case as UB, not ill-formed.
- // However we still diagnose this as an error since until the PDTS is fixed.
- S.Diag(FD.getLocation(),
- diag::err_coroutine_promise_requires_return_function)
- << PromiseRecordDecl;
- S.Diag(PromiseRecordDecl->getLocation(), diag::note_defined_here)
- << PromiseRecordDecl;
- return false;
+ // We need to set 'Fallthrough'. Otherwise the other analysis part might
+ // think the coroutine has defined a return_value method. So it might emit
+ // **false** positive warning. e.g.,
+ //
+ // promise_without_return_func foo() {
+ // co_await something();
+ // }
+ //
+ // Then AnalysisBasedWarning would emit a warning about `foo()` lacking a
+ // co_return statements, which isn't correct.
+ Fallthrough = S.ActOnNullStmt(PromiseRecordDecl->getLocation());
+ if (Fallthrough.isInvalid())
+ return false;
} else if (HasRVoid) {
- // If the unqualified-id return_void is found, flowing off the end of a
- // coroutine is equivalent to a co_return with no operand. Otherwise,
- // flowing off the end of a coroutine results in undefined behavior.
Fallthrough = S.BuildCoreturnStmt(FD.getLocation(), nullptr,
/*IsImplicit*/false);
Fallthrough = S.ActOnFinishFullStmt(Fallthrough.get());
diff --git a/clang/test/SemaCXX/coroutines-exp-namespace.cpp b/clang/test/SemaCXX/coroutines-exp-namespace.cpp
index 6fce00c09ca6f..a5ad37e338d2d 100644
--- a/clang/test/SemaCXX/coroutines-exp-namespace.cpp
+++ b/clang/test/SemaCXX/coroutines-exp-namespace.cpp
@@ -978,19 +978,6 @@ extern "C" int f(mismatch_gro_type_tag4) {
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
-struct bad_promise_no_return_func { // expected-note {{'bad_promise_no_return_func' defined here}}
- coro<bad_promise_no_return_func> get_return_object();
- suspend_always initial_suspend();
- suspend_always final_suspend() noexcept;
- void unhandled_exception();
-};
-// FIXME: The PDTS currently specifies this as UB, technically forbidding a
-// diagnostic.
-coro<bad_promise_no_return_func> no_return_value_or_return_void() {
- // expected-error at -1 {{'bad_promise_no_return_func' must declare either 'return_value' or 'return_void'}}
- co_await a;
-}
-
struct bad_await_suspend_return {
bool await_ready();
// expected-error at +1 {{return type of 'await_suspend' is required to be 'void' or 'bool' (have 'char')}}
diff --git a/clang/test/SemaCXX/coroutines.cpp b/clang/test/SemaCXX/coroutines.cpp
index e175e3f9fa64f..b461c881355b5 100644
--- a/clang/test/SemaCXX/coroutines.cpp
+++ b/clang/test/SemaCXX/coroutines.cpp
@@ -970,19 +970,36 @@ extern "C" int f(mismatch_gro_type_tag4) {
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
-struct bad_promise_no_return_func { // expected-note {{'bad_promise_no_return_func' defined here}}
- coro<bad_promise_no_return_func> get_return_object();
+struct promise_no_return_func {
+ coro<promise_no_return_func> get_return_object();
suspend_always initial_suspend();
suspend_always final_suspend() noexcept;
void unhandled_exception();
};
-// FIXME: The PDTS currently specifies this as UB, technically forbidding a
-// diagnostic.
-coro<bad_promise_no_return_func> no_return_value_or_return_void() {
- // expected-error at -1 {{'bad_promise_no_return_func' must declare either 'return_value' or 'return_void'}}
+// [dcl.fct.def.coroutine]/p6
+// If searches for the names return_void and return_value in the scope of
+// the promise type each find any declarations, the program is ill-formed.
+// [Note 1: If return_void is found, flowing off the end of a coroutine is
+// equivalent to a co_return with no operand. Otherwise, flowing off the end
+// of a coroutine results in undefined behavior ([stmt.return.coroutine]). —
+// end note]
+//
+// So it isn't ill-formed if the promise doesn't define return_value and return_void.
+// It is just a potential UB.
+coro<promise_no_return_func> no_return_value_or_return_void() {
co_await a;
}
+// The following two tests that it would emit correct diagnostic message
+// if we co_return in `promise_no_return_func`.
+coro<promise_no_return_func> no_return_value_or_return_void_2() {
+ co_return; // expected-error {{no member named 'return_void'}}
+}
+
+coro<promise_no_return_func> no_return_value_or_return_void_3() {
+ co_return 43; // expected-error {{no member named 'return_value'}}
+}
+
struct bad_await_suspend_return {
bool await_ready();
// expected-error at +1 {{return type of 'await_suspend' is required to be 'void' or 'bool' (have 'char')}}
More information about the cfe-commits
mailing list