Alexander_Droste added inline comments.
================
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++};
----------------
Alexander_Droste wrote:
> 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.
>
I'll give this pattern a try:
```
const Stmt *S = nullptr;
ProgramPoint ProgLoc = N->getLocation();
if (Optional<StmtPoint> SP = ProgLoc.getAs<StmtPoint>()) {
S = SP->getStmt();
}
// if S then S->getSourceRange
```
Looks like it could work.
http://reviews.llvm.org/D12761
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
