[clang-tools-extra] [clang-tidy] Refactor how NamedDecl are renamed (PR #88735)
https://github.com/PiotrZSL closed https://github.com/llvm/llvm-project/pull/88735 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Refactor how NamedDecl are renamed (PR #88735)
revane wrote: @PiotrZSL I finally got a windows environment set up and fixed the windows build error. Would you merge for me since I don't have permission? https://github.com/llvm/llvm-project/pull/88735 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Refactor how NamedDecl are renamed (PR #88735)
https://github.com/revane updated
https://github.com/llvm/llvm-project/pull/88735
>From 64c1b5b7d6c29c86e0e7f26f0eff3b7a52f95e7e Mon Sep 17 00:00:00 2001
From: Edwin Vane
Date: Thu, 28 Mar 2024 09:30:32 -0400
Subject: [PATCH] [clang-tidy] Refactor how NamedDecl are renamed
The handling of renaming failures and multiple usages related to those
failures is currently spread over several functions. Identifying the
failure NamedDecl for a given usage is also duplicated, once when
creating failures and again when identify usages. There are currently
two ways to a failed NamedDecl from a usage: use the canonical decl or
use the overridden method. With new methods about to be added, a cleanup
was in order.
The data flow is simplified as follows:
* The visitor always forwards NamedDecls to addUsage(NamedDecl).
* addUsage(NamedDecl) determines the failed NamedDecl and determines
potential new names based on that failure. Usages are registered using
addUsage(NamingCheckId).
* addUsage(NamingCheckId) is now protected and its single responsibility
is maintaining the integrity of the failure/usage map.
---
.../bugprone/ReservedIdentifierCheck.cpp | 5 +-
.../readability/IdentifierNamingCheck.cpp | 4 +
.../utils/RenamerClangTidyCheck.cpp | 196 ++
.../clang-tidy/utils/RenamerClangTidyCheck.h | 14 +-
4 files changed, 121 insertions(+), 98 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
index f6714d056518d..53956661d57d1 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
@@ -178,8 +178,11 @@ std::optional
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.");
+ // Implicit identifiers cannot fail.
+ if (Decl->isImplicit())
+return std::nullopt;
+
return getFailureInfoImpl(
Decl->getName(), isa(Decl->getDeclContext()),
/*IsMacro = */ false, getLangOpts(), Invert, AllowedIdentifiers);
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index dc30531ebda0e..27a12bfc58068 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -1374,6 +1374,10 @@ IdentifierNamingCheck::getFailureInfo(
std::optional
IdentifierNamingCheck::getDeclFailureInfo(const NamedDecl *Decl,
const SourceManager &SM) const {
+ // Implicit identifiers cannot be renamed.
+ if (Decl->isImplicit())
+return std::nullopt;
+
SourceLocation Loc = Decl->getLocation();
const FileStyle &FileStyle = getStyleForFile(SM.getFilename(Loc));
if (!FileStyle.isActive())
diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index 962a243ce94d4..f5ed617365403 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -61,6 +61,7 @@ struct
DenseMapInfo {
namespace clang::tidy {
namespace {
+
class NameLookup {
llvm::PointerIntPair Data;
@@ -78,6 +79,7 @@ class NameLookup {
operator bool() const { return !hasMultipleResolutions(); }
const NamedDecl *operator*() const { return getDecl(); }
};
+
} // namespace
static const NamedDecl *findDecl(const RecordDecl &RecDecl,
@@ -91,6 +93,44 @@ static const NamedDecl *findDecl(const RecordDecl &RecDecl,
return nullptr;
}
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
+ if (Method->size_overridden_methods() != 1)
+return nullptr;
+
+ while (true) {
+Method = *Method->begin_overridden_methods();
+assert(Method && "Overridden method shouldn't be null");
+unsigned NumOverrides = Method->size_overridden_methods();
+if (NumOverrides == 0)
+ return Method;
+if (NumOverrides > 1)
+ return nullptr;
+ }
+}
+
+static bool hasNoName(const NamedDecl *Decl) {
+ return !Decl->getIdentifier() || Decl->getName().empty();
+}
+
+static const NamedDecl *getFailureForNamedDecl(const NamedDecl *ND) {
+ const auto *Canonical = cast(ND->getCanonicalDecl());
+ if (Canonical != ND)
+return Canonical;
+
+ if (const auto *Method = dyn_cast(ND)) {
+if (const CXXMethodD
[clang-tools-extra] [clang-tidy] Refactor how NamedDecl are renamed (PR #88735)
https://github.com/revane updated
https://github.com/llvm/llvm-project/pull/88735
>From 5be7a57838b8fe7042e0719911670f6f369db2e8 Mon Sep 17 00:00:00 2001
From: Edwin Vane
Date: Thu, 28 Mar 2024 09:30:32 -0400
Subject: [PATCH] [clang-tidy] Refactor how NamedDecl are renamed
The handling of renaming failures and multiple usages related to those
failures is currently spread over several functions. Identifying the
failure NamedDecl for a given usage is also duplicated, once when
creating failures and again when identify usages. There are currently
two ways to a failed NamedDecl from a usage: use the canonical decl or
use the overridden method. With new methods about to be added, a cleanup
was in order.
The data flow is simplified as follows:
* The visitor always forwards NamedDecls to addUsage(NamedDecl).
* addUsage(NamedDecl) determines the failed NamedDecl and determines
potential new names based on that failure. Usages are registered using
addUsage(NamingCheckId).
* addUsage(NamingCheckId) is now protected and its single responsibility
is maintaining the integrity of the failure/usage map.
---
.../utils/RenamerClangTidyCheck.cpp | 196 ++
.../clang-tidy/utils/RenamerClangTidyCheck.h | 14 +-
2 files changed, 113 insertions(+), 97 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index 962a243ce94d48..f5ed617365403a 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -61,6 +61,7 @@ struct
DenseMapInfo {
namespace clang::tidy {
namespace {
+
class NameLookup {
llvm::PointerIntPair Data;
@@ -78,6 +79,7 @@ class NameLookup {
operator bool() const { return !hasMultipleResolutions(); }
const NamedDecl *operator*() const { return getDecl(); }
};
+
} // namespace
static const NamedDecl *findDecl(const RecordDecl &RecDecl,
@@ -91,6 +93,44 @@ static const NamedDecl *findDecl(const RecordDecl &RecDecl,
return nullptr;
}
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
+ if (Method->size_overridden_methods() != 1)
+return nullptr;
+
+ while (true) {
+Method = *Method->begin_overridden_methods();
+assert(Method && "Overridden method shouldn't be null");
+unsigned NumOverrides = Method->size_overridden_methods();
+if (NumOverrides == 0)
+ return Method;
+if (NumOverrides > 1)
+ return nullptr;
+ }
+}
+
+static bool hasNoName(const NamedDecl *Decl) {
+ return !Decl->getIdentifier() || Decl->getName().empty();
+}
+
+static const NamedDecl *getFailureForNamedDecl(const NamedDecl *ND) {
+ const auto *Canonical = cast(ND->getCanonicalDecl());
+ if (Canonical != ND)
+return Canonical;
+
+ if (const auto *Method = dyn_cast(ND)) {
+if (const CXXMethodDecl *Overridden = getOverrideMethod(Method))
+ Canonical = cast(Overridden->getCanonicalDecl());
+
+if (Canonical != ND)
+ return Canonical;
+ }
+
+ return ND;
+}
+
/// Returns a decl matching the \p DeclName in \p Parent or one of its base
/// classes. If \p AggressiveTemplateLookup is `true` then it will check
/// template dependent base classes as well.
@@ -132,24 +172,6 @@ static NameLookup findDeclInBases(const CXXRecordDecl
&Parent,
return NameLookup(Found); // If nullptr, decl wasn't found.
}
-/// Returns the function that \p Method is overridding. If There are none or
-/// multiple overrides it returns nullptr. If the overridden function itself is
-/// overridding then it will recurse up to find the first decl of the function.
-static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
- if (Method->size_overridden_methods() != 1)
-return nullptr;
-
- while (true) {
-Method = *Method->begin_overridden_methods();
-assert(Method && "Overridden method shouldn't be null");
-unsigned NumOverrides = Method->size_overridden_methods();
-if (NumOverrides == 0)
- return Method;
-if (NumOverrides > 1)
- return nullptr;
- }
-}
-
namespace {
/// Callback supplies macros to RenamerClangTidyCheck::checkMacro
@@ -192,10 +214,6 @@ class RenamerClangTidyVisitor
: Check(Check), SM(SM),
AggressiveDependentMemberLookup(AggressiveDependentMemberLookup) {}
- static bool hasNoName(const NamedDecl *Decl) {
-return !Decl->getIdentifier() || Decl->getName().empty();
- }
-
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }
@@ -246,29 +264,10 @@ class RenamerClangTidyVisitor
}
bool VisitNamedDecl(NamedDecl *Decl) {
-if (hasNoName(Decl))
-
[clang-tools-extra] [clang-tidy] Refactor how NamedDecl are renamed (PR #88735)
@@ -61,6 +61,44 @@ struct
DenseMapInfo {
namespace clang::tidy {
namespace {
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
revane wrote:
Ah. Good to know.
https://github.com/llvm/llvm-project/pull/88735
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Refactor how NamedDecl are renamed (PR #88735)
https://github.com/revane updated
https://github.com/llvm/llvm-project/pull/88735
>From 2249f8c4e28297d72d6f5d36e883b921116f33e4 Mon Sep 17 00:00:00 2001
From: Edwin Vane
Date: Thu, 28 Mar 2024 09:30:32 -0400
Subject: [PATCH] [clang-tidy] Refactor how NamedDecl are renamed
The handling of renaming failures and multiple usages related to those
failures is currently spread over several functions. Identifying the
failure NamedDecl for a given usage is also duplicated, once when
creating failures and again when identify usages. There are currently
two ways to a failed NamedDecl from a usage: use the canonical decl or
use the overridden method. With new methods about to be added, a cleanup
was in order.
The data flow is simplified as follows:
* The visitor always forwards NamedDecls to addUsage(NamedDecl).
* addUsage(NamedDecl) determines the failed NamedDecl and determines
potential new names based on that failure. Usages are registered using
addUsage(NamingCheckId).
* addUsage(NamingCheckId) is now protected and its single responsibility
is maintaining the integrity of the failure/usage map.
---
.../utils/RenamerClangTidyCheck.cpp | 196 ++
.../clang-tidy/utils/RenamerClangTidyCheck.h | 14 +-
2 files changed, 113 insertions(+), 97 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index 962a243ce94d48..f5ed617365403a 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -61,6 +61,7 @@ struct
DenseMapInfo {
namespace clang::tidy {
namespace {
+
class NameLookup {
llvm::PointerIntPair Data;
@@ -78,6 +79,7 @@ class NameLookup {
operator bool() const { return !hasMultipleResolutions(); }
const NamedDecl *operator*() const { return getDecl(); }
};
+
} // namespace
static const NamedDecl *findDecl(const RecordDecl &RecDecl,
@@ -91,6 +93,44 @@ static const NamedDecl *findDecl(const RecordDecl &RecDecl,
return nullptr;
}
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
+ if (Method->size_overridden_methods() != 1)
+return nullptr;
+
+ while (true) {
+Method = *Method->begin_overridden_methods();
+assert(Method && "Overridden method shouldn't be null");
+unsigned NumOverrides = Method->size_overridden_methods();
+if (NumOverrides == 0)
+ return Method;
+if (NumOverrides > 1)
+ return nullptr;
+ }
+}
+
+static bool hasNoName(const NamedDecl *Decl) {
+ return !Decl->getIdentifier() || Decl->getName().empty();
+}
+
+static const NamedDecl *getFailureForNamedDecl(const NamedDecl *ND) {
+ const auto *Canonical = cast(ND->getCanonicalDecl());
+ if (Canonical != ND)
+return Canonical;
+
+ if (const auto *Method = dyn_cast(ND)) {
+if (const CXXMethodDecl *Overridden = getOverrideMethod(Method))
+ Canonical = cast(Overridden->getCanonicalDecl());
+
+if (Canonical != ND)
+ return Canonical;
+ }
+
+ return ND;
+}
+
/// Returns a decl matching the \p DeclName in \p Parent or one of its base
/// classes. If \p AggressiveTemplateLookup is `true` then it will check
/// template dependent base classes as well.
@@ -132,24 +172,6 @@ static NameLookup findDeclInBases(const CXXRecordDecl
&Parent,
return NameLookup(Found); // If nullptr, decl wasn't found.
}
-/// Returns the function that \p Method is overridding. If There are none or
-/// multiple overrides it returns nullptr. If the overridden function itself is
-/// overridding then it will recurse up to find the first decl of the function.
-static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
- if (Method->size_overridden_methods() != 1)
-return nullptr;
-
- while (true) {
-Method = *Method->begin_overridden_methods();
-assert(Method && "Overridden method shouldn't be null");
-unsigned NumOverrides = Method->size_overridden_methods();
-if (NumOverrides == 0)
- return Method;
-if (NumOverrides > 1)
- return nullptr;
- }
-}
-
namespace {
/// Callback supplies macros to RenamerClangTidyCheck::checkMacro
@@ -192,10 +214,6 @@ class RenamerClangTidyVisitor
: Check(Check), SM(SM),
AggressiveDependentMemberLookup(AggressiveDependentMemberLookup) {}
- static bool hasNoName(const NamedDecl *Decl) {
-return !Decl->getIdentifier() || Decl->getName().empty();
- }
-
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }
@@ -246,29 +264,10 @@ class RenamerClangTidyVisitor
}
bool VisitNamedDecl(NamedDecl *Decl) {
-if (hasNoName(Decl))
-
[clang-tools-extra] [clang-tidy] Refactor how NamedDecl are renamed (PR #88735)
@@ -61,6 +61,44 @@ struct
DenseMapInfo {
namespace clang::tidy {
namespace {
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
PiotrZSL wrote:
anyonymous namespace -> static (per llvm codding guidelines)
leave only NameLookup in anonymous namespace
https://github.com/llvm/llvm-project/pull/88735
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Refactor how NamedDecl are renamed (PR #88735)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/88735 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Refactor how NamedDecl are renamed (PR #88735)
https://github.com/PiotrZSL approved this pull request. For me looks fine. Give it few days before merging, so others would have opportunity to review. https://github.com/llvm/llvm-project/pull/88735 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Refactor how NamedDecl are renamed (PR #88735)
revane wrote: @PiotrZSL Second-last in the sequence of PRs which fix another renaming issue. https://github.com/llvm/llvm-project/pull/88735 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Refactor how NamedDecl are renamed (PR #88735)
llvmbot wrote:
@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-clang-tidy
Author: Edwin Vane (revane)
Changes
The handling of renaming failures and multiple usages related to those failures
is currently spread over several functions. Identifying the failure NamedDecl
for a given usage is also duplicated, once when creating failures and again
when identify usages. There are currently two ways to a failed NamedDecl from a
usage: use the canonical decl or use the overridden method. With new methods
about to be added, a cleanup was in order.
The data flow is simplified as follows:
* The visitor always forwards NamedDecls to addUsage(NamedDecl).
* addUsage(NamedDecl) determines the failed NamedDecl and determines potential
new names based on that failure. Usages are registered using
addUsage(NamingCheckId).
* addUsage(NamingCheckId) is now protected and its single responsibility is
maintaining the integrity of the failure/usage map.
---
Full diff: https://github.com/llvm/llvm-project/pull/88735.diff
2 Files Affected:
- (modified) clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
(+103-91)
- (modified) clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h (+8-6)
``diff
diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index 962a243ce94d48..453d5a754f12fc 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -61,6 +61,44 @@ struct
DenseMapInfo {
namespace clang::tidy {
namespace {
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
+ if (Method->size_overridden_methods() != 1)
+return nullptr;
+
+ while (true) {
+Method = *Method->begin_overridden_methods();
+assert(Method && "Overridden method shouldn't be null");
+unsigned NumOverrides = Method->size_overridden_methods();
+if (NumOverrides == 0)
+ return Method;
+if (NumOverrides > 1)
+ return nullptr;
+ }
+}
+
+bool hasNoName(const NamedDecl *Decl) {
+ return !Decl->getIdentifier() || Decl->getName().empty();
+}
+
+const NamedDecl *getFailureForNamedDecl(const NamedDecl *ND) {
+ const auto *Canonical = cast(ND->getCanonicalDecl());
+ if (Canonical != ND)
+return Canonical;
+
+ if (const auto *Method = dyn_cast(ND)) {
+if (const CXXMethodDecl *Overridden = getOverrideMethod(Method))
+ Canonical = cast(Overridden->getCanonicalDecl());
+
+if (Canonical != ND)
+ return Canonical;
+ }
+
+ return ND;
+}
+
class NameLookup {
llvm::PointerIntPair Data;
@@ -132,24 +170,6 @@ static NameLookup findDeclInBases(const CXXRecordDecl
&Parent,
return NameLookup(Found); // If nullptr, decl wasn't found.
}
-/// Returns the function that \p Method is overridding. If There are none or
-/// multiple overrides it returns nullptr. If the overridden function itself is
-/// overridding then it will recurse up to find the first decl of the function.
-static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
- if (Method->size_overridden_methods() != 1)
-return nullptr;
-
- while (true) {
-Method = *Method->begin_overridden_methods();
-assert(Method && "Overridden method shouldn't be null");
-unsigned NumOverrides = Method->size_overridden_methods();
-if (NumOverrides == 0)
- return Method;
-if (NumOverrides > 1)
- return nullptr;
- }
-}
-
namespace {
/// Callback supplies macros to RenamerClangTidyCheck::checkMacro
@@ -192,10 +212,6 @@ class RenamerClangTidyVisitor
: Check(Check), SM(SM),
AggressiveDependentMemberLookup(AggressiveDependentMemberLookup) {}
- static bool hasNoName(const NamedDecl *Decl) {
-return !Decl->getIdentifier() || Decl->getName().empty();
- }
-
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }
@@ -246,29 +262,10 @@ class RenamerClangTidyVisitor
}
bool VisitNamedDecl(NamedDecl *Decl) {
-if (hasNoName(Decl))
- return true;
-
-const auto *Canonical = cast(Decl->getCanonicalDecl());
-if (Canonical != Decl) {
- Check->addUsage(Canonical, Decl->getLocation(), SM);
- return true;
-}
-
-// Fix overridden methods
-if (const auto *Method = dyn_cast(Decl)) {
- if (const CXXMethodDecl *Overridden = getOverrideMethod(Method)) {
-Check->addUsage(Overridden, Method->getLocation(), SM);
-return true; // Don't try to add the actual decl as a Failure.
- }
-}
-
-// Ignore ClassTemplateSpecializationDecl which are creating duplicate
-// replacements with CXXRecordDecl.
-
[clang-tools-extra] [clang-tidy] Refactor how NamedDecl are renamed (PR #88735)
https://github.com/revane created
https://github.com/llvm/llvm-project/pull/88735
The handling of renaming failures and multiple usages related to those failures
is currently spread over several functions. Identifying the failure NamedDecl
for a given usage is also duplicated, once when creating failures and again
when identify usages. There are currently two ways to a failed NamedDecl from a
usage: use the canonical decl or use the overridden method. With new methods
about to be added, a cleanup was in order.
The data flow is simplified as follows:
* The visitor always forwards NamedDecls to addUsage(NamedDecl).
* addUsage(NamedDecl) determines the failed NamedDecl and determines potential
new names based on that failure. Usages are registered using
addUsage(NamingCheckId).
* addUsage(NamingCheckId) is now protected and its single responsibility is
maintaining the integrity of the failure/usage map.
>From 16271c2988994257d33a3f6b2b98254be2587b58 Mon Sep 17 00:00:00 2001
From: Edwin Vane
Date: Thu, 28 Mar 2024 09:30:32 -0400
Subject: [PATCH] [clang-tidy] Refactor how NamedDecl are renamed
The handling of renaming failures and multiple usages related to those
failures is currently spread over several functions. Identifying the
failure NamedDecl for a given usage is also duplicated, once when
creating failures and again when identify usages. There are currently
two ways to a failed NamedDecl from a usage: use the canonical decl or
use the overridden method. With new methods about to be added, a cleanup
was in order.
The data flow is simplified as follows:
* The visitor always forwards NamedDecls to addUsage(NamedDecl).
* addUsage(NamedDecl) determines the failed NamedDecl and determines
potential new names based on that failure. Usages are registered using
addUsage(NamingCheckId).
* addUsage(NamingCheckId) is now protected and its single responsibility
is maintaining the integrity of the failure/usage map.
---
.../utils/RenamerClangTidyCheck.cpp | 194 ++
.../clang-tidy/utils/RenamerClangTidyCheck.h | 14 +-
2 files changed, 111 insertions(+), 97 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index 962a243ce94d48..453d5a754f12fc 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -61,6 +61,44 @@ struct
DenseMapInfo {
namespace clang::tidy {
namespace {
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
+ if (Method->size_overridden_methods() != 1)
+return nullptr;
+
+ while (true) {
+Method = *Method->begin_overridden_methods();
+assert(Method && "Overridden method shouldn't be null");
+unsigned NumOverrides = Method->size_overridden_methods();
+if (NumOverrides == 0)
+ return Method;
+if (NumOverrides > 1)
+ return nullptr;
+ }
+}
+
+bool hasNoName(const NamedDecl *Decl) {
+ return !Decl->getIdentifier() || Decl->getName().empty();
+}
+
+const NamedDecl *getFailureForNamedDecl(const NamedDecl *ND) {
+ const auto *Canonical = cast(ND->getCanonicalDecl());
+ if (Canonical != ND)
+return Canonical;
+
+ if (const auto *Method = dyn_cast(ND)) {
+if (const CXXMethodDecl *Overridden = getOverrideMethod(Method))
+ Canonical = cast(Overridden->getCanonicalDecl());
+
+if (Canonical != ND)
+ return Canonical;
+ }
+
+ return ND;
+}
+
class NameLookup {
llvm::PointerIntPair Data;
@@ -132,24 +170,6 @@ static NameLookup findDeclInBases(const CXXRecordDecl
&Parent,
return NameLookup(Found); // If nullptr, decl wasn't found.
}
-/// Returns the function that \p Method is overridding. If There are none or
-/// multiple overrides it returns nullptr. If the overridden function itself is
-/// overridding then it will recurse up to find the first decl of the function.
-static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
- if (Method->size_overridden_methods() != 1)
-return nullptr;
-
- while (true) {
-Method = *Method->begin_overridden_methods();
-assert(Method && "Overridden method shouldn't be null");
-unsigned NumOverrides = Method->size_overridden_methods();
-if (NumOverrides == 0)
- return Method;
-if (NumOverrides > 1)
- return nullptr;
- }
-}
-
namespace {
/// Callback supplies macros to RenamerClangTidyCheck::checkMacro
@@ -192,10 +212,6 @@ class RenamerClangTidyVisitor
: Check(Check), SM(SM),
AggressiveDependentMemberLookup(AggressiveDependentMemberLookup) {}
- static bool hasNoName(const NamedDecl *Decl) {
-return !Decl->getIdentifier() || Decl->getName
