https://github.com/NagyDonat created https://github.com/llvm/llvm-project/pull/204354
Part of my commit series to eliminate the class `NodeBuilder`. From 574dc1af0f90d7e4f95f949b716eaa30059ac075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Wed, 17 Jun 2026 15:03:08 +0200 Subject: [PATCH 1/2] [side] Don't enter the loop if we want to skip it Entering the loop and then immediately `break`ing with a condition that does not depend on the loop variable is worse than using `goto` -- because it IS `goto`, but looks like normal code. I really dislike the old code and seriously think that a clear `goto AfterLoop;` would be better than it, but I ended up skipping the loop in a different way, by clearing the range of the `for` loop. --- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 32da5e097c76e..fb72c04aa8256 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -3201,12 +3201,13 @@ void ExprEngine::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *Ex, const Expr *Arr = Ex->getCommonExpr()->getSourceExpr(); - for (auto *Node : CheckerPreStmt) { - - // The constructor visitior has already taken care of everything. - if (isa<CXXConstructExpr>(Ex->getSubExpr())) - break; + if (isa<CXXConstructExpr>(Ex->getSubExpr())) { + // The constructor visitior has already taken care of everything, so let's + // skip forward to the PostStmt handling after the 'for' loop. + CheckerPreStmt.clear(); + } + for (auto *Node : CheckerPreStmt) { const StackFrame *SF = Node->getStackFrame(); ProgramStateRef state = Node->getState(); From 32d89b64c473a3689f45311563484480137043f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Wed, 17 Jun 2026 15:08:50 +0200 Subject: [PATCH 2/2] Remove NodeBuilder from VisitArrayInitLoopExpr The constructor of the `NodeBuilder` was inserting the contents of `CheckerPreStmt` into `EvalSet`, which was usually irrelevant because the `generateNode` calls in the loop remove each element of `CheckerPreStmt` from `EvalSet`-- but I had to re-add the `EvalSet.insert(CheckerPreStmt)` call on the branch that skips the loop. --- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index fb72c04aa8256..0937f76308d74 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -3193,17 +3193,16 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D, void ExprEngine::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *Ex, ExplodedNode *Pred, ExplodedNodeSet &Dst) { + const Expr *Arr = Ex->getCommonExpr()->getSourceExpr(); + ExplodedNodeSet CheckerPreStmt; getCheckerManager().runCheckersForPreStmt(CheckerPreStmt, Pred, Ex, *this); ExplodedNodeSet EvalSet; - NodeBuilder Bldr(CheckerPreStmt, EvalSet, *currBldrCtx); - - const Expr *Arr = Ex->getCommonExpr()->getSourceExpr(); - if (isa<CXXConstructExpr>(Ex->getSubExpr())) { // The constructor visitior has already taken care of everything, so let's // skip forward to the PostStmt handling after the 'for' loop. + EvalSet.insert(CheckerPreStmt); CheckerPreStmt.clear(); } @@ -3282,7 +3281,7 @@ void ExprEngine::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *Ex, else Base = UnknownVal(); - Bldr.generateNode(Ex, Node, state->BindExpr(Ex, SF, Base)); + EvalSet.insert(Engine.makeNodeWithBinding(Node, Ex, Base)); } getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, Ex, *this); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
