[clang] [analyzer] Avoid use of `CallEvent`s with obsolete state (PR #160707)
DonĂ¡t Nagy via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 8 11:10:27 PDT 2025
NagyDonat wrote:
> You probably shouldn't write a lot of code that you're about to delete anyway. So in my opinion it doesn't make sense to make a unit test or a debug checker just for this patch.
>
> Maybe just assert that the states are the same before you pass it to the checker in evalCall? It's very slightly unobvious there with the whole loop thing and that's where the problem was anyway. Or just leave it as-is.
I think I will leave it as-is because the initialization of `UpdatedCall` and the `CheckerContext` are really close and they are both relying on the same `Pred` state:
```c++
ProgramStateRef State = Pred->getState();
CallEventRef<> UpdatedCall = Call.cloneWithState(State);
// Check if any of the EvalCall callbacks can evaluate the call.
for (const auto &EvalCallChecker : EvalCallCheckers) {
// TODO: Support the situation when the call doesn't correspond
// to any Expr.
ProgramPoint L = ProgramPoint::getProgramPoint(
UpdatedCall->getOriginExpr(), ProgramPoint::PostStmtKind,
Pred->getLocationContext(), EvalCallChecker.Checker);
bool evaluated = false;
{ // CheckerContext generates transitions (populates checkDest) on
// destruction, so introduce the scope to make sure it gets properly
// populated.
CheckerContext C(B, Eng, Pred, L);
evaluated = EvalCallChecker(*UpdatedCall, C);
}
```
> If you won't be able to do the follow-up commit then you can add a clever follow-up test instead.
I'm pretty sure that I will do the follow-up commit -- it is probably simpler than designing a testcase.
------
> Actually maybe just keep the strict bare minimum of information in the call event? Like a `SmallVector<SVal, ...>` of args, a potential return value as `Optional<SVal>`, the runtime definition that's been already computed elsewhere as a `Decl *`, and more subclass-specific stuff in the subclasses.
>
> If you don't want the data to go out of sync, just normalize your database.
I would really like to have `CallEvent` implemented this way (I strongly agree that this would be more elegant than the status quo), but I fear that _reaching_ this ideal state would be a difficult undertaking -- especially on the weird Objective-C areas that I really don't understand.
I strongly support this "keep the bare minimum in the call event" idea as a long-term development plan, but I cannot promise to implement it in the foreseeable future, so I would still like to merge this PR as a temporary solution. (If call events do end up being minimized, then it will be possible to discard the logic tweaked in this PR along with all the other code that updates the states attached to `CallEvent`s.)
Also note that this development plan is compatible with my other plans (making `CallEvent::getState()` protected and ensuring that `CallEvent::invalidateRegions()` always takes the state explicitly) because those are necessary first steps towards implementing a setup where `CallEvent` doesn't have a full state (not even as a private member).
https://github.com/llvm/llvm-project/pull/160707
More information about the cfe-commits
mailing list