[clang] a73baf6 - [coroutine] Suppress unreachable-code warning on coroutine statements. (#77454)

[via cfe-commits]

Author: Haojian Wu
Date: 2024-02-05T08:55:27+01:00
New Revision: a73baf620b8374805b7e927cc79cc157a30e0ac8

URL: https://github.com/llvm/llvm-project/commit/a73baf620b8374805b7e927cc79cc157a30e0ac8
DIFF: https://github.com/llvm/llvm-project/commit/a73baf620b8374805b7e927cc79cc157a30e0ac8.diff

LOG: [coroutine] Suppress unreachable-code warning on coroutine statements. (#77454)

This fixes #69219.

Consider an example:

```
CoTask my_coroutine() {
    std::abort();
    co_return 1; // unreachable code warning.
}
```

Clang emits a CFG-based unreachable warning on the `co_return` statement
(precisely the `1` subexpr). If we remove this statement, the program
semantic is changed (my_coroutine is not a coroutine anymore).

This patch fixes this issue by never considering coroutine statements as
dead statements.

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

Modified: 
    clang/lib/Analysis/ReachableCode.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index 1bf0d9aec8620..acbe1470b3899 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ParentMap.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/CFG.h"
@@ -453,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. `Block` is the CFGBlock containing the `DeadStmt`.
+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 &&
+          // For simplicity, we only check simple coroutine statements.
+          (llvm::isa<CoreturnStmt>(S) || llvm::isa<CoroutineSuspendExpr>(S))) {
+        CoroStmt = S;
+        break;
+      }
+    }
+  if (!CoroStmt)
+    return false;
+  struct Checker : RecursiveASTVisitor<Checker> {
+    const Stmt *DeadStmt;
+    bool CoroutineSubStmt = false;
+    Checker(const Stmt *S) : DeadStmt(S) {}
+    bool VisitStmt(const Stmt *S) {
+      if (S == DeadStmt)
+        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;
   }
 

diff  --git a/clang/test/SemaCXX/coroutine-unreachable-warning.cpp b/clang/test/SemaCXX/coroutine-unreachable-warning.cpp
new file mode 100644
index 0000000000000..35890ee83a1be
--- /dev/null
+++ b/clang/test/SemaCXX/coroutine-unreachable-warning.cpp
@@ -0,0 +1,76 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -std=c++20 -fsyntax-only -verify -Wunreachable-code
+
+#include "Inputs/std-coroutine.h"
+
+extern void abort(void) __attribute__((__noreturn__));
+
+struct task {
+  struct promise_type {
+    std::suspend_always initial_suspend();
+    std::suspend_always final_suspend() noexcept;
+    void return_void();
+    std::suspend_always yield_value(int) { return {}; }
+    task get_return_object();
+    void unhandled_exception();
+
+    struct Awaiter {
+      bool await_ready();
+      void await_suspend(auto);
+      int await_resume();
+    };
+    auto await_transform(const int& x) { return Awaiter{}; }
+  };
+};
+
+task test1() {
+  abort();
+  co_yield 1;
+}
+
+task test2() {
+  abort();
+  1;  // expected-warning {{code will never be executed}}
+  co_yield 1;
+}
+
+task test3() {
+  abort();
+  co_return;
+}
+
+task test4() {
+  abort();
+  1;  // expected-warning {{code will never be executed}}
+  co_return;
+}
+
+task test5() {
+  abort();
+  co_await 1;
+}
+
+task test6() {
+  abort();
+  1;  // expected-warning {{code will never be executed}}
+  co_await 3;
+}
+
+task test7() {
+  // coroutine statements are not considered unreachable.
+  co_await 1;
+  abort();
+  co_await 2;
+}
+
+task test8() {
+  // coroutine statements are not considered unreachable.
+  abort();
+  co_return;
+  1 + 1;  // expected-warning {{code will never be executed}}
+}
+
+task test9() {
+  abort();
+  // This warning is emitted on the declaration itself, rather the coroutine substmt.
+  int x = co_await 1; // expected-warning {{code will never be executed}}
+}