Author: modocache Date: Sat Nov 3 15:35:17 2018 New Revision: 346074 URL: http://llvm.org/viewvc/llvm-project?rev=346074&view=rev Log: [coroutines] Fix fallthrough warning on try/catch
Summary: The test case added in this diff would incorrectly warn that control flow may fall through without returning. Here's a standalone example: https://godbolt.org/z/dCwXEi The same program, but using `return` instead of `co_return`, does not produce a warning: https://godbolt.org/z/mVldqQ The issue was in how Clang analysis would structure its representation of the control-flow graph. Specifically, when constructing the CFG, `CFGBuilder::Visit` had special handling of a `ReturnStmt`, in which it would place object destructors in the same CFG block as a `return` statement, immediately after it. Doing so would allow the logic in `lib/Sema/AnalysisBasedWarning.cpp` `CheckFallThrough` to work properly in the program that used `return`, correctly determining that no "plain edges" preceded the exit block of the function. Because a `co_return` statement would not enjoy the same treatment when it was being built into the control-flow graph, object destructors would not be placed in the same CFG block as the `co_return`, thus resulting in a "plain edge" preceding the exit block of the function, and so the warning logic would be triggered. Add special casing for `co_return` to Clang analysis, thereby remedying the mistaken warning. Test Plan: `check-clang` Reviewers: GorNishanov, tks2103, rsmith Reviewed By: GorNishanov Subscribers: EricWF, lewissbaker, cfe-commits Differential Revision: https://reviews.llvm.org/D54075 Added: cfe/trunk/test/SemaCXX/coreturn-eh.cpp Modified: cfe/trunk/lib/Analysis/CFG.cpp Modified: cfe/trunk/lib/Analysis/CFG.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=346074&r1=346073&r2=346074&view=diff ============================================================================== --- cfe/trunk/lib/Analysis/CFG.cpp (original) +++ cfe/trunk/lib/Analysis/CFG.cpp Sat Nov 3 15:35:17 2018 @@ -571,7 +571,7 @@ private: CFGBlock *VisitObjCForCollectionStmt(ObjCForCollectionStmt *S); CFGBlock *VisitObjCMessageExpr(ObjCMessageExpr *E, AddStmtChoice asc); CFGBlock *VisitPseudoObjectExpr(PseudoObjectExpr *E); - CFGBlock *VisitReturnStmt(ReturnStmt *R); + CFGBlock *VisitReturnStmt(Stmt *S); CFGBlock *VisitSEHExceptStmt(SEHExceptStmt *S); CFGBlock *VisitSEHFinallyStmt(SEHFinallyStmt *S); CFGBlock *VisitSEHLeaveStmt(SEHLeaveStmt *S); @@ -2147,7 +2147,8 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, Ad return VisitPseudoObjectExpr(cast<PseudoObjectExpr>(S)); case Stmt::ReturnStmtClass: - return VisitReturnStmt(cast<ReturnStmt>(S)); + case Stmt::CoreturnStmtClass: + return VisitReturnStmt(S); case Stmt::SEHExceptStmtClass: return VisitSEHExceptStmt(cast<SEHExceptStmt>(S)); @@ -2877,22 +2878,24 @@ CFGBlock *CFGBuilder::VisitIfStmt(IfStmt return LastBlock; } -CFGBlock *CFGBuilder::VisitReturnStmt(ReturnStmt *R) { +CFGBlock *CFGBuilder::VisitReturnStmt(Stmt *S) { // If we were in the middle of a block we stop processing that block. // - // NOTE: If a "return" appears in the middle of a block, this means that the - // code afterwards is DEAD (unreachable). We still keep a basic block - // for that code; a simple "mark-and-sweep" from the entry block will be - // able to report such dead blocks. + // NOTE: If a "return" or "co_return" appears in the middle of a block, this + // means that the code afterwards is DEAD (unreachable). We still keep + // a basic block for that code; a simple "mark-and-sweep" from the entry + // block will be able to report such dead blocks. + assert(isa<ReturnStmt>(S) || isa<CoreturnStmt>(S)); // Create the new block. Block = createBlock(false); - addAutomaticObjHandling(ScopePos, LocalScope::const_iterator(), R); + addAutomaticObjHandling(ScopePos, LocalScope::const_iterator(), S); - findConstructionContexts( - ConstructionContextLayer::create(cfg->getBumpVectorContext(), R), - R->getRetValue()); + if (auto *R = dyn_cast<ReturnStmt>(S)) + findConstructionContexts( + ConstructionContextLayer::create(cfg->getBumpVectorContext(), R), + R->getRetValue()); // If the one of the destructors does not return, we already have the Exit // block as a successor. @@ -2901,7 +2904,7 @@ CFGBlock *CFGBuilder::VisitReturnStmt(Re // Add the return statement to the block. This may create new blocks if R // contains control-flow (short-circuit operations). - return VisitStmt(R, AddStmtChoice::AlwaysAdd); + return VisitStmt(S, AddStmtChoice::AlwaysAdd); } CFGBlock *CFGBuilder::VisitSEHExceptStmt(SEHExceptStmt *ES) { Added: cfe/trunk/test/SemaCXX/coreturn-eh.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/coreturn-eh.cpp?rev=346074&view=auto ============================================================================== --- cfe/trunk/test/SemaCXX/coreturn-eh.cpp (added) +++ cfe/trunk/test/SemaCXX/coreturn-eh.cpp Sat Nov 3 15:35:17 2018 @@ -0,0 +1,45 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts -fcxx-exceptions -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify -fblocks -Wall -Wextra -Wno-error=unreachable-code +// expected-no-diagnostics + +#include "Inputs/std-coroutine.h" + +using std::experimental::suspend_always; +using std::experimental::suspend_never; + +struct awaitable { + bool await_ready(); + void await_suspend(std::experimental::coroutine_handle<>); // FIXME: coroutine_handle + void await_resume(); +} a; + +struct object { ~object() {} }; + +struct promise_void_return_value { + void get_return_object(); + suspend_always initial_suspend(); + suspend_always final_suspend(); + void unhandled_exception(); + void return_value(object); +}; + +struct VoidTagReturnValue { + struct promise_type { + VoidTagReturnValue get_return_object(); + suspend_always initial_suspend(); + suspend_always final_suspend(); + void unhandled_exception(); + void return_value(object); + }; +}; + +template <typename T1> +struct std::experimental::coroutine_traits<void, T1> { using promise_type = promise_void_return_value; }; + +VoidTagReturnValue test() { + object x = {}; + try { + co_return {}; + } catch (...) { + throw; + } +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
