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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
