[clang] [coroutine] Suppress unreachable-code warning on coroutine statements. (PR #77454)
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 2 02:11:04 PST 2024
https://github.com/hokein updated https://github.com/llvm/llvm-project/pull/77454
>From 43810d2b18e1e31c5f15dc58c847c83b3c34d982 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Tue, 9 Jan 2024 12:29:45 +0100
Subject: [PATCH 1/3] [coroutine] Suppress unreachable-code warning on
coroutine statements.
---
clang/lib/Analysis/ReachableCode.cpp | 42 +++++++++++++++-
.../SemaCXX/coroutine-unreachable-warning.cpp | 50 +++++++++++++++++++
2 files changed, 91 insertions(+), 1 deletion(-)
create mode 100644 clang/test/SemaCXX/coroutine-unreachable-warning.cpp
diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index 1bf0d9aec8620..d839d2f999609 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"
@@ -60,6 +61,45 @@ 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()))
@@ -623,7 +663,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)) {
+ isBuiltinAssumeFalse(B, S, C) || isInCoroutineStmt(B, S)) {
return;
}
else if (isDeadReturn(B, 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..6ac5c34262b7e
--- /dev/null
+++ b/clang/test/SemaCXX/coroutine-unreachable-warning.cpp
@@ -0,0 +1,50 @@
+// 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();
+ };
+};
+
+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 std::suspend_never{};
+}
+
+task test6() {
+ abort();
+ 1; // expected-warning {{code will never be executed}}
+ co_await std::suspend_never{};
+}
>From ee6fa40a7864480a059225ef56ae6cf2187d5f18 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Wed, 31 Jan 2024 16:08:32 +0100
Subject: [PATCH 2/3] Never treat coroutine statements as valid dead
statements.
---
clang/lib/Analysis/ReachableCode.cpp | 91 ++++++++++---------
.../SemaCXX/coroutine-unreachable-warning.cpp | 17 +++-
2 files changed, 63 insertions(+), 45 deletions(-)
diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index d839d2f999609..1e92e6458a538 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/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)) {
diff --git a/clang/test/SemaCXX/coroutine-unreachable-warning.cpp b/clang/test/SemaCXX/coroutine-unreachable-warning.cpp
index 6ac5c34262b7e..26f80bef303b1 100644
--- a/clang/test/SemaCXX/coroutine-unreachable-warning.cpp
+++ b/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}}
+}
>From c4a22fbc29b4aa20085a575860cf2b66c908fbd4 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Fri, 2 Feb 2024 11:09:56 +0100
Subject: [PATCH 3/3] Address review comments, and add a test
---
clang/lib/Analysis/ReachableCode.cpp | 10 +++---
.../SemaCXX/coroutine-unreachable-warning.cpp | 33 ++++++++++++-------
2 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index 1e92e6458a538..ff97034a4a202 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -455,7 +455,7 @@ bool DeadCodeScan::isDeadCodeRoot(const clang::CFGBlock *Block) {
}
// Check if the given `DeadStmt` is a coroutine statement and is a substmt of
-// the coroutine statement.
+// 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;
@@ -468,6 +468,7 @@ static bool isInCoroutineStmt(const Stmt* DeadStmt, const CFGBlock *Block) {
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;
@@ -475,13 +476,12 @@ static bool isInCoroutineStmt(const Stmt* DeadStmt, const CFGBlock *Block) {
}
if (!CoroStmt)
return false;
-
struct Checker : RecursiveASTVisitor<Checker> {
- const Stmt *StmtToCheck;
+ const Stmt *DeadStmt;
bool CoroutineSubStmt = false;
- Checker(const Stmt *S) : StmtToCheck(S) {}
+ Checker(const Stmt *S) : DeadStmt(S) {}
bool VisitStmt(const Stmt *S) {
- if (S == StmtToCheck)
+ if (S == DeadStmt)
CoroutineSubStmt = true;
return true;
}
diff --git a/clang/test/SemaCXX/coroutine-unreachable-warning.cpp b/clang/test/SemaCXX/coroutine-unreachable-warning.cpp
index 26f80bef303b1..e9e07da48bc94 100644
--- a/clang/test/SemaCXX/coroutine-unreachable-warning.cpp
+++ b/clang/test/SemaCXX/coroutine-unreachable-warning.cpp
@@ -2,7 +2,7 @@
#include "Inputs/std-coroutine.h"
-extern void abort (void) __attribute__ ((__noreturn__));
+extern void abort(void) __attribute__((__noreturn__));
struct task {
struct promise_type {
@@ -12,6 +12,13 @@ struct task {
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{}; }
};
};
@@ -22,7 +29,7 @@ task test1() {
task test2() {
abort();
- 1; // expected-warning {{code will never be executed}}
+ 1; // expected-warning {{code will never be executed}}
co_yield 1;
}
@@ -33,33 +40,37 @@ task test3() {
task test4() {
abort();
- 1; // expected-warning {{code will never be executed}}
+ 1; // expected-warning {{code will never be executed}}
co_return;
}
-
task test5() {
abort();
- co_await std::suspend_never{};
+ co_await 1;
}
task test6() {
abort();
- 1; // expected-warning {{code will never be executed}}
- co_await std::suspend_never{};
+ 1; // expected-warning {{code will never be executed}}
+ co_await 3;
}
task test7() {
// coroutine statements are not considered unreachable.
- co_await std::suspend_never{};
+ co_await 1;
abort();
- co_await std::suspend_never{};
+ co_await 2;
}
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}}
+ 1 + 1; // expected-warning {{code will never be executed}}
+}
+
+task test9() {
+ abort();
+ // This warning is emit on the declaration itself, rather the coroutine substmt.
+ int x = co_await 1; // expected-warning {{code will never be executed}}
}
More information about the cfe-commits
mailing list