[clang] [NFC][analyzer] Remove the NodeBuilder from VisitArrayInitLoopExpr (PR #204354)

[Donát Nagy via cfe-commits]

https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/204354

>From 574dc1af0f90d7e4f95f949b716eaa30059ac075 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 17 Jun 2026 15:03:08 +0200
Subject: [PATCH 1/3] [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?= <donat.nagy at ericsson.com>
Date: Wed, 17 Jun 2026 15:08:50 +0200
Subject: [PATCH 2/3] 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);

>From 8c604b511695bac28f89221fdeba6b2663e09948 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 22 Jun 2026 11:22:47 +0200
Subject: [PATCH 3/3] Clarify comment

---
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 0937f76308d74..1ad24f63d2f9e 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3200,8 +3200,8 @@ void ExprEngine::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *Ex,
 
   ExplodedNodeSet EvalSet;
   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.
+    // The constructor visitor has already handled everything, so let's skip
+    // forward to PostStmt handling by clearing the range of the 'for' loop.
     EvalSet.insert(CheckerPreStmt);
     CheckerPreStmt.clear();
   }