https://github.com/NagyDonat created 
https://github.com/llvm/llvm-project/pull/200837

Remove NodeBuilder use from `ExprEngine::Visit` to reduce its size from 700 
lines to 600 lines of source code.

This commit also moves the "for instance method operators, make sure the 'this' 
argument has a valid region" logic (~10 lines) from the huge 
`ExprEngine::Visit` to the more specific `VisitCallExpr`; and applies a few 
other very minor code quality improvements.

------

Note for reviewers: "why is this NFC" justifications can be found in the 
descriptions of the individual commits. 

From 03dc35ac438b5a23ab2cea53f9578f316284b0fe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]>
Date: Fri, 24 Apr 2026 15:52:15 +0200
Subject: [PATCH 1/7] [NFC] Remove local builders in ExprEngine::Visit

This eliminates two `NodeBuilder`s from `ExprEngine::Visit` that were
only used locally, in a very simple and straightforward pattern. (The
destination set is a freshly declared empty set; the single
`generateNode` call can be replaced by `makePostStmtNode`.)

Follow-up changes will handle the "big fish", the `NodeBuilder Bldr`
which is referenced 95 times in the 700+ lines of this method.
---
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index fcd61cee34fe1..df63928b003ef 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1953,7 +1953,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
 
       ExplodedNodeSet Tmp;
-      NodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
 
       const Expr *ArgE;
       if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S))
@@ -1980,7 +1979,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
         if (IsTemporary)
           State = createTemporaryRegionIfNeeded(State, SF, cast<Expr>(S),
                                                 cast<Expr>(S));
-        Bldr2.generateNode(S, I, State);
+        Tmp.insert(Engine.makePostStmtNode(S, State, I));
       }
 
       getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
@@ -1999,7 +1998,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       getCheckerManager().runCheckersForPreStmt(preVisit, Pred, S, *this);
 
       ExplodedNodeSet Tmp;
-      NodeBuilder Bldr2(preVisit, Tmp, *currBldrCtx);
 
       const auto *Ex = cast<Expr>(S);
       QualType resultType = Ex->getType();
@@ -2023,7 +2021,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
             State = escapeValues(State, Val, PSK_EscapeOther);
           }
 
-        Bldr2.generateNode(S, N, State);
+        Tmp.insert(Engine.makePostStmtNode(S, State, N));
       }
 
       getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);

From 8d84feff87ae6a2cfb3d142eec42e6a9359fe9ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]>
Date: Mon, 11 May 2026 18:53:50 +0200
Subject: [PATCH 2/7] [NFC] Remove the "main" NodeBuilder from
 `ExprEngine::Visit`

This big commit removes the node builder `Bldr` which was referenced
almost 100 times. (I couldn't split it up because I don't want to
mix node builder methods and straightforward set manipulation.)

First note that `Visit` is only called once and and that call passes an
empty set as `DstTop`.

The constructor of `Bldr` inserts `Pred` into `DstTop`, but this is
cancelled out on almost all branches either by explicit
`Bldr.takeNodes(Pred)` calls or implicitly by passing `Pred` to a
`generateNode` or `generateSink` call.

This commit adds `DstTop.insert(Pred)` calls in the only two situations
where `Pred` is not removed:
- The cases `Expr::ConstantExprClass` and `Stmt::ExprWithCleanupsClass`
  where `Visit()` does nothing.
- The case of `Stmt::StmtExprClass` where the `DstTop.insert(Pred)` call
  has double duty: on two branches it inserts the node received as an
  argument, while on the branch when there is a relevant last
  expression, it inserts a freshly created node that was stored in the
  variable `Pred`.

Calls to `Bldr.generateNode` are replaced by the appropriate node making
method (`makeNode`, `makePostStmtNode`, `makeNodeWithBinding`) followed
by a call of `DstTop.insert()`.

Calls to `Bldr.generateSink` are just replaced by a node making method
(with MarkAsSink) because `ExplodedNodeSet` does not accept sink nodes
as members (calling `DstTop.insert()` would be a no-op).
---
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 194 +++++++------------
 1 file changed, 68 insertions(+), 126 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index df63928b003ef..7c2320bc92815 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1668,7 +1668,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 S->getBeginLoc(), "Error evaluating 
statement");
   ExplodedNodeSet Dst;
-  NodeBuilder Bldr(Pred, DstTop, *currBldrCtx);
 
   assert(!isa<Expr>(S) || S == cast<Expr>(S)->IgnoreParens());
 
@@ -1800,7 +1799,8 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
     case Stmt::OMPUnrollDirectiveClass:
     case Stmt::OMPMetaDirectiveClass:
     case Stmt::HLSLOutArgExprClass: {
-      const ExplodedNode *node = Bldr.generateSink(S, Pred, Pred->getState());
+      const ExplodedNode *node = Engine.makePostStmtNode(
+          S, Pred->getState(), Pred, /*MarkAsSink=*/true);
       Engine.addAbortedBlock(node, getCurrBlock());
       break;
     }
@@ -1842,40 +1842,35 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
 
     case Stmt::GNUNullExprClass: {
       // GNU __null is a pointer-width integer, not an actual pointer.
-      ProgramStateRef state = Pred->getState();
-      state = state->BindExpr(
-          cast<Expr>(S), Pred->getStackFrame(),
-          svalBuilder.makeIntValWithWidth(getContext().VoidPtrTy, 0));
-      Bldr.generateNode(S, Pred, state);
+      SVal Val = svalBuilder.makeIntValWithWidth(getContext().VoidPtrTy, 0);
+      DstTop.insert(Engine.makeNodeWithBinding(Pred, cast<Expr>(S), Val));
       break;
     }
 
     case Stmt::ObjCAtSynchronizedStmtClass:
-      Bldr.takeNodes(Pred);
       VisitObjCAtSynchronizedStmt(cast<ObjCAtSynchronizedStmt>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Expr::ConstantExprClass:
     case Stmt::ExprWithCleanupsClass:
+      DstTop.insert(Pred);
       // Handled due to fully linearised CFG.
       break;
 
     case Stmt::CXXBindTemporaryExprClass: {
-      Bldr.takeNodes(Pred);
       ExplodedNodeSet PreVisit;
       getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
       ExplodedNodeSet Next;
       VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), PreVisit, Next);
       getCheckerManager().runCheckersForPostStmt(Dst, Next, S, *this);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::ArrayInitLoopExprClass:
-      Bldr.takeNodes(Pred);
       VisitArrayInitLoopExpr(cast<ArrayInitLoopExpr>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     // Cases not handled yet; but will handle some day.
     case Stmt::DesignatedInitExprClass:
@@ -1931,24 +1926,21 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
     case Stmt::SYCLUniqueStableNameExprClass:
     case Stmt::OpenACCAsteriskSizeExprClass:
     case Stmt::TypeTraitExprClass: {
-      Bldr.takeNodes(Pred);
       ExplodedNodeSet preVisit;
       getCheckerManager().runCheckersForPreStmt(preVisit, Pred, S, *this);
       getCheckerManager().runCheckersForPostStmt(Dst, preVisit, S, *this);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::AttributedStmtClass: {
-      Bldr.takeNodes(Pred);
       VisitAttributedStmt(cast<AttributedStmt>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::CXXDefaultArgExprClass:
     case Stmt::CXXDefaultInitExprClass: {
-      Bldr.takeNodes(Pred);
       ExplodedNodeSet PreVisit;
       getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
 
@@ -1983,7 +1975,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       }
 
       getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
@@ -1992,8 +1984,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
     case Expr::ObjCArrayLiteralClass:
     case Expr::ObjCDictionaryLiteralClass:
     case Expr::ObjCBoxedExprClass: {
-      Bldr.takeNodes(Pred);
-
       ExplodedNodeSet preVisit;
       getCheckerManager().runCheckersForPreStmt(preVisit, Pred, S, *this);
 
@@ -2025,14 +2015,13 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       }
 
       getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::ArraySubscriptExprClass:
-      Bldr.takeNodes(Pred);
       VisitArraySubscriptExpr(cast<ArraySubscriptExpr>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::MatrixSingleSubscriptExprClass:
@@ -2045,36 +2034,33 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       break;
 
     case Stmt::GCCAsmStmtClass: {
-      Bldr.takeNodes(Pred);
       ExplodedNodeSet PreVisit;
       getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
       ExplodedNodeSet PostVisit;
       for (ExplodedNode *const N : PreVisit)
         VisitGCCAsmStmt(cast<GCCAsmStmt>(S), N, PostVisit);
       getCheckerManager().runCheckersForPostStmt(Dst, PostVisit, S, *this);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::MSAsmStmtClass:
-      Bldr.takeNodes(Pred);
       VisitMSAsmStmt(cast<MSAsmStmt>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::BlockExprClass:
-      Bldr.takeNodes(Pred);
       VisitBlockExpr(cast<BlockExpr>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::LambdaExprClass:
       if (AMgr.options.ShouldInlineLambdas) {
-        Bldr.takeNodes(Pred);
         VisitLambdaExpr(cast<LambdaExpr>(S), Pred, Dst);
-        Bldr.addNodes(Dst);
+        DstTop.insert(Dst);
       } else {
-        const ExplodedNode *node = Bldr.generateSink(S, Pred, 
Pred->getState());
+        const ExplodedNode *node = Engine.makePostStmtNode(
+            S, Pred->getState(), Pred, /*MarkAsSink=*/true);
         Engine.addAbortedBlock(node, getCurrBlock());
       }
       break;
@@ -2082,23 +2068,16 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
     case Stmt::BinaryOperatorClass: {
       const auto *B = cast<BinaryOperator>(S);
       if (B->isLogicalOp()) {
-        Bldr.takeNodes(Pred);
         VisitLogicalExpr(B, Pred, Dst);
-        Bldr.addNodes(Dst);
+        DstTop.insert(Dst);
         break;
-      }
-      else if (B->getOpcode() == BO_Comma) {
-        ProgramStateRef state = Pred->getState();
-        Bldr.generateNode(
-            B, Pred,
-            state->BindExpr(
-                B, Pred->getStackFrame(),
-                state->getSVal(B->getRHS(), Pred->getStackFrame())));
+      } else if (B->getOpcode() == BO_Comma) {
+        SVal Val =
+            Pred->getState()->getSVal(B->getRHS(), Pred->getStackFrame());
+        DstTop.insert(Engine.makeNodeWithBinding(Pred, B, Val));
         break;
       }
 
-      Bldr.takeNodes(Pred);
-
       if (AMgr.options.ShouldEagerlyAssume &&
           (B->isRelationalOp() || B->isEqualityOp())) {
         ExplodedNodeSet Tmp;
@@ -2108,7 +2087,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       else
         VisitBinaryOperator(cast<BinaryOperator>(S), Pred, Dst);
 
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
@@ -2125,47 +2104,43 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
           ProgramStateRef NewState =
               createTemporaryRegionIfNeeded(State, SF, OCE->getArg(0));
           if (NewState != State) {
-            Pred = Bldr.generateNode(OCE, Pred, NewState, /*tag=*/nullptr,
-                                     ProgramPoint::PreStmtKind);
-            // Did we cache out?
+            PreStmt PS(OCE, SF, /*tag=*/nullptr);
+            Pred = Engine.makeNode(PS, NewState, Pred);
             if (!Pred)
-              break;
+              break; // Cached out.
           }
         }
       }
+      // FIXME: Move this logic into `VisitCallExpr` to reduce the complexity
+      // of `Visit`.
       [[fallthrough]];
     }
 
     case Stmt::CallExprClass:
     case Stmt::CXXMemberCallExprClass:
     case Stmt::UserDefinedLiteralClass:
-      Bldr.takeNodes(Pred);
       VisitCallExpr(cast<CallExpr>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::CXXCatchStmtClass:
-      Bldr.takeNodes(Pred);
       VisitCXXCatchStmt(cast<CXXCatchStmt>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::CXXTemporaryObjectExprClass:
     case Stmt::CXXConstructExprClass:
-      Bldr.takeNodes(Pred);
       VisitCXXConstructExpr(cast<CXXConstructExpr>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::CXXInheritedCtorInitExprClass:
-      Bldr.takeNodes(Pred);
       VisitCXXInheritedCtorInitExpr(cast<CXXInheritedCtorInitExpr>(S), Pred,
                                     Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::CXXNewExprClass: {
-      Bldr.takeNodes(Pred);
 
       ExplodedNodeSet PreVisit;
       getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
@@ -2175,12 +2150,11 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
         VisitCXXNewExpr(cast<CXXNewExpr>(S), i, PostVisit);
 
       getCheckerManager().runCheckersForPostStmt(Dst, PostVisit, S, *this);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::CXXDeleteExprClass: {
-      Bldr.takeNodes(Pred);
       ExplodedNodeSet PreVisit;
       const auto *CDE = cast<CXXDeleteExpr>(S);
       getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
@@ -2190,59 +2164,52 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       for (const auto i : PostVisit)
         VisitCXXDeleteExpr(CDE, i, Dst);
 
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
       // FIXME: ChooseExpr is really a constant.  We need to fix
       //        the CFG do not model them as explicit control-flow.
 
     case Stmt::ChooseExprClass: { // __builtin_choose_expr
-      Bldr.takeNodes(Pred);
       const auto *C = cast<ChooseExpr>(S);
       VisitGuardedExpr(C, C->getLHS(), C->getRHS(), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::CompoundAssignOperatorClass:
-      Bldr.takeNodes(Pred);
       VisitBinaryOperator(cast<BinaryOperator>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::CompoundLiteralExprClass:
-      Bldr.takeNodes(Pred);
       VisitCompoundLiteralExpr(cast<CompoundLiteralExpr>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::BinaryConditionalOperatorClass:
     case Stmt::ConditionalOperatorClass: { // '?' operator
-      Bldr.takeNodes(Pred);
       const auto *C = cast<AbstractConditionalOperator>(S);
       VisitGuardedExpr(C, C->getTrueExpr(), C->getFalseExpr(), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::CXXThisExprClass:
-      Bldr.takeNodes(Pred);
       VisitCXXThisExpr(cast<CXXThisExpr>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::DeclRefExprClass: {
-      Bldr.takeNodes(Pred);
       const auto *DE = cast<DeclRefExpr>(S);
       VisitCommonDeclRefExpr(DE, DE->getDecl(), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::DeclStmtClass:
-      Bldr.takeNodes(Pred);
       VisitDeclStmt(cast<DeclStmt>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::ImplicitCastExprClass:
@@ -2255,19 +2222,17 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
     case Stmt::BuiltinBitCastExprClass:
     case Stmt::ObjCBridgedCastExprClass:
     case Stmt::CXXAddrspaceCastExprClass: {
-      Bldr.takeNodes(Pred);
       const auto *C = cast<CastExpr>(S);
       ExplodedNodeSet dstExpr;
       VisitCast(C, C->getSubExpr(), Pred, dstExpr);
 
       // Handle the postvisit checks.
       getCheckerManager().runCheckersForPostStmt(Dst, dstExpr, C, *this);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Expr::MaterializeTemporaryExprClass: {
-      Bldr.takeNodes(Pred);
       const auto *MTE = cast<MaterializeTemporaryExpr>(S);
       ExplodedNodeSet dstPrevisit;
       getCheckerManager().runCheckersForPreStmt(dstPrevisit, Pred, MTE, *this);
@@ -2275,72 +2240,63 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       for (const auto i : dstPrevisit)
         CreateCXXTemporaryObject(MTE, i, dstExpr);
       getCheckerManager().runCheckersForPostStmt(Dst, dstExpr, MTE, *this);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::InitListExprClass: {
       const InitListExpr *E = cast<InitListExpr>(S);
-      Bldr.takeNodes(Pred);
       ConstructInitList(E, E->inits(), E->isTransparent(), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Expr::CXXParenListInitExprClass: {
       const CXXParenListInitExpr *E = cast<CXXParenListInitExpr>(S);
-      Bldr.takeNodes(Pred);
       ConstructInitList(E, E->getInitExprs(), /*IsTransparent*/ false, Pred,
                         Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::MemberExprClass:
-      Bldr.takeNodes(Pred);
       VisitMemberExpr(cast<MemberExpr>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::AtomicExprClass:
-      Bldr.takeNodes(Pred);
       VisitAtomicExpr(cast<AtomicExpr>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::ObjCIvarRefExprClass:
-      Bldr.takeNodes(Pred);
       VisitLvalObjCIvarRefExpr(cast<ObjCIvarRefExpr>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::ObjCForCollectionStmtClass:
-      Bldr.takeNodes(Pred);
       VisitObjCForCollectionStmt(cast<ObjCForCollectionStmt>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::ObjCMessageExprClass:
-      Bldr.takeNodes(Pred);
       VisitObjCMessage(cast<ObjCMessageExpr>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::ObjCAtThrowStmtClass:
     case Stmt::CXXThrowExprClass:
       // FIXME: This is not complete.  We basically treat @throw as
       // an abort.
-      Bldr.generateSink(S, Pred, Pred->getState());
+      Engine.makePostStmtNode(S, Pred->getState(), Pred, /*MarkAsSink=*/true);
       break;
 
     case Stmt::ReturnStmtClass:
-      Bldr.takeNodes(Pred);
       VisitReturnStmt(cast<ReturnStmt>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::OffsetOfExprClass: {
-      Bldr.takeNodes(Pred);
       ExplodedNodeSet PreVisit;
       getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
 
@@ -2349,15 +2305,14 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
         VisitOffsetOfExpr(cast<OffsetOfExpr>(S), Node, PostVisit);
 
       getCheckerManager().runCheckersForPostStmt(Dst, PostVisit, S, *this);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::UnaryExprOrTypeTraitExprClass:
-      Bldr.takeNodes(Pred);
-      VisitUnaryExprOrTypeTraitExpr(cast<UnaryExprOrTypeTraitExpr>(S),
-                                    Pred, Dst);
-      Bldr.addNodes(Dst);
+      VisitUnaryExprOrTypeTraitExpr(cast<UnaryExprOrTypeTraitExpr>(S), Pred,
+                                    Dst);
+      DstTop.insert(Dst);
       break;
 
     case Stmt::StmtExprClass: {
@@ -2367,22 +2322,16 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
         // Empty statement expression.
         assert(SE->getType() == getContext().VoidTy
                && "Empty statement expression must have void type.");
-        break;
-      }
-
-      if (const auto *LastExpr =
-              dyn_cast<Expr>(*SE->getSubStmt()->body_rbegin())) {
-        ProgramStateRef state = Pred->getState();
-        Bldr.generateNode(
-            SE, Pred,
-            state->BindExpr(SE, Pred->getStackFrame(),
-                            state->getSVal(LastExpr, Pred->getStackFrame())));
+      } else if (const auto *LastExpr =
+                     dyn_cast<Expr>(*SE->getSubStmt()->body_rbegin())) {
+        SVal Val = Pred->getState()->getSVal(LastExpr, Pred->getStackFrame());
+        Pred = Engine.makeNodeWithBinding(Pred, SE, Val);
       }
+      DstTop.insert(Pred);
       break;
     }
 
     case Stmt::UnaryOperatorClass: {
-      Bldr.takeNodes(Pred);
       const auto *U = cast<UnaryOperator>(S);
       if (AMgr.options.ShouldEagerlyAssume && (U->getOpcode() == UO_LNot)) {
         ExplodedNodeSet Tmp;
@@ -2391,25 +2340,21 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       }
       else
         VisitUnaryOperator(U, Pred, Dst);
-      Bldr.addNodes(Dst);
+      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::PseudoObjectExprClass: {
-      Bldr.takeNodes(Pred);
       ProgramStateRef state = Pred->getState();
       const auto *PE = cast<PseudoObjectExpr>(S);
+      // FIXME: Simplify
       if (const Expr *Result = PE->getResultExpr()) {
         SVal V = state->getSVal(Result, Pred->getStackFrame());
-        Bldr.generateNode(
-            S, Pred, state->BindExpr(cast<Expr>(S), Pred->getStackFrame(), V));
+        DstTop.insert(Engine.makeNodeWithBinding(Pred, PE, V));
       }
       else
-        Bldr.generateNode(S, Pred,
-                          state->BindExpr(cast<Expr>(S), Pred->getStackFrame(),
-                                          UnknownVal()));
+        DstTop.insert(Engine.makeNodeWithBinding(Pred, PE, UnknownVal()));
 
-      Bldr.addNodes(Dst);
       break;
     }
 
@@ -2417,14 +2362,11 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       // ObjCIndirectCopyRestoreExpr implies passing a temporary for
       // correctness of lifetime management.  Due to limited analysis
       // of ARC, this is implemented as direct arg passing.
-      Bldr.takeNodes(Pred);
       ProgramStateRef state = Pred->getState();
       const auto *OIE = cast<ObjCIndirectCopyRestoreExpr>(S);
       const Expr *E = OIE->getSubExpr();
       SVal V = state->getSVal(E, Pred->getStackFrame());
-      Bldr.generateNode(
-          S, Pred, state->BindExpr(cast<Expr>(S), Pred->getStackFrame(), V));
-      Bldr.addNodes(Dst);
+      DstTop.insert(Engine.makeNodeWithBinding(Pred, OIE, V));
       break;
     }
   }

From 0968c82cae3619c6bb43afde1142ec1ceceb9dc5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]>
Date: Mon, 1 Jun 2026 14:42:56 +0200
Subject: [PATCH 3/7] [NFC] Unify 'DstTop' and 'Dst' in `ExprEngine::Visit`

At the beginning of `ExprEngine::Visit` the code introduces a local
(empty) `ExplodedNodeSet` `Dst`. Many branches (handling various
statement kinds) pass this `Dst` as the destination argument of the
appropriate `VisitXXXStatement` methods and then call
`DstTop.insert(Dst)` to place the produced nodes into `DstTop`, the
out-parameter of `ExprEngine::Visit`.

However, after recent simplifications (primarily in my commit
73ded458839c0a105b19476929f6206356f92991), the method
`ExprEngine::Visit` is only called from one location and there the
argument `DstTop` is an empty `ExplodedNodeSet`, so there is no reason
to use the local `Dst` instead of the out-parameter `DstTop` (which are
both completely equivalent empty sets).

This commit renames the out-parameter of `ExprEngine::Visit` to `Dst`
(from `DstTop`) and uses it directly in the branches that previously
called `DstTop.insert(Dst)` at the end.

Technically, this commit introduces the precondition that the
destination argument of `ExprEngine::Visit` must be empty -- but
currently this happens to be true and in will "go away" in the near
future, when I eliminate the trickly `NodeBuiler` logic from the
`VisitXXXStatement` methods that are called by `ExprEngine::Visit`.
---
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 57 +++-----------------
 1 file changed, 8 insertions(+), 49 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 7c2320bc92815..5c112520dba1a 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1664,10 +1664,9 @@ ProgramStateRef ExprEngine::escapeValues(ProgramStateRef 
State,
 }
 
 void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
-                       ExplodedNodeSet &DstTop) {
+                       ExplodedNodeSet &Dst) {
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 S->getBeginLoc(), "Error evaluating 
statement");
-  ExplodedNodeSet Dst;
 
   assert(!isa<Expr>(S) || S == cast<Expr>(S)->IgnoreParens());
 
@@ -1843,18 +1842,17 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
     case Stmt::GNUNullExprClass: {
       // GNU __null is a pointer-width integer, not an actual pointer.
       SVal Val = svalBuilder.makeIntValWithWidth(getContext().VoidPtrTy, 0);
-      DstTop.insert(Engine.makeNodeWithBinding(Pred, cast<Expr>(S), Val));
+      Dst.insert(Engine.makeNodeWithBinding(Pred, cast<Expr>(S), Val));
       break;
     }
 
     case Stmt::ObjCAtSynchronizedStmtClass:
       VisitObjCAtSynchronizedStmt(cast<ObjCAtSynchronizedStmt>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Expr::ConstantExprClass:
     case Stmt::ExprWithCleanupsClass:
-      DstTop.insert(Pred);
+      Dst.insert(Pred);
       // Handled due to fully linearised CFG.
       break;
 
@@ -1864,13 +1862,11 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       ExplodedNodeSet Next;
       VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), PreVisit, Next);
       getCheckerManager().runCheckersForPostStmt(Dst, Next, S, *this);
-      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::ArrayInitLoopExprClass:
       VisitArrayInitLoopExpr(cast<ArrayInitLoopExpr>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
     // Cases not handled yet; but will handle some day.
     case Stmt::DesignatedInitExprClass:
@@ -1929,13 +1925,11 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       ExplodedNodeSet preVisit;
       getCheckerManager().runCheckersForPreStmt(preVisit, Pred, S, *this);
       getCheckerManager().runCheckersForPostStmt(Dst, preVisit, S, *this);
-      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::AttributedStmtClass: {
       VisitAttributedStmt(cast<AttributedStmt>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
     }
 
@@ -1975,7 +1969,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       }
 
       getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
-      DstTop.insert(Dst);
       break;
     }
 
@@ -2015,13 +2008,11 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       }
 
       getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
-      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::ArraySubscriptExprClass:
       VisitArraySubscriptExpr(cast<ArraySubscriptExpr>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::MatrixSingleSubscriptExprClass:
@@ -2040,24 +2031,20 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       for (ExplodedNode *const N : PreVisit)
         VisitGCCAsmStmt(cast<GCCAsmStmt>(S), N, PostVisit);
       getCheckerManager().runCheckersForPostStmt(Dst, PostVisit, S, *this);
-      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::MSAsmStmtClass:
       VisitMSAsmStmt(cast<MSAsmStmt>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::BlockExprClass:
       VisitBlockExpr(cast<BlockExpr>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::LambdaExprClass:
       if (AMgr.options.ShouldInlineLambdas) {
         VisitLambdaExpr(cast<LambdaExpr>(S), Pred, Dst);
-        DstTop.insert(Dst);
       } else {
         const ExplodedNode *node = Engine.makePostStmtNode(
             S, Pred->getState(), Pred, /*MarkAsSink=*/true);
@@ -2069,12 +2056,11 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       const auto *B = cast<BinaryOperator>(S);
       if (B->isLogicalOp()) {
         VisitLogicalExpr(B, Pred, Dst);
-        DstTop.insert(Dst);
         break;
       } else if (B->getOpcode() == BO_Comma) {
         SVal Val =
             Pred->getState()->getSVal(B->getRHS(), Pred->getStackFrame());
-        DstTop.insert(Engine.makeNodeWithBinding(Pred, B, Val));
+        Dst.insert(Engine.makeNodeWithBinding(Pred, B, Val));
         break;
       }
 
@@ -2087,7 +2073,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       else
         VisitBinaryOperator(cast<BinaryOperator>(S), Pred, Dst);
 
-      DstTop.insert(Dst);
       break;
     }
 
@@ -2120,24 +2105,20 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
     case Stmt::CXXMemberCallExprClass:
     case Stmt::UserDefinedLiteralClass:
       VisitCallExpr(cast<CallExpr>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::CXXCatchStmtClass:
       VisitCXXCatchStmt(cast<CXXCatchStmt>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::CXXTemporaryObjectExprClass:
     case Stmt::CXXConstructExprClass:
       VisitCXXConstructExpr(cast<CXXConstructExpr>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::CXXInheritedCtorInitExprClass:
       VisitCXXInheritedCtorInitExpr(cast<CXXInheritedCtorInitExpr>(S), Pred,
                                     Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::CXXNewExprClass: {
@@ -2150,7 +2131,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
         VisitCXXNewExpr(cast<CXXNewExpr>(S), i, PostVisit);
 
       getCheckerManager().runCheckersForPostStmt(Dst, PostVisit, S, *this);
-      DstTop.insert(Dst);
       break;
     }
 
@@ -2164,7 +2144,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       for (const auto i : PostVisit)
         VisitCXXDeleteExpr(CDE, i, Dst);
 
-      DstTop.insert(Dst);
       break;
     }
       // FIXME: ChooseExpr is really a constant.  We need to fix
@@ -2173,43 +2152,36 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
     case Stmt::ChooseExprClass: { // __builtin_choose_expr
       const auto *C = cast<ChooseExpr>(S);
       VisitGuardedExpr(C, C->getLHS(), C->getRHS(), Pred, Dst);
-      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::CompoundAssignOperatorClass:
       VisitBinaryOperator(cast<BinaryOperator>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::CompoundLiteralExprClass:
       VisitCompoundLiteralExpr(cast<CompoundLiteralExpr>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::BinaryConditionalOperatorClass:
     case Stmt::ConditionalOperatorClass: { // '?' operator
       const auto *C = cast<AbstractConditionalOperator>(S);
       VisitGuardedExpr(C, C->getTrueExpr(), C->getFalseExpr(), Pred, Dst);
-      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::CXXThisExprClass:
       VisitCXXThisExpr(cast<CXXThisExpr>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::DeclRefExprClass: {
       const auto *DE = cast<DeclRefExpr>(S);
       VisitCommonDeclRefExpr(DE, DE->getDecl(), Pred, Dst);
-      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::DeclStmtClass:
       VisitDeclStmt(cast<DeclStmt>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::ImplicitCastExprClass:
@@ -2228,7 +2200,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
 
       // Handle the postvisit checks.
       getCheckerManager().runCheckersForPostStmt(Dst, dstExpr, C, *this);
-      DstTop.insert(Dst);
       break;
     }
 
@@ -2240,14 +2211,12 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       for (const auto i : dstPrevisit)
         CreateCXXTemporaryObject(MTE, i, dstExpr);
       getCheckerManager().runCheckersForPostStmt(Dst, dstExpr, MTE, *this);
-      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::InitListExprClass: {
       const InitListExpr *E = cast<InitListExpr>(S);
       ConstructInitList(E, E->inits(), E->isTransparent(), Pred, Dst);
-      DstTop.insert(Dst);
       break;
     }
 
@@ -2255,33 +2224,27 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       const CXXParenListInitExpr *E = cast<CXXParenListInitExpr>(S);
       ConstructInitList(E, E->getInitExprs(), /*IsTransparent*/ false, Pred,
                         Dst);
-      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::MemberExprClass:
       VisitMemberExpr(cast<MemberExpr>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::AtomicExprClass:
       VisitAtomicExpr(cast<AtomicExpr>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::ObjCIvarRefExprClass:
       VisitLvalObjCIvarRefExpr(cast<ObjCIvarRefExpr>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::ObjCForCollectionStmtClass:
       VisitObjCForCollectionStmt(cast<ObjCForCollectionStmt>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::ObjCMessageExprClass:
       VisitObjCMessage(cast<ObjCMessageExpr>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::ObjCAtThrowStmtClass:
@@ -2293,7 +2256,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
 
     case Stmt::ReturnStmtClass:
       VisitReturnStmt(cast<ReturnStmt>(S), Pred, Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::OffsetOfExprClass: {
@@ -2305,14 +2267,12 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
         VisitOffsetOfExpr(cast<OffsetOfExpr>(S), Node, PostVisit);
 
       getCheckerManager().runCheckersForPostStmt(Dst, PostVisit, S, *this);
-      DstTop.insert(Dst);
       break;
     }
 
     case Stmt::UnaryExprOrTypeTraitExprClass:
       VisitUnaryExprOrTypeTraitExpr(cast<UnaryExprOrTypeTraitExpr>(S), Pred,
                                     Dst);
-      DstTop.insert(Dst);
       break;
 
     case Stmt::StmtExprClass: {
@@ -2327,7 +2287,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
         SVal Val = Pred->getState()->getSVal(LastExpr, Pred->getStackFrame());
         Pred = Engine.makeNodeWithBinding(Pred, SE, Val);
       }
-      DstTop.insert(Pred);
+      Dst.insert(Pred);
       break;
     }
 
@@ -2340,7 +2300,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       }
       else
         VisitUnaryOperator(U, Pred, Dst);
-      DstTop.insert(Dst);
       break;
     }
 
@@ -2350,10 +2309,10 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
       // FIXME: Simplify
       if (const Expr *Result = PE->getResultExpr()) {
         SVal V = state->getSVal(Result, Pred->getStackFrame());
-        DstTop.insert(Engine.makeNodeWithBinding(Pred, PE, V));
+        Dst.insert(Engine.makeNodeWithBinding(Pred, PE, V));
       }
       else
-        DstTop.insert(Engine.makeNodeWithBinding(Pred, PE, UnknownVal()));
+        Dst.insert(Engine.makeNodeWithBinding(Pred, PE, UnknownVal()));
 
       break;
     }
@@ -2366,7 +2325,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       const auto *OIE = cast<ObjCIndirectCopyRestoreExpr>(S);
       const Expr *E = OIE->getSubExpr();
       SVal V = state->getSVal(E, Pred->getStackFrame());
-      DstTop.insert(Engine.makeNodeWithBinding(Pred, OIE, V));
+      Dst.insert(Engine.makeNodeWithBinding(Pred, OIE, V));
       break;
     }
   }

From 5b27af08993985c06b0afafe3adf84153354587f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]>
Date: Mon, 1 Jun 2026 15:10:53 +0200
Subject: [PATCH 4/7] [NFC] Simplify case Stmt::PseudoObjectExprClass

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

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 5c112520dba1a..b781c44e7f5d9 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2304,16 +2304,11 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
     }
 
     case Stmt::PseudoObjectExprClass: {
-      ProgramStateRef state = Pred->getState();
       const auto *PE = cast<PseudoObjectExpr>(S);
-      // FIXME: Simplify
-      if (const Expr *Result = PE->getResultExpr()) {
-        SVal V = state->getSVal(Result, Pred->getStackFrame());
-        Dst.insert(Engine.makeNodeWithBinding(Pred, PE, V));
-      }
-      else
-        Dst.insert(Engine.makeNodeWithBinding(Pred, PE, UnknownVal()));
-
+      SVal V = UnknownVal();
+      if (const Expr *Result = PE->getResultExpr())
+        V = Pred->getState()->getSVal(Result, Pred->getStackFrame());
+      Dst.insert(Engine.makeNodeWithBinding(Pred, PE, V));
       break;
     }
 

From 79887be6e53f73450d0fff8c32e26f700c5ee7ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]>
Date: Mon, 1 Jun 2026 15:12:13 +0200
Subject: [PATCH 5/7] [NFC] Simplify case
 Stmt::ObjCIndirectCopyRestoreExprClass

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

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index b781c44e7f5d9..840fa6654e104 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2316,10 +2316,9 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       // ObjCIndirectCopyRestoreExpr implies passing a temporary for
       // correctness of lifetime management.  Due to limited analysis
       // of ARC, this is implemented as direct arg passing.
-      ProgramStateRef state = Pred->getState();
       const auto *OIE = cast<ObjCIndirectCopyRestoreExpr>(S);
       const Expr *E = OIE->getSubExpr();
-      SVal V = state->getSVal(E, Pred->getStackFrame());
+      SVal V = Pred->getState()->getSVal(E, Pred->getStackFrame());
       Dst.insert(Engine.makeNodeWithBinding(Pred, OIE, V));
       break;
     }

From ac2e3d598504c54640dd548c31ebc73fd0cab3f8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]>
Date: Mon, 1 Jun 2026 15:24:48 +0200
Subject: [PATCH 6/7] [NFC] Refactor case Stmt::CXXOperatorCallExprClass:

Previously this case of `ExprEngine::Visit` invoked
`createTemporaryRegionIfNeeded` under certain conditions; then did a
`[[fallthrough]];` to the cases of `CallExprClass`,
`CXXMemberCallExprClass` and `UserDefinedLiteralClass` -- which was the
only callsite of `VisitCallExpr()`.

This commit moves the "for instance method operators make sure the
'this' argument has a valid region" logic into the method
`VisitCallExpr` to get rid of the `[[fallthrough]]` and reduce the
complexity of the already huge `ExprEngine::Visit`.

I'm very surprised to see that this "make sure the 'this' argument has a
valid region" logic is only triggered for operator calls (and not for
other sorts of calls), but this is an NFC commit so I'm preserving the
existing behavior (with a FIXME comment to highlight the question).
---
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp  | 26 +------------------
 .../Core/ExprEngineCallAndReturn.cpp          | 20 ++++++++++++++
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 840fa6654e104..8b1215cfa1c4c 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2076,31 +2076,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       break;
     }
 
-    case Stmt::CXXOperatorCallExprClass: {
-      const auto *OCE = cast<CXXOperatorCallExpr>(S);
-
-      // For instance method operators, make sure the 'this' argument has a
-      // valid region.
-      const Decl *Callee = OCE->getCalleeDecl();
-      if (const auto *MD = dyn_cast_or_null<CXXMethodDecl>(Callee)) {
-        if (MD->isImplicitObjectMemberFunction()) {
-          ProgramStateRef State = Pred->getState();
-          const StackFrame *SF = Pred->getStackFrame();
-          ProgramStateRef NewState =
-              createTemporaryRegionIfNeeded(State, SF, OCE->getArg(0));
-          if (NewState != State) {
-            PreStmt PS(OCE, SF, /*tag=*/nullptr);
-            Pred = Engine.makeNode(PS, NewState, Pred);
-            if (!Pred)
-              break; // Cached out.
-          }
-        }
-      }
-      // FIXME: Move this logic into `VisitCallExpr` to reduce the complexity
-      // of `Visit`.
-      [[fallthrough]];
-    }
-
+    case Stmt::CXXOperatorCallExprClass:
     case Stmt::CallExprClass:
     case Stmt::CXXMemberCallExprClass:
     case Stmt::UserDefinedLiteralClass:
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 0c6d35876db2f..7ecca2af626f3 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -590,6 +590,26 @@ static ProgramStateRef 
getInlineFailedState(ProgramStateRef State,
 
 void ExprEngine::VisitCallExpr(const CallExpr *CE, ExplodedNode *Pred,
                                ExplodedNodeSet &dst) {
+  if (const auto *OCE = dyn_cast<CXXOperatorCallExpr>(CE)) {
+    // For instance method operators, make sure the 'this' argument has a
+    // valid region.
+    // FIXME: Why is this only applied for operator calls and not other calls?
+    const Decl *Callee = OCE->getCalleeDecl();
+    if (const auto *MD = dyn_cast_or_null<CXXMethodDecl>(Callee)) {
+      if (MD->isImplicitObjectMemberFunction()) {
+        ProgramStateRef State = Pred->getState();
+        const StackFrame *SF = Pred->getStackFrame();
+        ProgramStateRef NewState =
+            createTemporaryRegionIfNeeded(State, SF, OCE->getArg(0));
+        if (NewState != State) {
+          PreStmt PS(OCE, SF, /*tag=*/nullptr);
+          Pred = Engine.makeNode(PS, NewState, Pred);
+          if (!Pred)
+            return; // Cached out.
+        }
+      }
+    }
+  }
   // Perform the previsit of the CallExpr.
   ExplodedNodeSet dstPreVisit;
   getCheckerManager().runCheckersForPreStmt(dstPreVisit, Pred, CE, *this);

From e8b93456b6625a21bcee3a441a03dea73cb53122 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]>
Date: Mon, 1 Jun 2026 16:04:07 +0200
Subject: [PATCH 7/7] [NFC] Capitalize a variable name

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

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 8b1215cfa1c4c..34ca88705cba6 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1798,9 +1798,9 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
     case Stmt::OMPUnrollDirectiveClass:
     case Stmt::OMPMetaDirectiveClass:
     case Stmt::HLSLOutArgExprClass: {
-      const ExplodedNode *node = Engine.makePostStmtNode(
+      const ExplodedNode *Node = Engine.makePostStmtNode(
           S, Pred->getState(), Pred, /*MarkAsSink=*/true);
-      Engine.addAbortedBlock(node, getCurrBlock());
+      Engine.addAbortedBlock(Node, getCurrBlock());
       break;
     }
 
@@ -2046,9 +2046,9 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       if (AMgr.options.ShouldInlineLambdas) {
         VisitLambdaExpr(cast<LambdaExpr>(S), Pred, Dst);
       } else {
-        const ExplodedNode *node = Engine.makePostStmtNode(
+        const ExplodedNode *Node = Engine.makePostStmtNode(
             S, Pred->getState(), Pred, /*MarkAsSink=*/true);
-        Engine.addAbortedBlock(node, getCurrBlock());
+        Engine.addAbortedBlock(Node, getCurrBlock());
       }
       break;
 

_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to