[clang] [NFC][analyzer] Eliminate some simple node builders (PR #204187)

[via cfe-commits]

=?utf-8?q?Donát?= Nagy <donat.nagy at ericsson.com>,
=?utf-8?q?Donát?= Nagy <donat.nagy at ericsson.com>,
=?utf-8?q?Donát?= Nagy <donat.nagy at ericsson.com>,
=?utf-8?q?Donát?= Nagy <donat.nagy at ericsson.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/204187 at github.com>


llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Donát Nagy (NagyDonat)

<details>
<summary>Changes</summary>

Replace four `NodeBuilder`s with direct use of the `makeNode` method family. This is part of my commit series that simplifies the engine by gradually eliminating the class `NodeBuilder`.

-------

"Why is this NFC" justifications can be found in the descriptions of the individual commits.

@<!-- -->necto This change touches the method `ProcessLifetimeEnd` which was recently published by you. I don't know much you already know about this refactoring effort (which I started several months ago), but if you are not yet familiar with it, I think this PR is a good and clear example of what I'm doing in this long commit series. For the motivation behind this project, you can see [this comment](https://github.com/llvm/llvm-project/pull/181431#issuecomment-3900458198) and the commit message of #<!-- -->182377 -- and I'm happy to answer any remaining questions.

---
Full diff: https://github.com/llvm/llvm-project/pull/204187.diff


1 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Core/ExprEngine.cpp (+12-20) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 32da5e097c76e..0ecef1c77f4cb 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1148,10 +1148,8 @@ void ExprEngine::ProcessLifetimeEnd(const Stmt *S, const VarDecl *D,
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 S->getBeginLoc(),
                                 "Error evaluating end of a lifetime");
-  ExplodedNodeSet Src;
-  NodeBuilder Bldr(Pred, Src, *currBldrCtx);
   LifetimeEnd PP(S, D, Pred->getStackFrame());
-  Bldr.generateNode(PP, Pred->getState(), Pred);
+  ExplodedNode *Src = Engine.makeNode(PP, Pred->getState(), Pred);
 
   ExplodedNodeSet Dst;
   getCheckerManager().runCheckersForLifetimeEnd(Dst, Src, D, *this);
@@ -1639,7 +1637,6 @@ void ExprEngine::VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE,
     Dst = PreVisit;
     return;
   }
-  NodeBuilder Builder(PreVisit, Dst, *currBldrCtx);
   for (ExplodedNode *Node : PreVisit) {
     ProgramStateRef State = Node->getState();
     const StackFrame *SF = Node->getStackFrame();
@@ -1650,7 +1647,7 @@ void ExprEngine::VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE,
       // temporary destructor nodes.
       State = addObjectUnderConstruction(State, BTE, SF, UnknownVal());
     }
-    Builder.generateNode(BTE, Node, State);
+    Dst.insert(Engine.makePostStmtNode(BTE, State, Node));
   }
 }
 
@@ -3352,7 +3349,6 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred,
     for (const auto I : CheckedSet)
       VisitCommonDeclRefExpr(M, Member, I, EvalSet);
   } else {
-    NodeBuilder Bldr(CheckedSet, EvalSet, *currBldrCtx);
     ExplodedNodeSet Tmp;
 
     for (const auto I : CheckedSet) {
@@ -3366,9 +3362,8 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred,
           state = createTemporaryRegionIfNeeded(state, SF, BaseExpr);
 
         SVal MDVal = svalBuilder.getFunctionPointer(MD);
-        state = state->BindExpr(M, SF, MDVal);
 
-        Bldr.generateNode(M, I, state);
+        EvalSet.insert(Engine.makeNodeWithBinding(I, M, MDVal, state));
         continue;
       }
 
@@ -3412,12 +3407,13 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred,
             L = UnknownVal();
         }
 
-        Bldr.generateNode(M, I, state->BindExpr(M, SF, L), nullptr,
-                          ProgramPoint::PostLValueKind);
+        EvalSet.insert(Engine.makeNodeWithBinding(
+            I, M, L, state, ProgramPoint::PostLValueKind));
       } else {
-        Bldr.takeNodes(I);
+        // FIXME: When evalLoad no longer uses NodeBuilders, eliminate Tmp and
+        // pass EvalSet as the first argument of evalLoad.
         evalLoad(Tmp, M, M, I, state, L);
-        Bldr.addNodes(Tmp);
+        EvalSet.insert(Tmp);
       }
     }
   }
@@ -3434,16 +3430,14 @@ void ExprEngine::VisitAtomicExpr(const AtomicExpr *AE, ExplodedNode *Pred,
   // FIXME: Ideally we should model the behavior of the atomics precisely here.
 
   ExplodedNodeSet AfterInvalidateSet;
-  NodeBuilder Bldr(AfterPreSet, AfterInvalidateSet, *currBldrCtx);
 
   for (const auto I : AfterPreSet) {
     ProgramStateRef State = I->getState();
     const StackFrame *SF = I->getStackFrame();
 
     SmallVector<SVal, 8> ValuesToInvalidate;
-    for (unsigned SI = 0, Count = AE->getNumSubExprs(); SI != Count; SI++) {
-      const Expr *SubExpr = AE->getSubExprs()[SI];
-      SVal SubExprVal = State->getSVal(SubExpr, SF);
+    for (const Stmt *SubExpr : AE->children()) {
+      SVal SubExprVal = State->getSVal(cast<Expr>(SubExpr), SF);
       ValuesToInvalidate.push_back(SubExprVal);
     }
 
@@ -3452,10 +3446,8 @@ void ExprEngine::VisitAtomicExpr(const AtomicExpr *AE, ExplodedNode *Pred,
                                      /*CausedByPointerEscape*/ true,
                                      /*Symbols=*/nullptr);
 
-    SVal ResultVal = UnknownVal();
-    State = State->BindExpr(AE, SF, ResultVal);
-    Bldr.generateNode(AE, I, State, nullptr,
-                      ProgramPoint::PostStmtKind);
+    AfterInvalidateSet.insert(
+        Engine.makeNodeWithBinding(I, AE, UnknownVal(), State));
   }
 
   getCheckerManager().runCheckersForPostStmt(Dst, AfterInvalidateSet, AE, *this);

``````````

</details>


https://github.com/llvm/llvm-project/pull/204187