[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 29 09:34:13 PDT 2018


aaron.ballman added a comment.

In https://reviews.llvm.org/D53817#1278933, @lebedev.ri wrote:

> In https://reviews.llvm.org/D53817#1278916, @JonasToth wrote:
>
> > How does this patch change the behaviour for macros without location?
>
>
> It doesn't.
>
> > They will be diagnosed at the beginning of the TU?
>
>
>
> > It is probably better to ignore these kind of macros, as there is no way around them (compile time configuration can only be done with macros?!) and therefore warnings for theses macros are false positives. Using `constexpr` constructs does not help either.
>
> I agree that it might be good idea to add an *option* to not diagnose such macros.
>  But i think it is not the spirit of the check/rule to ignore them completely [by default],
>  because those macros are still horrible, and can affect all kinds of things,
>  just like the 'normal' macros.


I'm not certain an option makes sense in practice. Predefined macros and macros that come from the command line are a different beast from macros defined by the developer -- I don't think we should be diagnosing those cases as they're generally outside of programmer control. Predefined macros are entirely outside of the user's control. Macros from the command line are most often going to be controlling build toggles, but there is no reason to assume the user can control those (like 3rd party packages) or that they can be replaced with an alternative like `constexpr`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53817





More information about the cfe-commits mailing list