================
@@ -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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
