[PATCH] D104136: [analyzer] Add better tracking for RetainCountChecker leak warnings

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D104136#2823834 <https://reviews.llvm.org/D104136#2823834>, @Szelethus wrote:

> Aha, alright! So the tracker tracked back to where the tracked expression got computed in node `N`, which is the return value of some identity function. Unless its explicitly told that there is a link in between the return value and the parameter, the tracker can't figure this out that it could track further (why would it, right?). So it asks its handlers whether they want to do anything with `N`, which `IdentityHandler` does -- it digs out the node where the parameter was evaluated, and tells the tracker to go on with that. Awesome!

That's exactly it!

> I guess we could put `IdentityHandler` in an anonymous namespace?

Sure thing!



================
Comment at: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp:945-947
+    ExprEngine &Eng = C.getStateManager().getOwningEngine();
+    // Let's mark this place with a special tag.
+    Tag = Eng.getDataTags().make<IdentityTag>(CE, BindReturnTo);
----------------
Szelethus wrote:
> I don't know ObjC at all, but is this identity obvious? Do you not need a `NoteTag` to say that "Foo is an identity function, its return value equals the parameter"? Or, in the possession of the actual `PathSensitiveBugReport` that you can retrieve in the handler, you could drop a note there, maybe?
These are usually some type of casts.  And we already provide notes when this value gets into some other region (like `'newPtr' initialized to the value of 'originalPtr'`).  I think that they are enough.  And if we want something better, I think we need to tweak that messaging (`StoreHandler` now allows that) instead of crowding user with even more notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104136