[PATCH] D67932: [analyzer] Fix accidentally skipping the call during inlined defensive check suppression.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked an inline comment as done.
NoQ added a comment.

Yeah, i think we should avoid such peeking and instead try to do everything in one pass. I.e., if we need to peek at the node above us, just make a visitor that delays the decision until it has precisely the information it needs. I guess i'll be slooowly moving in this direction.

Also every time i see special handling of `DeclRefExpr`s i get pretty worried. It usually means that this code almost never works.



================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1912-1922
 /// Find the ExplodedNode where the lvalue (the value of 'Ex')
 /// was computed.
 static const ExplodedNode* findNodeForExpression(const ExplodedNode *N,
                                                  const Expr *Inner) {
   while (N) {
     if (N->getStmtForDiagnostics() == Inner)
       return N;
----------------
Szelethus wrote:
> > When `bugreporter::trackExpressionValue()` is invoked on a `DeclRefExpr`, it tries to do most of its computations over the node in which this `DeclRefExpr` is computed, rather than on the error node (or whatever node is stuffed into it). I'm quite confused about the idea behind it and i highly doubt that it actually works correctly, but one reason why we can't simply use the error node may be that the binding to that variable might have already disappeared from the state by the time the bug is found.
> 
> So, its possible that this function, and its uses in `trackExpressionValue` is completely unnecessary?
Of course it's necessary! Why would you ever write a system of mutually recursive visitors and then never implement an ad-hoc visitor within one of these visitors that allows you to quadratically visit nodes while you visit nodes, before actually visiting them in the real visitor? (/sarcasm)


Repository:
  rC Clang

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

https://reviews.llvm.org/D67932