dcoughlin marked 3 inline comments as done.
================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:229
@@ -228,2 +228,3 @@
+ /// checkers should use generateErrorNode() instead.
ExplodedNode *generateSink(ProgramStateRef State = nullptr,
ExplodedNode *Pred = nullptr,
----------------
zaks.anna wrote:
> Most likely there are not much uses of this left and to avoid confusion we
> could require State and Pred inputs. What do you think?
There are 7 uses left. Requiring State and Pred seems like the right thing to
me. I will change it.
================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:321
@@ +320,3 @@
+ // sink, we assume that a client requesting a transition to a state that is
+ // the same as the predecessor state has made a mistake. We return the
+ // predecessor and rather than cache out.
----------------
jordan_rose wrote:
> What does "has made a mistake" mean? What is the mistake and how will they
> discover it?
The mistake this guard protects against is adding a transition from a state to
itself, which would normally cause a cache out. My understanding is that the
whole point of this guard is to silently swallow that mistake. I found that
surprising, which is why I added the comment.
================
Comment at: lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp:53
@@ -52,3 +52,3 @@
- if (ExplodedNode *N = C.addTransition()) {
+ if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
if (!BT)
----------------
zaks.anna wrote:
> jordan_rose wrote:
> > zaks.anna wrote:
> > > Can this ever fail? In some cases we just assume it won't in others we
> > > tests..
> > >
> > > Maybe it only fails when we cache out?
> > It does fail when we cache out, and I think we can still cache out if Pred
> > has a different tag the second time around.
> There some instances in this patch where we do not check if the returned node
> is null. We should be consistent.
Ok, I'll go through and add checks where they are missing.
http://reviews.llvm.org/D12780
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
