[clang-tools-extra] r351081 - [clangd] Add Limit parameter for xref.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 14 10:11:10 PST 2019
Author: hokein
Date: Mon Jan 14 10:11:09 2019
New Revision: 351081
URL: http://llvm.org/viewvc/llvm-project?rev=351081&view=rev
Log:
[clangd] Add Limit parameter for xref.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D56597
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/XRefs.h
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/MemIndex.cpp
clang-tools-extra/trunk/clangd/index/Merge.cpp
clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=351081&r1=351080&r2=351081&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Mon Jan 14 10:11:09 2019
@@ -710,7 +710,7 @@ void ClangdLSPServer::onChangeConfigurat
void ClangdLSPServer::onReference(const ReferenceParams &Params,
Callback<std::vector<Location>> Reply) {
Server->findReferences(Params.textDocument.uri.file(), Params.position,
- std::move(Reply));
+ CCOpts.Limit, std::move(Reply));
}
void ClangdLSPServer::onSymbolInfo(const TextDocumentPositionParams &Params,
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=351081&r1=351080&r2=351081&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Jan 14 10:11:09 2019
@@ -500,13 +500,13 @@ void ClangdServer::documentSymbols(llvm:
Bind(Action, std::move(CB)));
}
-void ClangdServer::findReferences(PathRef File, Position Pos,
+void ClangdServer::findReferences(PathRef File, Position Pos, uint32_t Limit,
Callback<std::vector<Location>> CB) {
- auto Action = [Pos, this](Callback<std::vector<Location>> CB,
- llvm::Expected<InputsAndAST> InpAST) {
+ auto Action = [Pos, Limit, this](Callback<std::vector<Location>> CB,
+ llvm::Expected<InputsAndAST> InpAST) {
if (!InpAST)
return CB(InpAST.takeError());
- CB(clangd::findReferences(InpAST->AST, Pos, Index));
+ CB(clangd::findReferences(InpAST->AST, Pos, Limit, Index));
};
WorkScheduler.runWithAST("References", File, Bind(Action, std::move(CB)));
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=351081&r1=351080&r2=351081&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Mon Jan 14 10:11:09 2019
@@ -181,7 +181,7 @@ public:
Callback<std::vector<DocumentSymbol>> CB);
/// Retrieve locations for symbol references.
- void findReferences(PathRef File, Position Pos,
+ void findReferences(PathRef File, Position Pos, uint32_t Limit,
Callback<std::vector<Location>> CB);
/// Run formatting for \p Rng inside \p File with content \p Code.
Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=351081&r1=351080&r2=351081&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Mon Jan 14 10:11:09 2019
@@ -704,7 +704,9 @@ llvm::Optional<Hover> getHover(ParsedAST
}
std::vector<Location> findReferences(ParsedAST &AST, Position Pos,
- const SymbolIndex *Index) {
+ uint32_t Limit, const SymbolIndex *Index) {
+ if (!Limit)
+ Limit = std::numeric_limits<uint32_t>::max();
std::vector<Location> Results;
const SourceManager &SM = AST.getASTContext().getSourceManager();
auto MainFilePath =
@@ -733,26 +735,30 @@ std::vector<Location> findReferences(Par
}
// Now query the index for references from other files.
- if (!Index)
- return Results;
- RefsRequest Req;
- for (const Decl *D : TargetDecls) {
- // 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))
- Req.IDs.insert(*ID);
+ if (Index && Results.size() < Limit) {
+ RefsRequest Req;
+ Req.Limit = Limit;
+
+ for (const Decl *D : TargetDecls) {
+ // 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))
+ Req.IDs.insert(*ID);
+ }
+ if (Req.IDs.empty())
+ return Results;
+ Index->refs(Req, [&](const Ref &R) {
+ auto LSPLoc = toLSPLocation(R.Location, *MainFilePath);
+ // Avoid indexed results for the main file - the AST is authoritative.
+ if (LSPLoc && LSPLoc->uri.file() != *MainFilePath)
+ Results.push_back(std::move(*LSPLoc));
+ });
}
- if (Req.IDs.empty())
- return Results;
- Index->refs(Req, [&](const Ref &R) {
- auto LSPLoc = toLSPLocation(R.Location, *MainFilePath);
- // Avoid indexed results for the main file - the AST is authoritative.
- if (LSPLoc && LSPLoc->uri.file() != *MainFilePath)
- Results.push_back(std::move(*LSPLoc));
- });
+ if (Results.size() > Limit)
+ Results.resize(Limit);
return Results;
}
Modified: clang-tools-extra/trunk/clangd/XRefs.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.h?rev=351081&r1=351080&r2=351081&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.h (original)
+++ clang-tools-extra/trunk/clangd/XRefs.h Mon Jan 14 10:11:09 2019
@@ -35,7 +35,9 @@ std::vector<DocumentHighlight> findDocum
llvm::Optional<Hover> getHover(ParsedAST &AST, Position Pos);
/// Returns reference locations of the symbol at a specified \p Pos.
+/// \p Limit limits the number of results returned (0 means no limit).
std::vector<Location> findReferences(ParsedAST &AST, Position Pos,
+ uint32_t Limit,
const SymbolIndex *Index = nullptr);
/// Get info about symbols at \p Pos.
Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=351081&r1=351080&r2=351081&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Mon Jan 14 10:11:09 2019
@@ -476,6 +476,10 @@ struct LookupRequest {
struct RefsRequest {
llvm::DenseSet<SymbolID> IDs;
RefKind Filter = RefKind::All;
+ /// If set, limit the number of refers returned from the index. The index may
+ /// choose to return less than this, e.g. it tries to avoid returning stale
+ /// results.
+ llvm::Optional<uint32_t> Limit;
};
/// Interface for symbol indexes that can be used for searching or
Modified: clang-tools-extra/trunk/clangd/index/MemIndex.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/MemIndex.cpp?rev=351081&r1=351080&r2=351081&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/MemIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp Mon Jan 14 10:11:09 2019
@@ -69,13 +69,18 @@ void MemIndex::lookup(const LookupReques
void MemIndex::refs(const RefsRequest &Req,
llvm::function_ref<void(const Ref &)> Callback) const {
trace::Span Tracer("MemIndex refs");
+ uint32_t Remaining =
+ Req.Limit.getValueOr(std::numeric_limits<uint32_t>::max());
for (const auto &ReqID : Req.IDs) {
auto SymRefs = Refs.find(ReqID);
if (SymRefs == Refs.end())
continue;
- for (const auto &O : SymRefs->second)
- if (static_cast<int>(Req.Filter & O.Kind))
+ for (const auto &O : SymRefs->second) {
+ if (Remaining > 0 && static_cast<int>(Req.Filter & O.Kind)) {
+ --Remaining;
Callback(O);
+ }
+ }
}
}
Modified: clang-tools-extra/trunk/clangd/index/Merge.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Merge.cpp?rev=351081&r1=351080&r2=351081&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Merge.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Merge.cpp Mon Jan 14 10:11:09 2019
@@ -87,6 +87,8 @@ void MergedIndex::lookup(
void MergedIndex::refs(const RefsRequest &Req,
llvm::function_ref<void(const Ref &)> Callback) const {
trace::Span Tracer("MergedIndex refs");
+ uint32_t Remaining =
+ Req.Limit.getValueOr(std::numeric_limits<uint32_t>::max());
// We don't want duplicated refs from the static/dynamic indexes,
// and we can't reliably duplicate them because offsets may differ slightly.
// We consider the dynamic index authoritative and report all its refs,
@@ -99,10 +101,18 @@ void MergedIndex::refs(const RefsRequest
Dynamic->refs(Req, [&](const Ref &O) {
DynamicIndexFileURIs.insert(O.Location.FileURI);
Callback(O);
+ --Remaining;
});
+ assert(Remaining >= 0);
+ if (Remaining == 0)
+ return;
+ // We return less than Req.Limit if static index returns more refs for dirty
+ // files.
Static->refs(Req, [&](const Ref &O) {
- if (!DynamicIndexFileURIs.count(O.Location.FileURI))
+ if (Remaining > 0 && !DynamicIndexFileURIs.count(O.Location.FileURI)) {
+ --Remaining;
Callback(O);
+ }
});
}
Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.cpp?rev=351081&r1=351080&r2=351081&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp Mon Jan 14 10:11:09 2019
@@ -236,10 +236,15 @@ void Dex::lookup(const LookupRequest &Re
void Dex::refs(const RefsRequest &Req,
llvm::function_ref<void(const Ref &)> Callback) const {
trace::Span Tracer("Dex refs");
+ uint32_t Remaining =
+ Req.Limit.getValueOr(std::numeric_limits<uint32_t>::max());
for (const auto &ID : Req.IDs)
- for (const auto &Ref : Refs.lookup(ID))
- if (static_cast<int>(Req.Filter & Ref.Kind))
+ for (const auto &Ref : Refs.lookup(ID)) {
+ if (Remaining > 0 && static_cast<int>(Req.Filter & Ref.Kind)) {
+ --Remaining;
Callback(Ref);
+ }
+ }
}
size_t Dex::estimateMemoryUsage() const {
Modified: clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DexTests.cpp?rev=351081&r1=351080&r2=351081&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/DexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DexTests.cpp Mon Jan 14 10:11:09 2019
@@ -667,18 +667,26 @@ TEST(DexTests, Refs) {
auto Foo = symbol("foo");
auto Bar = symbol("bar");
AddRef(Foo, "foo.h", RefKind::Declaration);
+ AddRef(Foo, "foo.cc", RefKind::Definition);
AddRef(Foo, "reffoo.h", RefKind::Reference);
AddRef(Bar, "bar.h", RefKind::Declaration);
- std::vector<std::string> Files;
RefsRequest Req;
Req.IDs.insert(Foo.ID);
Req.Filter = RefKind::Declaration | RefKind::Definition;
+
+ std::vector<std::string> Files;
Dex(std::vector<Symbol>{Foo, Bar}, Refs).refs(Req, [&](const Ref &R) {
Files.push_back(R.Location.FileURI);
});
+ EXPECT_THAT(Files, UnorderedElementsAre("foo.h", "foo.cc"));
- EXPECT_THAT(Files, ElementsAre("foo.h"));
+ Req.Limit = 1;
+ Files.clear();
+ Dex(std::vector<Symbol>{Foo, Bar}, Refs).refs(Req, [&](const Ref &R) {
+ Files.push_back(R.Location.FileURI);
+ });
+ EXPECT_THAT(Files, ElementsAre(AnyOf("foo.h", "foo.cc")));
}
} // namespace
Modified: clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp?rev=351081&r1=351080&r2=351081&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp Mon Jan 14 10:11:09 2019
@@ -19,6 +19,7 @@
using testing::_;
using testing::AllOf;
+using testing::AnyOf;
using testing::ElementsAre;
using testing::Pair;
using testing::Pointee;
@@ -292,7 +293,6 @@ TEST(MergeIndexTest, Refs) {
Request.IDs = {Foo.ID};
RefSlab::Builder Results;
Merge.refs(Request, [&](const Ref &O) { Results.insert(Foo.ID, O); });
-
EXPECT_THAT(
std::move(Results).build(),
ElementsAre(Pair(
@@ -300,6 +300,14 @@ TEST(MergeIndexTest, Refs) {
FileURI("unittest:///test.cc")),
AllOf(RefRange(Test2Code.range("Foo")),
FileURI("unittest:///test2.cc"))))));
+
+ Request.Limit = 1;
+ RefSlab::Builder Results2;
+ Merge.refs(Request, [&](const Ref &O) { Results2.insert(Foo.ID, O); });
+ EXPECT_THAT(std::move(Results2).build(),
+ ElementsAre(Pair(
+ _, ElementsAre(AnyOf(FileURI("unittest:///test.cc"),
+ FileURI("unittest:///test2.cc"))))));
}
MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=351081&r1=351080&r2=351081&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Mon Jan 14 10:11:09 2019
@@ -1219,7 +1219,7 @@ TEST(FindReferences, WithinAST) {
std::vector<Matcher<Location>> ExpectedLocations;
for (const auto &R : T.ranges())
ExpectedLocations.push_back(RangeIs(R));
- EXPECT_THAT(findReferences(AST, T.point()),
+ EXPECT_THAT(findReferences(AST, T.point(), 0),
ElementsAreArray(ExpectedLocations))
<< Test;
}
@@ -1266,7 +1266,7 @@ TEST(FindReferences, ExplicitSymbols) {
std::vector<Matcher<Location>> ExpectedLocations;
for (const auto &R : T.ranges())
ExpectedLocations.push_back(RangeIs(R));
- EXPECT_THAT(findReferences(AST, T.point()),
+ EXPECT_THAT(findReferences(AST, T.point(), 0),
ElementsAreArray(ExpectedLocations))
<< Test;
}
@@ -1281,7 +1281,7 @@ TEST(FindReferences, NeedsIndex) {
auto AST = TU.build();
// References in main file are returned without index.
- EXPECT_THAT(findReferences(AST, Main.point(), /*Index=*/nullptr),
+ EXPECT_THAT(findReferences(AST, Main.point(), 0, /*Index=*/nullptr),
ElementsAre(RangeIs(Main.range())));
Annotations IndexedMain(R"cpp(
int main() { [[f^oo]](); }
@@ -1292,13 +1292,17 @@ TEST(FindReferences, NeedsIndex) {
IndexedTU.Code = IndexedMain.code();
IndexedTU.Filename = "Indexed.cpp";
IndexedTU.HeaderCode = Header;
- EXPECT_THAT(findReferences(AST, Main.point(), IndexedTU.index().get()),
+ EXPECT_THAT(findReferences(AST, Main.point(), 0, IndexedTU.index().get()),
ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range())));
+ EXPECT_EQ(1u, findReferences(AST, Main.point(), /*Limit*/ 1,
+ IndexedTU.index().get())
+ .size());
+
// If the main file is in the index, we don't return duplicates.
// (even if the references are in a different location)
TU.Code = ("\n\n" + Main.code()).str();
- EXPECT_THAT(findReferences(AST, Main.point(), TU.index().get()),
+ EXPECT_THAT(findReferences(AST, Main.point(), 0, TU.index().get()),
ElementsAre(RangeIs(Main.range())));
}
@@ -1328,7 +1332,7 @@ TEST(FindReferences, NoQueryForLocalSymb
Annotations File(T.AnnotatedCode);
RecordingIndex Rec;
auto AST = TestTU::withCode(File.code()).build();
- findReferences(AST, File.point(), &Rec);
+ findReferences(AST, File.point(), 0, &Rec);
if (T.WantQuery)
EXPECT_NE(Rec.RefIDs, None) << T.AnnotatedCode;
else
More information about the cfe-commits
mailing list