Szelethus marked 4 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? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65724/new/ https://reviews.llvm.org/D65724 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
