[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

llvm-svn: 346074

1 files changed, 15 insertions, 12 deletions

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 442aaf35afc..c7724e200b7 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -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, AddStmtChoice asc) {
       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 *I) {
   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(ReturnStmt *R) {
 
   // 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) {