[PATCH] D69745: [analyzer] Checker: check::BeginAnalysis

Csaba Dabis via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 4 08:16:51 PST 2019


Charusso marked 3 inline comments as done.
Charusso added a comment.

In D69745#1732348 <https://reviews.llvm.org/D69745#1732348>, @Szelethus wrote:

> YES PLEASE. Debug checkers that only dump from `check::EndAnalysis` won't rely on the analysis not actually crashing. Which is ironically exactly when we want to use them.


Thanks for the idea! I am not that cool with debug checkers and I think you are the first user who made a creative decision how to use this. My use case is the `Preprocessor` only if I do not touch the checker-registry based construction.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp:199
+  ///
+  /// This callback should be used to construct the checker's fields.
+  ///
----------------
Szelethus wrote:
> Szelethus wrote:
> > Hmmm, what use case do you have in mind? Do we actually want this to happen? By the time checkers are constructed, the AST is already built, alongside all the other data structures that will be inspected by the analyzer. Since checkers are constructed once for each clang invocation (not for every `BeginAnalysis` -- `EndAnalysis` cycle), I would imagine that the checker registry functions are more appropriate for such initializations.
> > 
> > I'm only aware of one non-debug checker that uses `EndAnalysis` (maybe `RetailCount`?), that clears some sort of a locally stored map that it shouldn't really have anyways (since checkers are supposed to be stateless), and I struggle to think of a realistic, supported use case for this callback other than debug checkers.
> Turns out `deadcode.UnreachableCode` uses `EndAnalysis` quite cleverly as well.
Yes, good point. It is made for the `Preprocessor` mainly which is very path-sensitive, but that is not that good idea to construct e.g. the `BugType` multiple times. So that, I do not want to restrict creativity and I have removed that line.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69745/new/

https://reviews.llvm.org/D69745





More information about the cfe-commits mailing list