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

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Hmm, so before this patch, we just used `LVNode` everywhere and ignored `InputNode`. It may not have made much sense, but its still not as confusing as using both if the creation of `LVNode` is unnecessary overall. Could we just remove it?



================
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;
----------------
> 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?


================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2007-2009
+          // Note that LVNode may be too late; the lvalue may have been computed
+          // before the inlined call was evaluated. InputNode may as well be
+          // too late, because the symbol is already dead; this, however,
----------------
Too late in terms of too close to the root of the `ExplodedGraph`, or the other way around? This is a bitconfusing since visitors are inspecting the graph in reverse.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67932