NoQ added a comment.
This is a major improvement. `MallocChecker` will have to catch up on that.
Hopefully through increased code re-use.
================
Comment at:
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp:651
+
+ return Result;
+}
----------------
I feel semi-irrational urge to add comments whenever NRVO is employed.
================
Comment at:
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp:967-968
+
+ // While looking for the last var bindings, we can still find
+ // `AllocFirstBinding` to be one of them. In situations like this,
+ // it would still be the easiest case to explain to our users.
----------------
Given that the current state doesn't satisfy the requirement on line 945, this
can happen for two reasons:
1. The symbol was there when the variable died so we simply tracked it back to
the last moment of time the variable was alive.
2. The symbol was there but was overwritten later (and then the variable may
have also, but haven't necessarily, died).
Case 2 is bad because we're back in business for reporting the wrong variable;
it's still more rare than before the patch but i think the problem remains.
Something like this should probably demonstrate it:
```lang=c++
void foo() {
Object *Original = allocate(...);
Original = allocate();
Original->release();
}
```
A potential solution may be to add a check in `getAllVarBindingsForSymbol` that
the first node in which we find our symbol corresponds to a `*PurgeDeadSymbols`
program point. It's a straightforward way to find out whether we're in case 1
or in case 2.
================
Comment at:
clang/test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist:5993
+ <key>message</key>
+ <string>Object leaked: object allocated and stored into 'New'
is not referenced later in this execution path and has a retain count of
+1</string>
+ </dict>
----------------
This patch seems to add 3 entirely new warnings here. Why do they suddenly
start showing up? Are they good?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100839/new/
https://reviews.llvm.org/D100839
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
