Szelethus added inline comments.
================
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:
> Szelethus wrote:
> > 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?
> >
> >
> Essentially, yes. `trackExpressionValue` (and `Tracker::track`) ascend the
> graph searching for a node where the give expression is evaluated. The
> problem here is that whenever we ask to track some expression, not only
> `trackExpressionValue` will do its traversal (much longer), but also this
> handler will do its (shorter). It would be better not to do this altogether,
> but I personally don't see any solutions how we can cut on this part.
> At the same time, I don't think that it is performance critical, considering
> that the part of the graph where `N->getStmtForDiagnostics() == E` is small.
Ah, so, this is your patch:
```
E: Error Node
C: Identity function call
V: Argument evaluation
A: Origin of the argument value
---->: Nodes traversed by the tracker
====>: Nodes traversed by the handler
* * * *
E->O->O->O->O->C->O->O->O->V->O->O->O->A->X->X->X
--------------->
============>----------->
```
This is fine, because its just a single ascend, made a little worse by the fact
that we do it for each tracked variable. But later, with more handlers, it
might get worse (per variable):
```
* * * *
E->O->O->O->O->C->O->O->O->V->O->O->O->A->X->X->X
--------------->
============>----------->
===============>----->
==============>------------------>
=================================>
======================>
```
Now, I can't immediately imagine a case where someone would add THAT many
handlers, or track that many variables, even in extreme circumstances, but I
don't posses a crystal ball either :^)
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
