[PATCH] D65724: [analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field
Kristóf Umann via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 14 12:16:14 PDT 2019
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));
> 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
More information about the cfe-commits