[PATCH] D55400: [analyzer] Move out tracking retain count for OSObjects into a separate checker

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 23 00:20:41 PST 2019


Szelethus added a comment.

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

> > Hmmm, does this mess with options that bad? Could you please clarify?
>
> `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.
>  Do you have better ideas on how this could be arranged?


Sure, in fact its already implemented and got accepted by @NoQ, I just didnt have the time to commit and fine tune :)

The solution is essentially to create `CheckerManager::getChecker` alongside `registerChecker`, and making them in order assert if the checker is unregistered, and assert when the checked is already registered. In an earlier patch, I implement handling of dependencies on a higher level, essentially making `CheckerRegistry` take care of this, instead of making checker registry functions register multiple checkers, or, like in this case, register a common base. Before the registry function for either RetainCound or OSObjectRetainCount is called (which will now invoke `getChecker`), the registry function for RetainCountBase will be called, that will register the actual checker object.

You can take a look at how this would look like in D54438 <https://reviews.llvm.org/D54438>.

Would love to hear your thoughts on this! :)

> I think the current situation is a mess - ideally I would prefer to be able to access the options in the constructor, but we can't even do that,
>  since `registerChecker` sets the checker name and is called after the object has been constructed.
>  It seems that it would only make sense if the checker name is known at the time the checker is constructed: probably the function `registerXChecker` should get it as an argument.

The proposed solution solves this issue as well.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55400





More information about the cfe-commits mailing list