Alexander_Droste 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.
I have watched it some months ago but I'm sure it is a good idea to revisit the talk. Thanks for the idea! ================ 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; ---------------- zaks.anna wrote: > There is no reason to add an empty transition to the graph. Maybe you want to > get the predecessor instead? Yes, that's better! ================ 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++}; ---------------- zaks.anna wrote: > 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. > I need the RequestID to distinct function calls of the same type and location using the request multiple times along the path. Like in a loop: ``` for int i ... nonblocking(.., request) // double nonblocking ``` > Do you absolutely need to store LastUser and VariableName? Absolutely not. :D I will remove the string member. The variable name retrieval can be delayed to the point where the report is created. I can also remove the enum, as the state of the request can be derived from the LastUser. The reason why I added the enum member, is that I was concerned about that the CallEventRef can be invalid to reference again if the call is inlined from an implementation defined in a header. As this seems not to be the case, I can remove the redundancy. > 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. Ah, I get it. Until now that seemed a bit overly complicated to me. http://reviews.llvm.org/D12761 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits