[clangd] Report only decl of overridding method in xref.

See: https://github.com/clangd/clangd/issues/668

```
struct A { virtual void foo() = 0; };
struct B : A { void foo() override; };
```

Find refs on `A::foo()` will show:
- decls of `A::foo()`
- decls of `B::foo()`
- refs to `A::foo()`
- no refs to `B::foo()`.

Differential Revision: https://reviews.llvm.org/D95812

clang-tools-extra/clangd/XRefs.cpp

@@ -1300,7 +1300,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
     return {};
   }
 
-  llvm::DenseSet<SymbolID> IDs, Overrides;
+  llvm::DenseSet<SymbolID> IDs;
 
   const auto *IdentifierAtCursor =
       syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
@@ -1339,25 +1339,19 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
       if (auto ID = getSymbolID(D))
         Targets.insert(ID);
 
+    RelationsRequest OverriddenBy;
     if (Index) {
-      RelationsRequest FindOverrides;
-      FindOverrides.Predicate = RelationKind::OverriddenBy;
+      OverriddenBy.Predicate = RelationKind::OverriddenBy;
       for (const NamedDecl *ND : Decls) {
-        // Special case: virtual void meth^od() = 0 includes refs of overrides.
+        // Special case: Inlcude declaration of overridding methods.
         if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(ND)) {
-          if (CMD->isPure())
+          if (CMD->isVirtual())
             if (IdentifierAtCursor && SM.getSpellingLoc(CMD->getLocation()) ==
                                           IdentifierAtCursor->location())
               if (auto ID = getSymbolID(CMD))
-                FindOverrides.Subjects.insert(ID);
+                OverriddenBy.Subjects.insert(ID);
         }
       }
-      if (!FindOverrides.Subjects.empty())
-        Index->relations(FindOverrides,
-                         [&](const SymbolID &Subject, const Symbol &Object) {
-                           Overrides.insert(Object.ID);
-                         });
-      Targets.insert(Overrides.begin(), Overrides.end());
     }
 
     // We traverse the AST to find references in the main file.
@@ -1376,17 +1370,37 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
       ReferencesResult::Reference Result;
       Result.Loc.range = Ref.range(SM);
       Result.Loc.uri = URIMainFile;
-      // Overrides are always considered references, not defs/decls.
-      if (!Overrides.contains(Ref.Target)) {
-        if (Ref.Role & static_cast<unsigned>(index::SymbolRole::Declaration))
-          Result.Attributes |= ReferencesResult::Declaration;
-        // clang-index doesn't report definitions as declarations, but they are.
-        if (Ref.Role & static_cast<unsigned>(index::SymbolRole::Definition))
-          Result.Attributes |=
-              ReferencesResult::Definition | ReferencesResult::Declaration;
-      }
+      if (Ref.Role & static_cast<unsigned>(index::SymbolRole::Declaration))
+        Result.Attributes |= ReferencesResult::Declaration;
+      // clang-index doesn't report definitions as declarations, but they are.
+      if (Ref.Role & static_cast<unsigned>(index::SymbolRole::Definition))
+        Result.Attributes |=
+            ReferencesResult::Definition | ReferencesResult::Declaration;
       Results.References.push_back(std::move(Result));
     }
+    // Add decl/def of overridding methods.
+    if (Index && Results.References.size() <= Limit &&
+        !OverriddenBy.Subjects.empty())
+      Index->relations(
+          OverriddenBy, [&](const SymbolID &Subject, const Symbol &Object) {
+            if (auto LSPLoc =
+                    toLSPLocation(Object.CanonicalDeclaration, *MainFilePath)) {
+              ReferencesResult::Reference Result;
+              Result.Loc = std::move(*LSPLoc);
+              Result.Attributes =
+                  ReferencesResult::Declaration | ReferencesResult::Override;
+              Results.References.push_back(std::move(Result));
+            }
+            if (auto LSPLoc = toLSPLocation(Object.Definition, *MainFilePath)) {
+              ReferencesResult::Reference Result;
+              Result.Loc = std::move(*LSPLoc);
+              Result.Attributes = ReferencesResult::Declaration |
+                                  ReferencesResult::Definition |
+                                  ReferencesResult::Override;
+              Results.References.push_back(std::move(Result));
+            }
+          });
+
     if (Index && Results.References.size() <= Limit) {
       for (const Decl *D : Decls) {
         // Not all symbols can be referenced from outside (e.g.
@@ -1429,13 +1443,11 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
     });
   };
   QueryIndex(std::move(IDs), /*AllowAttributes=*/true);
-  // FIXME: Currently we surface decls/defs/refs of derived methods.
-  // Maybe we'd prefer decls/defs of derived methods, and refs of base methods?
-  QueryIndex(std::move(Overrides), /*AllowAttributes=*/false);
   if (Results.References.size() > Limit) {
     Results.HasMore = true;
     Results.References.resize(Limit);
   }
+  // FIXME: Report refs of base methods.
   return Results;
 }
 
@@ -1507,6 +1519,8 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
     OS << " [decl]";
   if (R.Attributes & ReferencesResult::Definition)
     OS << " [def]";
+  if (R.Attributes & ReferencesResult::Override)
+    OS << " [override]";
   return OS;
 }
 

clang-tools-extra/clangd/XRefs.h

@@ -83,6 +83,8 @@ struct ReferencesResult {
   enum ReferenceAttributes : unsigned {
     Declaration = 1 << 0,
     Definition = 1 << 1,
+    // The occurrence is an override of the target base method.
+    Override = 1 << 2,
   };
   struct Reference {
     Location Loc;

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

@@ -1706,6 +1706,15 @@ void checkFindRefs(llvm::StringRef Test, bool UseIndex = false) {
   for (const auto &R : T.ranges("decl"))
     ExpectedLocations.push_back(
         AllOf(RangeIs(R), AttrsAre(ReferencesResult::Declaration)));
+  for (const auto &R : T.ranges("overridedecl"))
+    ExpectedLocations.push_back(AllOf(
+        RangeIs(R),
+        AttrsAre(ReferencesResult::Declaration | ReferencesResult::Override)));
+  for (const auto &R : T.ranges("overridedef"))
+    ExpectedLocations.push_back(
+        AllOf(RangeIs(R), AttrsAre(ReferencesResult::Declaration |
+                                   ReferencesResult::Definition |
+                                   ReferencesResult::Override)));
   EXPECT_THAT(
       findReferences(AST, T.point(), 0, UseIndex ? TU.index().get() : nullptr)
           .References,
@@ -1871,10 +1880,11 @@ TEST(FindReferences, IncludeOverrides) {
         };
         class Derived : public Base {
         public:
-          void [[func]]() override;
+          void $overridedecl[[func]]() override;
         };
+        void Derived::$overridedef[[func]]() {}
         void test(Derived* D) {
-          D->[[func]]();
+          D->func();  // No references to the overrides.
         })cpp";
   checkFindRefs(Test, /*UseIndex=*/true);
 }