Alexander_Droste added inline comments. ================ Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:43 @@ +42,3 @@ +private: + const std::unique_ptr<MPICheckerPathSensitive> CheckerSens; + ---------------- zaks.anna wrote: > Alexander_Droste wrote: > > zaks.anna wrote: > > > I'd stress "path" instead of "sensitive" in the name. Also, this > > > indirection is redundant if you remove the AST checks. > > If sufficient, I would rename `MPICheckerPathSensitive` to `MPICheckerPath` > > then. > > Do you mind the indirection? > I indirection buying us anything? I think it makes the code more difficult to > follow, I think, I like the entry point class being separated from the class which does the actual checking. But the necessity for `dynamicInit()` maybe outweighs that. I can collapse the two.
================ Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp:74 @@ +73,3 @@ + // A wait has no matching nonblocking call. + BugReporter.reportUnmatchedWait(PreCallEvent, ReqRegion, ExplNode); + } ---------------- zaks.anna wrote: > Alexander_Droste wrote: > > zaks.anna wrote: > > > This is called in a loop. Should you break once the first error is > > > reported? > > > > > > Also, as before you should use the CheckerContext API to produce a node > > > on which the error should be reported. > > > > > > > > I don't think break should be called after the first error is reported. > > Each memory region > > represents a different request, why this should be rated as multiple > > errors. > You might be right, I am not 100% sure what's more user-friendly.. > Presumably the fixes are different in each case? Could you add a test case > where multiple errors on the same call site are reported? (Ups can use > expected-warning@+1 to report multiple warnings on the same line.) This is comparable to calling free on different elements, for which memory was never allocated. So each report informs about a distinct missing nonblocking call. I'll give `expected-warning@+1` a try. http://reviews.llvm.org/D12761 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
