[PATCH] D12780: [analyzer] Add generateErrorNode() APIs to CheckerContext

[Anna Zaks via cfe-commits]

zaks.anna added inline comments.

================
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,
----------------
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?

================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:244
@@ +243,3 @@
+                                  const ProgramPointTag *Tag = nullptr) {
+    return generateSink(State, /*Pred=*/nullptr,
+                       (Tag ? Tag : Location.getTag()));
----------------
Please, use a non-null Pred for clarity

================
Comment at: lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:89
@@ -88,3 +88,3 @@
                                          CheckerContext &C) const {
-  ExplodedNode *N = C.getPredecessor();
+  ExplodedNode *N = C.generateNonFatalErrorNode();
   const LocationContext *LC = N->getLocationContext();
----------------
We should not generate a transition on an early exit here..

================
Comment at: lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:116
@@ -115,3 +115,3 @@
                                                  CheckerContext &C) const {
-  ExplodedNode *N = C.getPredecessor();
+  ExplodedNode *N = C.generateNonFatalErrorNode();
   const LocationContext *LC = N->getLocationContext();
----------------
Same here, no reason to generate a transition unless we are ready to report a bug. I'just pass C.generateNonFatalErrorNode() to  llvm::make_unique<BugReport>.

================
Comment at: lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp:53
@@ -52,3 +52,3 @@
 
-  if (ExplodedNode *N = C.addTransition()) {
+  if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
     if (!BT)
----------------
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?


http://reviews.llvm.org/D12780