franchiotta added inline comments.
================
Comment at:
include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:444-448
@@ +443,7 @@
+
+ SymbolRef
+ getSymbolFromStmt(const Stmt *S, const LocationContext *LCtx) const;
+
+ const MemRegion*
+ getRegionFromStmt(const Stmt *S, const LocationContext *LCtx) const;
+
----------------
krememek wrote:
> krememek wrote:
> > Can we add documentation comments for these? Seems like generally useful
> > utility methods. We could also probably just make these public.
> Actually, I'm wondering if we really need to add these at all. They are just
> one liners that easily could be written where they are used.
Right. Removing these methods, and adding the one-liners directly where they
are used.
================
Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:653-654
@@ -654,3 +652,4 @@
+ *LCtx) const {
if (const Expr *E = dyn_cast_or_null<Expr>(S))
S = E->IgnoreParens();
----------------
krememek wrote:
> Is this even needed? I think Environment::getSVal() already handles
> parenthesis and other ignored expressions. This looks like dead code.
>
> This can probably just be an inline method in ProgramState.h, that just
> forwards to getSVal(S, LCtx).getAsSymbol().
>
> Alternatively, if this is only called once, we don't need to add a method at
> all, since it is just a one liner.
Yes, you are right. It is not needed since it is handle by
ignoreTransparentExprs method in Environment module. I will not add this method
at all.
================
Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:660-663
@@ +659,6 @@
+
+const MemRegion* ProgramState::getRegionFromStmt(const Stmt *S, const
LocationContext
+ *LCtx) const {
+ return getSVal(S, LCtx).getAsRegion();
+}
+
----------------
krememek wrote:
> This is just a one-liner. Do we really need this method at all? It is only
> called once.
We don't. I will add the one-liner directly where it is used.
================
Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:672-676
@@ -660,7 +671,7 @@
- const MemRegion *R = getSVal(S, LCtx).getAsRegion();
+ const MemRegion *R = getRegionFromStmt(S, LCtx);
addTaint(R, Kind);
// Cannot add taint, so just return the state.
return this;
}
----------------
krememek wrote:
> This looks fishy. 'addTaint' returns a new state, but then the return value
> is ignored, and 'this' is returned.
Yes, it does.. I will return at the time the last addTaint is invoked.
================
Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:704-708
@@ +703,7 @@
+
+ const MemRegion *R = getRegionFromStmt(S, LCtx);
+ removeTaint(R, Kind);
+
+ // Cannot remove taint, so just return the state.
+ return this;
+}
----------------
krememek wrote:
> This looks fishy. 'removeTaint' returns a new state, but then the return
> value is ignored. 'ProgramState' values are immutable, so this method
> appears to do nothing.
Yes, you are right. I will return at the time the last addTaint is invoked.
http://reviews.llvm.org/D11700
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
