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

[Richard via Phabricator via cfe-commits]

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.
https://en.cppreference.com/w/cpp/language/character_literal


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116386/new/

https://reviews.llvm.org/D116386