diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11071,8 +11071,6 @@ "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 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -1432,9 +1432,13 @@ 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 @@ << 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 --- a/clang/test/SemaCXX/coroutines-exp-namespace.cpp +++ b/clang/test/SemaCXX/coroutines-exp-namespace.cpp @@ -978,19 +978,6 @@ 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 --- a/clang/test/SemaCXX/coroutines.cpp +++ b/clang/test/SemaCXX/coroutines.cpp @@ -970,19 +970,36 @@ 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')}}