From: Chuanqi Xu Date: Fri, 24 Dec 2021 05:33:52 +0000 (+0800) Subject: [C++20] [Coroutines] Allow promise_type to not define return_void or return_value X-Git-Tag: upstream/15.0.7~22242 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=097208dbf07786f3da84aec5b5f571c6e95a10e2;p=platform%2Fupstream%2Fllvm.git [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 --- diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f50c5a0..3b6341d 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 336e0c5..e89cecd 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 6fce00c..a5ad37e 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 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 no_return_value_or_return_void() { - // expected-error@-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@+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 e175e3f..b461c88 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 get_return_object(); +struct promise_no_return_func { + coro 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 no_return_value_or_return_void() { - // expected-error@-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 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 no_return_value_or_return_void_2() { + co_return; // expected-error {{no member named 'return_void'}} +} + +coro 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@+1 {{return type of 'await_suspend' is required to be 'void' or 'bool' (have 'char')}}