Szelethus added inline comments.
================
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);
----------------
vsavchenko wrote:
> 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.
Fair enough!
================
Comment at:
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp:953-957
+ // Let's traverse...
+ for (const ExplodedNode *N = ExprNode;
+ // ...all the nodes corresponding to the given expression...
+ N != nullptr && N->getStmtForDiagnostics() == E;
+ N = N->getFirstPred()) {
----------------
vsavchenko wrote:
> NoQ wrote:
> > I guess this part should ultimately be written in one place, eg.
> > `ExplodedNode::findTag()` or something like that.
> >
> > I'd also really like to explore the possibility to further limit the
> > variety of nodes traversed here. What nodes are typically traversed here?
> > Is it checker-tagged nodes or is it purge dead symbol nodes or something
> > else?
> Yes, `ExplodedNode::findTag()` sounds like a great idea!
>
> I mean it is hard to tell without calculating statistics right here and
> running it on a bunch of projects. However, it is always possible to write
> the code that will have it the other way. My take on it is that it is
> probably a mix of things.
>
> I'd also prefer to traverse less, do you have any specific ideas here?
> I'd also really like to explore the possibility to further limit the variety
> of nodes traversed here. What nodes are typically traversed here? Is it
> checker-tagged nodes or is it purge dead symbol nodes or something else?
Is there something I'm not seeing here? Trackers basically ascend //a// path
from the error node to at most the root of the ExplodedGraph (not the trimmed
version, as `Tracker::track()` is called before any of that process happens),
so its not much slower than `trackExpressionValue`, right?
Or does this, and likely many other future handlers run such a loop more often
then what I imagine?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104136/new/
https://reviews.llvm.org/D104136
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
