[clangd] Go-to-definition on pure virtual method decls jumps to all o…

…verrides. Reviewed By: usaxena95 Differential Revision: https://reviews.llvm.org/D92299
syntacore · Dec 14, 2020 · 63ec9e4 · 63ec9e4
1 parent 08e287a
commit 63ec9e4
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 26 deletions.
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
@@ -292,6 +292,35 @@ const NamedDecl *getPreferredDecl(const NamedDecl *D) {
   return D;
 }
 
+std::vector<LocatedSymbol> findOverrides(llvm::DenseSet<SymbolID> IDs,
+                                         const SymbolIndex *Index,
+                                         llvm::StringRef MainFilePath) {
+  if (IDs.empty())
+    return {};
+  RelationsRequest Req;
+  Req.Predicate = RelationKind::OverriddenBy;
+  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;
+    }
+    Results.emplace_back();
+    Results.back().Name = Object.Name.str();
+    Results.back().PreferredDeclaration = *DeclLoc;
+    auto DefLoc = indexToLSPLocation(Object.Definition, MainFilePath);
+    if (!DefLoc) {
+      elog("Failed to convert location: {0}", DefLoc.takeError());
+      return;
+    }
+    Results.back().Definition = *DefLoc;
+  });
+  return Results;
+}
+
 // Decls are more complicated.
 // The AST contains at least a declaration, maybe a definition.
 // These are up-to-date, and so generally preferred over index results.
@@ -330,10 +359,19 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
       DeclRelation::TemplatePattern | DeclRelation::Alias;
   auto Candidates =
       getDeclAtPositionWithRelations(AST, CurLoc, Relations, NodeKind);
+  llvm::DenseSet<SymbolID> VirtualMethods;
   for (const auto &E : Candidates) {
     const NamedDecl *D = E.first;
-    // Special case: void foo() ^override: jump to the overridden method.
     if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(D)) {
+      // Special case: virtual void ^method() = 0: jump to all overrides.
+      // FIXME: extend it to ^virtual, unfortunately, virtual location is not
+      // saved in the AST.
+      if (CMD->isPure()) {
+        if (TouchedIdentifier && SM.getSpellingLoc(CMD->getLocation()) ==
+                                     TouchedIdentifier->location())
+          VirtualMethods.insert(getSymbolID(CMD));
+      }
+      // Special case: void foo() ^override: jump to the overridden method.
       const InheritableAttr *Attr = D->getAttr<OverrideAttr>();
       if (!Attr)
         Attr = D->getAttr<FinalAttr>();
@@ -420,6 +458,8 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
     });
   }
 
+  auto Overrides = findOverrides(VirtualMethods, Index, MainFilePath);
+  Result.insert(Result.end(), Overrides.begin(), Overrides.end());
   return Result;
 }
 
@@ -1145,36 +1185,14 @@ std::vector<LocatedSymbol> findImplementations(ParsedAST &AST, Position Pos,
          CurLoc.takeError());
     return {};
   }
-  std::vector<LocatedSymbol> Results;
   DeclRelationSet Relations =
       DeclRelation::TemplatePattern | DeclRelation::Alias;
-  RelationsRequest Req;
-  Req.Predicate = RelationKind::OverriddenBy;
+  llvm::DenseSet<SymbolID> VirtualMethods;
   for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations))
     if (const CXXMethodDecl *CXXMD = llvm::dyn_cast<CXXMethodDecl>(ND))
       if (CXXMD->isVirtual())
-        Req.Subjects.insert(getSymbolID(ND));
-
-  if (Req.Subjects.empty())
-    return Results;
-  Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
-    auto DeclLoc =
-        indexToLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
-    if (!DeclLoc) {
-      elog("Find implementation: {0}", DeclLoc.takeError());
-      return;
-    }
-    LocatedSymbol Loc;
-    Loc.Name = Object.Name.str();
-    Loc.PreferredDeclaration = *DeclLoc;
-    auto DefLoc = indexToLSPLocation(Object.Definition, *MainFilePath);
-    if (DefLoc)
-      Loc.Definition = *DefLoc;
-    else
-      llvm::consumeError(DefLoc.takeError());
-    Results.push_back(Loc);
-  });
-  return Results;
+        VirtualMethods.insert(getSymbolID(ND));
+  return findOverrides(std::move(VirtualMethods), Index, *MainFilePath);
 }
 
 ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,

diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -349,6 +349,22 @@ TEST(LocateSymbol, WithIndex) {
       ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range())));
 }
 
+TEST(LocateSymbol, FindOverrides) {
+  auto Code = Annotations(R"cpp(
+    class Foo {
+      virtual void $1[[fo^o]]() = 0;
+    };
+    class Bar : public Foo {
+      void $2[[foo]]() override;
+    };
+  )cpp");
+  TestTU TU = TestTU::withCode(Code.code());
+  auto AST = TU.build();
+  EXPECT_THAT(locateSymbolAt(AST, Code.point(), TU.index().get()),
+              UnorderedElementsAre(Sym("foo", Code.range("1"), llvm::None),
+                                   Sym("foo", Code.range("2"), llvm::None)));
+}
+
 TEST(LocateSymbol, WithIndexPreferredLocation) {
   Annotations SymbolHeader(R"cpp(
         class $p[[Proto]] {};