[PATCH] D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks"

Wed Sep 4 14:10:58 PDT 2019

gribozavr added a comment.

In D67140#1658315 <https://reviews.llvm.org/D67140#1658315>, @aaron.ballman wrote:

> In D67140#1656831 <https://reviews.llvm.org/D67140#1656831>, @NoQ wrote:
>
> > Honestly, i'm much more worried about message capitalization :)
>
>
> Likewise. I wish the static analyzer would follow the usual conventions followed by clang and clang-tidy. ;-)

I have the opposite opinion -- I wish that ClangTidy used complete sentences, and multiple sentences if it makes sense. The sentence fragments are too brief to explain complex and nuanced topics that ClangTidy communicates about. ClangTidy often plays the role of a developer education tool. It is not a guard like a compiler; developers can totally ignore ClangTidy if they disagree with the message. The better we can explain the problem, the more likely it is the developer will act on the message. I believe static analysis tools would be better off if we could write multiple sentences in the diagnostic.

Even for compiler messages, a sentence fragment is sometimes too concise.

> In D67140#1657421 <https://reviews.llvm.org/D67140#1657421>, @alexfh wrote:
> 
>> Historically, clang-tidy only used the term "check" (to denote the thing that checks something, rather than the rule being checked or the act of checking), and we tried to keep its use consistent. However, "checker" is a more precise and less ambiguous way to convey this meaning. I support to use the term "checker" in clang-tidy, as long as someone is willing to update the code and documentation (except for verbs, e.g. the `check()` method ;). Also note that there's a non-trivial number of out-of-tree check(er)s out there. They will need to be updated as well.
>>
>> Adding Aaron in case he has a different opinion.
> 
> 
> My primary concern is with needless churn for out-of-tree clients. They don't get any real added benefit from the change in nomenclature, but renaming `ClangTidyCheck` to `ClangTidyChecker` will break every single out of tree clang-tidy checker. I've not used the plugin infrastructure for clang-tidy, but will this cause plugins to fail to load? If so, is it a silent failure or a noisy one?

A lot of the changes we do in Clang break many clients. I think we should try to find the best API.

We could add a deprecated typedef for, say, one release, if you think that would be significantly helpful for out-of-tree users.

The pre-compiled plugins will fail to load, but they will fail to load regardless of whether we make this change or not. The plugins will have to be recompiled anyway (and they will fail to compile), since Clang does not provide a stable library ABI.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67140