[PATCH] D65724: [analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field

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

Szelethus marked 2 inline comments as done.
Szelethus added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:192-194
+  if (const auto *DRE = dyn_cast<DeclRefExpr>(CondVarExpr))
+    if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl()))
+      return State->getSVal(State->getLValue(VD, LCtx));
----------------
Szelethus wrote:
> xazax.hun wrote:
> > NoQ wrote:
> > > Ugh, so this code intentionally avoids the complexity of `ExprEngine::VisitCommonDeclRefExpr()` so that to work differently with references. Okay.
> > > 
> > > @xazax.hun, do you think this works correctly with lambda captures? (when the `DeclRefExpr` to a variable might evaluate to a field within the lambda at run-time).
> > Good point! This definitely worth a test case somewhere somehow!
> > 
> > I would expect `DRE->getDecl()` to return the captured declaration rather than the field and thus `State->getSVal(State->getLValue(VD, LCtx))` to compute the wrong answer. I might be a bit rusty on this topic so of course we would need a carefully crafted test case to make sure :)
> So, we seem to be okay with this code in the context of this patch, but have voiced concerns that there probably is a testcase on which this doesn't work. What kind of `FIXME` message shall I put in before commiting?
Also, could you please detail why we think this doesn't/shouldn't work?


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

https://reviews.llvm.org/D65724