[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:244
@@ +243,3 @@
+                                  const ProgramPointTag *Tag = nullptr) {
+    return generateSink(State, /*Pred=*/nullptr,
+                       (Tag ? Tag : Location.getTag()));
----------------
xazax.hun wrote:
> zaks.anna wrote:
> > Please, use a non-null Pred for clarity
> The following workflow is not supported by this API: a checker that generates multiple transition in the same callback (the generated nodes are added sequentially to the path), and one of the might be an error node.
> 
> This also applies to generateNonFatalErrorNode.
> 
> In case we would like to improve the documentation it might be useful to give some pointers to the users which when should an error node be considered as fatal.
 - We could introduce another API that requires a Pred node.
 - The workflow is supported right now by directly using addTransition API.

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


http://reviews.llvm.org/D12780