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 <[email protected]> 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 05b7a81142a698e1921a639df352217a363aad06 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 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 <[email protected]> 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 ea6102c7674bfe1c1abf1783d5eb908af63a35be Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 f24e8f2c2b7a876b917618ea7b4f322f0222bcc4 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 1a95bbe0b2d73b2f223657d42fe5b9ab6682f807 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 56cd78164c5249a84fcdbaf195f0595867348899 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 45b308718f9467f12e6ce056e25b30331f931fe8 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 5c8e2262fa20f4b67d57dc02b1274cd75dce64a7 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 c5818e97d1a2b0b97bb3a2cfe596840137279703 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 386a006ed77b92b8e000fccd089eed7f16897fc1 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 adf905ab32761bb48b6fd5bab2f28a9f835ab953 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 cab5a8d89b447df1e2c9dd0f0585f8ad10670646 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 1fd70071f612f0460eab08c0a8f44a611db0a341 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 5956ab079ce52b1a37ddc9e466ca147a6bf1359b Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 805cb25e65a6b5fe5babfe35882c512ce1181195 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 832b286f79bf81c0bd8aa32985a9e55dcd29721a Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 b6017d5662f9eeb1c144923f9cbc63e5a4cbae1c Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
