[clang-tools-extra] fbeff2e - [clangd] Report only decl of overridding method in xref.
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 2 04:06:39 PST 2021
Author: Utkarsh Saxena
Date: 2021-02-02T13:06:20+01:00
New Revision: fbeff2ec2bc6e44b92931207b0063f83ff7a3b3a
URL: https://github.com/llvm/llvm-project/commit/fbeff2ec2bc6e44b92931207b0063f83ff7a3b3a
DIFF: https://github.com/llvm/llvm-project/commit/fbeff2ec2bc6e44b92931207b0063f83ff7a3b3a.diff
LOG: [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
Added:
Modified:
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index 55b55da80fda..b6a0be4a0a0a 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/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;
}
diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h
index e4a9f7411d96..3f69106611dc 100644
--- a/clang-tools-extra/clangd/XRefs.h
+++ b/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;
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 30a8b0ade1ee..966656e47d8f 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/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);
}
More information about the cfe-commits
mailing list