Reapply [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression #146281
+284
−93
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yronglin
requested review from
Xazax-hun,
jvoung,
hokein,
steakhal,
martinboehme and
Sirraide
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang Author: None (yronglin) ChangesThis PR reapply #127338. Patch is 22.68 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/146281.diff 12 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 01f3b7a557a5c..929c688ee7c8c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1064,6 +1064,9 @@ Static Analyzer
---------------
- Fixed a crash when C++20 parenthesized initializer lists are used. This issue
was causing a crash in clang-tidy. (#GH136041)
+- Clang currently support extending lifetime of object bound to
+ reference members of aggregates in CFG and ExprEngine, that are
+ created from default member initializer.
New features
^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/ParentMap.h b/clang/include/clang/AST/ParentMap.h
index 86e2f048a3445..5853a0be0a483 100644
--- a/clang/include/clang/AST/ParentMap.h
+++ b/clang/include/clang/AST/ParentMap.h
@@ -13,6 +13,8 @@
#ifndef LLVM_CLANG_AST_PARENTMAP_H
#define LLVM_CLANG_AST_PARENTMAP_H
+#include "llvm/Support/Casting.h"
+
namespace clang {
class Stmt;
class Expr;
@@ -39,6 +41,25 @@ class ParentMap {
Stmt *getParentIgnoreParenImpCasts(Stmt *) const;
Stmt *getOuterParenParent(Stmt *) const;
+ template <typename... Ts> Stmt *getOuterMostAncestor(Stmt *S) const {
+ Stmt *Res = nullptr;
+ while (S) {
+ if (llvm::isa<Ts...>(S))
+ Res = S;
+ S = getParent(S);
+ }
+ return Res;
+ }
+
+ template <typename... Ts> Stmt *getInnerMostAncestor(Stmt *S) const {
+ while (S) {
+ if (llvm::isa<Ts...>(S))
+ return S;
+ S = getParent(S);
+ }
+ return nullptr;
+ }
+
const Stmt *getParent(const Stmt* S) const {
return getParent(const_cast<Stmt*>(S));
}
@@ -51,6 +72,16 @@ class ParentMap {
return getParentIgnoreParenCasts(const_cast<Stmt*>(S));
}
+ template <typename... Ts>
+ const Stmt *getOuterMostAncestor(const Stmt *S) const {
+ return getOuterMostAncestor<Ts...>(const_cast<Stmt *>(S));
+ }
+
+ template <typename... Ts>
+ const Stmt *getInnerMostAncestor(const Stmt *S) const {
+ return getInnerMostAncestor<Ts...>(const_cast<Stmt *>(S));
+ }
+
bool hasParent(const Stmt *S) const { return getParent(S) != nullptr; }
bool isConsumedExpr(Expr *E) const;
diff --git a/clang/lib/AST/ParentMap.cpp b/clang/lib/AST/ParentMap.cpp
index e62e71bf5a514..580613b2618fb 100644
--- a/clang/lib/AST/ParentMap.cpp
+++ b/clang/lib/AST/ParentMap.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/ParentMap.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/StmtObjC.h"
#include "llvm/ADT/DenseMap.h"
@@ -103,6 +104,22 @@ static void BuildParentMap(MapTy& M, Stmt* S,
BuildParentMap(M, SubStmt, OVMode);
}
break;
+ case Stmt::CXXDefaultArgExprClass:
+ if (auto *Arg = dyn_cast<CXXDefaultArgExpr>(S)) {
+ if (Arg->hasRewrittenInit()) {
+ M[Arg->getExpr()] = S;
+ BuildParentMap(M, Arg->getExpr(), OVMode);
+ }
+ }
+ break;
+ case Stmt::CXXDefaultInitExprClass:
+ if (auto *Init = dyn_cast<CXXDefaultInitExpr>(S)) {
+ if (Init->hasRewrittenInit()) {
+ M[Init->getExpr()] = S;
+ BuildParentMap(M, Init->getExpr(), OVMode);
+ }
+ }
+ break;
default:
for (Stmt *SubStmt : S->children()) {
if (SubStmt) {
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index cf7595952be27..76671976db471 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -578,6 +578,10 @@ class CFGBuilder {
private:
// Visitors to walk an AST and construct the CFG.
+ CFGBlock *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Default,
+ AddStmtChoice asc);
+ CFGBlock *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Default,
+ AddStmtChoice asc);
CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
CFGBlock *VisitAttributedStmt(AttributedStmt *A, AddStmtChoice asc);
@@ -2345,16 +2349,10 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
asc, ExternallyDestructed);
case Stmt::CXXDefaultArgExprClass:
+ return VisitCXXDefaultArgExpr(cast<CXXDefaultArgExpr>(S), asc);
+
case Stmt::CXXDefaultInitExprClass:
- // FIXME: The expression inside a CXXDefaultArgExpr is owned by the
- // called function's declaration, not by the caller. If we simply add
- // this expression to the CFG, we could end up with the same Expr
- // appearing multiple times (PR13385).
- //
- // It's likewise possible for multiple CXXDefaultInitExprs for the same
- // expression to be used in the same function (through aggregate
- // initialization).
- return VisitStmt(S, asc);
+ return VisitCXXDefaultInitExpr(cast<CXXDefaultInitExpr>(S), asc);
case Stmt::CXXBindTemporaryExprClass:
return VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), asc);
@@ -2524,6 +2522,44 @@ CFGBlock *CFGBuilder::VisitChildren(Stmt *S) {
return B;
}
+CFGBlock *CFGBuilder::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Arg,
+ AddStmtChoice asc) {
+ if (Arg->hasRewrittenInit()) {
+ if (asc.alwaysAdd(*this, Arg)) {
+ autoCreateBlock();
+ appendStmt(Block, Arg);
+ }
+ return VisitStmt(Arg->getExpr()->IgnoreParens(), asc);
+ }
+
+ // We can't add the default argument if it's not rewritten because the
+ // expression inside a CXXDefaultArgExpr is owned by the called function's
+ // declaration, not by the caller, we could end up with the same expression
+ // appearing multiple times.
+ return VisitStmt(Arg, asc);
+}
+
+CFGBlock *CFGBuilder::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Init,
+ AddStmtChoice asc) {
+ if (Init->hasRewrittenInit()) {
+ if (asc.alwaysAdd(*this, Init)) {
+ autoCreateBlock();
+ appendStmt(Block, Init);
+ }
+
+ // Unlike CXXDefaultArgExpr::getExpr stripped off the top level FullExpr and
+ // ConstantExpr, CXXDefaultInitExpr::getExpr does not do this, so we don't
+ // need to ignore ParenExprs, because the top level will not be a ParenExpr.
+ return VisitStmt(Init->getExpr(), asc);
+ }
+
+ // We can't add the default initializer if it's not rewritten because multiple
+ // CXXDefaultInitExprs for the same sub-expression to be used in the same
+ // function (through aggregate initialization). we could end up with the same
+ // expression appearing multiple times.
+ return VisitStmt(Init, asc);
+}
+
CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
if (asc.alwaysAdd(*this, ILE)) {
autoCreateBlock();
diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index 739c47b12e8c4..a8f6df5f39e5d 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -25,6 +25,7 @@
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/BitVector.h"
+#include <cstddef>
#include <optional>
using namespace clang;
@@ -396,6 +397,7 @@ namespace {
SmallVector<const CFGBlock *, 10> WorkList;
Preprocessor &PP;
ASTContext &C;
+ AnalysisDeclContext &AC;
typedef SmallVector<std::pair<const CFGBlock *, const Stmt *>, 12>
DeferredLocsTy;
@@ -403,10 +405,10 @@ namespace {
DeferredLocsTy DeferredLocs;
public:
- DeadCodeScan(llvm::BitVector &reachable, Preprocessor &PP, ASTContext &C)
+ DeadCodeScan(llvm::BitVector &reachable, Preprocessor &PP, AnalysisDeclContext &AC)
: Visited(reachable.size()),
Reachable(reachable),
- PP(PP), C(C) {}
+ PP(PP), C(AC.getASTContext()), AC(AC) {}
void enqueue(const CFGBlock *block);
unsigned scanBackwards(const CFGBlock *Start,
@@ -453,48 +455,7 @@ bool DeadCodeScan::isDeadCodeRoot(const clang::CFGBlock *Block) {
return isDeadRoot;
}
-// 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 : DynamicRecursiveASTVisitor {
- const Stmt *DeadStmt;
- bool CoroutineSubStmt = false;
- Checker(const Stmt *S) : DeadStmt(S) {
- // Statements captured in the CFG can be implicit.
- ShouldVisitImplicitCode = true;
- }
-
- bool VisitStmt(Stmt *S) override {
- if (S == DeadStmt)
- CoroutineSubStmt = true;
- return true;
- }
- };
- Checker checker(DeadStmt);
- checker.TraverseStmt(const_cast<Stmt *>(CoroStmt));
- return checker.CoroutineSubStmt;
-}
-
-static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
+static bool isValidDeadStmt(ParentMap &PM, const Stmt *S, const clang::CFGBlock *Block) {
if (S->getBeginLoc().isInvalid())
return false;
if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(S))
@@ -502,21 +463,27 @@ static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
// 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);
+ return !PM.getInnerMostAncestor<CoreturnStmt, CoroutineSuspendExpr>(S);
}
const Stmt *DeadCodeScan::findDeadCode(const clang::CFGBlock *Block) {
+ auto &PM = AC.getParentMap();
+
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, Block))
+ auto *RewrittenParent =
+ PM.getOuterMostAncestor<CXXDefaultArgExpr, CXXDefaultInitExpr>(S);
+ if (RewrittenParent)
+ S = RewrittenParent;
+ if (isValidDeadStmt(AC.getParentMap(), S, Block))
return S;
}
CFGTerminator T = Block->getTerminator();
if (T.isStmtBranch()) {
const Stmt *S = T.getStmt();
- if (S && isValidDeadStmt(S, Block))
+ if (S && isValidDeadStmt(AC.getParentMap(), S, Block))
return S;
}
@@ -762,7 +729,7 @@ void FindUnreachableCode(AnalysisDeclContext &AC, Preprocessor &PP,
if (reachable[block->getBlockID()])
continue;
- DeadCodeScan DS(reachable, PP, AC.getASTContext());
+ DeadCodeScan DS(reachable, PP, AC);
numReachable += DS.scanBackwards(block, CB);
if (numReachable == cfg->getNumBlockIDs())
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index a3f534ee6712e..eb487fe214db3 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5623,8 +5623,10 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
/*SkipImmediateInvocations=*/NestedDefaultChecking))
return ExprError();
+ Expr *RewrittenExpr = (Init == Param->getDefaultArg() ? nullptr : Init);
return CXXDefaultArgExpr::Create(Context, InitializationContext->Loc, Param,
- Init, InitializationContext->Context);
+ RewrittenExpr,
+ InitializationContext->Context);
}
static FieldDecl *FindFieldDeclInstantiationPattern(const ASTContext &Ctx,
@@ -5742,10 +5744,11 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
return ExprError();
}
Init = Res.get();
-
+ Expr *RewrittenInit =
+ (Init == Field->getInClassInitializer() ? nullptr : Init);
return CXXDefaultInitExpr::Create(Context, InitializationContext->Loc,
Field, InitializationContext->Context,
- Init);
+ RewrittenInit);
}
// DR1351:
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index c77ef26da568d..418399f98268b 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1995,33 +1995,45 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
ExplodedNodeSet Tmp;
StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
- const Expr *ArgE;
- if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S))
+ bool HasRebuiltInit = false;
+ const Expr *ArgE = nullptr;
+ if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S)) {
ArgE = DefE->getExpr();
- else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S))
+ HasRebuiltInit = DefE->hasRewrittenInit();
+ } else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S)) {
ArgE = DefE->getExpr();
- else
+ HasRebuiltInit = DefE->hasRewrittenInit();
+ } else
llvm_unreachable("unknown constant wrapper kind");
- bool IsTemporary = false;
- if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) {
- ArgE = MTE->getSubExpr();
- IsTemporary = true;
- }
+ if (HasRebuiltInit) {
+ for (auto *N : PreVisit) {
+ ProgramStateRef state = N->getState();
+ const LocationContext *LCtx = N->getLocationContext();
+ state = state->BindExpr(S, LCtx, state->getSVal(ArgE, LCtx));
+ Bldr2.generateNode(S, N, state);
+ }
+ } else {
+ // If it's not rewritten, the contents of these expressions are not
+ // actually part of the current function, so we fall back to constant
+ // evaluation.
+ bool IsTemporary = false;
+ if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) {
+ ArgE = MTE->getSubExpr();
+ IsTemporary = true;
+ }
+
+ std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
+ const LocationContext *LCtx = Pred->getLocationContext();
+ for (auto *I : PreVisit) {
+ ProgramStateRef State = I->getState();
+ State = State->BindExpr(S, LCtx, ConstantVal.value_or(UnknownVal()));
+ if (IsTemporary)
+ State = createTemporaryRegionIfNeeded(State, LCtx, cast<Expr>(S),
+ cast<Expr>(S));
- std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
- if (!ConstantVal)
- ConstantVal = UnknownVal();
-
- const LocationContext *LCtx = Pred->getLocationContext();
- for (const auto I : PreVisit) {
- ProgramStateRef State = I->getState();
- State = State->BindExpr(S, LCtx, *ConstantVal);
- if (IsTemporary)
- State = createTemporaryRegionIfNeeded(State, LCtx,
- cast<Expr>(S),
- cast<Expr>(S));
- Bldr2.generateNode(S, I, State);
+ Bldr2.generateNode(S, I, State);
+ }
}
getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
diff --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp
index a8e30f1759e9f..5294819da2aa6 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -294,7 +294,7 @@ union U {
// CHECK-NEXT: `-DeclStmt {{.*}}
// CHECK-NEXT: `-VarDecl {{.*}} g 'U':'GH112560::U' listinit
// CHECK-NEXT: `-InitListExpr {{.*}} 'U':'GH112560::U' contains-errors field Field {{.*}} 'f' 'int'
-// CHECK-NEXT: `-CXXDefaultInitExpr {{.*}} 'int' contains-errors has rewritten init
+// CHECK-NEXT: `-CXXDefaultInitExpr {{.*}} 'int' contains-errors
// CHECK-NEXT: `-RecoveryExpr {{.*}} 'int' contains-errors
// DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
void foo() {
diff --git a/clang/test/Analysis/lifetime-extended-regions.cpp b/clang/test/Analysis/lifetime-extended-regions.cpp
index 4458ad294af7c..02a1210d9af92 100644
--- a/clang/test/Analysis/lifetime-extended-regions.cpp
+++ b/clang/test/Analysis/lifetime-extended-regions.cpp
@@ -121,11 +121,10 @@ void aggregateWithReferences() {
clang_analyzer_dump(viaReference.rx); // expected-warning-re {{&lifetime_extended_object{int, viaReference, S{{[0-9]+}}} }}
clang_analyzer_dump(viaReference.ry); // expected-warning-re {{&lifetime_extended_object{Composite, viaReference, S{{[0-9]+}}} }}
- // FIXME: clang currently support extending lifetime of object bound to reference members of aggregates,
- // that are created from default member initializer. But CFG and ExprEngine need to be updated to address this change.
- // The following expect warning: {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
+ // The lifetime of object bound to reference members of aggregates,
+ // that are created from default member initializer was extended.
RefAggregate defaultInitExtended{i};
- clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }}
+ clang_analyzer_dump(defaultInitExtended.ry); // expected-warning-re {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
}
void lambda() {
diff --git a/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp b/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
index 8e428c0ef0427..37824c16f4f05 100644
--- a/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
+++ b/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -274,16 +274,16 @@ void f() {
// CHECK: ClassTemplateSpecializationDecl {{.*}} struct A definition
// CHECK: CXXConstructorDecl {{.*}} implicit used constexpr A 'void () noexcept'
// CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int'
-// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
+// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int'
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 1
// CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int'
-// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
+// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int'
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 2
// CHECK-NEXT: CXXCtorInitializer Field {{.*}} 'a' 'int'
-// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
+// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int'
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 3
// CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int'
-// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
+// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int'
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 4
// CHECK-NEXT: CompoundStmt {{.*}}
diff --git a/clang/test/SemaCXX/warn-unreachable.cpp b/clang/test/SemaCXX/warn-unreachable.cpp
index e6f5bc5ef8e12..f8a8d17049abb 100644
--- a/clang/test/SemaCXX/warn-unreachable.cpp
+++ b/clang/test/SemaCXX/warn-unreachable.cpp
@@ -414,3 +414,107 @@ void tautological_compare(bool x, int y) {
calledFun();
}
+
+namespace test_rebuilt_default_arg {
+struct A {
+ explicit A(int = __builtin_LINE());
+};
+
+int h(int a) {
+ return 3;
+ A(); // expected-warning {{will never be executed}}
+}
+
+struct Temp {
+ Temp();
+ ~Temp();
+};
+
+struct B {
+ explicit B(const Temp &t = Temp());
+};
+int f(int a) {
+ return 3;
+ B(); // expected-warning {{will never be executed}}
+}
+} // namespace test_rebuilt_default_arg
+namespace test_rebuilt_default_init {
+
+struct A {
+ A();
+ ~A();
+};
+
+struct B {
+ const A &t = A();
+};
+int f(int a) {
+ return 3;
+ A{}; // expected-warning {{will never be executed}}
+}
+} // namespace test_rebuilt_default_init
+
+// This issue reported by the comments in https://github.com/llvm/llvm-project/pull/117437.
+// All block-level expressions should have already been IgnoreParens()ed.
+namespace gh117437_ignore_parens_in_default_arg {
+ ...
[truncated]
|
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: None (yronglin) ChangesThis PR reapply #127338. Patch is 22.68 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/146281.diff 12 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 01f3b7a557a5c..929c688ee7c8c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1064,6 +1064,9 @@ Static Analyzer
---------------
- Fixed a crash when C++20 parenthesized initializer lists are used. This issue
was causing a crash in clang-tidy. (#GH136041)
+- Clang currently support extending lifetime of object bound to
+ reference members of aggregates in CFG and ExprEngine, that are
+ created from default member initializer.
New features
^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/ParentMap.h b/clang/include/clang/AST/ParentMap.h
index 86e2f048a3445..5853a0be0a483 100644
--- a/clang/include/clang/AST/ParentMap.h
+++ b/clang/include/clang/AST/ParentMap.h
@@ -13,6 +13,8 @@
#ifndef LLVM_CLANG_AST_PARENTMAP_H
#define LLVM_CLANG_AST_PARENTMAP_H
+#include "llvm/Support/Casting.h"
+
namespace clang {
class Stmt;
class Expr;
@@ -39,6 +41,25 @@ class ParentMap {
Stmt *getParentIgnoreParenImpCasts(Stmt *) const;
Stmt *getOuterParenParent(Stmt *) const;
+ template <typename... Ts> Stmt *getOuterMostAncestor(Stmt *S) const {
+ Stmt *Res = nullptr;
+ while (S) {
+ if (llvm::isa<Ts...>(S))
+ Res = S;
+ S = getParent(S);
+ }
+ return Res;
+ }
+
+ template <typename... Ts> Stmt *getInnerMostAncestor(Stmt *S) const {
+ while (S) {
+ if (llvm::isa<Ts...>(S))
+ return S;
+ S = getParent(S);
+ }
+ return nullptr;
+ }
+
const Stmt *getParent(const Stmt* S) const {
return getParent(const_cast<Stmt*>(S));
}
@@ -51,6 +72,16 @@ class ParentMap {
return getParentIgnoreParenCasts(const_cast<Stmt*>(S));
}
+ template <typename... Ts>
+ const Stmt *getOuterMostAncestor(const Stmt *S) const {
+ return getOuterMostAncestor<Ts...>(const_cast<Stmt *>(S));
+ }
+
+ template <typename... Ts>
+ const Stmt *getInnerMostAncestor(const Stmt *S) const {
+ return getInnerMostAncestor<Ts...>(const_cast<Stmt *>(S));
+ }
+
bool hasParent(const Stmt *S) const { return getParent(S) != nullptr; }
bool isConsumedExpr(Expr *E) const;
diff --git a/clang/lib/AST/ParentMap.cpp b/clang/lib/AST/ParentMap.cpp
index e62e71bf5a514..580613b2618fb 100644
--- a/clang/lib/AST/ParentMap.cpp
+++ b/clang/lib/AST/ParentMap.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/ParentMap.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/StmtObjC.h"
#include "llvm/ADT/DenseMap.h"
@@ -103,6 +104,22 @@ static void BuildParentMap(MapTy& M, Stmt* S,
BuildParentMap(M, SubStmt, OVMode);
}
break;
+ case Stmt::CXXDefaultArgExprClass:
+ if (auto *Arg = dyn_cast<CXXDefaultArgExpr>(S)) {
+ if (Arg->hasRewrittenInit()) {
+ M[Arg->getExpr()] = S;
+ BuildParentMap(M, Arg->getExpr(), OVMode);
+ }
+ }
+ break;
+ case Stmt::CXXDefaultInitExprClass:
+ if (auto *Init = dyn_cast<CXXDefaultInitExpr>(S)) {
+ if (Init->hasRewrittenInit()) {
+ M[Init->getExpr()] = S;
+ BuildParentMap(M, Init->getExpr(), OVMode);
+ }
+ }
+ break;
default:
for (Stmt *SubStmt : S->children()) {
if (SubStmt) {
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index cf7595952be27..76671976db471 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -578,6 +578,10 @@ class CFGBuilder {
private:
// Visitors to walk an AST and construct the CFG.
+ CFGBlock *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Default,
+ AddStmtChoice asc);
+ CFGBlock *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Default,
+ AddStmtChoice asc);
CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
CFGBlock *VisitAttributedStmt(AttributedStmt *A, AddStmtChoice asc);
@@ -2345,16 +2349,10 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
asc, ExternallyDestructed);
case Stmt::CXXDefaultArgExprClass:
+ return VisitCXXDefaultArgExpr(cast<CXXDefaultArgExpr>(S), asc);
+
case Stmt::CXXDefaultInitExprClass:
- // FIXME: The expression inside a CXXDefaultArgExpr is owned by the
- // called function's declaration, not by the caller. If we simply add
- // this expression to the CFG, we could end up with the same Expr
- // appearing multiple times (PR13385).
- //
- // It's likewise possible for multiple CXXDefaultInitExprs for the same
- // expression to be used in the same function (through aggregate
- // initialization).
- return VisitStmt(S, asc);
+ return VisitCXXDefaultInitExpr(cast<CXXDefaultInitExpr>(S), asc);
case Stmt::CXXBindTemporaryExprClass:
return VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), asc);
@@ -2524,6 +2522,44 @@ CFGBlock *CFGBuilder::VisitChildren(Stmt *S) {
return B;
}
+CFGBlock *CFGBuilder::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Arg,
+ AddStmtChoice asc) {
+ if (Arg->hasRewrittenInit()) {
+ if (asc.alwaysAdd(*this, Arg)) {
+ autoCreateBlock();
+ appendStmt(Block, Arg);
+ }
+ return VisitStmt(Arg->getExpr()->IgnoreParens(), asc);
+ }
+
+ // We can't add the default argument if it's not rewritten because the
+ // expression inside a CXXDefaultArgExpr is owned by the called function's
+ // declaration, not by the caller, we could end up with the same expression
+ // appearing multiple times.
+ return VisitStmt(Arg, asc);
+}
+
+CFGBlock *CFGBuilder::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Init,
+ AddStmtChoice asc) {
+ if (Init->hasRewrittenInit()) {
+ if (asc.alwaysAdd(*this, Init)) {
+ autoCreateBlock();
+ appendStmt(Block, Init);
+ }
+
+ // Unlike CXXDefaultArgExpr::getExpr stripped off the top level FullExpr and
+ // ConstantExpr, CXXDefaultInitExpr::getExpr does not do this, so we don't
+ // need to ignore ParenExprs, because the top level will not be a ParenExpr.
+ return VisitStmt(Init->getExpr(), asc);
+ }
+
+ // We can't add the default initializer if it's not rewritten because multiple
+ // CXXDefaultInitExprs for the same sub-expression to be used in the same
+ // function (through aggregate initialization). we could end up with the same
+ // expression appearing multiple times.
+ return VisitStmt(Init, asc);
+}
+
CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
if (asc.alwaysAdd(*this, ILE)) {
autoCreateBlock();
diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index 739c47b12e8c4..a8f6df5f39e5d 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -25,6 +25,7 @@
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/BitVector.h"
+#include <cstddef>
#include <optional>
using namespace clang;
@@ -396,6 +397,7 @@ namespace {
SmallVector<const CFGBlock *, 10> WorkList;
Preprocessor &PP;
ASTContext &C;
+ AnalysisDeclContext &AC;
typedef SmallVector<std::pair<const CFGBlock *, const Stmt *>, 12>
DeferredLocsTy;
@@ -403,10 +405,10 @@ namespace {
DeferredLocsTy DeferredLocs;
public:
- DeadCodeScan(llvm::BitVector &reachable, Preprocessor &PP, ASTContext &C)
+ DeadCodeScan(llvm::BitVector &reachable, Preprocessor &PP, AnalysisDeclContext &AC)
: Visited(reachable.size()),
Reachable(reachable),
- PP(PP), C(C) {}
+ PP(PP), C(AC.getASTContext()), AC(AC) {}
void enqueue(const CFGBlock *block);
unsigned scanBackwards(const CFGBlock *Start,
@@ -453,48 +455,7 @@ bool DeadCodeScan::isDeadCodeRoot(const clang::CFGBlock *Block) {
return isDeadRoot;
}
-// 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 : DynamicRecursiveASTVisitor {
- const Stmt *DeadStmt;
- bool CoroutineSubStmt = false;
- Checker(const Stmt *S) : DeadStmt(S) {
- // Statements captured in the CFG can be implicit.
- ShouldVisitImplicitCode = true;
- }
-
- bool VisitStmt(Stmt *S) override {
- if (S == DeadStmt)
- CoroutineSubStmt = true;
- return true;
- }
- };
- Checker checker(DeadStmt);
- checker.TraverseStmt(const_cast<Stmt *>(CoroStmt));
- return checker.CoroutineSubStmt;
-}
-
-static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
+static bool isValidDeadStmt(ParentMap &PM, const Stmt *S, const clang::CFGBlock *Block) {
if (S->getBeginLoc().isInvalid())
return false;
if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(S))
@@ -502,21 +463,27 @@ static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
// 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);
+ return !PM.getInnerMostAncestor<CoreturnStmt, CoroutineSuspendExpr>(S);
}
const Stmt *DeadCodeScan::findDeadCode(const clang::CFGBlock *Block) {
+ auto &PM = AC.getParentMap();
+
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, Block))
+ auto *RewrittenParent =
+ PM.getOuterMostAncestor<CXXDefaultArgExpr, CXXDefaultInitExpr>(S);
+ if (RewrittenParent)
+ S = RewrittenParent;
+ if (isValidDeadStmt(AC.getParentMap(), S, Block))
return S;
}
CFGTerminator T = Block->getTerminator();
if (T.isStmtBranch()) {
const Stmt *S = T.getStmt();
- if (S && isValidDeadStmt(S, Block))
+ if (S && isValidDeadStmt(AC.getParentMap(), S, Block))
return S;
}
@@ -762,7 +729,7 @@ void FindUnreachableCode(AnalysisDeclContext &AC, Preprocessor &PP,
if (reachable[block->getBlockID()])
continue;
- DeadCodeScan DS(reachable, PP, AC.getASTContext());
+ DeadCodeScan DS(reachable, PP, AC);
numReachable += DS.scanBackwards(block, CB);
if (numReachable == cfg->getNumBlockIDs())
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index a3f534ee6712e..eb487fe214db3 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5623,8 +5623,10 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
/*SkipImmediateInvocations=*/NestedDefaultChecking))
return ExprError();
+ Expr *RewrittenExpr = (Init == Param->getDefaultArg() ? nullptr : Init);
return CXXDefaultArgExpr::Create(Context, InitializationContext->Loc, Param,
- Init, InitializationContext->Context);
+ RewrittenExpr,
+ InitializationContext->Context);
}
static FieldDecl *FindFieldDeclInstantiationPattern(const ASTContext &Ctx,
@@ -5742,10 +5744,11 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
return ExprError();
}
Init = Res.get();
-
+ Expr *RewrittenInit =
+ (Init == Field->getInClassInitializer() ? nullptr : Init);
return CXXDefaultInitExpr::Create(Context, InitializationContext->Loc,
Field, InitializationContext->Context,
- Init);
+ RewrittenInit);
}
// DR1351:
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index c77ef26da568d..418399f98268b 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1995,33 +1995,45 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
ExplodedNodeSet Tmp;
StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
- const Expr *ArgE;
- if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S))
+ bool HasRebuiltInit = false;
+ const Expr *ArgE = nullptr;
+ if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S)) {
ArgE = DefE->getExpr();
- else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S))
+ HasRebuiltInit = DefE->hasRewrittenInit();
+ } else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S)) {
ArgE = DefE->getExpr();
- else
+ HasRebuiltInit = DefE->hasRewrittenInit();
+ } else
llvm_unreachable("unknown constant wrapper kind");
- bool IsTemporary = false;
- if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) {
- ArgE = MTE->getSubExpr();
- IsTemporary = true;
- }
+ if (HasRebuiltInit) {
+ for (auto *N : PreVisit) {
+ ProgramStateRef state = N->getState();
+ const LocationContext *LCtx = N->getLocationContext();
+ state = state->BindExpr(S, LCtx, state->getSVal(ArgE, LCtx));
+ Bldr2.generateNode(S, N, state);
+ }
+ } else {
+ // If it's not rewritten, the contents of these expressions are not
+ // actually part of the current function, so we fall back to constant
+ // evaluation.
+ bool IsTemporary = false;
+ if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) {
+ ArgE = MTE->getSubExpr();
+ IsTemporary = true;
+ }
+
+ std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
+ const LocationContext *LCtx = Pred->getLocationContext();
+ for (auto *I : PreVisit) {
+ ProgramStateRef State = I->getState();
+ State = State->BindExpr(S, LCtx, ConstantVal.value_or(UnknownVal()));
+ if (IsTemporary)
+ State = createTemporaryRegionIfNeeded(State, LCtx, cast<Expr>(S),
+ cast<Expr>(S));
- std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
- if (!ConstantVal)
- ConstantVal = UnknownVal();
-
- const LocationContext *LCtx = Pred->getLocationContext();
- for (const auto I : PreVisit) {
- ProgramStateRef State = I->getState();
- State = State->BindExpr(S, LCtx, *ConstantVal);
- if (IsTemporary)
- State = createTemporaryRegionIfNeeded(State, LCtx,
- cast<Expr>(S),
- cast<Expr>(S));
- Bldr2.generateNode(S, I, State);
+ Bldr2.generateNode(S, I, State);
+ }
}
getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
diff --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp
index a8e30f1759e9f..5294819da2aa6 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -294,7 +294,7 @@ union U {
// CHECK-NEXT: `-DeclStmt {{.*}}
// CHECK-NEXT: `-VarDecl {{.*}} g 'U':'GH112560::U' listinit
// CHECK-NEXT: `-InitListExpr {{.*}} 'U':'GH112560::U' contains-errors field Field {{.*}} 'f' 'int'
-// CHECK-NEXT: `-CXXDefaultInitExpr {{.*}} 'int' contains-errors has rewritten init
+// CHECK-NEXT: `-CXXDefaultInitExpr {{.*}} 'int' contains-errors
// CHECK-NEXT: `-RecoveryExpr {{.*}} 'int' contains-errors
// DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
void foo() {
diff --git a/clang/test/Analysis/lifetime-extended-regions.cpp b/clang/test/Analysis/lifetime-extended-regions.cpp
index 4458ad294af7c..02a1210d9af92 100644
--- a/clang/test/Analysis/lifetime-extended-regions.cpp
+++ b/clang/test/Analysis/lifetime-extended-regions.cpp
@@ -121,11 +121,10 @@ void aggregateWithReferences() {
clang_analyzer_dump(viaReference.rx); // expected-warning-re {{&lifetime_extended_object{int, viaReference, S{{[0-9]+}}} }}
clang_analyzer_dump(viaReference.ry); // expected-warning-re {{&lifetime_extended_object{Composite, viaReference, S{{[0-9]+}}} }}
- // FIXME: clang currently support extending lifetime of object bound to reference members of aggregates,
- // that are created from default member initializer. But CFG and ExprEngine need to be updated to address this change.
- // The following expect warning: {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
+ // The lifetime of object bound to reference members of aggregates,
+ // that are created from default member initializer was extended.
RefAggregate defaultInitExtended{i};
- clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }}
+ clang_analyzer_dump(defaultInitExtended.ry); // expected-warning-re {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
}
void lambda() {
diff --git a/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp b/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
index 8e428c0ef0427..37824c16f4f05 100644
--- a/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
+++ b/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -274,16 +274,16 @@ void f() {
// CHECK: ClassTemplateSpecializationDecl {{.*}} struct A definition
// CHECK: CXXConstructorDecl {{.*}} implicit used constexpr A 'void () noexcept'
// CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int'
-// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
+// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int'
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 1
// CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int'
-// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
+// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int'
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 2
// CHECK-NEXT: CXXCtorInitializer Field {{.*}} 'a' 'int'
-// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
+// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int'
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 3
// CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int'
-// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
+// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int'
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 4
// CHECK-NEXT: CompoundStmt {{.*}}
diff --git a/clang/test/SemaCXX/warn-unreachable.cpp b/clang/test/SemaCXX/warn-unreachable.cpp
index e6f5bc5ef8e12..f8a8d17049abb 100644
--- a/clang/test/SemaCXX/warn-unreachable.cpp
+++ b/clang/test/SemaCXX/warn-unreachable.cpp
@@ -414,3 +414,107 @@ void tautological_compare(bool x, int y) {
calledFun();
}
+
+namespace test_rebuilt_default_arg {
+struct A {
+ explicit A(int = __builtin_LINE());
+};
+
+int h(int a) {
+ return 3;
+ A(); // expected-warning {{will never be executed}}
+}
+
+struct Temp {
+ Temp();
+ ~Temp();
+};
+
+struct B {
+ explicit B(const Temp &t = Temp());
+};
+int f(int a) {
+ return 3;
+ B(); // expected-warning {{will never be executed}}
+}
+} // namespace test_rebuilt_default_arg
+namespace test_rebuilt_default_init {
+
+struct A {
+ A();
+ ~A();
+};
+
+struct B {
+ const A &t = A();
+};
+int f(int a) {
+ return 3;
+ A{}; // expected-warning {{will never be executed}}
+}
+} // namespace test_rebuilt_default_init
+
+// This issue reported by the comments in https://github.com/llvm/llvm-project/pull/117437.
+// All block-level expressions should have already been IgnoreParens()ed.
+namespace gh117437_ignore_parens_in_default_arg {
+ ...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
yronglin
commented
Jun 29, 2025
| 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, Block)) | ||
| auto *RewrittenParent = | ||
| PM.getOuterMostAncestor<CXXDefaultArgExpr, CXXDefaultInitExpr>(S); |
There was a problem hiding this comment.
I try to fix Nico's example(#128195) about the wrong unrachable code location. The root cause of this issue is that this PR introduce "rewritten" expressions in CFGBlock. When we are finding the real dead code, we should try to ignore the "rewritten" sub-expression nodes in CXXDefaultInitExpr/CXXDefaultArgExpr, but I'm not sure this option is make sense. What do you think?
…ult init expression Co-authored-by: Jan Voung <[email protected]> Signed-off-by: yronglin <[email protected]>
yronglin
force-pushed
the
reapply_cfg_default_init
branch
from
3a15187 to
5839b3d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR reapply #127338.
Fixes: #128068, #128195