[cfe-dev] [analyzer] Refactoring AnalyzerOptions

[Artem Dergachev via cfe-dev]

What do you think about refactoring Checkers.td into a .def-file and 
listing checker options there? Eg.,

     CHECKER(Malloc, core,
         "Check for memory leaks, double free, and use-after-free 
problems.")
     OPTION(Malloc, Optimistic, bool, false,
         "A useless option that needs to be removed.")

     CHECKER(PthreadLock, alpha.unix,
         "Simple lock -> unlock checker"

We could also de-duplicate packages (though i don't think that's 
necessary, as it's a matter of simple string prefix comparison):

     BEGIN_PACKAGE(unix, alpha)
         CHECKER(PthreadLock, "Simple lock -> unlock checker")
         CHECKER(...)
         OPTION(...)
     END_PACKAGE

=====

As an unrelated note, i've been dreaming for a while now about replacing 
packages with hashtags for more flexibility. Eg.,

     CHECKER(PthreadLock, "Simple lock -> unlock checker", "#alpha 
#posix #pathsensitive #threading")

Of course, we can always keep packages around for backwards compatibility.


On 10/26/18 3:17 PM, Kristóf Umann wrote:
> Too bad I cant edit mails.
>
> Where I talked about extracting all isn't easily accessable fields and 
> related methods to CheckerManagerData, I actually meant *easily* 
> accessible (since some checkers actually need to access LangOptions, 
> as well as AnalyerOptions, both avaible when other similar -help 
> options are handled).
>
> On 26 Oct 2018 20:58, "Kristóf Umann" <dkszelethus at gmail.com 
> <mailto:dkszelethus at gmail.com>> wrote:
>
>     Hi!
>
>     We did have a thread about this but with a very misleading title,
>     so here's a link to that, and I'll get into this mail:
>     http://lists.llvm.org/pipermail/cfe-dev/2018-October/059664.html
>     <http://lists.llvm.org/pipermail/cfe-dev/2018-October/059664.html>
>
>     AnalyzerOptions shouldn't be mutable once fully initialized (which
>     should be achieveble by the time the actual analysis begins), and
>     the greatest enemies of this idea are checker options, because
>     * we can either forget about collecting all checker options and
>     possibly diagnose them, and let checkers use AnalyzerOptions as a
>     sort of set of user supplied options. This is the current state of
>     things, and should my non-checker option refactoring effort go
>     through, AnalyzerOptions can be made const straight away.
>     * we could supply a mutable AnalyzerOptions object to the checkers
>     when registering, let them register and evaluate their options,
>     and make it immutable for the rest of the analysis.
>
>     I'm highly in favour of the second option, but it's a non-trivial
>     issue, mostly because of external plugins, which is why I'm
>     looking for some feedback on my ideas.
>
>     In order to register (and, more importantly, initialize) checkers,
>     one needs to have access to a CheckerManager object, which isn't
>     trivial to create, which makes it impossible to implement a help
>     flag (like -analyzer-checker-help or the proposed
>     -analyzer-config-help). I'm proposing two possible solutions.
>
>     1. Extract everything that isn't easily accessible to a new
>     CheckerManagerData class, make CheckerManager only responsible for
>     interacting (but not registering) checkers. I've got a fork on
>     which I managed to get this working, but I disliked this approach,
>     and went on to find a better solution.
>
>     2. Force checkers to properly register their options in a new,
>     registerOptionsFor##CHECKERNAME function, which would take
>     AnalyzerOptions as a parameter, alongside register##CHECKERNAME.
>     This would add one more complication to the already
>     very-not-trivial registering process, but could also be
>     autogenerated using tblgen.
>
>     It's clear to me that the second option is superior to the second,
>     but going forward with either is a lot of work, so I'm looking for
>     feedback.
>
>     Thanks to everyone who already took the time to help me with this
>     effort!
>
>     Cheers,
>     Kristóf
>