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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

>> 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.

Yes and no, the CPPCG do acknowledge the fact, that macros are (still)
necessary for some jobs, like compile-time configurations of values and
require that the macros should at least be super visible in the code
(enforceable with the regex option in this check).

Turning off assertions for release builds would be a good example of
necessary macros. I would say this check must take this into account.

As for printing the name in the diagnostic: Not sure if this really adds
value. The location the diagnostic is pointing to is the name of macro
itself. The unclear diag comes from the fact that the macro is not
defined in code, so i think we should find a position on that issue first.

My opinion is, that macros without location (however compiler defined
macros) should not be diagnosed. It is unlikely that complicated
functions or the like a defined via compiler flags. These tend to be
used for integral constants to turn things on/off.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53817