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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
