[clang] [NFC][analyzer] Remove StmtNodeBuilder (PR #181431)

[Donát Nagy via cfe-commits]

================
@@ -267,8 +260,9 @@ class NodeBuilder {
   NodeBuilder(const ExplodedNodeSet &SrcSet, ExplodedNodeSet &DstSet,
               const NodeBuilderContext &Ctx)
       : C(Ctx), Frontier(DstSet) {
-    Frontier.insert(SrcSet);
-    assert(hasNoSinksInFrontier());
+    for (ExplodedNode *Node : SrcSet)
+      if (!Node->isSink())
+        Frontier.insert(Node);
   }
----------------
NagyDonat wrote:

> What if the DstSet already has elements?

`DstSet` is almost always (although not always) empty. In particular, it is empty in the concrete motivational example where `BranchNodeBuilder` receives nodes from the checker callback.

> Is it really the same?

I think this is an NFC change because it only changes the behavior under a hypothetical situation that is AFAIK currently unreachable (I didn't see any reports about the failure of this assertion). I'm only removing this assertion to simplify the code and remove a "pitfall" from future changes that could introduce something that rarely triggers this assertion.

Also note that there is no analogous assertion in the other constructor of `NodeBuilder` (which only takes a single `SrcNode` instead of a `SrcSet`). We can say that we are aligning the behavior of this constructor with the other one.

Another factor is that that removing the assertion doesn't influence the behavior of production builds (where `assert` is a no-op). In fact there the more significant (although logically clearly beneficial) change is that in production builds after this change sink nodes would not be added to the `Frontier`. (Which is still NFC, because currently AFAIK no sink node can reach this point.)

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