LegalizeAdulthood created this revision.
LegalizeAdulthood added a reviewer: alexfh.
Herald added subscribers: carlosgalvezp, kbarton, xazax.hun, nemanjai.
LegalizeAdulthood requested review of this revision.
Herald added a project: clang-tools-extra.
Previously, any macro that didn't look like a varargs macro
or a function style macro was reported with a warning that
it should be replaced with a constexpr const declaration.
This is only reasonable when the macro body contains constants
and not expansions like ",", "[[noreturn]]", "__declspec(xxx)",
etc.
So instead of always issuing a warning about every macro that
doesn't look like a varargs or function style macro, examine the
tokens in the macro and only warn about the macro if it contains
only comment and constant tokens.
Fixes #39945
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D116386
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t -- -header-filter=.* -system-headers --
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage -std=c++17-or-later %t -- -header-filter=.* -system-headers --
#ifndef INCLUDE_GUARD
#define INCLUDE_GUARD
@@ -6,6 +6,21 @@
#define PROBLEMATIC_CONSTANT 0
// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT' used to declare a constant; consider using a 'constexpr' constant
+#define PROBLEMATIC_CONSTANT_CHAR '0'
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT_CHAR' used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_CONSTANT_WIDE_CHAR L'0'
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT_WIDE_CHAR' used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_CONSTANT_UTF8_CHAR u8'0'
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT_UTF8_CHAR' used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_CONSTANT_UTF16_CHAR u'0'
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT_UTF16_CHAR' used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_CONSTANT_UTF32_CHAR U'0'
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT_UTF32_CHAR' used to declare a constant; consider using a 'constexpr' constant
+
#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro 'PROBLEMATIC_FUNCTION' used; consider a 'constexpr' template function
@@ -15,4 +30,17 @@
#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro 'PROBLEMATIC_VARIADIC2' used; consider using a 'constexpr' variadic template function
+// These are all examples of common macros that shouldn't have constexpr suggestions.
+#define COMMA ,
+
+#define NORETURN [[noreturn]]
+
+#define DEPRECATED attribute((deprecated))
+
+#if LIB_EXPORTS
+#define DLLEXPORTS __declspec(dllexport)
+#else
+#define DLLEXPORTS __declspec(dllimport)
+#endif
+
#endif
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
@@ -78,22 +78,52 @@
this, SM, AllowedRegexp, CheckCapsOnly, IgnoreCommandLineMacros));
}
+namespace {
+
+bool isConstantToken(const MacroDirective *MD) {
+ for (const auto &Token : MD->getMacroInfo()->tokens()) {
+ switch (Token.getKind()) {
+ case tok::comment:
+ case tok::numeric_constant:
+ case tok::char_constant:
+ case tok::wide_char_constant:
+ case tok::utf8_char_constant:
+ case tok::utf16_char_constant:
+ case tok::utf32_char_constant:
+ case tok::string_literal:
+ case tok::wide_string_literal:
+ case tok::header_name:
+ case tok::utf8_string_literal:
+ case tok::utf16_string_literal:
+ case tok::utf32_string_literal:
+ break;
+ default:
+ return false;
+ }
+ }
+ return true;
+}
+
+} // namespace
+
void MacroUsageCheck::warnMacro(const MacroDirective *MD, StringRef MacroName) {
- StringRef Message =
- "macro '%0' used to declare a constant; consider using a 'constexpr' "
- "constant";
+ StringRef Message;
+ if (isConstantToken(MD))
+ Message = "macro '%0' used to declare a constant; consider using a "
+ "'constexpr' constant";
/// A variadic macro is function-like at the same time. Therefore variadic
/// macros are checked first and will be excluded for the function-like
/// diagnostic.
- if (MD->getMacroInfo()->isVariadic())
+ else if (MD->getMacroInfo()->isVariadic())
Message = "variadic macro '%0' used; consider using a 'constexpr' "
"variadic template function";
else if (MD->getMacroInfo()->isFunctionLike())
Message = "function-like macro '%0' used; consider a 'constexpr' template "
"function";
- diag(MD->getLocation(), Message) << MacroName;
+ if (!Message.empty())
+ diag(MD->getLocation(), Message) << MacroName;
}
void MacroUsageCheck::warnNaming(const MacroDirective *MD,
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
