carlosgalvezp 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;
+inline bool isCapsOnly(StringRef Name) {
+ return llvm::all_of(Name, [](const char C) {
----------------
Shouldn't it still be "static"? "inline" will not give internal linkage. I
think it's also unnecessary in this case since there's no risk for multiple
definitions (it's not defined in a header file)
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:11
+`ES.31
<https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es31-dont-use-macros-for-constants-or-functions>`_,
and
+`ES.32
<https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es32-use-all_caps-for-all-macro-names>`_.
+
----------------
Is ES.32 really checked by this check? I don't see any example or test that
indicates that.
I'm also unsure if ES.32 should be handled here or via the
"readability-identifier-naming" check, which is where you enforce a particular
naming convention for different identifiers. Setting ALL_CAPS for macros there
would be an effective way of solving ES.32.
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
