[PATCH] D158295: [NFC][clang][analyzer] Avoid potential dereferencing of null pointer value

[Balázs Benics via Phabricator via cfe-commits]

steakhal requested changes to this revision.
steakhal added a comment.
This revision now requires changes to proceed.

Thanks for the PR.
I went over the code and concluded that it can never be null.
However, the code could be improved, while making this explicit.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp:1416-1433
   // Update counts from autorelease pools
   for (const auto &I: state->get<RefBindings>()) {
     SymbolRef Sym = I.first;
     if (SymReaper.isDead(Sym)) {
       static CheckerProgramPointTag Tag(this, "DeadSymbolAutorelease");
       const RefVal &V = I.second;
       state = handleAutoreleaseCounts(state, Pred, &Tag, C, Sym, V);
----------------
So `state->get<RefBindings>()` returns a map, and we iterate over the Key&Val pairs of that map.

```
const RefVal *getRefBinding(ProgramStateRef State, SymbolRef Sym) {
  return State->get<RefBindings>(Sym);
}
```

So, this just returns the associated value with `Sym` in the map.
Instead of this lookup ,we really should have just used the `I.second`.

Now the question is, could have map a `Sym` to a null pointer?

```
static ProgramStateRef setRefBinding(ProgramStateRef State, SymbolRef Sym,
                                     RefVal Val) {
  assert(Sym != nullptr);
  return State->set<RefBindings>(Sym, Val);
}
```
Nop. It's never supposed to be null.

---

Could you please just decompose in the loop with a structured-binding to `auto [Sym, V]` and use `V` instead of `T`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158295/new/

https://reviews.llvm.org/D158295