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...@gmail.com> Date: Tue, 9 Jan 2024 12:29:45 +0100 Subject: [PATCH 1/2] [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 c8a195122b3fbde6351b3d455c0c75140849f002 Mon Sep 17 00:00:00 2001 From: Haojian Wu <hokein...@gmail.com> Date: Wed, 31 Jan 2024 16:08:32 +0100 Subject: [PATCH 2/2] Never treat coroutine statements as valid dead statements. --- clang/lib/Analysis/ReachableCode.cpp | 87 +++++++++---------- .../SemaCXX/coroutine-unreachable-warning.cpp | 17 +++- 2 files changed, 59 insertions(+), 45 deletions(-) diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp index d839d2f999609..fb303dea82472 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,64 @@ 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 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; + } + // 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 +662,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}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits