[PATCH] D116386: [clang-tidy] Narrow cppguidelines-macro-usage to actual constants

Richard via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 19 10:38:09 PST 2022

LegalizeAdulthood marked 2 inline comments as done.
LegalizeAdulthood added inline comments.

Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:23
-namespace {
-bool isCapsOnly(StringRef Name) {
-  return std::all_of(Name.begin(), Name.end(), [](const char C) {
-    if (std::isupper(C) || std::isdigit(C) || C == '_')
-      return true;
-    return false;
+static inline bool isCapsOnly(StringRef Name) {
+  return llvm::all_of(Name, [](const char C) {
carlosgalvezp wrote:
> Nit: `inline` can be removed.
Yeah, my IDE flagged it but since you asked for the `static` .... `:)`

Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:99
+  if (!Message.empty())
+    diag(MD->getLocation(), Message) << MacroName;
carlosgalvezp wrote:
> Nit: tab with spaces.
Not sure how that tab got in there, LOL.
Probably my IDE "helping" me in an unwanted fashion.

Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage.cpp:1
-// 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 --
carlosgalvezp wrote:
> I'm curious as to why this is needed. If I remove it the test fails, on line 15, but the `u8` prefix was introduced already since C++11?
The `u''` (UTF-16) and `U''` (UTF-32) character literals were added in C++11.
The `u8''` (UTF-8) character literal was added in C++17.



More information about the cfe-commits mailing list