Alexander_Droste added a comment. Sorry about the delay.
================ 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: > Alexander_Droste wrote: > > 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. > I suspect the only thing you need in Request is the enum; everything else can > be determined while visiting the path. > > This should be in ento namespace not in the clang namespace. I think you're right that the enum is sufficient. Thus for better diagnostics the source range of the CallEventRef (LastUser) should be obtained by the BugReportVisitor. Is it possible to obtain a CallEventRef based on a memory region? So conceptually this would mean to find the last function call (before the error node) using a specific region. http://reviews.llvm.org/D12761 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits