[PATCH] D38171: Implement clang-tidy check aliases.

András Leitereg via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 26 15:31:29 PDT 2017


leanil added a comment.

In https://reviews.llvm.org/D38171#880021, @xazax.hun wrote:

> In https://reviews.llvm.org/D38171#878808, @alexfh wrote:
>
> > András, that's definitely an interesting idea. However, it might be interesting to explore a more principled approach:
> >
> > 1. Make `clang-diagnostic-*` checks first-class citizens and take full control of all diagnostics, i.e. disable all Clang diagnostics by default, and enable the ones that correspond to the enabled clang-diagnostic checks.
>
>
> I agree that this is a good idea. I think it could be done in this patch. But to be sure, could you elaborate on what do you mean by first-class citizen?
>
> > 2. Make aliases first-class citizens (there was a proposal as well, but we didn't arrive to a consensus at that time). That would include the ability to configure an alias name for any check including clang-diagnostic- and clang-analyzer- checks.
>
> Do you have a link to the proposal? What do you mean by the ability to configure? As in having a config file or registering aliases in modules like now? If you mean the former, I think that is better addressed in a follow-up patch.


I also think this sounds good, though I'm not quite sure it can be done in this patch. Anyway, we should definitely specify what we expect from first-class citizen checks. Please correct & extend the list:

- can be enabled or disabled through a unique name
- can have config options
- can have aliases
- inherits ClangTidyCheck

I see the last one as a requirement for handling everything uniformly (without workarounds like this patch), but also as a main point where an alias differs from a "real" check.


https://reviews.llvm.org/D38171





More information about the cfe-commits mailing list