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

[Alexander Droste via cfe-commits]

Alexander_Droste added a comment.

Hi Anna,

> I am fine with committing it and iterating with smaller updates in tree if it is more convenient for you.


This sounds good! The last thing I'll change before are the improvements you pointed out.

> One task that I would like to very strongly encourage is running this on a lot of code.


Good idea. I'll do that.

Thanks a lot for all the time and effort you invested into the review!


================
Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:87
@@ +86,3 @@
+      }
+      // A wait has no matching nonblocking call.
+      BReporter->reportUnmatchedWait(PreCallEvent, ReqRegion, ErrorNode);
----------------
zaks.anna wrote:
> This is done, right?
Yep.

================
Comment at: test/Analysis/MPIChecker.cpp:99
@@ +98,3 @@
+  MPI_Wait(&req, MPI_STATUS_IGNORE);
+}
+
----------------
zaks.anna wrote:
> This are explaining the path on which the problem occurs; the users will see them as well. There should not be a lot of those, you do not have a lot of conditions. Would it be reasonable to change the tests to incorporate those. Other alternative is to have another tests file that tests the notes in that mode.
> 
> What do you think?
I'm fine with adding the notes to this test file.

================
Comment at: test/Analysis/MPIChecker.cpp:114
@@ +113,3 @@
+
+void doubleNonblocking4() {
+  int rank = 0;
----------------
zaks.anna wrote:
> > I would then simply create a new pair of .cpp and .h files in the test folder 
> > where I define those functions so that the MPI-Checker tests can use them.
> 
> You do not have to do that. You could just declare the functions and not define them. It will be equivalent to having the definitions in the other TUs.
> 
> 
Like you suggested, I'll simply declare the functions without definition.


http://reviews.llvm.org/D12761