Never treat coroutine statements as valid dead statements.

Author: hokein

clang/lib/Analysis/ReachableCode.cpp

@@ -61,45 +61,6 @@ static bool isTrivialDoWhile(const CFGBlock *B, const Stmt *S) {
   return false;
 }
 
-// Check if the block starts with a coroutine statement and see if the given
-// unreachable 'S' is the substmt of the coroutine statement.
-//
-// We suppress the unreachable warning for cases where an unreachable code is
-// a substmt of the coroutine statement, becase removing it will change the
-// function semantic if this is the only coroutine statement of the coroutine.
-static bool isInCoroutineStmt(const CFGBlock *Block, const Stmt* S) {
-  // The coroutine statement, co_return, co_await, or co_yield.
-  const Stmt* CoroStmt = nullptr;
-  // Find the first coroutine statement in the block.
-  for (CFGBlock::const_iterator I = Block->begin(), E = Block->end(); I != E;
-       ++I)
-    if (std::optional<CFGStmt> CS = I->getAs<CFGStmt>()) {
-      const Stmt *S = CS->getStmt();
-      if (llvm::isa<CoreturnStmt>(S) || llvm::isa<CoroutineSuspendExpr>(S)) {
-        CoroStmt = S ;
-        break;
-      }
-    }
-  if (!CoroStmt)
-    return false;
-
-  struct Checker : RecursiveASTVisitor<Checker> {
-    const Stmt *StmtToCheck;
-    bool CoroutineSubStmt = false;
-    Checker(const Stmt *S) : StmtToCheck(S) {}
-    bool VisitStmt(const Stmt *S) {
-      if (S == StmtToCheck)
-        CoroutineSubStmt = true;
-      return true;
-    }
-    // The 'S' stmt captured in the CFG can be implicit.
-    bool shouldVisitImplicitCode() const { return true; }
-  };
-  Checker checker(S);
-  checker.TraverseStmt(const_cast<Stmt *>(CoroStmt));
-  return checker.CoroutineSubStmt;
-}
-
 static bool isBuiltinUnreachable(const Stmt *S) {
   if (const auto *DRE = dyn_cast<DeclRefExpr>(S))
     if (const auto *FDecl = dyn_cast<FunctionDecl>(DRE->getDecl()))
@@ -493,26 +454,68 @@ bool DeadCodeScan::isDeadCodeRoot(const clang::CFGBlock *Block) {
   return isDeadRoot;
 }
 
-static bool isValidDeadStmt(const Stmt *S) {
+// Check if the given `DeadStmt` is a coroutine statement and is a substmt of
+// the coroutine statement.
+static bool isInCoroutineStmt(const Stmt* DeadStmt, const CFGBlock *Block) {
+  // The coroutine statement, co_return, co_await, or co_yield.
+  const Stmt* CoroStmt = nullptr;
+  // Find the first coroutine statement after the DeadStmt in the block.
+  bool AfterDeadStmt = false;
+  for (CFGBlock::const_iterator I = Block->begin(), E = Block->end(); I != E;
+       ++I)
+    if (std::optional<CFGStmt> CS = I->getAs<CFGStmt>()) {
+      const Stmt *S = CS->getStmt();
+      if (S == DeadStmt)
+        AfterDeadStmt = true;
+      if (AfterDeadStmt &&
+          (llvm::isa<CoreturnStmt>(S) || llvm::isa<CoroutineSuspendExpr>(S))) {
+        CoroStmt = S;
+        break;
+      }
+    }
+  if (!CoroStmt)
+    return false;
+
+  struct Checker : RecursiveASTVisitor<Checker> {
+    const Stmt *StmtToCheck;
+    bool CoroutineSubStmt = false;
+    Checker(const Stmt *S) : StmtToCheck(S) {}
+    bool VisitStmt(const Stmt *S) {
+      if (S == StmtToCheck)
+        CoroutineSubStmt = true;
+      return true;
+    }
+    // Statements captured in the CFG can be implicit.
+    bool shouldVisitImplicitCode() const { return true; }
+  };
+  Checker checker(DeadStmt);
+  checker.TraverseStmt(const_cast<Stmt *>(CoroStmt));
+  return checker.CoroutineSubStmt;
+}
+
+static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
   if (S->getBeginLoc().isInvalid())
     return false;
   if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(S))
     return BO->getOpcode() != BO_Comma;
-  return true;
+  // Coroutine statements are never considered dead statements, because removing
+  // them may change the function semantic if it is the only coroutine statement
+  // of the coroutine.
+  return !isInCoroutineStmt(S, Block);
 }
 
 const Stmt *DeadCodeScan::findDeadCode(const clang::CFGBlock *Block) {
   for (CFGBlock::const_iterator I = Block->begin(), E = Block->end(); I!=E; ++I)
     if (std::optional<CFGStmt> CS = I->getAs<CFGStmt>()) {
       const Stmt *S = CS->getStmt();
-      if (isValidDeadStmt(S))
+      if (isValidDeadStmt(S, Block))
         return S;
     }
 
   CFGTerminator T = Block->getTerminator();
   if (T.isStmtBranch()) {
     const Stmt *S = T.getStmt();
-    if (S && isValidDeadStmt(S))
+    if (S && isValidDeadStmt(S, Block))
       return S;
   }
 
@@ -663,7 +666,7 @@ void DeadCodeScan::reportDeadCode(const CFGBlock *B,
   if (isa<BreakStmt>(S)) {
     UK = reachable_code::UK_Break;
   } else if (isTrivialDoWhile(B, S) || isBuiltinUnreachable(S) ||
-             isBuiltinAssumeFalse(B, S, C) || isInCoroutineStmt(B, S)) {
+             isBuiltinAssumeFalse(B, S, C)) {
     return;
   }
   else if (isDeadReturn(B, S)) {

clang/test/SemaCXX/coroutine-unreachable-warning.cpp

@@ -10,7 +10,7 @@ struct task {
     std::suspend_always final_suspend() noexcept;
     void return_void();
     std::suspend_always yield_value(int) { return {}; }
-      task get_return_object();
+    task get_return_object();
     void unhandled_exception();
   };
 };
@@ -48,3 +48,18 @@ task test6() {
   1; // expected-warning {{code will never be executed}}
   co_await std::suspend_never{};
 }
+
+task test7() {
+  // coroutine statements are not considered unreachable.
+  co_await std::suspend_never{};
+  abort();
+  co_await std::suspend_never{};
+}
+
+task test8() {
+  // coroutine statements are not considered unreachable.
+  // co_await std::suspend_never{};
+  abort();
+  co_return;
+  1 + 1; // expected-warning {{code will never be executed}}
+}