[PATCH] D12761: MPI-Checker patch for Clang Static Analyzer

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

I've been mostly looking at the path sensitive checks. Maybe clang-tidy team would be interested in reviewing the syntactic checks.


================
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Container.h:31
@@ +30,2 @@
+
+#endif
----------------
Alexander_Droste wrote:
> I just found `/include/llvm/ADT/STLExtras.h`. Wouldn't that be a good fit for the `contains()` function?
This sounds right.
I suggest submitting separate patches to the corresponding components.

================
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp:107
@@ +106,3 @@
+      State = State->remove<RequestMap>(Req.first);
+    }
+  }
----------------
Alexander_Droste wrote:
> I need `addTransition()` later to take removed requests into account.
What you have now, is generating a fork - two successors from the initial predecessor. You need to use the addTransition method that takes a predecessor and pass the ErrorNode there. You should also use the State from that node, not the initial state.

================
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Utility.h:54
@@ +53,3 @@
+/// \returns variable name for memory region
+std::string variableName(const clang::ento::MemRegion *MR);
+
----------------
Since this takes no arguments but MemRegion, seems like it could go on MemRegion. Please, submit this as a separate patch.




http://reviews.llvm.org/D12761