diff --git a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp index aaf10f63fb070..fb04e6e0fa936 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 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 -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 -getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace, +getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace, bool IsMacro, const LangOptions &LangOpts, bool Invert, ArrayRef 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(Decl->getDeclContext()), - getLangOpts(), Invert, AllowedIdentifiers); + return getFailureInfoImpl( + Decl->getName(), isa(Decl->getDeclContext()), + /*IsMacro = */ false, getLangOpts(), Invert, AllowedIdentifiers); } std::optional 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 a93ff58c60e2c..000b80aadc023 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 + `, 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 2baae86fd9de3..1951167b41093 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