[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
Victor Chernyakin via cfe-commits
cfe-commits at lists.llvm.org
Sun Dec 7 14:44:46 PST 2025
https://github.com/localspook created https://github.com/llvm/llvm-project/pull/171059
I've structured this PR as a series of independent commits, so it's probably easiest to review commit-by-commit. There's some subjectivity involved with refactoring, so if you think any of the changes is a downgrade, I'm happy to discuss it.
>From c53880940787c9c2f81b0466f82e41831b87b407 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 01:38:48 -0800
Subject: [PATCH 01/18] Inline `addNodeToInterfaceMap`
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 11 ++---------
.../clang-tidy/fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index df40d166bd404..45fea99ece90b 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,13 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-// Adds a node to the interface map, if it was not present in the map
-// previously.
-void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
- bool IsInterface) {
- InterfaceMap.try_emplace(Node, IsInterface);
-}
-
// Returns "true" if the boolean "isInterface" has been set to the
// interface status of the current Node. Return "false" if the
// interface status for the current node is not yet known.
@@ -69,13 +62,13 @@ bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
continue;
assert(Base->isCompleteDefinition());
if (!isInterface(Base)) {
- addNodeToInterfaceMap(Node, false);
+ InterfaceMap.try_emplace(Node, false);
return false;
}
}
const bool CurrentClassIsInterface = isCurrentClassInterface(Node);
- addNodeToInterfaceMap(Node, CurrentClassIsInterface);
+ InterfaceMap.try_emplace(Node, CurrentClassIsInterface);
return CurrentClassIsInterface;
}
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index b9055eb58a300..a333b3c67882e 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
@@ -30,7 +30,6 @@ class MultipleInheritanceCheck : public ClangTidyCheck {
void onEndOfTranslationUnit() override { InterfaceMap.clear(); }
private:
- void addNodeToInterfaceMap(const CXXRecordDecl *Node, bool IsInterface);
bool getInterfaceStatus(const CXXRecordDecl *Node, bool &IsInterface) const;
bool isCurrentClassInterface(const CXXRecordDecl *Node) const;
bool isInterface(const CXXRecordDecl *Node);
>From 32cbb6efa3331d055faffefc354c9d81fc2d967f Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 01:41:21 -0800
Subject: [PATCH 02/18] Inline `getInterfaceStatus`
At the call site, it was just as much code to call
this function as it is to inline it. This also
avoids an uninitialized variable.
---
.../fuchsia/MultipleInheritanceCheck.cpp | 18 +++---------------
.../fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 45fea99ece90b..71dbf6acf906f 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,18 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-// Returns "true" if the boolean "isInterface" has been set to the
-// interface status of the current Node. Return "false" if the
-// interface status for the current node is not yet known.
-bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
- bool &IsInterface) const {
- auto Pair = InterfaceMap.find(Node);
- if (Pair == InterfaceMap.end())
- return false;
- IsInterface = Pair->second;
- return true;
-}
-
bool MultipleInheritanceCheck::isCurrentClassInterface(
const CXXRecordDecl *Node) const {
// Interfaces should have no fields.
@@ -49,9 +37,9 @@ bool MultipleInheritanceCheck::isCurrentClassInterface(
bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
// Short circuit the lookup if we have analyzed this record before.
- bool PreviousIsInterfaceResult = false;
- if (getInterfaceStatus(Node, PreviousIsInterfaceResult))
- return PreviousIsInterfaceResult;
+ const auto CachedValue = InterfaceMap.find(Node);
+ if (CachedValue != InterfaceMap.end())
+ return CachedValue->second;
// To be an interface, all base classes must be interfaces as well.
for (const auto &I : Node->bases()) {
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index a333b3c67882e..feb86da4309fb 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
@@ -30,7 +30,6 @@ class MultipleInheritanceCheck : public ClangTidyCheck {
void onEndOfTranslationUnit() override { InterfaceMap.clear(); }
private:
- bool getInterfaceStatus(const CXXRecordDecl *Node, bool &IsInterface) const;
bool isCurrentClassInterface(const CXXRecordDecl *Node) const;
bool isInterface(const CXXRecordDecl *Node);
>From 91019fc7102a923e4ba114ee65dada5f3949c2cf Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 01:42:01 -0800
Subject: [PATCH 03/18] Prefer preincrement
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 71dbf6acf906f..01caa501e3221 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -79,7 +79,7 @@ void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
continue;
assert(Base->isCompleteDefinition());
if (!isInterface(Base))
- NumConcrete++;
+ ++NumConcrete;
}
// Check virtual bases to see if there is more than one concrete
@@ -90,7 +90,7 @@ void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
continue;
assert(Base->isCompleteDefinition());
if (!isInterface(Base))
- NumConcrete++;
+ ++NumConcrete;
}
if (NumConcrete > 1) {
>From 1e50d9c70ec47d105fc89354401acf2861bb98f6 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 01:44:00 -0800
Subject: [PATCH 04/18] Remove unnecessary check for if node was matched
---
.../fuchsia/MultipleInheritanceCheck.cpp | 55 +++++++++----------
1 file changed, 27 insertions(+), 28 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 01caa501e3221..fd8e6a655b359 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -67,36 +67,35 @@ void MultipleInheritanceCheck::registerMatchers(MatchFinder *Finder) {
}
void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
- if (const auto *D = Result.Nodes.getNodeAs<CXXRecordDecl>("decl")) {
- // Check against map to see if the class inherits from multiple
- // concrete classes
- unsigned NumConcrete = 0;
- for (const auto &I : D->bases()) {
- if (I.isVirtual())
- continue;
- const auto *Base = I.getType()->getAsCXXRecordDecl();
- if (!Base)
- continue;
- assert(Base->isCompleteDefinition());
- if (!isInterface(Base))
- ++NumConcrete;
- }
+ const auto &D = *Result.Nodes.getNodeAs<CXXRecordDecl>("decl");
+ // Check against map to see if the class inherits from multiple
+ // concrete classes
+ unsigned NumConcrete = 0;
+ for (const auto &I : D.bases()) {
+ if (I.isVirtual())
+ continue;
+ const auto *Base = I.getType()->getAsCXXRecordDecl();
+ if (!Base)
+ continue;
+ assert(Base->isCompleteDefinition());
+ if (!isInterface(Base))
+ ++NumConcrete;
+ }
- // Check virtual bases to see if there is more than one concrete
- // non-virtual base.
- for (const auto &V : D->vbases()) {
- const auto *Base = V.getType()->getAsCXXRecordDecl();
- if (!Base)
- continue;
- assert(Base->isCompleteDefinition());
- if (!isInterface(Base))
- ++NumConcrete;
- }
+ // Check virtual bases to see if there is more than one concrete
+ // non-virtual base.
+ for (const auto &V : D.vbases()) {
+ const auto *Base = V.getType()->getAsCXXRecordDecl();
+ if (!Base)
+ continue;
+ assert(Base->isCompleteDefinition());
+ if (!isInterface(Base))
+ ++NumConcrete;
+ }
- if (NumConcrete > 1) {
- diag(D->getBeginLoc(), "inheriting multiple classes that aren't "
- "pure virtual is discouraged");
- }
+ if (NumConcrete > 1) {
+ diag(D.getBeginLoc(), "inheriting multiple classes that aren't "
+ "pure virtual is discouraged");
}
}
>From d71221a16632660192207570ff348c66d214b430 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 01:46:22 -0800
Subject: [PATCH 05/18] Deduplication `try_emplace` calls
---
.../fuchsia/MultipleInheritanceCheck.cpp | 27 +++++++++----------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index fd8e6a655b359..507ecc6973a24 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -41,21 +41,20 @@ bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
if (CachedValue != InterfaceMap.end())
return CachedValue->second;
- // To be an interface, all base classes must be interfaces as well.
- for (const auto &I : Node->bases()) {
- if (I.isVirtual())
- continue;
- const auto *Base = I.getType()->getAsCXXRecordDecl();
- if (!Base)
- continue;
- assert(Base->isCompleteDefinition());
- if (!isInterface(Base)) {
- InterfaceMap.try_emplace(Node, false);
- return false;
+ const bool CurrentClassIsInterface = [&] {
+ // To be an interface, all base classes must be interfaces as well.
+ for (const auto &I : Node->bases()) {
+ if (I.isVirtual())
+ continue;
+ const auto *Base = I.getType()->getAsCXXRecordDecl();
+ if (!Base)
+ continue;
+ assert(Base->isCompleteDefinition());
+ if (!isInterface(Base))
+ return false;
}
- }
-
- const bool CurrentClassIsInterface = isCurrentClassInterface(Node);
+ return isCurrentClassInterface(Node);
+ }();
InterfaceMap.try_emplace(Node, CurrentClassIsInterface);
return CurrentClassIsInterface;
}
>From 333cb8087fc1c71953b79a72b24f9921b3e5d9c8 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 01:47:47 -0800
Subject: [PATCH 06/18] Inline `isCurrentClassInterface`
Now all the interface criteria is in one place.
---
.../fuchsia/MultipleInheritanceCheck.cpp | 22 ++++++++-----------
.../fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 507ecc6973a24..cbc76f81356cb 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,18 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-bool MultipleInheritanceCheck::isCurrentClassInterface(
- const CXXRecordDecl *Node) const {
- // Interfaces should have no fields.
- if (!Node->field_empty())
- return false;
-
- // Interfaces should have exclusively pure methods.
- return llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) {
- return M->isUserProvided() && !M->isPureVirtual() && !M->isStatic();
- });
-}
-
bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
// Short circuit the lookup if we have analyzed this record before.
const auto CachedValue = InterfaceMap.find(Node);
@@ -53,7 +41,15 @@ bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
if (!isInterface(Base))
return false;
}
- return isCurrentClassInterface(Node);
+
+ // Interfaces should have no fields.
+ if (!Node->field_empty())
+ return false;
+
+ // Interfaces should have exclusively pure methods.
+ return llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) {
+ return M->isUserProvided() && !M->isPureVirtual() && !M->isStatic();
+ });
}();
InterfaceMap.try_emplace(Node, CurrentClassIsInterface);
return CurrentClassIsInterface;
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index feb86da4309fb..699037e8e38e0 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
@@ -30,7 +30,6 @@ class MultipleInheritanceCheck : public ClangTidyCheck {
void onEndOfTranslationUnit() override { InterfaceMap.clear(); }
private:
- bool isCurrentClassInterface(const CXXRecordDecl *Node) const;
bool isInterface(const CXXRecordDecl *Node);
// Contains the identity of each named CXXRecord as an interface. This is
>From a5a0e5ce5bd4a1a0f05d547440e39ef630e15e0f Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 01:50:32 -0800
Subject: [PATCH 07/18] Explicitly spell type where it isn't visible in the
initializer
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index cbc76f81356cb..4ca56cbb02167 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -31,7 +31,7 @@ bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
const bool CurrentClassIsInterface = [&] {
// To be an interface, all base classes must be interfaces as well.
- for (const auto &I : Node->bases()) {
+ for (const CXXBaseSpecifier &I : Node->bases()) {
if (I.isVirtual())
continue;
const auto *Base = I.getType()->getAsCXXRecordDecl();
@@ -66,7 +66,7 @@ void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
// Check against map to see if the class inherits from multiple
// concrete classes
unsigned NumConcrete = 0;
- for (const auto &I : D.bases()) {
+ for (const CXXBaseSpecifier &I : D.bases()) {
if (I.isVirtual())
continue;
const auto *Base = I.getType()->getAsCXXRecordDecl();
@@ -79,7 +79,7 @@ void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
// Check virtual bases to see if there is more than one concrete
// non-virtual base.
- for (const auto &V : D.vbases()) {
+ for (const CXXBaseSpecifier &V : D.vbases()) {
const auto *Base = V.getType()->getAsCXXRecordDecl();
if (!Base)
continue;
>From 4a4c4dcfb88419bcdf5d8e9352a618c1e7abaccd Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 01:51:21 -0800
Subject: [PATCH 08/18] Deduplicate `isCompleteDefinition` assertions
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 4ca56cbb02167..c371150be6bec 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -24,6 +24,8 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
} // namespace
bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
+ assert(Node->isCompleteDefinition());
+
// Short circuit the lookup if we have analyzed this record before.
const auto CachedValue = InterfaceMap.find(Node);
if (CachedValue != InterfaceMap.end())
@@ -37,7 +39,6 @@ bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
const auto *Base = I.getType()->getAsCXXRecordDecl();
if (!Base)
continue;
- assert(Base->isCompleteDefinition());
if (!isInterface(Base))
return false;
}
@@ -72,7 +73,6 @@ void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Base = I.getType()->getAsCXXRecordDecl();
if (!Base)
continue;
- assert(Base->isCompleteDefinition());
if (!isInterface(Base))
++NumConcrete;
}
@@ -83,7 +83,6 @@ void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Base = V.getType()->getAsCXXRecordDecl();
if (!Base)
continue;
- assert(Base->isCompleteDefinition());
if (!isInterface(Base))
++NumConcrete;
}
>From 6e5958e18c2ced9b1ba5ce540f251281d907491d Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 01:53:16 -0800
Subject: [PATCH 09/18] Remove unhelpful comment
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index c371150be6bec..752e58d05f2d6 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -57,7 +57,6 @@ bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
}
void MultipleInheritanceCheck::registerMatchers(MatchFinder *Finder) {
- // Match declarations which have bases.
Finder->addMatcher(cxxRecordDecl(hasBases(), isDefinition()).bind("decl"),
this);
}
>From e6fd89bc6190f8886ba11c553a6da8af9ba34fb2 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 01:57:20 -0800
Subject: [PATCH 10/18] Correct stale (?) comment
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 752e58d05f2d6..63efaa27d27d2 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -63,8 +63,7 @@ void MultipleInheritanceCheck::registerMatchers(MatchFinder *Finder) {
void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
const auto &D = *Result.Nodes.getNodeAs<CXXRecordDecl>("decl");
- // Check against map to see if the class inherits from multiple
- // concrete classes
+ // Check to see if the class inherits from multiple concrete classes.
unsigned NumConcrete = 0;
for (const CXXBaseSpecifier &I : D.bases()) {
if (I.isVirtual())
>From b4f7436fe0751502caf0389f1c1a4ffee4c76352 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 02:05:30 -0800
Subject: [PATCH 11/18] Deduplicate null checks
---
.../fuchsia/MultipleInheritanceCheck.cpp | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 63efaa27d27d2..3972a0be3a2ef 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -24,6 +24,9 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
} // namespace
bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
+ if (!Node)
+ return false;
+
assert(Node->isCompleteDefinition());
// Short circuit the lookup if we have analyzed this record before.
@@ -36,10 +39,7 @@ bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
for (const CXXBaseSpecifier &I : Node->bases()) {
if (I.isVirtual())
continue;
- const auto *Base = I.getType()->getAsCXXRecordDecl();
- if (!Base)
- continue;
- if (!isInterface(Base))
+ if (!isInterface(I.getType()->getAsCXXRecordDecl()))
return false;
}
@@ -68,20 +68,14 @@ void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
for (const CXXBaseSpecifier &I : D.bases()) {
if (I.isVirtual())
continue;
- const auto *Base = I.getType()->getAsCXXRecordDecl();
- if (!Base)
- continue;
- if (!isInterface(Base))
+ if (!isInterface(I.getType()->getAsCXXRecordDecl()))
++NumConcrete;
}
// Check virtual bases to see if there is more than one concrete
// non-virtual base.
for (const CXXBaseSpecifier &V : D.vbases()) {
- const auto *Base = V.getType()->getAsCXXRecordDecl();
- if (!Base)
- continue;
- if (!isInterface(Base))
+ if (!isInterface(V.getType()->getAsCXXRecordDecl()))
++NumConcrete;
}
>From 7f8e3106f1cf58feae88c6e918c61c33290f7098 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 02:07:21 -0800
Subject: [PATCH 12/18] Merge a couple `if` statements
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 3972a0be3a2ef..e22040e14d27e 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -37,9 +37,7 @@ bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
const bool CurrentClassIsInterface = [&] {
// To be an interface, all base classes must be interfaces as well.
for (const CXXBaseSpecifier &I : Node->bases()) {
- if (I.isVirtual())
- continue;
- if (!isInterface(I.getType()->getAsCXXRecordDecl()))
+ if (!I.isVirtual() && !isInterface(I.getType()->getAsCXXRecordDecl()))
return false;
}
@@ -66,9 +64,7 @@ void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
// Check to see if the class inherits from multiple concrete classes.
unsigned NumConcrete = 0;
for (const CXXBaseSpecifier &I : D.bases()) {
- if (I.isVirtual())
- continue;
- if (!isInterface(I.getType()->getAsCXXRecordDecl()))
+ if (!I.isVirtual() && !isInterface(I.getType()->getAsCXXRecordDecl()))
++NumConcrete;
}
>From 514d85357e988bfec7cde59b349ecacd4181f0d4 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 02:26:18 -0800
Subject: [PATCH 13/18] pure and pure virtual are very different things
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index e22040e14d27e..7687176d33a8f 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -45,7 +45,7 @@ bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
if (!Node->field_empty())
return false;
- // Interfaces should have exclusively pure methods.
+ // Interfaces should have exclusively pure virtual methods.
return llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) {
return M->isUserProvided() && !M->isPureVirtual() && !M->isStatic();
});
>From 6ae8fcdc4a6be896283908bf1576e0dbab08af9f Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 13:55:02 -0800
Subject: [PATCH 14/18] Make isInterface take a CXXBaseSpecifier
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 9 +++++----
.../clang-tidy/fuchsia/MultipleInheritanceCheck.h | 2 +-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 7687176d33a8f..a5fba372a9a16 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,7 +23,8 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
+bool MultipleInheritanceCheck::isInterface(const CXXBaseSpecifier &Base) {
+ const CXXRecordDecl *const Node = Base.getType()->getAsCXXRecordDecl();
if (!Node)
return false;
@@ -37,7 +38,7 @@ bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
const bool CurrentClassIsInterface = [&] {
// To be an interface, all base classes must be interfaces as well.
for (const CXXBaseSpecifier &I : Node->bases()) {
- if (!I.isVirtual() && !isInterface(I.getType()->getAsCXXRecordDecl()))
+ if (!I.isVirtual() && !isInterface(I))
return false;
}
@@ -64,14 +65,14 @@ void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
// Check to see if the class inherits from multiple concrete classes.
unsigned NumConcrete = 0;
for (const CXXBaseSpecifier &I : D.bases()) {
- if (!I.isVirtual() && !isInterface(I.getType()->getAsCXXRecordDecl()))
+ if (!I.isVirtual() && !isInterface(I))
++NumConcrete;
}
// Check virtual bases to see if there is more than one concrete
// non-virtual base.
for (const CXXBaseSpecifier &V : D.vbases()) {
- if (!isInterface(V.getType()->getAsCXXRecordDecl()))
+ if (!isInterface(V))
++NumConcrete;
}
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index 699037e8e38e0..eb5c94241996e 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
@@ -30,7 +30,7 @@ class MultipleInheritanceCheck : public ClangTidyCheck {
void onEndOfTranslationUnit() override { InterfaceMap.clear(); }
private:
- bool isInterface(const CXXRecordDecl *Node);
+ bool isInterface(const CXXBaseSpecifier& Base);
// Contains the identity of each named CXXRecord as an interface. This is
// used to memoize lookup speeds and improve performance from O(N^2) to O(N),
>From 8c24a9bec1b355fc69f8d41bf31dfff93f41f390 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 14:02:14 -0800
Subject: [PATCH 15/18] Use count_if algorithm
---
.../fuchsia/MultipleInheritanceCheck.cpp | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index a5fba372a9a16..c58a9adc6d386 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -63,18 +63,15 @@ void MultipleInheritanceCheck::registerMatchers(MatchFinder *Finder) {
void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
const auto &D = *Result.Nodes.getNodeAs<CXXRecordDecl>("decl");
// Check to see if the class inherits from multiple concrete classes.
- unsigned NumConcrete = 0;
- for (const CXXBaseSpecifier &I : D.bases()) {
- if (!I.isVirtual() && !isInterface(I))
- ++NumConcrete;
- }
+ unsigned NumConcrete =
+ llvm::count_if(D.bases(), [&](const CXXBaseSpecifier &I) {
+ return !I.isVirtual() && !isInterface(I);
+ });
// Check virtual bases to see if there is more than one concrete
// non-virtual base.
- for (const CXXBaseSpecifier &V : D.vbases()) {
- if (!isInterface(V))
- ++NumConcrete;
- }
+ NumConcrete += llvm::count_if(
+ D.vbases(), [&](const CXXBaseSpecifier &V) { return !isInterface(V); });
if (NumConcrete > 1) {
diag(D.getBeginLoc(), "inheriting multiple classes that aren't "
>From 0bbf692a8bb6edf47a5b947aaf9cae0e2c3def1c Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 14:03:41 -0800
Subject: [PATCH 16/18] Use any_of algorithm
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index c58a9adc6d386..ccefadfbfdd49 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -37,10 +37,10 @@ bool MultipleInheritanceCheck::isInterface(const CXXBaseSpecifier &Base) {
const bool CurrentClassIsInterface = [&] {
// To be an interface, all base classes must be interfaces as well.
- for (const CXXBaseSpecifier &I : Node->bases()) {
- if (!I.isVirtual() && !isInterface(I))
- return false;
- }
+ if (llvm::any_of(Node->bases(), [&](const CXXBaseSpecifier &I) {
+ return !I.isVirtual() && !isInterface(I);
+ }))
+ return false;
// Interfaces should have no fields.
if (!Node->field_empty())
>From 332efab2bea824e61e314a12437879af982c3ef2 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 14:26:54 -0800
Subject: [PATCH 17/18] Reduce lambda down to conditional expression
---
.../fuchsia/MultipleInheritanceCheck.cpp | 28 +++++++++----------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index ccefadfbfdd49..efbe339c955ed 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -35,22 +35,20 @@ bool MultipleInheritanceCheck::isInterface(const CXXBaseSpecifier &Base) {
if (CachedValue != InterfaceMap.end())
return CachedValue->second;
- const bool CurrentClassIsInterface = [&] {
- // To be an interface, all base classes must be interfaces as well.
- if (llvm::any_of(Node->bases(), [&](const CXXBaseSpecifier &I) {
- return !I.isVirtual() && !isInterface(I);
- }))
- return false;
-
- // Interfaces should have no fields.
- if (!Node->field_empty())
- return false;
+ // To be an interface, a class must have...
+ const bool CurrentClassIsInterface =
+ // ...no bases that aren't interfaces...
+ llvm::none_of(Node->bases(),
+ [&](const CXXBaseSpecifier &I) {
+ return !I.isVirtual() && !isInterface(I);
+ }) &&
+ // ...no fields, and...
+ Node->field_empty() &&
+ // ...no methods that aren't pure virtual.
+ llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) {
+ return M->isUserProvided() && !M->isPureVirtual() && !M->isStatic();
+ });
- // Interfaces should have exclusively pure virtual methods.
- return llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) {
- return M->isUserProvided() && !M->isPureVirtual() && !M->isStatic();
- });
- }();
InterfaceMap.try_emplace(Node, CurrentClassIsInterface);
return CurrentClassIsInterface;
}
>From 9284d780a0ad58286f519932aac803f437dd4530 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 14:28:02 -0800
Subject: [PATCH 18/18] Shorten matcher
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index efbe339c955ed..aa5f4f847c762 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -17,9 +17,7 @@ namespace clang::tidy::fuchsia {
namespace {
AST_MATCHER(CXXRecordDecl, hasBases) {
- if (Node.hasDefinition())
- return Node.getNumBases() > 0;
- return false;
+ return Node.hasDefinition() && Node.getNumBases() > 0;
}
} // namespace
More information about the cfe-commits
mailing list