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

[Carlos Galvez via cfe-commits]

Author: Carlos Galvez
Date: 2023-07-30T12:10:48Z
New Revision: 2992d084774f44e7626a7d640fe6c30163db450e

URL: https://github.com/llvm/llvm-project/commit/2992d084774f44e7626a7d640fe6c30163db450e
DIFF: https://github.com/llvm/llvm-project/commit/2992d084774f44e7626a7d640fe6c30163db450e.diff

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

Fixes #64130

Differential Revision: https://reviews.llvm.org/D156608

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
index aaf10f63fb070d..fb04e6e0fa9361 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
@@ -102,13 +102,15 @@ static std::optional<std::string> getUnderscoreCapitalFixup(StringRef Name) {
 }
 
 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;
 }
@@ -123,7 +125,7 @@ static std::string getNonReservedFixup(std::string 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());
@@ -158,15 +160,16 @@ getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace,
       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;
 }
@@ -177,16 +180,17 @@ ReservedIdentifierCheck::getDeclFailureInfo(const NamedDecl *Decl,
   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

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a93ff58c60e2c0..000b80aadc0237 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -140,6 +140,10 @@ 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
 ^^^^^^^^^^^^^^
 

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
index 2baae86fd9de36..1951167b41093d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
@@ -45,7 +45,7 @@ struct S {};
 
 #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]
@@ -115,9 +115,9 @@ struct S {};
 
 //
 
+// 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]
@@ -171,10 +171,9 @@ 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