steakhal wrote:

Let me forward couple seemingly relevant comments:

  - CoreEngine::makeNode is const, so the existing const CoreEngine 
&getCoreEngine() const already suffices for addTransitionImpl (that's how 
NodeBuilder::generateNode reached it). Is
  the non-const overload deliberate — signalling that node creation mutates the 
graph — or can it be dropped so CheckerContext.h doesn't expose a mutable 
CoreEngine to every checker?
  - ExplodedNode still has friend class NodeBuilder; (ExplodedGraph.h:69), 
keeping the name alive as an incomplete type. Delete it here? Look for similar 
stale spellings.
  - The currBldrCtx comment block and now-unused getBuilderContext() — fold 
into this PR, or leave to the announced NodeBuilderContext follow-up?
  - On your own EndFunction question: the asymmetry looks worth fixing, but as 
a separate non-NFC change with a test — please keep it out of this PR.

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

Reply via email to