r303868 - [coroutines] Diagnose when promise types fail to declare either return_void or return_value.
Eric Fiselier via cfe-commits
cfe-commits at lists.llvm.org
Thu May 25 07:59:40 PDT 2017
Author: ericwf
Date: Thu May 25 09:59:39 2017
New Revision: 303868
URL: http://llvm.org/viewvc/llvm-project?rev=303868&view=rev
Log:
[coroutines] Diagnose when promise types fail to declare either return_void or return_value.
Summary:
According to the PDTS it's perfectly legal to have a promise type that defines neither `return_value` nor `return_void`. However a coroutine that uses such a promise type will almost always have UB, because it can never `co_return`.
This patch changes Clang to diagnose such cases as an error. It also cleans up some of the diagnostic messages relating to member lookup in the promise type.
Reviewers: GorNishanov, rsmith
Reviewed By: GorNishanov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D33534
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/SemaCXX/coreturn.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=303868&r1=303867&r2=303868&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu May 25 09:59:39 2017
@@ -6303,6 +6303,8 @@ def warn_ambiguous_suitable_delete_funct
InGroup<DiagGroup<"ambiguous-delete">>;
def note_member_declared_here : Note<
"member %0 declared here">;
+def note_member_first_declared_here : Note<
+ "member %0 first declared here">;
def err_decrement_bool : Error<"cannot decrement expression of type bool">;
def warn_increment_bool : Warning<
"incrementing expression of type bool is deprecated and "
@@ -8918,8 +8920,6 @@ def err_return_in_coroutine : Error<
"return statement not allowed in coroutine; did you mean 'co_return'?">;
def note_declared_coroutine_here : Note<
"function is a coroutine due to use of '%0' here">;
-def note_promise_member_declared_here : Note<
- "'%0' is declared here">;
def err_coroutine_objc_method : Error<
"Objective-C methods as coroutines are not yet supported">;
def err_coroutine_unevaluated_context : Error<
@@ -8954,8 +8954,10 @@ def err_coroutine_promise_type_incomplet
def err_coroutine_type_missing_specialization : Error<
"this function cannot be a coroutine: missing definition of "
"specialization %q0">;
-def err_coroutine_promise_return_ill_formed : Error<
- "%0 declares both 'return_value' and 'return_void'">;
+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<
Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=303868&r1=303867&r2=303868&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Thu May 25 09:59:39 2017
@@ -23,14 +23,22 @@
using namespace clang;
using namespace sema;
-static bool lookupMember(Sema &S, const char *Name, CXXRecordDecl *RD,
- SourceLocation Loc) {
+static LookupResult lookupMember(Sema &S, const char *Name, CXXRecordDecl *RD,
+ SourceLocation Loc, bool &Res) {
DeclarationName DN = S.PP.getIdentifierInfo(Name);
LookupResult LR(S, DN, Loc, Sema::LookupMemberName);
// Suppress diagnostics when a private member is selected. The same warnings
// will be produced again when building the call.
LR.suppressDiagnostics();
- return S.LookupQualifiedName(LR, RD);
+ Res = S.LookupQualifiedName(LR, RD);
+ return LR;
+}
+
+static bool lookupMember(Sema &S, const char *Name, CXXRecordDecl *RD,
+ SourceLocation Loc) {
+ bool Res;
+ lookupMember(S, Name, RD, Loc, Res);
+ return Res;
}
/// Look up the std::coroutine_traits<...>::promise_type for the given
@@ -861,9 +869,8 @@ bool CoroutineStmtBuilder::makeReturnOnA
StmtResult ReturnStmt =
S.BuildReturnStmt(Loc, ReturnObjectOnAllocationFailure.get());
if (ReturnStmt.isInvalid()) {
- S.Diag(Found.getFoundDecl()->getLocation(),
- diag::note_promise_member_declared_here)
- << DN.getAsString();
+ S.Diag(Found.getFoundDecl()->getLocation(), diag::note_member_declared_here)
+ << DN;
S.Diag(Fn.FirstCoroutineStmtLoc, diag::note_declared_coroutine_here)
<< Fn.getFirstCoroutineStmtKeyword();
return false;
@@ -993,13 +1000,32 @@ bool CoroutineStmtBuilder::makeOnFallthr
// [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.
- const bool HasRVoid = lookupMember(S, "return_void", PromiseRecordDecl, Loc);
- const bool HasRValue = lookupMember(S, "return_value", PromiseRecordDecl, Loc);
+ bool HasRVoid, HasRValue;
+ LookupResult LRVoid =
+ lookupMember(S, "return_void", PromiseRecordDecl, Loc, HasRVoid);
+ LookupResult LRValue =
+ lookupMember(S, "return_value", PromiseRecordDecl, Loc, HasRValue);
StmtResult Fallthrough;
if (HasRVoid && HasRValue) {
// FIXME Improve this diagnostic
- S.Diag(FD.getLocation(), diag::err_coroutine_promise_return_ill_formed)
+ S.Diag(FD.getLocation(),
+ diag::err_coroutine_promise_incompatible_return_functions)
+ << PromiseRecordDecl;
+ S.Diag(LRVoid.getRepresentativeDecl()->getLocation(),
+ diag::note_member_first_declared_here)
+ << LRVoid.getLookupName();
+ S.Diag(LRValue.getRepresentativeDecl()->getLocation(),
+ diag::note_member_first_declared_here)
+ << 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;
} else if (HasRVoid) {
@@ -1074,8 +1100,8 @@ bool CoroutineStmtBuilder::makeReturnObj
static void noteMemberDeclaredHere(Sema &S, Expr *E, FunctionScopeInfo &Fn) {
if (auto *MbrRef = dyn_cast<CXXMemberCallExpr>(E)) {
auto *MethodDecl = MbrRef->getMethodDecl();
- S.Diag(MethodDecl->getLocation(), diag::note_promise_member_declared_here)
- << MethodDecl->getName();
+ S.Diag(MethodDecl->getLocation(), diag::note_member_declared_here)
+ << MethodDecl;
}
S.Diag(Fn.FirstCoroutineStmtLoc, diag::note_declared_coroutine_here)
<< Fn.getFirstCoroutineStmtKeyword();
Modified: cfe/trunk/test/SemaCXX/coreturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/coreturn.cpp?rev=303868&r1=303867&r2=303868&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/coreturn.cpp (original)
+++ cfe/trunk/test/SemaCXX/coreturn.cpp Thu May 25 09:59:39 2017
@@ -138,16 +138,3 @@ VoidTagReturnValue test11(bool b) {
if (b)
co_return 42;
} // expected-warning {{control may reach end of coroutine}}
-
-// FIXME: Promise types that declare neither return_value nor return_void
-// should be ill-formed. This test should be removed when that is fixed.
-VoidTagNoReturn test12() {
- co_await a;
-} // expected-warning {{control reaches end of coroutine}}
-
-// FIXME: Promise types that declare neither return_value nor return_void
-// should be ill-formed. This test should be removed when that is fixed.
-VoidTagNoReturn test13(bool b) {
- if (b)
- co_await a;
-} // expected-warning {{control reaches end of coroutine}}
Modified: cfe/trunk/test/SemaCXX/coroutines.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/coroutines.cpp?rev=303868&r1=303867&r2=303868&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/coroutines.cpp (original)
+++ cfe/trunk/test/SemaCXX/coroutines.cpp Thu May 25 09:59:39 2017
@@ -347,6 +347,7 @@ namespace dependent_operator_co_await_lo
::adl_ns::coawait_arg_type final_suspend();
transformed await_transform(transform_awaitable);
void unhandled_exception();
+ void return_void();
};
template <class AwaitArg>
struct basic_promise {
@@ -355,6 +356,7 @@ namespace dependent_operator_co_await_lo
awaitable initial_suspend();
awaitable final_suspend();
void unhandled_exception();
+ void return_void();
};
awaitable operator co_await(await_arg_1);
@@ -473,6 +475,7 @@ struct bad_promise_1 {
suspend_always initial_suspend();
suspend_always final_suspend();
void unhandled_exception();
+ void return_void();
};
coro<bad_promise_1> missing_get_return_object() { // expected-error {{no member named 'get_return_object' in 'bad_promise_1'}}
co_await a;
@@ -483,6 +486,7 @@ struct bad_promise_2 {
// FIXME: We shouldn't offer a typo-correction here!
suspend_always final_suspend(); // expected-note {{here}}
void unhandled_exception();
+ void return_void();
};
// FIXME: This shouldn't happen twice
coro<bad_promise_2> missing_initial_suspend() { // expected-error {{no member named 'initial_suspend' in 'bad_promise_2'}}
@@ -494,6 +498,7 @@ struct bad_promise_3 {
// FIXME: We shouldn't offer a typo-correction here!
suspend_always initial_suspend(); // expected-note {{here}}
void unhandled_exception();
+ void return_void();
};
coro<bad_promise_3> missing_final_suspend() { // expected-error {{no member named 'final_suspend' in 'bad_promise_3'}}
co_await a;
@@ -503,6 +508,7 @@ struct bad_promise_4 {
coro<bad_promise_4> get_return_object();
not_awaitable initial_suspend();
suspend_always final_suspend();
+ void return_void();
};
// FIXME: This diagnostic is terrible.
coro<bad_promise_4> bad_initial_suspend() { // expected-error {{no member named 'await_ready' in 'not_awaitable'}}
@@ -514,6 +520,7 @@ struct bad_promise_5 {
coro<bad_promise_5> get_return_object();
suspend_always initial_suspend();
not_awaitable final_suspend();
+ void return_void();
};
// FIXME: This diagnostic is terrible.
coro<bad_promise_5> bad_final_suspend() { // expected-error {{no member named 'await_ready' in 'not_awaitable'}}
@@ -526,8 +533,8 @@ struct bad_promise_6 {
suspend_always initial_suspend();
suspend_always final_suspend();
void unhandled_exception();
- void return_void();
- void return_value(int) const;
+ void return_void(); // expected-note 2 {{member 'return_void' first declared here}}
+ void return_value(int) const; // expected-note 2 {{member 'return_value' first declared here}}
void return_value(int);
};
coro<bad_promise_6> bad_implicit_return() { // expected-error {{'bad_promise_6' declares both 'return_value' and 'return_void'}}
@@ -751,7 +758,7 @@ struct mismatch_gro_type_tag1 {};
template<>
struct std::experimental::coroutine_traits<int, mismatch_gro_type_tag1> {
struct promise_type {
- void get_return_object() {} //expected-note {{'get_return_object' is declared here}}
+ void get_return_object() {} //expected-note {{member 'get_return_object' declared here}}
suspend_always initial_suspend() { return {}; }
suspend_always final_suspend() { return {}; }
void return_void() {}
@@ -768,7 +775,7 @@ struct mismatch_gro_type_tag2 {};
template<>
struct std::experimental::coroutine_traits<int, mismatch_gro_type_tag2> {
struct promise_type {
- void* get_return_object() {} //expected-note {{'get_return_object' is declared here}}
+ void *get_return_object() {} //expected-note {{member 'get_return_object' declared here}}
suspend_always initial_suspend() { return {}; }
suspend_always final_suspend() { return {}; }
void return_void() {}
@@ -786,7 +793,7 @@ template<>
struct std::experimental::coroutine_traits<int, mismatch_gro_type_tag3> {
struct promise_type {
int get_return_object() {}
- static void get_return_object_on_allocation_failure() {} //expected-note {{'get_return_object_on_allocation_failure' is declared here}}
+ static void get_return_object_on_allocation_failure() {} //expected-note {{member 'get_return_object_on_allocation_failure' declared here}}
suspend_always initial_suspend() { return {}; }
suspend_always final_suspend() { return {}; }
void return_void() {}
@@ -805,7 +812,7 @@ template<>
struct std::experimental::coroutine_traits<int, mismatch_gro_type_tag4> {
struct promise_type {
int get_return_object() {}
- static char* get_return_object_on_allocation_failure() {} //expected-note {{'get_return_object_on_allocation_failure' is declared here}}
+ static char *get_return_object_on_allocation_failure() {} //expected-note {{member 'get_return_object_on_allocation_failure' declared}}
suspend_always initial_suspend() { return {}; }
suspend_always final_suspend() { return {}; }
void return_void() {}
@@ -818,14 +825,15 @@ 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 {
+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();
void unhandled_exception();
};
-// FIXME: Make this ill-formed because the promise type declares
-// neither return_value(...) or return_void().
+// 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;
}
More information about the cfe-commits
mailing list