[clang-tools-extra] [clang-tidy] Guard `readability-identifier-naming` recursion in dependent base lookup (PR #204913)
Zihan Qin via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 19 23:29:08 PDT 2026
https://github.com/qzhhhi updated https://github.com/llvm/llvm-project/pull/204913
>From 729724741c21af552cc7a358d65ae0a1702ac740 Mon Sep 17 00:00:00 2001
From: qzhhhi <zihanqin2048 at gmail.com>
Date: Sat, 20 Jun 2026 05:39:33 +0000
Subject: [PATCH 1/2] [clang-tidy] Guard `readability-identifier-naming`
recursion in dependent base lookup
Prevent `readability-identifier-naming` from recursing indefinitely in dependent base lookup when `AggressiveDependentMemberLookup` maps a dependent template base back to the same record definition.
Track visited `CXXRecordDecl` definitions while walking base classes and stop revisiting the same definition. Add a regression test covering the dependent base cycle reproducer.
---
.../utils/RenamerClangTidyCheck.cpp | 21 +++++++++++++++----
...identifier-naming-dependent-base-cycle.cpp | 17 +++++++++++++++
2 files changed, 34 insertions(+), 4 deletions(-)
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-dependent-base-cycle.cpp
diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index cb7ef19827675..d55c8f2e05c71 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -17,6 +17,8 @@
#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/PointerIntPair.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include <optional>
#define DEBUG_TYPE "clang-tidy"
@@ -118,6 +120,8 @@ static const NamedDecl *getFailureForNamedDecl(const NamedDecl *ND) {
return ND;
}
+using RecursionProtectionSet = llvm::SmallPtrSet<const CXXRecordDecl *, 4>;
+
/// 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.
@@ -125,9 +129,17 @@ static const NamedDecl *getFailureForNamedDecl(const NamedDecl *ND) {
/// flag indicating the multiple resolutions.
static NameLookup findDeclInBases(const CXXRecordDecl &Parent,
StringRef DeclName,
- bool AggressiveTemplateLookup) {
+ bool AggressiveTemplateLookup,
+ RecursionProtectionSet &Visited) {
if (!Parent.hasDefinition())
return NameLookup(nullptr);
+
+ const auto *Definition = Parent.getDefinition();
+ if (!Visited.insert(Definition).second)
+ return NameLookup(nullptr);
+ auto RemoveFromVisited =
+ llvm::scope_exit([&Visited, Definition] { Visited.erase(Definition); });
+
if (const NamedDecl *InClassRef = findDecl(Parent, DeclName))
return NameLookup(InClassRef);
const NamedDecl *Found = nullptr;
@@ -144,8 +156,8 @@ static NameLookup findDeclInBases(const CXXRecordDecl &Parent,
}
if (!Record)
continue;
- if (auto Search =
- findDeclInBases(*Record, DeclName, AggressiveTemplateLookup)) {
+ if (auto Search = findDeclInBases(*Record, DeclName,
+ AggressiveTemplateLookup, Visited)) {
if (*Search) {
if (Found)
return NameLookup(
@@ -301,8 +313,9 @@ class RenamerClangTidyVisitor
return true;
const StringRef DependentName = DeclName.getAsIdentifierInfo()->getName();
+ RecursionProtectionSet Visited;
if (const NameLookup Resolved = findDeclInBases(
- *Base, DependentName, AggressiveDependentMemberLookup)) {
+ *Base, DependentName, AggressiveDependentMemberLookup, Visited)) {
if (*Resolved)
Check->addUsage(*Resolved,
DepMemberRef->getMemberNameInfo().getSourceRange(), SM);
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-dependent-base-cycle.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-dependent-base-cycle.cpp
new file mode 100644
index 0000000000000..9dd966f92edf3
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-dependent-base-cycle.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN: -config='{CheckOptions: { \
+// RUN: readability-identifier-naming.AggressiveDependentMemberLookup: true \
+// RUN: }}' -- -fno-delayed-template-parsing
+
+template <class T>
+struct A;
+
+template <class T>
+struct A<const T> {
+ int x;
+};
+
+template <class T>
+struct A : A<const T> {
+ A() { this->x; }
+};
>From 60c2ed99c06db8c97970ed4d5cb207aec6cd6869 Mon Sep 17 00:00:00 2001
From: qzhhhi <zihanqin2048 at gmail.com>
Date: Sat, 20 Jun 2026 06:28:45 +0000
Subject: [PATCH 2/2] Apply review feedback
---
.../clang-tidy/utils/RenamerClangTidyCheck.cpp | 1 -
clang-tools-extra/docs/ReleaseNotes.rst | 3 +++
.../identifier-naming-dependent-base-cycle.cpp | 17 -----------------
.../identifier-naming-member-decl-usage.cpp | 15 +++++++++++++++
4 files changed, 18 insertions(+), 18 deletions(-)
delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-dependent-base-cycle.cpp
diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index d55c8f2e05c71..bfdcd381b4122 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -18,7 +18,6 @@
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/ScopeExit.h"
-#include "llvm/ADT/SmallPtrSet.h"
#include <optional>
#define DEBUG_TYPE "clang-tidy"
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d167f5e90f88a..9f49a968e0b9b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -791,6 +791,9 @@ Changes in existing checks
- Fixed a false positive where function templates could be diagnosed as generic
identifiers when `DefaultCase` was enabled.
+ - Fixed a crash in dependent base lookup when
+ ``AggressiveDependentMemberLookup`` is enabled.
+
- Improved :doc:`readability-implicit-bool-conversion
<clang-tidy/checks/readability/implicit-bool-conversion>` check:
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-dependent-base-cycle.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-dependent-base-cycle.cpp
deleted file mode 100644
index 9dd966f92edf3..0000000000000
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-dependent-base-cycle.cpp
+++ /dev/null
@@ -1,17 +0,0 @@
-// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
-// RUN: -config='{CheckOptions: { \
-// RUN: readability-identifier-naming.AggressiveDependentMemberLookup: true \
-// RUN: }}' -- -fno-delayed-template-parsing
-
-template <class T>
-struct A;
-
-template <class T>
-struct A<const T> {
- int x;
-};
-
-template <class T>
-struct A : A<const T> {
- A() { this->x; }
-};
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-member-decl-usage.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-member-decl-usage.cpp
index 61c3aeb043934..ce7065dc0643e 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-member-decl-usage.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-member-decl-usage.cpp
@@ -216,3 +216,18 @@ struct Derived : DependentBase<T> {
};
} // namespace unresolved_dependance
+
+namespace dependent_base_cycle {
+template <typename T>
+struct CycleBase;
+
+template <typename T>
+struct CycleBase<const T> {
+ int Value;
+};
+
+template <typename T>
+struct CycleBase : CycleBase<const T> {
+ CycleBase() { this->Value; }
+};
+} // namespace dependent_base_cycle
More information about the cfe-commits
mailing list