[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:166-173
+  // The declaration of the value may rely on a pointer so take its l-value.
+  if (const auto *DRE = dyn_cast_or_null<DeclRefExpr>(CondVarExpr)) {
+    if (const auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl())) {
+      SVal DeclSVal = State->getSVal(State->getLValue(VD, LCtx));
+      if (auto DeclCI = DeclSVal.getAs<nonloc::ConcreteInt>())
+        return &DeclCI->getValue();
+    }
----------------
I'll take a closer look at the rest of the patch, but this code snippet looks suspicious to me at a glance. Does it ever add anything on top of the "hard-coded" case? When it does, is it correct?

I mean, the "hard-coded" case does not actually correspond to an integer literal expression; it corresponds to any circumstances in which the Static Analyzer could compute that the value is going to be exactly that concrete integer on this execution path. If the Static Analyzer could not compute that, i doubt that this simple procedure will do better.

It might be that this is needed because you're looking at a wrong `ExplodedNode` on which the condition expression is either not computed yet or already dropped. In this case i'd prefer to try harder to find the right node, because `getSVal(CondVarExpr, LCtx)` is just so much more powerful.


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

https://reviews.llvm.org/D53076