njames93 added a comment.

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

> 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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to