diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp index 652dec9bcc2a9..df40d166bd404 100644 --- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp +++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp @@ -23,13 +23,11 @@ AST_MATCHER(CXXRecordDecl, hasBases) { } } // namespace -// Adds a node (by name) to the interface map, if it was not present in the map +// Adds a node to the interface map, if it was not present in the map // previously. void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node, bool IsInterface) { - assert(Node->getIdentifier()); - const StringRef Name = Node->getIdentifier()->getName(); - InterfaceMap.insert(std::make_pair(Name, IsInterface)); + InterfaceMap.try_emplace(Node, IsInterface); } // Returns "true" if the boolean "isInterface" has been set to the @@ -37,9 +35,7 @@ void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node, // interface status for the current node is not yet known. bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node, bool &IsInterface) const { - assert(Node->getIdentifier()); - const StringRef Name = Node->getIdentifier()->getName(); - auto Pair = InterfaceMap.find(Name); + auto Pair = InterfaceMap.find(Node); if (Pair == InterfaceMap.end()) return false; IsInterface = Pair->second; @@ -59,9 +55,6 @@ bool MultipleInheritanceCheck::isCurrentClassInterface( } bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) { - if (!Node->getIdentifier()) - return false; - // Short circuit the lookup if we have analyzed this record before. bool PreviousIsInterfaceResult = false; if (getInterfaceStatus(Node, PreviousIsInterfaceResult)) diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h index 2e268432c17cf..b9055eb58a300 100644 --- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h +++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h @@ -38,7 +38,7 @@ class MultipleInheritanceCheck : public ClangTidyCheck { // 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), // where N is the number of classes. - llvm::StringMap InterfaceMap; + llvm::DenseMap InterfaceMap; }; } // namespace clang::tidy::fuchsia diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 42160fa9cb51c..d1fb1cba3e45a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -441,6 +441,12 @@ Changes in existing checks correctly ignore ``std::array`` and other array-like containers when `IgnoreArrays` option is set to `true`. +- Improved :doc:`fuchsia-multiple-inheritance + ` + by fixing an issue where the check would only analyze the first class with + a given name in the program, missing any subsequent classes with that same + name (declared in a different scope). + - Improved :doc:`google-readability-casting ` check by adding fix-it notes for downcasts and casts to void pointer. diff --git a/clang-tools-extra/test/clang-tidy/checkers/fuchsia/multiple-inheritance.cpp b/clang-tools-extra/test/clang-tidy/checkers/fuchsia/multiple-inheritance.cpp index d53b3fde7736b..c60649f52cb94 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/fuchsia/multiple-inheritance.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/fuchsia/multiple-inheritance.cpp @@ -148,3 +148,18 @@ void test_no_crash() { auto foo = []() {}; WithTemplBase(); } + +struct S1 {}; +struct S2 {}; + +struct S3 : S1, S2 {}; + +namespace N { + +struct S1 { int i; }; +struct S2 { int i; }; + +// CHECK-MESSAGES: [[@LINE+1]]:1: warning: inheriting multiple classes that aren't pure virtual is discouraged [fuchsia-multiple-inheritance] +struct S3 : S1, S2 {}; + +} // namespace N