[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

Thu Jan 31 02:00:32 PST 2019

Szelethus added a comment.

In D54438#1378425 <https://reviews.llvm.org/D54438#1378425>, @Szelethus wrote:

> In D54438#1375858 <https://reviews.llvm.org/D54438#1375858>, @george.karpenkov wrote:
>
> > After this landed, options for RetainCountChecker stopped working - e.g. I can't use `osx.cocoa.RetainCount:blah=X`.
> >  Do you know why is this the case / how to fix it?
>
>
> That's bad. I'll look into this ASAP, might take a day or two though if that's alright.

Nvm, just poppoed into my head. I suspect that this has been a bug for a while, this patch merely unearthed it. As you said:

In D55400#1367046 <https://reviews.llvm.org/D55400#1367046>, @george.karpenkov wrote:

> `registerChecker` gets-or-creates a checker object. A checker name (used for getting the options) is set the first time it's created.
>  The checker which was created first "wins" and gets to name the resulting checker.
>  In practice it basically means that options and checkers reusing the same class do not work.

This patch now ensures that //every time// `osx.cocoa.RetaiCount` and/or `osx.OSObjectREtainCount= is enabled, `osx.RetainCountBase`˙is registered first, which makes the checker object's name just that, making (incorrectly ofc) `AnalyzerOptions` look for `osx.RetainCountBase:blah=X`.

Should we not have multiple checkers belong to the same chekcer object, the approach of requsting this checker object for it's options would make sense, but since this isn't the case, this bug exposes a weakness of `AnalyzerOptions::getChecker*Option`, namely that these subcheckers aren't able to possess any options at all, and even if one manages to get it working, as you said, merely changing the order of `-analyzer-checker=` could mess things up.

So clearly, `AnalyzerOptions` should just take the actual checker name as parameter, which each checker knows via the `Name` field, or via `CheckerManager::getCurrentCheckName` in the checker registry function. I'll try to sort this out tonight.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D54438