[cfe-dev] [analyzer] Restructuring the interface of the RetainCountChecker family

[Kristóf Umann via cfe-dev]

On Wed, 28 Aug 2019 at 05:12, Kristóf Umann <dkszelethus at gmail.com> wrote:

> Hi!
>
> This family of checkers is not under my authority, nor am I that
> knowledgeable about them, but even as a non-user, I find its interface
> confusing. While this doesn't affect me much, it is also the greatest
> sinner of how modeling/diagnostic checkers should be structured according
> to our previous discussions.
>
> I'd be happy to fix it (in fact, I'd prefer to, just to gain a better
> understanding of how the new checker system should look like), but I
> obviously can't make decisions on how it should look like -- could you help
> me please?
>
> Here is the problem:
> * OSObjectRetainCountChecker and RetainCountChecker are subcheckers
> of RetainCountBase, yet they seem to fine-tune how the modeling should be
> done, rather then what diagnostics should be emitted. Shouldn't we turn
> them into checker options of RetainCountBase instead?
>
> * OSObjectRetainCountChecker and the checker option
> osx.cocoa.RetainCount:CheckOSObject have a super weird interaction -- they
> are supposed to do the same thing (optionally enable some modeling
> RetainCount does), and exist purely for backward compatibility reasons, but
> in a way that I personally find impossible to understand.
>
> The option was added by George, and was tied to osx.cocoa.RetainCount
> rather then osx.OSObjectRetainCountChecker, yet the option is unused unless
> osx.OSObjectRetainCountChecker itself is enabled, making it the only
> checker option ever to have 3 stances: enabled, disabled, and unspecified,
> the only remaining option not retrievable with debug.ConfigDumper, and is
> also the single reason why we can't make AnalyzerOptions' config table
> private.
>
> https://reviews.llvm.org/rGd1081ec5082ba6ba26809c66e410b127ca5819a8
>
> Would it be possible to just simply make this a *regular* option that
> belongs to osx.RetainCountBase? Are there any users relying on this
> behavior?
>

Another possible solutions:
* Supply the shouldRegister* functions with more data (AnalyzerOptions in
particular), don't even enable the checker if the option is false.
* When we're parsing the checker options in CompilerInvocation.cpp,
manually turn this into an -analyzer-disable-checker.


> * RefCountBug::RefCountBugType is an enum for various types of retain
> count related errors. Shouldn't we turn these into subcheckers?
>
> Mind that we can always do this in a backwards-compatible manner, and
> aside from the Schrödinger option, we can also preserve the original
> behavior.
>
> Cheers!
> Husi
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190828/6852345c/attachment.html>