vsavchenko 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()) {
----------------
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.


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

Reply via email to