aaron.ballman added a comment.
I think this check is going to be extraordinarily chatty. For instance, macros
are often used to hide compiler details in signatures, such as use of
attributes. This construct cannot be replaced with anything else because the
macro isn't defining an object or value. Another example where this will be bad
is for conditional processing where the macro is later used in a `#if`,
`#elif`, `#ifdef`, or `#ifndef` preprocessor conditional, as this also cannot
be replaced with a `constexpr` variable. Without handling things like this, I
don't see how this rule can provide utility to real world code. Do you have
ideas for how to handle these kind of situations?
================
Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:21
+public:
+ MacroUsageCallbacks(MacroUsageCheck *Check) : Check{Check} {}
+ void MacroDefined(const Token &MacroNameTok,
----------------
I'd use `Check(Check)` instead of curly braces.
================
Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:28
+
+ Check->warnMacro(MD);
+ }
----------------
I don't think this warning would be appropriate in all language modes. For
instance, macros in C code, or in C++98 mode, etc.
================
Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:42
+void MacroUsageCheck::warnMacro(const MacroDirective *MD) {
+ std::string DiagnosticMessage = "don't use macros to declare constants";
+
----------------
These diagnostics should be reworded to not use contractions and instead
explain what the issue is. e.g., macro used to define a constant; consider
using 'constexpr' instead (or something along those lines).
================
Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:44
+
+ auto *Info = MD->getMacroInfo();
+ if (Info->isFunctionLike())
----------------
Don't use `auto`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41648
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
