Skip to content

Commit

Permalink
[coroutine] Suppress unreachable-code warning on coroutine statements. (
Browse files Browse the repository at this point in the history
#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.
  • Loading branch information
hokein committed Feb 5, 2024
1 parent cfa0833 commit a73baf6
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 4 deletions.
51 changes: 47 additions & 4 deletions clang/lib/Analysis/ReachableCode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
}

Expand Down
76 changes: 76 additions & 0 deletions clang/test/SemaCXX/coroutine-unreachable-warning.cpp
Original file line number Diff line number Diff line change
@@ -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}}
}

0 comments on commit a73baf6

Please sign in to comment.