diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -83,6 +83,7 @@ enum ReferenceAttributes : unsigned { Declaration = 1 << 0, Definition = 1 << 1, + OverriddenBy = 1 << 2, }; struct Reference { Location Loc; diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1300,7 +1300,7 @@ return {}; } - llvm::DenseSet IDs, Overrides; + llvm::DenseSet IDs; const auto *IdentifierAtCursor = syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens()); @@ -1339,25 +1339,19 @@ 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(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,29 @@ 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(index::SymbolRole::Declaration)) - Result.Attributes |= ReferencesResult::Declaration; - // clang-index doesn't report definitions as declarations, but they are. - if (Ref.Role & static_cast(index::SymbolRole::Definition)) - Result.Attributes |= - ReferencesResult::Definition | ReferencesResult::Declaration; - } + if (Ref.Role & static_cast(index::SymbolRole::Declaration)) + Result.Attributes |= ReferencesResult::Declaration; + // clang-index doesn't report definitions as declarations, but they are. + if (Ref.Role & static_cast(index::SymbolRole::Definition)) + Result.Attributes |= + ReferencesResult::Definition | ReferencesResult::Declaration; Results.References.push_back(std::move(Result)); } + // Add declaration 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::OverriddenBy; + 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 +1435,11 @@ }); }; 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; } diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -1706,6 +1706,10 @@ for (const auto &R : T.ranges("decl")) ExpectedLocations.push_back( AllOf(RangeIs(R), AttrsAre(ReferencesResult::Declaration))); + for (const auto &R : T.ranges("override")) + ExpectedLocations.push_back( + AllOf(RangeIs(R), AttrsAre(ReferencesResult::Declaration | + ReferencesResult::OverriddenBy))); EXPECT_THAT( findReferences(AST, T.point(), 0, UseIndex ? TU.index().get() : nullptr) .References, @@ -1862,7 +1866,7 @@ checkFindRefs(Test); } -TEST(FindReferences, IncludeOverrides) { +TEST(FindReferences, IncludeOverrideDeclaration) { llvm::StringRef Test = R"cpp( class Base { @@ -1871,10 +1875,10 @@ }; class Derived : public Base { public: - void [[func]]() override; + void $override[[func]]() override; }; void test(Derived* D) { - D->[[func]](); + D->func(); // No references to the overrides. })cpp"; checkFindRefs(Test, /*UseIndex=*/true); }