[clangd] go-to-implementation on a base class jumps to all subclasses.

[njames93]

How does this handle Dependent base classes (including CRTP). Can tests be added to demonstrate that behaviour if its supported.

clang-tools-extra/clangd/XRefs.cpp

1252

nit: Make this else if or add a continue statement to the the previous if. We know ND can't be a CXXRecordDecl its a CXXMethodDecl.
super-nit: Maybe call this CXXRD or just RD, D implies we only know its a Decl but nothing more.

clang-tools-extra/clangd/unittests/XRefsTests.cpp

1645

If you're changing this line, may as well change use StringRef here.

[hokein]

address review comment.

[hokein]

In D92749#2436658, @njames93 wrote:

How does this handle Dependent base classes (including CRTP). Can tests be added to demonstrate that behaviour if its supported.

Thanks, this is a good point.

[usaxena95]

LGTM. Thanks!

clang-tools-extra/clangd/XRefs.h

 std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
                                                       Position Pos);
 
 struct ReferencesResult {
   std::vector<Location> References;
   bool HasMore = false;
 };
 
-/// Returns implementations of the virtual function at a specified \p Pos.
+/// Returns implementations at a specified \p Pos:
+///   - overrides for a virtual method;
+///   - subclasses for a base class;
 std::vector<LocatedSymbol> findImplementations(ParsedAST &AST, Position Pos,
                                                const SymbolIndex *Index);
 
 /// Returns references of the symbol at a specified \p Pos.
 /// \p Limit limits the number of results returned (0 means no limit).
 ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
                                 const SymbolIndex *Index = nullptr);
 

clang-tools-extra/clangd/XRefs.cpp

if (const auto *DefinitionID = ID->getDefinition())
       return DefinitionID;
   if (const auto *PD = dyn_cast<ObjCProtocolDecl>(D))
     if (const auto *DefinitionID = PD->getDefinition())
       return DefinitionID;
 
   return D;
 }
 
-std::vector<LocatedSymbol> findOverrides(llvm::DenseSet<SymbolID> IDs,
+std::vector<LocatedSymbol> findImplementors(llvm::DenseSet<SymbolID> IDs,
+                                            RelationKind Predicate,
                                             const SymbolIndex *Index,
                                             llvm::StringRef MainFilePath) {
   if (IDs.empty())
     return {};
   RelationsRequest Req;
-  Req.Predicate = RelationKind::OverriddenBy;
+  Req.Predicate = Predicate;
   Req.Subjects = std::move(IDs);
   std::vector<LocatedSymbol> Results;
   Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
     auto DeclLoc =
         indexToLSPLocation(Object.CanonicalDeclaration, MainFilePath);
     if (!DeclLoc) {
       elog("Find overrides: {0}", DeclLoc.takeError());
       return;
Index->lookup(QueryRequest, [&](const Symbol &Sym) {
                 getPreferredLocation(R.PreferredDeclaration,
                                      Sym.CanonicalDeclaration, Scratch),
                 MainFilePath))
           R.PreferredDeclaration = *Loc;
       }
     });
   }
 
-  auto Overrides = findOverrides(VirtualMethods, Index, MainFilePath);
+  auto Overrides = findImplementors(VirtualMethods, RelationKind::OverriddenBy,
+                                    Index, MainFilePath);
   Result.insert(Result.end(), Overrides.begin(), Overrides.end());
   return Result;
 }
 
 std::vector<LocatedSymbol> locateSymbolForType(const ParsedAST &AST,
                                                const QualType &Type) {
   const auto &SM = AST.getSourceManager();
   auto MainFilePath =
std::vector<LocatedSymbol> findImplementations(ParsedAST &AST, Position Pos,
   auto CurLoc = sourceLocationInMainFile(SM, Pos);
   if (!CurLoc) {
     elog("Failed to convert position to source location: {0}",
          CurLoc.takeError());
     return {};
   }
   DeclRelationSet Relations =
       DeclRelation::TemplatePattern | DeclRelation::Alias;
-  llvm::DenseSet<SymbolID> VirtualMethods;
-  for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations))
-    if (const CXXMethodDecl *CXXMD = llvm::dyn_cast<CXXMethodDecl>(ND))
-      if (CXXMD->isVirtual())
-        VirtualMethods.insert(getSymbolID(ND));
-  return findOverrides(std::move(VirtualMethods), Index, *MainFilePath);
+  llvm::DenseSet<SymbolID> IDs;
+  RelationKind QueryKind;
+  for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations)) {
+    if (const auto *CXXMD = llvm::dyn_cast<CXXMethodDecl>(ND)) {
+      if (CXXMD->isVirtual()) {
+        IDs.insert(getSymbolID(ND));
+        QueryKind = RelationKind::OverriddenBy;
+      }
+    } else if (const auto *RD = dyn_cast<CXXRecordDecl>(ND)) {
+      IDs.insert(getSymbolID(RD));
+      QueryKind = RelationKind::BaseOf;
+    }
+  }
+  return findImplementors(std::move(IDs), QueryKind, Index, *MainFilePath);
 }
 
 ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
                                 const SymbolIndex *Index) {
   if (!Limit)
     Limit = std::numeric_limits<uint32_t>::max();
   ReferencesResult Results;
   const SourceManager &SM = AST.getSourceManager();

clang-tools-extra/clangd/unittests/XRefsTests.cpp

if (T.ranges().empty())
       EXPECT_THAT(Nearby, Eq(llvm::None)) << Test;
     else
       EXPECT_EQ(Nearby, T.range()) << Test;
   }
 }
 
 TEST(FindImplementations, Inheritance) {
   llvm::StringRef Test = R"cpp(
-    struct Base {
+    struct $0^Base {
       virtual void F$1^oo();
       void C$4^oncrete();
     };
-    struct Child1 : Base {
+    struct $0[[Child1]] : Base {
       void $1[[Fo$3^o]]() override;
       virtual void B$2^ar();
       void Concrete();  // No implementations for concrete methods.
     };
     struct Child2 : Child1 {
       void $3[[Foo]]() override;
       void $2[[Bar]]() override;
     };
     void FromReference() {
-      Base* B;
+      $0^Base* B;
       B->Fo$1^o();
       B->C$4^oncrete();
       &Base::Fo$1^o;
       Child1 * C1;
       C1->B$2^ar();
       C1->Fo$3^o();
     }
+    // CRTP should work.
+    template<typename T>
+    struct $5^TemplateBase {};
+    struct $5[[Child3]] : public TemplateBase<Child3> {};
   )cpp";
 
   Annotations Code(Test);
   auto TU = TestTU::withCode(Code.code());
   auto AST = TU.build();
-  for (const std::string &Label : {"1", "2", "3", "4"}) {
+  for (StringRef Label : {"0", "1", "2", "3", "4", "5"}) {
     for (const auto &Point : Code.points(Label)) {
       EXPECT_THAT(findImplementations(AST, Point, TU.index().get()),
                   UnorderedPointwise(DeclRange(), Code.ranges(Label)))
           << Code.code() << " at " << Point << " for Label " << Label;
     }
   }
 }