isuckatcs wrote: If it's guaranteed that `ExprNode` is for `E`, I think it would be a good idea to assert it in the function.
```c++ assert(ExprNode->getStmtForDiagnostics() == E); ``` However I guess this assumption is made from the only currently known callsite of the function, which is: ```c++ Tracker::Result Tracker::track(const Expr *E, const ExplodedNode *N, TrackingOptions Opts) { if (!E || !N) return {}; const Expr *Inner = peelOffOuterExpr(E, N); const ExplodedNode *LVNode = findNodeForExpression(N, Inner); // <- Assertion guaranteed here if (!LVNode) return {}; Result CombinedResult; // Iterate through the handlers in the order according to their priorities. for (ExpressionHandlerPtr &Handler : ExpressionHandlers) { CombinedResult.combineWith(Handler->handle(Inner, N, LVNode, Opts)); if (CombinedResult.WasInterrupted) { // There is no need to confuse our users here. // We got interrupted, but our users don't need to know about it. CombinedResult.WasInterrupted = false; break; } } return CombinedResult; } ``` Calling the function from `Tracker::track()` indeed guarantees the assertion, however I can't find anything that guarantees that `PRValueHandler::handle()` cannot be called from anywhere else. In which case the assertion will fail and we'll start seeing crashes. While the change has no effect now, it could introduce a crash later, so I would leave `PRValueHandler::handle()` as it is. https://github.com/llvm/llvm-project/pull/75076 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits