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

[Gábor Horváth via cfe-commits]

xazax.hun added a comment.

In general I like this change, the node handling of the checkers are more readable and reflects the intent in a clearer way.  I have some comments inline.


================
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()));
----------------
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.

================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:322
@@ -290,1 +321,3 @@
+    // the same as the predecessor state has made a mistake. We return the
+    // predecessor and rather than cache out.
     if (!State || (State == Pred->getState() && !Tag && !MarkAsSink))
----------------
As a slightly related note: is it documented anywhere what "cache out" means? Maybe it would be great to refer to that document or write it if it is not written yet.


http://reviews.llvm.org/D12780