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

[Alexander Droste via cfe-commits]

Alexander_Droste added inline comments.

================
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td:524
@@ +523,3 @@
+def MPIChecker : Checker<"MPI-Checker">,
+  HelpText<"Checks MPI code written in C">,
+  DescFile<"MPIChecker.cpp">;
----------------
gribozavr wrote:
> Does it only works with C code, or does it work with C++ code that uses C bindings for MPI?  I think it is safe to describe it as "Checks MPI code".
I guess you're right, it should work with C++ code that uses C bindings for MPI
but I've only evaluated and tested the checker on plain C code.
Should we delay that decision to the end of the review?


================
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerAST.cpp:22
@@ +21,3 @@
+
+void MPICheckerAST::initMPITypeContainer() {
+  mpiTypes_ = {"MPI_BYTE",
----------------
gribozavr wrote:
> MPICH has its header files annotated for Clang for a few years already, I think we should be using the annotations instead of textually matching the constants.  Moreover, some users are using those annotations for their own datatypes.
I agree that using annotations is preferable, as the type matching is checked
during normal compilation but as you mentioned,
it necessary that those are implemented in a specific MPI implementation.
The check would support type matching for any implementation.

Custom buffer types and custom MPI types are simply skipped why that should not
impose a problem. Further, I planned to support custom types in the future. They could get 
passed as something like a list of pairs to the checker 
{{"customBufType", "customMPI"},...}.

================
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerAST.hpp:101
@@ +100,3 @@
+  MPIFunctionClassifier funcClassifier_;
+  llvm::SmallVector<std::string, 32> mpiTypes_;
+  MPIBugReporter bugReporter_;
----------------
gribozavr wrote:
> This should be a hashtable-based set, like `llvm::DenseSet`.  Scanning this vector over and over again is wasteful.
`llvm::DenseSet<std::string>` does not compile. 
`error: no member named 'getEmptyKey'  ..`
Is there an alternative?


http://reviews.llvm.org/D12761