From 0834a353d3d8b3c730d0073ed3b1d6ad92402f2b Mon Sep 17 00:00:00 2001 From: Eric Fiselier Date: Thu, 27 Oct 2016 07:30:31 +0000 Subject: [PATCH] [coroutines] Build fallthrough and set_exception statements. Summary: This patch adds semantic checking and building of the fall-through `co_return;` statement as well as the `p.set_exception(std::current_exception())` call for handling uncaught exceptions. The fall-through statement is built and checked according to: > [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. 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. Similarly the `set_exception` call is only built when that unqualified-id is found in the scope of class P. Additionally this patch adds fall-through warnings for non-void returning coroutines. Since it's surprising undefined behavior I thought it would be important to add the warning right away. Reviewers: majnemer, GorNishanov, rsmith Subscribers: mehdi_amini, cfe-commits Differential Revision: https://reviews.llvm.org/D25349 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@285271 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/StmtCXX.h | 5 +- include/clang/Basic/DiagnosticSemaKinds.td | 13 ++++ lib/Sema/AnalysisBasedWarnings.cpp | 55 +++++++++++--- lib/Sema/SemaCoroutine.cpp | 86 ++++++++++++++++++++-- lib/Sema/TreeTransform.h | 1 + test/SemaCXX/coreturn.cpp | 73 ++++++++++++++++++ test/SemaCXX/coroutines.cpp | 44 ++++++++++- 7 files changed, 256 insertions(+), 21 deletions(-) create mode 100644 test/SemaCXX/coreturn.cpp diff --git a/include/clang/AST/StmtCXX.h b/include/clang/AST/StmtCXX.h index 1d29c228a4..d1a91cc18f 100644 --- a/include/clang/AST/StmtCXX.h +++ b/include/clang/AST/StmtCXX.h @@ -405,10 +405,13 @@ public: SourceLocation getLocStart() const LLVM_READONLY { return CoreturnLoc; } SourceLocation getLocEnd() const LLVM_READONLY { - return getOperand()->getLocEnd(); + return getOperand() ? getOperand()->getLocEnd() : getLocStart(); } child_range children() { + if (!getOperand()) + return child_range(SubStmts + SubStmt::PromiseCall, + SubStmts + SubStmt::Count); return child_range(SubStmts, SubStmts + SubStmt::Count); } diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index b9e7c56d3d..bd0d2eb279 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -512,6 +512,12 @@ def err_maybe_falloff_nonvoid_block : Error< "control may reach end of non-void block">; def err_falloff_nonvoid_block : Error< "control reaches end of non-void block">; +def warn_maybe_falloff_nonvoid_coroutine : Warning< + "control may reach end of non-void coroutine">, + InGroup; +def warn_falloff_nonvoid_coroutine : Warning< + "control reaches end of non-void coroutine">, + InGroup; def warn_suggest_noreturn_function : Warning< "%select{function|method}0 %1 could be declared with attribute 'noreturn'">, InGroup, DefaultIgnore; @@ -8643,6 +8649,13 @@ def err_implied_std_coroutine_traits_promise_type_not_class : Error< def err_coroutine_traits_missing_specialization : Error< "this function cannot be a coroutine: missing definition of " "specialization %q0">; +def err_implied_std_current_exception_not_found : Error< + "you need to include before defining a coroutine that implicitly " + "uses 'set_exception'">; +def err_malformed_std_current_exception : Error< + "'std::current_exception' must be a function">; +def err_coroutine_promise_return_ill_formed : Error< + "%0 declares both 'return_value' and 'return_void'">; } let CategoryName = "Documentation Issue" in { diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp index 9b3fbd84b0..5953d020b4 100644 --- a/lib/Sema/AnalysisBasedWarnings.cpp +++ b/lib/Sema/AnalysisBasedWarnings.cpp @@ -365,7 +365,7 @@ static ControlFlowKind CheckFallThrough(AnalysisDeclContext &AC) { CFGStmt CS = ri->castAs(); const Stmt *S = CS.getStmt(); - if (isa(S)) { + if (isa(S) || isa(S)) { HasLiveReturn = true; continue; } @@ -416,7 +416,7 @@ struct CheckFallThroughDiagnostics { unsigned diag_AlwaysFallThrough_HasNoReturn; unsigned diag_AlwaysFallThrough_ReturnsNonVoid; unsigned diag_NeverFallThroughOrReturn; - enum { Function, Block, Lambda } funMode; + enum { Function, Block, Lambda, Coroutine } funMode; SourceLocation FuncLoc; static CheckFallThroughDiagnostics MakeForFunction(const Decl *Func) { @@ -452,6 +452,19 @@ struct CheckFallThroughDiagnostics { return D; } + static CheckFallThroughDiagnostics MakeForCoroutine(const Decl *Func) { + CheckFallThroughDiagnostics D; + D.FuncLoc = Func->getLocation(); + D.diag_MaybeFallThrough_HasNoReturn = 0; + D.diag_MaybeFallThrough_ReturnsNonVoid = + diag::warn_maybe_falloff_nonvoid_coroutine; + D.diag_AlwaysFallThrough_HasNoReturn = 0; + D.diag_AlwaysFallThrough_ReturnsNonVoid = + diag::warn_falloff_nonvoid_coroutine; + D.funMode = Coroutine; + return D; + } + static CheckFallThroughDiagnostics MakeForBlock() { CheckFallThroughDiagnostics D; D.diag_MaybeFallThrough_HasNoReturn = @@ -494,7 +507,13 @@ struct CheckFallThroughDiagnostics { (!ReturnsVoid || D.isIgnored(diag::warn_suggest_noreturn_block, FuncLoc)); } - + if (funMode == Coroutine) { + return (ReturnsVoid || + D.isIgnored(diag::warn_maybe_falloff_nonvoid_function, FuncLoc) || + D.isIgnored(diag::warn_maybe_falloff_nonvoid_coroutine, + FuncLoc)) && + (!HasNoReturn); + } // For blocks / lambdas. return ReturnsVoid && !HasNoReturn; } @@ -514,11 +533,14 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, bool ReturnsVoid = false; bool HasNoReturn = false; - if (const FunctionDecl *FD = dyn_cast(D)) { - ReturnsVoid = FD->getReturnType()->isVoidType(); + if (const auto *FD = dyn_cast(D)) { + if (const auto *CBody = dyn_cast(Body)) + ReturnsVoid = CBody->getFallthroughHandler() != nullptr; + else + ReturnsVoid = FD->getReturnType()->isVoidType(); HasNoReturn = FD->isNoReturn(); } - else if (const ObjCMethodDecl *MD = dyn_cast(D)) { + else if (const auto *MD = dyn_cast(D)) { ReturnsVoid = MD->getReturnType()->isVoidType(); HasNoReturn = MD->hasAttr(); } @@ -1986,13 +2008,22 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P, // Warning: check missing 'return' if (P.enableCheckFallThrough) { + auto IsCoro = [&]() { + if (auto *FD = dyn_cast(D)) + if (FD->getBody() && isa(FD->getBody())) + return true; + return false; + }; const CheckFallThroughDiagnostics &CD = - (isa(D) ? CheckFallThroughDiagnostics::MakeForBlock() - : (isa(D) && - cast(D)->getOverloadedOperator() == OO_Call && - cast(D)->getParent()->isLambda()) - ? CheckFallThroughDiagnostics::MakeForLambda() - : CheckFallThroughDiagnostics::MakeForFunction(D)); + (isa(D) + ? CheckFallThroughDiagnostics::MakeForBlock() + : (isa(D) && + cast(D)->getOverloadedOperator() == OO_Call && + cast(D)->getParent()->isLambda()) + ? CheckFallThroughDiagnostics::MakeForLambda() + : (IsCoro() + ? CheckFallThroughDiagnostics::MakeForCoroutine(D) + : CheckFallThroughDiagnostics::MakeForFunction(D))); CheckFallThroughForBody(S, D, Body, blkExpr, CD, AC); } diff --git a/lib/Sema/SemaCoroutine.cpp b/lib/Sema/SemaCoroutine.cpp index 323878e2d5..5231e23c1c 100644 --- a/lib/Sema/SemaCoroutine.cpp +++ b/lib/Sema/SemaCoroutine.cpp @@ -378,6 +378,34 @@ StmtResult Sema::BuildCoreturnStmt(SourceLocation Loc, Expr *E) { return Res; } +static ExprResult buildStdCurrentExceptionCall(Sema &S, SourceLocation Loc) { + NamespaceDecl *Std = S.getStdNamespace(); + if (!Std) { + S.Diag(Loc, diag::err_implied_std_current_exception_not_found); + return ExprError(); + } + LookupResult Result(S, &S.PP.getIdentifierTable().get("current_exception"), + Loc, Sema::LookupOrdinaryName); + if (!S.LookupQualifiedName(Result, Std)) { + S.Diag(Loc, diag::err_implied_std_current_exception_not_found); + return ExprError(); + } + + // FIXME The STL is free to provide more than one overload. + FunctionDecl *FD = Result.getAsSingle(); + if (!FD) { + S.Diag(Loc, diag::err_malformed_std_current_exception); + return ExprError(); + } + ExprResult Res = S.BuildDeclRefExpr(FD, FD->getType(), VK_LValue, Loc); + Res = S.ActOnCallExpr(/*Scope*/ nullptr, Res.get(), Loc, None, Loc); + if (Res.isInvalid()) { + S.Diag(Loc, diag::err_malformed_std_current_exception); + return ExprError(); + } + return Res; +} + void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) { FunctionScopeInfo *Fn = getCurFunction(); assert(Fn && !Fn->CoroutineStmts.empty() && "not a coroutine"); @@ -432,10 +460,59 @@ void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) { if (FinalSuspend.isInvalid()) return FD->setInvalidDecl(); - // FIXME: Perform analysis of set_exception call. - - // FIXME: Try to form 'p.return_void();' expression statement to handle + // Try to form 'p.return_void();' expression statement to handle // control flowing off the end of the coroutine. + // Also try to form 'p.set_exception(std::current_exception());' to handle + // uncaught exceptions. + ExprResult SetException; + StmtResult Fallthrough; + if (Fn->CoroutinePromise && + !Fn->CoroutinePromise->getType()->isDependentType()) { + CXXRecordDecl *RD = Fn->CoroutinePromise->getType()->getAsCXXRecordDecl(); + assert(RD && "Type should have already been checked"); + // [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. + DeclarationName RVoidDN = PP.getIdentifierInfo("return_void"); + LookupResult RVoidResult(*this, RVoidDN, Loc, Sema::LookupMemberName); + const bool HasRVoid = LookupQualifiedName(RVoidResult, RD); + + DeclarationName RValueDN = PP.getIdentifierInfo("return_value"); + LookupResult RValueResult(*this, RValueDN, Loc, Sema::LookupMemberName); + const bool HasRValue = LookupQualifiedName(RValueResult, RD); + + if (HasRVoid && HasRValue) { + // FIXME Improve this diagnostic + Diag(FD->getLocation(), diag::err_coroutine_promise_return_ill_formed) + << RD; + return FD->setInvalidDecl(); + } 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 = BuildCoreturnStmt(FD->getLocation(), nullptr); + Fallthrough = ActOnFinishFullStmt(Fallthrough.get()); + if (Fallthrough.isInvalid()) + return FD->setInvalidDecl(); + } + + // [dcl.fct.def.coroutine]/3 + // The unqualified-id set_exception is found in the scope of P by class + // member access lookup (3.4.5). + DeclarationName SetExDN = PP.getIdentifierInfo("set_exception"); + LookupResult SetExResult(*this, SetExDN, Loc, Sema::LookupMemberName); + if (LookupQualifiedName(SetExResult, RD)) { + // Form the call 'p.set_exception(std::current_exception())' + SetException = buildStdCurrentExceptionCall(*this, Loc); + if (SetException.isInvalid()) + return FD->setInvalidDecl(); + Expr *E = SetException.get(); + SetException = buildPromiseCall(*this, Fn, Loc, "set_exception", E); + SetException = ActOnFinishFullExpr(SetException.get(), Loc); + if (SetException.isInvalid()) + return FD->setInvalidDecl(); + } + } // Build implicit 'p.get_return_object()' expression and form initialization // of return type from it. @@ -462,6 +539,5 @@ void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) { // Build body for the coroutine wrapper statement. Body = new (Context) CoroutineBodyStmt( Body, PromiseStmt.get(), InitialSuspend.get(), FinalSuspend.get(), - /*SetException*/nullptr, /*Fallthrough*/nullptr, - ReturnObject.get(), ParamMoves); + SetException.get(), Fallthrough.get(), ReturnObject.get(), ParamMoves); } diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h index a461b33c4d..26d7222445 100644 --- a/lib/Sema/TreeTransform.h +++ b/lib/Sema/TreeTransform.h @@ -6654,6 +6654,7 @@ template StmtResult TreeTransform::TransformCoroutineBodyStmt(CoroutineBodyStmt *S) { // The coroutine body should be re-formed by the caller if necessary. + // FIXME: The coroutine body is always rebuilt by ActOnFinishFunctionBody return getDerived().TransformStmt(S->getBody()); } diff --git a/test/SemaCXX/coreturn.cpp b/test/SemaCXX/coreturn.cpp new file mode 100644 index 0000000000..4c41ae95c5 --- /dev/null +++ b/test/SemaCXX/coreturn.cpp @@ -0,0 +1,73 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify -fblocks -Wno-unreachable-code -Wno-unused-value + +struct awaitable { + bool await_ready(); + void await_suspend(); // FIXME: coroutine_handle + void await_resume(); +} a; + +struct suspend_always { + bool await_ready() { return false; } + void await_suspend() {} + void await_resume() {} +}; + +struct suspend_never { + bool await_ready() { return true; } + void await_suspend() {} + void await_resume() {} +}; + +namespace std { +namespace experimental { + +template +struct coroutine_traits { using promise_type = typename Ret::promise_type; }; + +template +struct coroutine_handle {}; +} +} + +struct promise_void { + void get_return_object(); + suspend_always initial_suspend(); + suspend_always final_suspend(); + void return_void(); +}; + +struct promise_float { + float get_return_object(); + suspend_always initial_suspend(); + suspend_always final_suspend(); + void return_void(); +}; + +struct promise_int { + int get_return_object(); + suspend_always initial_suspend(); + suspend_always final_suspend(); + void return_value(int); +}; + +template +struct std::experimental::coroutine_traits { using promise_type = promise_void; }; + +template +struct std::experimental::coroutine_traits { using promise_type = promise_float; }; + +template +struct std::experimental::coroutine_traits { using promise_type = promise_int; }; + +void test0() { co_await a; } +float test1() { co_await a; } + +int test2() { + co_await a; +} // expected-warning {{control reaches end of non-void coroutine}} + +int test3() { + co_await a; +b: + goto b; +} diff --git a/test/SemaCXX/coroutines.cpp b/test/SemaCXX/coroutines.cpp index dc3980cef0..f170201a2b 100644 --- a/test/SemaCXX/coroutines.cpp +++ b/test/SemaCXX/coroutines.cpp @@ -118,9 +118,6 @@ struct promise_void { void get_return_object(); suspend_always initial_suspend(); suspend_always final_suspend(); - awaitable yield_value(int); - awaitable yield_value(yielded_thing); - not_awaitable yield_value(void()); void return_void(); }; @@ -313,6 +310,47 @@ coro bad_final_suspend() { // expected-error {{no member named 'a co_await a; } +struct bad_promise_6 { + coro get_return_object(); + suspend_always initial_suspend(); + suspend_always final_suspend(); + void return_void(); + void return_value(int) const; + void return_value(int); +}; +coro bad_implicit_return() { // expected-error {{'bad_promise_6' declares both 'return_value' and 'return_void'}} + co_await a; +} + +struct bad_promise_7 { + coro get_return_object(); + suspend_always initial_suspend(); + suspend_always final_suspend(); + void return_void(); + void set_exception(int *); +}; +coro no_std_current_exc() { + // expected-error@-1 {{you need to include before defining a coroutine that implicitly uses 'set_exception'}} + co_await a; +} + +namespace std { +int *current_exception(); +} + +struct bad_promise_8 { + coro get_return_object(); + suspend_always initial_suspend(); + suspend_always final_suspend(); + void return_void(); + void set_exception(); // expected-note {{function not viable}} + void set_exception(int *) __attribute__((unavailable)); // expected-note {{explicitly made unavailable}} + void set_exception(void *); // expected-note {{candidate function}} +}; +coro calls_set_exception() { + // expected-error@-1 {{call to unavailable member function 'set_exception'}} + co_await a; +} template<> struct std::experimental::coroutine_traits { using promise_type = promise; }; -- 2.40.0