This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Supply all checkers with a shouldRegister function
ClosedPublic

Authored by Szelethus on Dec 7 2018, 3:38 AM.

Details

Summary

Introduce the boolean ento::shouldRegister##CHECKERNAME(const LangOptions &LO) function very similarly to ento::register##CHECKERNAME. This will force every checker to implement this function, but maybe it isn't that bad: I saw a lot of ObjC or C++ specific checkers that should probably not register themselves based on some LangOptions (mine too), but they do anyways.

A big benefit of this is that all registry functions now register their checker, once it is called, registration is guaranteed.

This patch is a part of a greater effort to reinvent checker registration, more info here: D54438#1315953

Diff Detail

Repository
rC Clang

Event Timeline

Szelethus created this revision.Dec 7 2018, 3:38 AM

The checker file changes in this patch are pretty boring, here are those that don't always return true in shouldRegister:

  • CastSizeChecker
  • GTestChecker
  • NSAutoreleasePoolChecker
  • ObjCAtSyncChecker
NoQ accepted this revision.Dec 7 2018, 2:25 PM

Yeah, this part looks easy :)

NoQ added a comment.Dec 10 2018, 4:50 PM

Should we also pass CheckerManager into shouldRegister...? Or is it entirely useless?

In D55424#1326397, @NoQ wrote:

Should we also pass CheckerManager into shouldRegister...? Or is it entirely useless?

I wouldn't say useless, but I'm struggling to come up with an example where registering would depend on non-language dependent reasons. I'm stuck on rebasing my patches, so I'll give it a thought whether it's worth the chore to change everything (but where I'd like to leave a system for the long term, doing extra work shouldn't be a major concern).

teemperor resigned from this revision.Jan 21 2019, 3:02 AM
This revision is now accepted and ready to land.Jan 21 2019, 3:02 AM
This revision was automatically updated to reflect the committed changes.
In D55424#1326397, @NoQ wrote:

Should we also pass CheckerManager into shouldRegister...? Or is it entirely useless?

I wouldn't say useless, but I'm struggling to come up with an example where registering would depend on non-language dependent reasons. I'm stuck on rebasing my patches, so I'll give it a thought whether it's worth the chore to change everything (but where I'd like to leave a system for the long term, doing extra work shouldn't be a major concern).

For now, I decided against it just to make progress with the project, but if the need arises, I'll be happy to change it.