[clang-tidy] Do not warn on macros starting with underscore and lowercase letter in bugprone-reserved-identifier

[carlosgalvezp]

Summary

Fixes #64130

[carlosgalvezp]

Remove extra newline

[PiotrZSL]

LGTM

clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp

 
 static std::optional<std::string> getUnderscoreCapitalFixup(StringRef Name) {
   if (startsWithUnderscoreCapital(Name))
     return std::string(Name.drop_front(1));
   return std::nullopt;
 }
 
 static bool startsWithUnderscoreInGlobalNamespace(StringRef Name,
-                                                  bool IsInGlobalNamespace) {
-  return IsInGlobalNamespace && Name.size() >= 1 && Name[0] == '_';
+                                                  bool IsInGlobalNamespace,
+                                                  bool IsMacro) {
+  return !IsMacro && IsInGlobalNamespace && Name.size() >= 1 && Name[0] == '_';
 }
 
 static std::optional<std::string>
-getUnderscoreGlobalNamespaceFixup(StringRef Name, bool IsInGlobalNamespace) {
-  if (startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace))
+getUnderscoreGlobalNamespaceFixup(StringRef Name, bool IsInGlobalNamespace,
+                                  bool IsMacro) {
+  if (startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace, IsMacro))
     return std::string(Name.drop_front(1));
   return std::nullopt;
 }
 
 static std::string getNonReservedFixup(std::string Name) {
   assert(!Name.empty());
   if (Name[0] == '_' || std::isupper(Name[0]))
     Name.insert(Name.begin(), '_');
   else
     Name.insert(Name.begin(), 2, '_');
   return Name;
 }
 
 static std::optional<RenamerClangTidyCheck::FailureInfo>
-getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace,
+getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace, bool IsMacro,
                    const LangOptions &LangOpts, bool Invert,
                    ArrayRef<llvm::Regex> AllowedIdentifiers) {
   assert(!Name.empty());
 
   if (llvm::any_of(AllowedIdentifiers, [&](const llvm::Regex &Regex) {
         return Regex.match(Name);
       })) {
     return std::nullopt;
auto InProgressFixup = [&] {
                  Info,
                  [](const FailureInfo &Info) { return StringRef(Info.Fixup); })
           .value_or(Name);
     };
     if (auto Fixup = getDoubleUnderscoreFixup(InProgressFixup(), LangOpts))
       AppendFailure(DoubleUnderscoreTag, std::move(*Fixup));
     if (auto Fixup = getUnderscoreCapitalFixup(InProgressFixup()))
       AppendFailure(UnderscoreCapitalTag, std::move(*Fixup));
-    if (auto Fixup = getUnderscoreGlobalNamespaceFixup(InProgressFixup(),
-                                                       IsInGlobalNamespace))
+    if (auto Fixup = getUnderscoreGlobalNamespaceFixup(
+            InProgressFixup(), IsInGlobalNamespace, IsMacro))
       AppendFailure(GlobalUnderscoreTag, std::move(*Fixup));
 
     return Info;
   }
   if (!(hasReservedDoubleUnderscore(Name, LangOpts) ||
         startsWithUnderscoreCapital(Name) ||
-        startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace)))
+        startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace,
+                                              IsMacro)))
     return FailureInfo{NonReservedTag, getNonReservedFixup(std::string(Name))};
   return std::nullopt;
 }
 
 std::optional<RenamerClangTidyCheck::FailureInfo>
 ReservedIdentifierCheck::getDeclFailureInfo(const NamedDecl *Decl,
                                             const SourceManager &) const {
   assert(Decl && Decl->getIdentifier() && !Decl->getName().empty() &&
          !Decl->isImplicit() &&
          "Decl must be an explicit identifier with a name.");
-  return getFailureInfoImpl(Decl->getName(),
-                            isa<TranslationUnitDecl>(Decl->getDeclContext()),
-                            getLangOpts(), Invert, AllowedIdentifiers);
+  return getFailureInfoImpl(
+      Decl->getName(), isa<TranslationUnitDecl>(Decl->getDeclContext()),
+      /*IsMacro = */ false, getLangOpts(), Invert, AllowedIdentifiers);
 }
 
 std::optional<RenamerClangTidyCheck::FailureInfo>
 ReservedIdentifierCheck::getMacroFailureInfo(const Token &MacroNameTok,
                                              const SourceManager &) const {
   return getFailureInfoImpl(MacroNameTok.getIdentifierInfo()->getName(), true,
-                            getLangOpts(), Invert, AllowedIdentifiers);
+                            /*IsMacro = */ true, getLangOpts(), Invert,
+                            AllowedIdentifiers);
 }
 
 RenamerClangTidyCheck::DiagInfo
 ReservedIdentifierCheck::getDiagInfo(const NamingCheckId &ID,
                                      const NamingCheckFailure &Failure) const {
   return DiagInfo{Message, [&](DiagnosticBuilder &Diag) {
                     Diag << ID.second
                          << getMessageSelectIndex(Failure.Info.KindName);
                   }};
 }
 
 } // namespace clang::tidy::bugprone

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`readability-reference-to-constructed-temporary
   of a temporary object that has just been constructed.
 
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Fixed bug in :doc:`bugprone-reserved-identifier
+  <clang-tidy/checks/bugprone/reserved-identifier>`, so that it does not warn
+  on macros starting with underscore and lowercase letter.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp

 struct S {};
 
 } // namespace _Ns
 
 //
 
 #define __macro(m) int m = 0
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '__macro', which is a reserved identifier [bugprone-reserved-identifier]
-// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
+// CHECK-FIXES: {{^}}#define _macro(m) int m = 0{{$}}
 
 namespace __ns {
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses identifier '__ns', which is a reserved identifier [bugprone-reserved-identifier]
 // CHECK-FIXES: {{^}}namespace ns {{{$}}
 class __object {
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses identifier '__object', which is a reserved identifier [bugprone-reserved-identifier]
   // CHECK-FIXES: {{^}}class _object {{{$}}
   int __member;

 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: declaration uses identifier 'templateParam___t', which is a reserved identifier [bugprone-reserved-identifier]
 // CHECK-FIXES: {{^}}template <typename templateParam_t>{{$}}
 struct S {};
 
 } // namespace ns___n
 
 //
 
+// OK, this rule does not apply to macros:
+// https://github.com/llvm/llvm-project/issues/64130#issuecomment-1655751676
 #define _macro(m) int m = 0
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '_macro', which is reserved in the global namespace [bugprone-reserved-identifier]
-// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
 
 namespace _ns {
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses identifier '_ns', which is reserved in the global namespace [bugprone-reserved-identifier]
 // CHECK-FIXES: {{^}}namespace ns {{{$}}
 int _i;
 // no warning
 } // namespace _ns
 class _object {

 int _________;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses identifier '_________', which is a reserved identifier; cannot be fixed automatically [bugprone-reserved-identifier]
 // CHECK-FIXES: {{^}}int _________;{{$}}
 
 int _;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses identifier '_', which is reserved in the global namespace; cannot be fixed automatically [bugprone-reserved-identifier]
 // CHECK-FIXES: {{^}}int _;{{$}}
 
+// This should not trigger a warning
 // https://github.com/llvm/llvm-project/issues/52895
 #define _5_kmph_rpm 459
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '_5_kmph_rpm', which is reserved in the global namespace; cannot be fixed automatically [bugprone-reserved-identifier]
-// CHECK-FIXES: {{^}}#define _5_kmph_rpm 459{{$}}
 
 // these should pass
 #define MACRO(m) int m = 0
 
 namespace Ns {
 class Object {
   int Member;
 };