zaks.anna added a comment.

We gave a talk about building checkers a while back; even though it does not 
talk about the bug reporter visitors, it might be worth watching if you haven't 
already seen it.

http://llvm.org/devmtg/2012-11/ 
Building a Checker in 24 hours


================
Comment at: 
tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp:68
@@ +67,3 @@
+  const CallEventRef<> CERef = PreCallEvent.cloneWithState(State);
+  const ExplodedNode *const ExplNode = Ctx.addTransition();
+  llvm::SmallVector<const MemRegion *, 2> ReqRegions;
----------------
There is no reason to add an empty transition to the graph. Maybe you want to 
get the predecessor instead?

================
Comment at: 
tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp:106
@@ +105,3 @@
+    static CheckerProgramPointTag Tag("MPI-Checker", "MissingWait");
+    ExplodedNode *const ExplNode =
+        Ctx.generateNonFatalErrorNode(Ctx.getState(), &Tag);
----------------
generateNonFatalErrorNode will add a transition to the exploded graph, which 
means that you should only generate it if there is an error and you do not need 
to call addTransition later in the function. 

================
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h:67
@@ +66,3 @@
+  // Every time a request is 'set' a new 'RequestId' gets created.
+  // Therefore, the 'UserKind' does not need to be profiled.
+  const int RequestId{RequestIdCount++};
----------------
Still it looks like you have several pieces of information in the state that 
are redundant.. Looks like you've added more now.

For example, why do we need RequestId? Each report will only talk about a 
single request, is this not the case?

Do you absolutely need to store LastUser and VariableName?

Note that storing in the state is very expensive; on the other hand, we can 
afford to perform costly analysis on post-processing of an error report. This 
is why all other checkers store minimal information in the state, usually just 
a single enum and determine all the other information during the walk over the 
error path in BugReporterVisitor. The visitor will visit all nodes that you 
visited while reporting this bug, so it should have access to all the 
information you need.



http://reviews.llvm.org/D12761



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to