Author: Victor Chernyakin Date: 2025-12-08T07:19:04-08:00 New Revision: a6fc5a1d77e78333f26a44b145ad43149e9ada17
URL: https://github.com/llvm/llvm-project/commit/a6fc5a1d77e78333f26a44b145ad43149e9ada17 DIFF: https://github.com/llvm/llvm-project/commit/a6fc5a1d77e78333f26a44b145ad43149e9ada17.diff LOG: [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (#171059) Added: Modified: clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp index df40d166bd404..4a10cb4085a41 100644 --- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp +++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp @@ -17,106 +17,61 @@ 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 -// 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. -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::isInterface(const CXXBaseSpecifier &Base) { + const CXXRecordDecl *const Node = Base.getType()->getAsCXXRecordDecl(); + if (!Node) + return true; -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(); - }); -} + assert(Node->isCompleteDefinition()); -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; - - // 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)) { - addNodeToInterfaceMap(Node, false); - return false; - } - } - - const bool CurrentClassIsInterface = isCurrentClassInterface(Node); - addNodeToInterfaceMap(Node, CurrentClassIsInterface); + if (const auto CachedValue = InterfaceMap.find(Node); + CachedValue != InterfaceMap.end()) + return CachedValue->second; + + // 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(); + }); + + InterfaceMap.try_emplace(Node, CurrentClassIsInterface); return CurrentClassIsInterface; } void MultipleInheritanceCheck::registerMatchers(MatchFinder *Finder) { - // Match declarations which have bases. Finder->addMatcher(cxxRecordDecl(hasBases(), isDefinition()).bind("decl"), this); } 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++; - } - - // 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"); - } - } + const auto &D = *Result.Nodes.getNodeAs<CXXRecordDecl>("decl"); + // Check to see if the class inherits from multiple concrete classes. + 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. + NumConcrete += llvm::count_if( + D.vbases(), [&](const CXXBaseSpecifier &V) { return !isInterface(V); }); + + if (NumConcrete > 1) + diag(D.getBeginLoc(), "inheriting multiple classes that aren't " + "pure virtual is discouraged"); } } // namespace clang::tidy::fuchsia diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h index b9055eb58a300..4dcbd0c7893c5 100644 --- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h +++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h @@ -30,10 +30,7 @@ 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); + 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), _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
