[clang-tools-extra] 8712df7 - [clangd] references: decls of overrides of x are refs to x, not decls
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 1 08:19:31 PST 2021
Author: Sam McCall
Date: 2021-02-01T17:19:19+01:00
New Revision: 8712df7a621d1d00a3fd4641ef72639a8faa6284
URL: https://github.com/llvm/llvm-project/commit/8712df7a621d1d00a3fd4641ef72639a8faa6284
DIFF: https://github.com/llvm/llvm-project/commit/8712df7a621d1d00a3fd4641ef72639a8faa6284.diff
LOG: [clangd] references: decls of overrides of x are refs to x, not decls
This requires a second index query for refs to overrides, as the refs
call doesn't tell you which ref points at which symbol.
Differential Revision: https://reviews.llvm.org/D95451
Added:
Modified:
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/index/Index.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 8123b9bc452f..55b55da80fda 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -872,6 +872,7 @@ class ReferenceFinder : public index::IndexDataConsumer {
struct Reference {
syntax::Token SpelledTok;
index::SymbolRoleSet Role;
+ SymbolID Target;
Range range(const SourceManager &SM) const {
return halfOpenToRange(SM, SpelledTok.range(SM).toCharRange(SM));
@@ -906,13 +907,15 @@ class ReferenceFinder : public index::IndexDataConsumer {
SourceLocation Loc,
index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
const SourceManager &SM = AST.getSourceManager();
- if (!isInsideMainFile(Loc, SM) ||
- TargetIDs.find(getSymbolID(D)) == TargetIDs.end())
+ if (!isInsideMainFile(Loc, SM))
+ return true;
+ SymbolID ID = getSymbolID(D);
+ if (!TargetIDs.contains(ID))
return true;
const auto &TB = AST.getTokens();
Loc = SM.getFileLoc(Loc);
if (const auto *Tok = TB.spelledTokenAt(Loc))
- References.push_back({*Tok, Roles});
+ References.push_back({*Tok, Roles, ID});
return true;
}
@@ -1297,7 +1300,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
return {};
}
- RefsRequest Req;
+ llvm::DenseSet<SymbolID> IDs, Overrides;
const auto *IdentifierAtCursor =
syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
@@ -1322,7 +1325,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
Results.References.push_back(std::move(Result));
}
}
- Req.IDs.insert(MacroSID);
+ IDs.insert(MacroSID);
}
} else {
// Handle references to Decls.
@@ -1336,7 +1339,6 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
if (auto ID = getSymbolID(D))
Targets.insert(ID);
- llvm::DenseSet<SymbolID> Overrides;
if (Index) {
RelationsRequest FindOverrides;
FindOverrides.Predicate = RelationKind::OverriddenBy;
@@ -1374,16 +1376,18 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
ReferencesResult::Reference Result;
Result.Loc.range = Ref.range(SM);
Result.Loc.uri = URIMainFile;
- 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;
+ // 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;
+ }
Results.References.push_back(std::move(Result));
}
if (Index && Results.References.size() <= Limit) {
- Req.IDs = std::move(Overrides);
for (const Decl *D : Decls) {
// Not all symbols can be referenced from outside (e.g.
// function-locals).
@@ -1392,13 +1396,17 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
if (D->getParentFunctionOrMethod())
continue;
if (auto ID = getSymbolID(D))
- Req.IDs.insert(ID);
+ IDs.insert(ID);
}
}
}
// Now query the index for references from other files.
- if (!Req.IDs.empty() && Index && Results.References.size() <= Limit) {
+ auto QueryIndex = [&](llvm::DenseSet<SymbolID> IDs, bool AllowAttributes) {
+ RefsRequest Req;
+ Req.IDs = std::move(IDs);
Req.Limit = Limit;
+ if (Req.IDs.empty() || !Index || Results.References.size() > Limit)
+ return;
Results.HasMore |= Index->refs(Req, [&](const Ref &R) {
// No need to continue process if we reach the limit.
if (Results.References.size() > Limit)
@@ -1409,15 +1417,21 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
return;
ReferencesResult::Reference Result;
Result.Loc = std::move(*LSPLoc);
- if ((R.Kind & RefKind::Declaration) == RefKind::Declaration)
- Result.Attributes |= ReferencesResult::Declaration;
- // FIXME: our index should definitely store def | decl separately!
- if ((R.Kind & RefKind::Definition) == RefKind::Definition)
- Result.Attributes |=
- ReferencesResult::Declaration | ReferencesResult::Definition;
+ if (AllowAttributes) {
+ if ((R.Kind & RefKind::Declaration) == RefKind::Declaration)
+ Result.Attributes |= ReferencesResult::Declaration;
+ // FIXME: our index should definitely store def | decl separately!
+ if ((R.Kind & RefKind::Definition) == RefKind::Definition)
+ Result.Attributes |=
+ ReferencesResult::Declaration | ReferencesResult::Definition;
+ }
Results.References.push_back(std::move(Result));
});
- }
+ };
+ 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);
diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h
index c961aa9d8bd9..775ce88841f9 100644
--- a/clang-tools-extra/clangd/index/Index.h
+++ b/clang-tools-extra/clangd/index/Index.h
@@ -104,11 +104,12 @@ class SymbolIndex {
lookup(const LookupRequest &Req,
llvm::function_ref<void(const Symbol &)> Callback) const = 0;
- /// Finds all occurrences (e.g. references, declarations, definitions) of a
- /// symbol and applies \p Callback on each result.
+ /// Finds all occurrences (e.g. references, declarations, definitions) of
+ /// symbols and applies \p Callback on each result.
///
/// Results should be returned in arbitrary order.
/// The returned result must be deep-copied if it's used outside Callback.
+ /// FIXME: there's no indication which result references which symbol.
///
/// Returns true if there will be more results (limited by Req.Limit);
virtual bool refs(const RefsRequest &Req,
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index ee44d38187d1..30a8b0ade1ee 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1871,7 +1871,7 @@ TEST(FindReferences, IncludeOverrides) {
};
class Derived : public Base {
public:
- void $decl[[func]]() override; // FIXME: ref, not decl
+ void [[func]]() override;
};
void test(Derived* D) {
D->[[func]]();
More information about the cfe-commits
mailing list