Author: Zeyi Xu Date: 2026-03-14T16:34:41+08:00 New Revision: b73a64fc3eb01635deb115b8c836cbca0cab1856
URL: https://github.com/llvm/llvm-project/commit/b73a64fc3eb01635deb115b8c836cbca0cab1856 DIFF: https://github.com/llvm/llvm-project/commit/b73a64fc3eb01635deb115b8c836cbca0cab1856.diff LOG: [clang-tidy] Fix virtual inheritance FP in misc-multiple-inheritance (#186103) Avoid double-counting concrete bases introduced through virtual inheritance in `misc-multiple-inheritance`. As of AI-Usage: Gemini 3 is used for pre-commit reviewing. Closes https://github.com/llvm/llvm-project/issues/186059 Added: Modified: clang-tools-extra/clang-tidy/misc/MultipleInheritanceCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/misc/multiple-inheritance.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/misc/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/misc/MultipleInheritanceCheck.cpp index 557b4559697b9..72e6aa6ac0b47 100644 --- a/clang-tools-extra/clang-tidy/misc/MultipleInheritanceCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/MultipleInheritanceCheck.cpp @@ -58,16 +58,35 @@ 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 = - llvm::count_if(D.bases(), [&](const CXXBaseSpecifier &I) { - return !I.isVirtual() && !isInterface(I); - }); + // Collect the direct and virtual concrete bases of the class. + SmallVector<const CXXRecordDecl *> DirectConcreteBases; + for (const CXXBaseSpecifier &Base : D.bases()) + if (!Base.isVirtual() && !isInterface(Base)) + DirectConcreteBases.push_back(Base.getType()->getAsCXXRecordDecl()); + + SmallVector<const CXXRecordDecl *> VirtualConcreteBases; + for (const CXXBaseSpecifier &VBase : D.vbases()) + if (!isInterface(VBase)) + VirtualConcreteBases.push_back(VBase.getType()->getAsCXXRecordDecl()); - // Check virtual bases to see if there is more than one concrete - // non-virtual base. + unsigned NumConcrete = DirectConcreteBases.size(); + + // Count only virtual concrete bases that introduce an additional + // implementation base, skipping those already represented by a more derived + // concrete base. NumConcrete += llvm::count_if( - D.vbases(), [&](const CXXBaseSpecifier &V) { return !isInterface(V); }); + VirtualConcreteBases, [&](const CXXRecordDecl *VirtualBase) { + const bool HiddenByMoreDerivedVirtualBase = llvm::any_of( + VirtualConcreteBases, [&](const CXXRecordDecl *OtherVirtualBase) { + return VirtualBase != OtherVirtualBase && + OtherVirtualBase->isVirtuallyDerivedFrom(VirtualBase); + }); + const bool HiddenByDirectConcreteBase = llvm::any_of( + DirectConcreteBases, [&](const CXXRecordDecl *DirectBase) { + return DirectBase->isVirtuallyDerivedFrom(VirtualBase); + }); + return !HiddenByMoreDerivedVirtualBase && !HiddenByDirectConcreteBase; + }); if (NumConcrete > 1) diag(D.getBeginLoc(), "inheriting multiple classes that aren't " diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f0607996c5ff2..4b207609d598d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -258,6 +258,10 @@ Changes in existing checks - Fixed false positive where an array of pointers to ``const`` was incorrectly diagnosed as allowing the pointee to be made ``const``. +- Improved :doc:`misc-multiple-inheritance + <clang-tidy/checks/misc/multiple-inheritance>` by avoiding false positives when + virtual inheritance causes concrete bases to be counted more than once. + - Improved :doc:`misc-throw-by-value-catch-by-reference <clang-tidy/checks/misc/throw-by-value-catch-by-reference>` check: diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/multiple-inheritance.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/multiple-inheritance.cpp index 6004ab3d812ea..257e16ab18f2f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/multiple-inheritance.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/multiple-inheritance.cpp @@ -163,3 +163,22 @@ struct S2 { int i; }; struct S3 : S1, S2 {}; } // namespace N + +namespace M { + +class basic_ios { int state; }; +class ostream : virtual public basic_ios { int more_state; }; +class OStringStream final : public ostream {}; + +struct A { int x; }; +struct B : A {}; +// CHECK-MESSAGES: [[@LINE+1]]:1: warning: inheriting multiple classes that aren't pure virtual is discouraged [misc-multiple-inheritance] +struct C : A, B {}; + +struct VA { virtual void f(); }; +struct VB : VA { virtual void g(); }; +struct VI : virtual VA { virtual void h() = 0; }; +// CHECK-MESSAGES: [[@LINE+1]]:1: warning: inheriting multiple classes that aren't pure virtual is discouraged [misc-multiple-inheritance] +struct VD : VI, VB {}; + +} // namespace M _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
