[clangd] Don't trim xrefs references if we overran the limit

This preserves all the results we've processed already rather than
throwing them away in the end.
It has some performance implications on the edge cases, in the worst case we
might issue 1 relations and 2 xrefs requests in extra to deduce `HasMore`
correctly.

Fixes https://github.com/clangd/clangd/issues/204.

Differential Revision: https://reviews.llvm.org/D116043
This commit is contained in:
Kadir Cetinkaya 2021-12-20 15:51:09 +01:00
parent 37e6bd8bc8
commit 81967b4fa7
No known key found for this signature in database
GPG Key ID: E39E36B8D2057ED6
1 changed files with 55 additions and 56 deletions

View File

@ -1330,8 +1330,6 @@ void getOverriddenMethods(const CXXMethodDecl *CMD,
ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
const SymbolIndex *Index) {
if (!Limit)
Limit = std::numeric_limits<uint32_t>::max();
ReferencesResult Results;
const SourceManager &SM = AST.getSourceManager();
auto MainFilePath =
@ -1347,7 +1345,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
return {};
}
llvm::DenseSet<SymbolID> IDs, OverriddenMethods;
llvm::DenseSet<SymbolID> IDsToQuery, OverriddenMethods;
const auto *IdentifierAtCursor =
syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
@ -1372,7 +1370,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
Results.References.push_back(std::move(Result));
}
}
IDs.insert(MacroSID);
IDsToQuery.insert(MacroSID);
}
} else {
// Handle references to Decls.
@ -1381,10 +1379,19 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
DeclRelation::TemplatePattern | DeclRelation::Alias;
std::vector<const NamedDecl *> Decls =
getDeclAtPosition(AST, *CurLoc, Relations);
llvm::DenseSet<SymbolID> Targets;
for (const NamedDecl *D : Decls)
if (auto ID = getSymbolID(D))
Targets.insert(ID);
llvm::DenseSet<SymbolID> TargetsInMainFile;
for (const NamedDecl *D : Decls) {
auto ID = getSymbolID(D);
if (!ID)
continue;
TargetsInMainFile.insert(ID);
// Not all symbols can be referenced from outside (e.g. function-locals).
// TODO: we could skip TU-scoped symbols here (e.g. static functions) if
// we know this file isn't a header. The details might be tricky.
if (D->getParentFunctionOrMethod())
continue;
IDsToQuery.insert(ID);
}
RelationsRequest OverriddenBy;
if (Index) {
@ -1403,7 +1410,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
}
// We traverse the AST to find references in the main file.
auto MainFileRefs = findRefs(Targets, AST, /*PerToken=*/false);
auto MainFileRefs = findRefs(TargetsInMainFile, AST, /*PerToken=*/false);
// We may get multiple refs with the same location and different Roles, as
// cross-reference is only interested in locations, we deduplicate them
// by the location to avoid emitting duplicated locations.
@ -1427,56 +1434,52 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
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) {
const auto LSPLocDecl =
toLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
const auto LSPLocDef =
toLSPLocation(Object.Definition, *MainFilePath);
if (LSPLocDecl && LSPLocDecl != LSPLocDef) {
ReferencesResult::Reference Result;
Result.Loc = std::move(*LSPLocDecl);
Result.Attributes =
ReferencesResult::Declaration | ReferencesResult::Override;
Results.References.push_back(std::move(Result));
}
if (LSPLocDef) {
ReferencesResult::Reference Result;
Result.Loc = std::move(*LSPLocDef);
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.
// function-locals).
// TODO: we could skip TU-scoped symbols here (e.g. static functions) if
// we know this file isn't a header. The details might be tricky.
if (D->getParentFunctionOrMethod())
continue;
if (auto ID = getSymbolID(D))
IDs.insert(ID);
}
if (Index && !OverriddenBy.Subjects.empty()) {
Index->relations(OverriddenBy, [&](const SymbolID &Subject,
const Symbol &Object) {
if (Limit && Results.References.size() >= Limit) {
Results.HasMore = true;
return;
}
const auto LSPLocDecl =
toLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
const auto LSPLocDef = toLSPLocation(Object.Definition, *MainFilePath);
if (LSPLocDecl && LSPLocDecl != LSPLocDef) {
ReferencesResult::Reference Result;
Result.Loc = std::move(*LSPLocDecl);
Result.Attributes =
ReferencesResult::Declaration | ReferencesResult::Override;
Results.References.push_back(std::move(Result));
}
if (LSPLocDef) {
ReferencesResult::Reference Result;
Result.Loc = std::move(*LSPLocDef);
Result.Attributes = ReferencesResult::Declaration |
ReferencesResult::Definition |
ReferencesResult::Override;
Results.References.push_back(std::move(Result));
}
});
}
}
// Now query the index for references from other files.
auto QueryIndex = [&](llvm::DenseSet<SymbolID> IDs, bool AllowAttributes,
bool AllowMainFileSymbols) {
if (IDs.empty() || !Index || Results.HasMore)
return;
RefsRequest Req;
Req.IDs = std::move(IDs);
Req.Limit = Limit;
if (Req.IDs.empty() || !Index || Results.References.size() > Limit)
return;
if (Limit) {
if (Limit < Results.References.size()) {
// We've already filled our quota, still check the index to correctly
// return the `HasMore` info.
Req.Limit = 0;
} else {
// Query index only for the remaining size.
Req.Limit = Limit - Results.References.size();
}
}
Results.HasMore |= Index->refs(Req, [&](const Ref &R) {
// No need to continue process if we reach the limit.
if (Results.References.size() > Limit)
return;
auto LSPLoc = toLSPLocation(R.Location, *MainFilePath);
// Avoid indexed results for the main file - the AST is authoritative.
if (!LSPLoc ||
@ -1495,17 +1498,13 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
Results.References.push_back(std::move(Result));
});
};
QueryIndex(std::move(IDs), /*AllowAttributes=*/true,
QueryIndex(std::move(IDsToQuery), /*AllowAttributes=*/true,
/*AllowMainFileSymbols=*/false);
// For a virtual method: Occurrences of BaseMethod should be treated as refs
// and not as decl/def. Allow symbols from main file since AST does not report
// these.
QueryIndex(std::move(OverriddenMethods), /*AllowAttributes=*/false,
/*AllowMainFileSymbols=*/true);
if (Results.References.size() > Limit) {
Results.HasMore = true;
Results.References.resize(Limit);
}
return Results;
}