[clang-tools-extra] r342138 - [clangd] Cleanup FuzzyFindRequest filtering limit semantics
Kirill Bobyrev via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 13 07:27:04 PDT 2018
Author: omtcyfz
Date: Thu Sep 13 07:27:03 2018
New Revision: 342138
URL: http://llvm.org/viewvc/llvm-project?rev=342138&view=rev
Log:
[clangd] Cleanup FuzzyFindRequest filtering limit semantics
As discussed during D51860 review, it is better to use `llvm::Optional`
here as it has clear semantics which reflect intended behavior.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D52028
Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/FindSymbols.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/MemIndex.cpp
clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
clang-tools-extra/trunk/test/clangd/Inputs/requests.json
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=342138&r1=342137&r2=342138&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu Sep 13 07:27:03 2018
@@ -1375,7 +1375,7 @@ private:
// Build the query.
FuzzyFindRequest Req;
if (Opts.Limit)
- Req.MaxCandidateCount = Opts.Limit;
+ Req.Limit = Opts.Limit;
Req.Query = Filter->pattern();
Req.RestrictForCodeCompletion = true;
Req.Scopes = QueryScopes;
Modified: clang-tools-extra/trunk/clangd/FindSymbols.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FindSymbols.cpp?rev=342138&r1=342137&r2=342138&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/FindSymbols.cpp (original)
+++ clang-tools-extra/trunk/clangd/FindSymbols.cpp Thu Sep 13 07:27:03 2018
@@ -117,8 +117,9 @@ getWorkspaceSymbols(StringRef Query, int
if (IsGlobalQuery || !Names.first.empty())
Req.Scopes = {Names.first};
if (Limit)
- Req.MaxCandidateCount = Limit;
- TopN<ScoredSymbolInfo, ScoredSymbolGreater> Top(Req.MaxCandidateCount);
+ Req.Limit = Limit;
+ TopN<ScoredSymbolInfo, ScoredSymbolGreater> Top(
+ Req.Limit ? *Req.Limit : std::numeric_limits<size_t>::max());
FuzzyMatcher Filter(Req.Query);
Index->fuzzyFind(Req, [HintPath, &Top, &Filter](const Symbol &Sym) {
// Prefer the definition over e.g. a function declaration in a header
Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=342138&r1=342137&r2=342138&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Thu Sep 13 07:27:03 2018
@@ -177,14 +177,14 @@ std::shared_ptr<SymbolIndex> SwapIndex::
bool fromJSON(const llvm::json::Value &Parameters, FuzzyFindRequest &Request) {
json::ObjectMapper O(Parameters);
- int64_t MaxCandidateCount;
+ int64_t Limit;
bool OK =
O && O.map("Query", Request.Query) && O.map("Scopes", Request.Scopes) &&
- O.map("MaxCandidateCount", MaxCandidateCount) &&
+ O.map("Limit", Limit) &&
O.map("RestrictForCodeCompletion", Request.RestrictForCodeCompletion) &&
O.map("ProximityPaths", Request.ProximityPaths);
- if (OK && MaxCandidateCount <= std::numeric_limits<uint32_t>::max())
- Request.MaxCandidateCount = MaxCandidateCount;
+ if (OK && Limit <= std::numeric_limits<uint32_t>::max())
+ Request.Limit = Limit;
return OK;
}
@@ -192,7 +192,7 @@ llvm::json::Value toJSON(const FuzzyFind
return json::Object{
{"Query", Request.Query},
{"Scopes", json::Array{Request.Scopes}},
- {"MaxCandidateCount", Request.MaxCandidateCount},
+ {"Limit", Request.Limit},
{"RestrictForCodeCompletion", Request.RestrictForCodeCompletion},
{"ProximityPaths", json::Array{Request.ProximityPaths}},
};
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=342138&r1=342137&r2=342138&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Thu Sep 13 07:27:03 2018
@@ -437,9 +437,7 @@ struct FuzzyFindRequest {
std::vector<std::string> Scopes;
/// \brief The number of top candidates to return. The index may choose to
/// return more than this, e.g. if it doesn't know which candidates are best.
- // FIXME: Use llvm::Optional; semantically, the absence of MaxCandidateCount
- // is equivalent to setting this field to default value as below.
- uint32_t MaxCandidateCount = std::numeric_limits<uint32_t>::max();
+ llvm::Optional<uint32_t> Limit;
/// If set to true, only symbols for completion support will be considered.
bool RestrictForCodeCompletion = false;
/// Contextually relevant files (e.g. the file we're code-completing in).
@@ -447,9 +445,9 @@ struct FuzzyFindRequest {
std::vector<std::string> ProximityPaths;
bool operator==(const FuzzyFindRequest &Req) const {
- return std::tie(Query, Scopes, MaxCandidateCount, RestrictForCodeCompletion,
+ return std::tie(Query, Scopes, Limit, RestrictForCodeCompletion,
ProximityPaths) ==
- std::tie(Req.Query, Req.Scopes, Req.MaxCandidateCount,
+ std::tie(Req.Query, Req.Scopes, Req.Limit,
Req.RestrictForCodeCompletion, Req.ProximityPaths);
}
bool operator!=(const FuzzyFindRequest &Req) const { return !(*this == Req); }
@@ -476,7 +474,7 @@ public:
/// each matched symbol before returning.
/// If returned Symbols are used outside Callback, they must be deep-copied!
///
- /// Returns true if there may be more results (limited by MaxCandidateCount).
+ /// Returns true if there may be more results (limited by Req.Limit).
virtual bool
fuzzyFind(const FuzzyFindRequest &Req,
llvm::function_ref<void(const Symbol &)> Callback) const = 0;
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=342138&r1=342137&r2=342138&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/MemIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp Thu Sep 13 07:27:03 2018
@@ -29,7 +29,8 @@ bool MemIndex::fuzzyFind(
assert(!StringRef(Req.Query).contains("::") &&
"There must be no :: in query.");
- TopN<std::pair<float, const Symbol *>> Top(Req.MaxCandidateCount);
+ TopN<std::pair<float, const Symbol *>> Top(
+ Req.Limit ? *Req.Limit : std::numeric_limits<size_t>::max());
FuzzyMatcher Filter(Req.Query);
bool More = false;
for (const auto Pair : Index) {
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=342138&r1=342137&r2=342138&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp Thu Sep 13 07:27:03 2018
@@ -179,8 +179,8 @@ bool Dex::fuzzyFind(const FuzzyFindReque
// FIXME(kbobyrev): Pre-scoring retrieval threshold should be adjusted as
// using 100x of the requested number might not be good in practice, e.g.
// when the requested number of items is small.
- const size_t ItemsToRetrieve = 100 * Req.MaxCandidateCount;
- auto Root = createLimit(move(QueryIterator), ItemsToRetrieve);
+ auto Root = Req.Limit ? createLimit(move(QueryIterator), *Req.Limit * 100)
+ : move(QueryIterator);
using IDAndScore = std::pair<DocID, float>;
std::vector<IDAndScore> IDAndScores = consume(*Root);
@@ -188,7 +188,8 @@ bool Dex::fuzzyFind(const FuzzyFindReque
auto Compare = [](const IDAndScore &LHS, const IDAndScore &RHS) {
return LHS.second > RHS.second;
};
- TopN<IDAndScore, decltype(Compare)> Top(Req.MaxCandidateCount, Compare);
+ TopN<IDAndScore, decltype(Compare)> Top(
+ Req.Limit ? *Req.Limit : std::numeric_limits<size_t>::max(), Compare);
for (const auto &IDAndScore : IDAndScores) {
const DocID SymbolDocID = IDAndScore.first;
const auto *Sym = Symbols[SymbolDocID];
@@ -205,7 +206,7 @@ bool Dex::fuzzyFind(const FuzzyFindReque
More = true;
}
- // Apply callback to the top Req.MaxCandidateCount items in the descending
+ // Apply callback to the top Req.Limit items in the descending
// order of cumulative score.
for (const auto &Item : std::move(Top).items())
Callback(*Symbols[Item.first]);
Modified: clang-tools-extra/trunk/test/clangd/Inputs/requests.json
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/Inputs/requests.json?rev=342138&r1=342137&r2=342138&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/Inputs/requests.json (original)
+++ clang-tools-extra/trunk/test/clangd/Inputs/requests.json Thu Sep 13 07:27:03 2018
@@ -1,7 +1,7 @@
-[{"MaxCandidateCount":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]},
-{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
-{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
-{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
-{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
-{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::", "clang::", "clang::clangd::dex::"]},
-{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":[""]}]
+[{"Limit":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]},
+{"Limit":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"Limit":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"Limit":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"Limit":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"Limit":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::", "clang::", "clang::clangd::dex::"]},
+{"Limit":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":[""]}]
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=342138&r1=342137&r2=342138&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/DexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DexTests.cpp Thu Sep 13 07:27:03 2018
@@ -482,7 +482,7 @@ TEST(DexTest, FuzzyMatchQ) {
URISchemes);
FuzzyFindRequest Req;
Req.Query = "lol";
- Req.MaxCandidateCount = 2;
+ Req.Limit = 2;
EXPECT_THAT(match(*I, Req),
UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
}
@@ -498,6 +498,7 @@ TEST(DexTest, DexDeduplicate) {
FuzzyFindRequest Req;
Req.Query = "2";
Dex I(Symbols, URISchemes);
+ EXPECT_FALSE(Req.Limit);
EXPECT_THAT(match(I, Req), ElementsAre("2", "2"));
}
@@ -505,10 +506,11 @@ TEST(DexTest, DexLimitedNumMatches) {
auto I = Dex::build(generateNumSymbols(0, 100), URISchemes);
FuzzyFindRequest Req;
Req.Query = "5";
- Req.MaxCandidateCount = 3;
+ Req.Limit = 3;
bool Incomplete;
auto Matches = match(*I, Req, &Incomplete);
- EXPECT_EQ(Matches.size(), Req.MaxCandidateCount);
+ EXPECT_TRUE(Req.Limit);
+ EXPECT_EQ(Matches.size(), *Req.Limit);
EXPECT_TRUE(Incomplete);
}
@@ -518,7 +520,7 @@ TEST(DexTest, FuzzyMatch) {
URISchemes);
FuzzyFindRequest Req;
Req.Query = "lol";
- Req.MaxCandidateCount = 2;
+ Req.Limit = 2;
EXPECT_THAT(match(*I, Req),
UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
}
@@ -596,7 +598,7 @@ TEST(DexTest, ProximityPathsBoosting) {
FuzzyFindRequest Req;
Req.Query = "abc";
// The best candidate can change depending on the proximity paths.
- Req.MaxCandidateCount = 1;
+ Req.Limit = 1;
// FuzzyFind request comes from the file which is far from the root: expect
// CloseSymbol to come out.
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=342138&r1=342137&r2=342138&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp Thu Sep 13 07:27:03 2018
@@ -78,10 +78,11 @@ TEST(MemIndexTest, MemIndexLimitedNumMat
auto I = MemIndex::build(generateNumSymbols(0, 100), RefSlab());
FuzzyFindRequest Req;
Req.Query = "5";
- Req.MaxCandidateCount = 3;
+ Req.Limit = 3;
bool Incomplete;
auto Matches = match(*I, Req, &Incomplete);
- EXPECT_EQ(Matches.size(), Req.MaxCandidateCount);
+ EXPECT_TRUE(Req.Limit);
+ EXPECT_EQ(Matches.size(), *Req.Limit);
EXPECT_TRUE(Incomplete);
}
@@ -91,7 +92,7 @@ TEST(MemIndexTest, FuzzyMatch) {
RefSlab());
FuzzyFindRequest Req;
Req.Query = "lol";
- Req.MaxCandidateCount = 2;
+ Req.Limit = 2;
EXPECT_THAT(match(*I, Req),
UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
}
More information about the cfe-commits
mailing list