a.sidorin added a comment.
Hi Artem! Thank you for this patch. It looks very promising, but I have some
questions and remarks.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:187
const Expr *Result) {
- SVal V = State->getSVal(Ex, LC);
+ SVal ExV = State->getSVal(Ex, LC);
if (!Result) {
----------------
If we are touching names, should we rename Ex to InitWithAdjustments (or smth
like this) and ExV correspondingly?
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:281
+ // Try to recover some path sensitivity in case we couldn't compute the
value.
+ if (ExV.isUnknown())
+ ExV = getSValBuilder().conjureSymbolVal(Result, LC, Ex->getType(),
----------------
Should we do all these operations with ExV/Reg if the InitV is known? There is
a FIXME but I think it is related to all this code, not to the bindLoc only.
And what happens if we remove this code?
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:285
+
+ // FIXME: Why do we need to do that if WipeV was known to begin with?
+ State = State->bindLoc(Reg, ExV, LC);
----------------
Seems like WipeV in comment should be InitV?
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:286
+ // FIXME: Why do we need to do that if WipeV was known to begin with?
+ State = State->bindLoc(Reg, ExV, LC);
+
----------------
If I understand correcly, if we call `bindLoc()`, we call
`checkRegionChanges()` callbacks. And if we `bindLoc()` twice, we call them
twice too. Is this what we want here?
https://reviews.llvm.org/D30534
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
