carlosgalvezp accepted this revision.
carlosgalvezp added a comment.
This revision is now accepted and ready to land.
Looks reasonable to me! I'm fairly new here so I might not be the most
"authoritative reviewer", let me know if you'd like someone else to give the
final approval
================
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) {
----------------
Nit: `inline` can be removed.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:99
+ if (!Message.empty())
+ diag(MD->getLocation(), Message) << MacroName;
}
----------------
Nit: tab with spaces.
================
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 --
----------------
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?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116386/new/
https://reviews.llvm.org/D116386
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
