[clang] [Analyzer][NFC] Remove redundant function call (PR #75076)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 11 10:02:01 PST 2023
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
More information about the cfe-commits
mailing list