[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:52:48 PST 2025
https://github.com/localspook updated https://github.com/llvm/llvm-project/pull/171059
>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/20] 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 05b7a81142a698e1921a639df352217a363aad06 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/20] 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 out parameter.
---
.../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 b7b7dbe9c87c7079df64eb67a68c175866792479 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/20] 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 ea6102c7674bfe1c1abf1783d5eb908af63a35be 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/20] 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 f24e8f2c2b7a876b917618ea7b4f322f0222bcc4 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/20] 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 1a95bbe0b2d73b2f223657d42fe5b9ab6682f807 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/20] 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 56cd78164c5249a84fcdbaf195f0595867348899 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/20] 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 45b308718f9467f12e6ce056e25b30331f931fe8 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/20] 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 5c8e2262fa20f4b67d57dc02b1274cd75dce64a7 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/20] 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 c5818e97d1a2b0b97bb3a2cfe596840137279703 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/20] 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 386a006ed77b92b8e000fccd089eed7f16897fc1 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/20] 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 adf905ab32761bb48b6fd5bab2f28a9f835ab953 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/20] 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 cab5a8d89b447df1e2c9dd0f0585f8ad10670646 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/20] 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 1fd70071f612f0460eab08c0a8f44a611db0a341 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/20] 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 5956ab079ce52b1a37ddc9e466ca147a6bf1359b 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/20] 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 805cb25e65a6b5fe5babfe35882c512ce1181195 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/20] 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 832b286f79bf81c0bd8aa32985a9e55dcd29721a 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/20] 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 b6017d5662f9eeb1c144923f9cbc63e5a4cbae1c 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/20] 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
>From b8644f4d2bcb48b0f26fb0d29bff4a7611372f3e Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 14:49:24 -0800
Subject: [PATCH 19/20] Formatting
---
clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index eb5c94241996e..4dcbd0c7893c5 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 CXXBaseSpecifier& Base);
+ 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 75f0c09c6208861ccc00c4b635c08e799305ff71 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 7 Dec 2025 14:52:24 -0800
Subject: [PATCH 20/20] Oops, fix "Deduplicate null checks" commit not being
NFC
---
.../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 aa5f4f847c762..ae4772c197360 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -24,7 +24,7 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
bool MultipleInheritanceCheck::isInterface(const CXXBaseSpecifier &Base) {
const CXXRecordDecl *const Node = Base.getType()->getAsCXXRecordDecl();
if (!Node)
- return false;
+ return true;
assert(Node->isCompleteDefinition());
More information about the cfe-commits
mailing list