[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D72566#1815913 <https://reviews.llvm.org/D72566#1815913>, @njames93 wrote:

> In D72566#1815904 <https://reviews.llvm.org/D72566#1815904>, @lebedev.ri wrote:
>
> > I agree that the current alias situation is not ideal, though i'm not sure
> >  how much we can fix it since it crosses user interface boundary
> >  (i.e. what fixes won't lead to some regressions from the previous expected behavior)
>
>
>   If you're expecting a test to run twice usually something has gone wrong


That is not what i'm talking about.
I'm mainly worried about checks with custom config options:
It is likely that said options were specified only for one check,
not all of it's aliases too. So now the fact whether or not
these options will be used will depend on the registering order of checks.

>> To be noted, this will make adding of new aliases in non-up-stream modules more involved,
>>  since now the aliases will no longer be confined to the new module, but to the original modules.
> 
> That's how it currently is for adding new alias definitions. 
>  When you create an alias definition now you need the implementation files available.
>  However this approach doesn't need to modify the original module and appears mostly transparent to it.
>  Its also made in such a way that if you set a check as aliasing to a now existent name, it would just give you another way to invoke said check
> 
>> If we proceed as-is, i think we also need one more config switch "CheckAliasesAreEnabled = 1".
>>  And we should completely ignore aliases unless it's enabled.
> 
> That does sound like something I can get behind
> 
> There is one major nit I can think of and that is if you create an alias to the wrong check it would overwrite said check. I may look into adjusting that




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72566