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
