[clang-tidy] Don't cache classes by name in fuchsia-multiple-inheritance

clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp

@@ -23,23 +23,19 @@ 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
 // 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 {
-  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))

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<bool> InterfaceMap;
+  llvm::DenseMap<const CXXRecordDecl *, bool> InterfaceMap;
 };
 
 } // namespace clang::tidy::fuchsia

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
+  <clang-tidy/checks/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
   <clang-tidy/checks/google/readability-casting>` check by adding fix-it
   notes for downcasts and casts to void pointer.

clang-tools-extra/test/clang-tidy/checkers/fuchsia/multiple-inheritance.cpp

@@ -148,3 +148,18 @@ void test_no_crash() {
   auto foo = []() {};
   WithTemplBase<decltype(foo)>();
 }
+
+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