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

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

Thanks you so much! One of the reasons why I love working on this is because the great feedback I get. I don't wanna seem annoyingly grateful, but it's been a very enjoyable experience so far.

> This should not cause a warnings because it should be fine to compile different translation units with different flags within the same project, while the decision to enable the checker explicitly is done once per project.

Okay, I don't insist on it :)

> Now, normally these decisions are handled by the Clang Driver - the gcc compatibility wrapper for clang that converts usual gcc-compatible run-lines into `-cc1` run-lines. Eg., the `unix` package is only enabled on UNIX targets and the `osx` package is only enabled on macOS/iOS targets and the `core` package is enabled on all targets and `security.insecureAPI` is disabled on PS4 //because that's what the Driver automatically appends to the -cc1 run-line//. Even though `scan-build` does not call the analyzer through the Driver, it invokes a `-###` run-line to obtain flags first, so it still indirectly consults the Driver.
> 
> But the problem with the Driver is that nobody knows about this layer of decision-making (unlike the obvious `Checkers.td`) and therefore it's kinda messy to have all checkers hardcoded separately within the Driver. It kinda makes more sense to split the checkers into packages and let the Driver choose which packages are enabled.
>  So, generally, i suggest not to jump onto this as long as the analyzer as a whole works more or less correctly. Restricting functionality for the purpose of avoiding mistakes should be done with extreme care because, well, it restricts functionality. Before restricting funcitonality, we need to be more or less certain that nobody will ever need such functionality or that we will be able to provide a safe replacement when necessary without also introducing too much complexity (aka bugs). Which is why i recommend not to bother too much about it. There are much more terrible bugs all over the place, no need to struggle that hard to prevent these small bugs.
> 
> Another approach to the original problem would be to replace the language options check in registerBlahBlahChecker() with a set of language options checks in every checker callback. It's annoying to write and clutters the checker but it preserves all the functionality without messing with the registration process. So if you simply want to move these checks out of the way of your work, this is probably the way to go.

Do you mean something along the lines of supplying a boolean `shouldRegister##CHECKERNAME` function to checkers? That would essentially achieve the same thing, but still allows me to move `CheckerManager::registerChecker` ultimately out of checker registry function.


Repository:
  rC Clang

https://reviews.llvm.org/D54438