[clang-tools-extra] r343775 - [cland] Dex: fix/simplify short-trigram generation
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 4 07:01:55 PDT 2018
Author: sammccall
Date: Thu Oct 4 07:01:55 2018
New Revision: 343775
URL: http://llvm.org/viewvc/llvm-project?rev=343775&view=rev
Log:
[cland] Dex: fix/simplify short-trigram generation
Summary:
1) Instead of x$$ for a short-query trigram, just use x
2) Make rules more coherent: prefixes of length 1-2, and first char + next head
3) Fix Dex::fuzzyFind to mark results as incomplete, because
short-trigram rules only yield a subset of results.
Reviewers: ioeric
Subscribers: ilya-biryukov, jkorous, mgrang, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52808
Modified:
clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
clang-tools-extra/trunk/clangd/index/dex/Trigram.h
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
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=343775&r1=343774&r2=343775&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp Thu Oct 4 07:01:55 2018
@@ -156,7 +156,9 @@ bool Dex::fuzzyFind(const FuzzyFindReque
"There must be no :: in query.");
trace::Span Tracer("Dex fuzzyFind");
FuzzyMatcher Filter(Req.Query);
- bool More = false;
+ // For short queries we use specialized trigrams that don't yield all results.
+ // Prevent clients from postfiltering them for longer queries.
+ bool More = !Req.Query.empty() && Req.Query.size() < 3;
std::vector<std::unique_ptr<Iterator>> TopLevelChildren;
const auto TrigramTokens = generateQueryTrigrams(Req.Query);
Modified: clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp?rev=343775&r1=343774&r2=343775&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp Thu Oct 4 07:01:55 2018
@@ -23,11 +23,6 @@ namespace clang {
namespace clangd {
namespace dex {
-/// This is used to mark unigrams and bigrams and distinct them from complete
-/// trigrams. Since '$' is not present in valid identifier names, it is safe to
-/// use it as the special symbol.
-static const char END_MARKER = '$';
-
std::vector<Token> generateIdentifierTrigrams(llvm::StringRef Identifier) {
// Apply fuzzy matching text segmentation.
std::vector<CharRole> Roles(Identifier.size());
@@ -47,40 +42,22 @@ std::vector<Token> generateIdentifierTri
// not available then 0 is stored.
std::vector<std::array<unsigned, 3>> Next(LowercaseIdentifier.size());
unsigned NextTail = 0, NextHead = 0, NextNextHead = 0;
- // Store two first HEAD characters in the identifier (if present).
- std::deque<char> TwoHeads;
for (int I = LowercaseIdentifier.size() - 1; I >= 0; --I) {
Next[I] = {{NextTail, NextHead, NextNextHead}};
NextTail = Roles[I] == Tail ? I : 0;
if (Roles[I] == Head) {
NextNextHead = NextHead;
NextHead = I;
- TwoHeads.push_front(LowercaseIdentifier[I]);
- if (TwoHeads.size() > 2)
- TwoHeads.pop_back();
}
}
DenseSet<Token> UniqueTrigrams;
- auto add = [&](std::string Chars) {
+ auto Add = [&](std::string Chars) {
UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
};
- if (TwoHeads.size() == 2)
- add({{TwoHeads.front(), TwoHeads.back(), END_MARKER}});
-
- if (!LowercaseIdentifier.empty())
- add({{LowercaseIdentifier.front(), END_MARKER, END_MARKER}});
-
- if (LowercaseIdentifier.size() >= 2)
- add({{LowercaseIdentifier[0], LowercaseIdentifier[1], END_MARKER}});
-
- if (LowercaseIdentifier.size() >= 3)
- add({{LowercaseIdentifier[0], LowercaseIdentifier[1],
- LowercaseIdentifier[2]}});
-
- // Iterate through valid seqneces of three characters Fuzzy Matcher can
+ // Iterate through valid sequneces of three characters Fuzzy Matcher can
// process.
for (size_t I = 0; I < LowercaseIdentifier.size(); ++I) {
// Skip delimiters.
@@ -92,66 +69,47 @@ std::vector<Token> generateIdentifierTri
for (const unsigned K : Next[J]) {
if (K == 0)
continue;
- add({{LowercaseIdentifier[I], LowercaseIdentifier[J],
+ Add({{LowercaseIdentifier[I], LowercaseIdentifier[J],
LowercaseIdentifier[K]}});
}
}
}
+ // Emit short-query trigrams: FooBar -> f, fo, fb.
+ if (!LowercaseIdentifier.empty())
+ Add({LowercaseIdentifier[0]});
+ if (LowercaseIdentifier.size() >= 2)
+ Add({LowercaseIdentifier[0], LowercaseIdentifier[1]});
+ for (size_t I = 1; I < LowercaseIdentifier.size(); ++I)
+ if (Roles[I] == Head) {
+ Add({LowercaseIdentifier[0], LowercaseIdentifier[I]});
+ break;
+ }
- std::vector<Token> Result;
- for (const auto &Trigram : UniqueTrigrams)
- Result.push_back(Trigram);
-
- return Result;
+ return {UniqueTrigrams.begin(), UniqueTrigrams.end()};
}
std::vector<Token> generateQueryTrigrams(llvm::StringRef Query) {
+ std::string LowercaseQuery = Query.lower();
+ if (Query.size() < 3) // short-query trigrams only
+ return {Token(Token::Kind::Trigram, LowercaseQuery)};
+
// Apply fuzzy matching text segmentation.
std::vector<CharRole> Roles(Query.size());
calculateRoles(Query, llvm::makeMutableArrayRef(Roles.data(), Query.size()));
- // Additional pass is necessary to count valid identifier characters.
- // Depending on that, this function might return incomplete trigram.
- unsigned ValidSymbolsCount = 0;
- for (const auto Role : Roles)
- if (Role == Head || Role == Tail)
- ++ValidSymbolsCount;
-
- std::string LowercaseQuery = Query.lower();
-
DenseSet<Token> UniqueTrigrams;
-
- // If the number of symbols which can form fuzzy matching trigram is not
- // sufficient, generate a single incomplete trigram for query.
- if (ValidSymbolsCount < 3) {
- std::string Chars =
- LowercaseQuery.substr(0, std::min<size_t>(3UL, Query.size()));
- Chars.append(3 - Chars.size(), END_MARKER);
- UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
- } else {
- std::deque<char> Chars;
- for (size_t I = 0; I < LowercaseQuery.size(); ++I) {
- // If current symbol is delimiter, just skip it.
- if (Roles[I] != Head && Roles[I] != Tail)
- continue;
-
- Chars.push_back(LowercaseQuery[I]);
-
- if (Chars.size() > 3)
- Chars.pop_front();
-
- if (Chars.size() == 3) {
- UniqueTrigrams.insert(
- Token(Token::Kind::Trigram, std::string(begin(Chars), end(Chars))));
- }
- }
+ std::string Chars;
+ for (unsigned I = 0; I < Query.size(); ++I) {
+ if (Roles[I] != Head && Roles[I] != Tail)
+ continue; // Skip delimiters.
+ Chars.push_back(LowercaseQuery[I]);
+ if (Chars.size() > 3)
+ Chars.erase(Chars.begin());
+ if (Chars.size() == 3)
+ UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
}
- std::vector<Token> Result;
- for (const auto &Trigram : UniqueTrigrams)
- Result.push_back(Trigram);
-
- return Result;
+ return {UniqueTrigrams.begin(), UniqueTrigrams.end()};
}
} // namespace dex
Modified: clang-tools-extra/trunk/clangd/index/dex/Trigram.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Trigram.h?rev=343775&r1=343774&r2=343775&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/dex/Trigram.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Trigram.h Thu Oct 4 07:01:55 2018
@@ -33,26 +33,21 @@ namespace clangd {
namespace dex {
/// Returns list of unique fuzzy-search trigrams from unqualified symbol.
+/// The trigrams give the 3-character query substrings this symbol can match.
///
-/// First, given Identifier (unqualified symbol name) is segmented using
-/// FuzzyMatch API and lowercased. After segmentation, the following technique
-/// is applied for generating trigrams: for each letter or digit in the input
-/// string the algorithms looks for the possible next and skip-1-next characters
-/// which can be jumped to during fuzzy matching. Each combination of such three
-/// characters is inserted into the result.
-///
+/// The symbol's name is broken into segments, e.g. "FooBar" has two segments.
/// Trigrams can start at any character in the input. Then we can choose to move
-/// to the next character, move to the start of the next segment, or skip over a
-/// segment.
+/// to the next character, move to the start of the next segment, or stop.
+///
+/// Short trigrams (length 1-2) are used for short queries. These are:
+/// - prefixes of the identifier, of length 1 and 2
+/// - the first character + next head character
+///
+/// For "FooBar" we get the following trigrams:
+/// {f, fo, fb, foo, fob, fba, oob, oba, bar}.
///
-/// This also generates incomplete trigrams for short query scenarios:
-/// * Empty trigram: "$$$".
-/// * Unigram: the first character of the identifier.
-/// * Bigrams: a 2-char prefix of the identifier and a bigram of the first two
-/// HEAD characters (if they exist).
-//
-/// Note: the returned list of trigrams does not have duplicates, if any trigram
-/// belongs to more than one class it is only inserted once.
+/// Trigrams are lowercase, as trigram matching is case-insensitive.
+/// Trigrams in the returned list are deduplicated.
std::vector<Token> generateIdentifierTrigrams(llvm::StringRef Identifier);
/// Returns list of unique fuzzy-search trigrams given a query.
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=343775&r1=343774&r2=343775&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/DexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DexTests.cpp Thu Oct 4 07:01:55 2018
@@ -367,53 +367,54 @@ trigramsAre(std::initializer_list<std::s
TEST(DexTrigrams, IdentifierTrigrams) {
EXPECT_THAT(generateIdentifierTrigrams("X86"),
- trigramsAre({"x86", "x$$", "x8$"}));
+ trigramsAre({"x86", "x", "x8"}));
- EXPECT_THAT(generateIdentifierTrigrams("nl"), trigramsAre({"nl$", "n$$"}));
+ EXPECT_THAT(generateIdentifierTrigrams("nl"), trigramsAre({"nl", "n"}));
- EXPECT_THAT(generateIdentifierTrigrams("n"), trigramsAre({"n$$"}));
+ EXPECT_THAT(generateIdentifierTrigrams("n"), trigramsAre({"n"}));
EXPECT_THAT(generateIdentifierTrigrams("clangd"),
- trigramsAre({"c$$", "cl$", "cla", "lan", "ang", "ngd"}));
+ trigramsAre({"c", "cl", "cla", "lan", "ang", "ngd"}));
EXPECT_THAT(generateIdentifierTrigrams("abc_def"),
- trigramsAre({"a$$", "abc", "abd", "ade", "bcd", "bde", "cde",
- "def", "ab$", "ad$"}));
+ trigramsAre({"a", "ab", "ad", "abc", "abd", "ade", "bcd", "bde",
+ "cde", "def"}));
EXPECT_THAT(generateIdentifierTrigrams("a_b_c_d_e_"),
- trigramsAre({"a$$", "a_$", "a_b", "abc", "abd", "acd", "ace",
- "bcd", "bce", "bde", "cde", "ab$"}));
+ trigramsAre({"a", "a_", "ab", "abc", "abd", "acd", "ace", "bcd",
+ "bce", "bde", "cde"}));
EXPECT_THAT(generateIdentifierTrigrams("unique_ptr"),
- trigramsAre({"u$$", "uni", "unp", "upt", "niq", "nip", "npt",
- "iqu", "iqp", "ipt", "que", "qup", "qpt", "uep",
- "ept", "ptr", "un$", "up$"}));
+ trigramsAre({"u", "un", "up", "uni", "unp", "upt", "niq", "nip",
+ "npt", "iqu", "iqp", "ipt", "que", "qup", "qpt",
+ "uep", "ept", "ptr"}));
EXPECT_THAT(
generateIdentifierTrigrams("TUDecl"),
- trigramsAre({"t$$", "tud", "tde", "ude", "dec", "ecl", "tu$", "td$"}));
+ trigramsAre({"t", "tu", "td", "tud", "tde", "ude", "dec", "ecl"}));
EXPECT_THAT(generateIdentifierTrigrams("IsOK"),
- trigramsAre({"i$$", "iso", "iok", "sok", "is$", "io$"}));
+ trigramsAre({"i", "is", "io", "iso", "iok", "sok"}));
+ auto X = generateIdentifierTrigrams("abc_defGhij__klm");
EXPECT_THAT(
generateIdentifierTrigrams("abc_defGhij__klm"),
- trigramsAre({"a$$", "abc", "abd", "abg", "ade", "adg", "adk", "agh",
- "agk", "bcd", "bcg", "bde", "bdg", "bdk", "bgh", "bgk",
- "cde", "cdg", "cdk", "cgh", "cgk", "def", "deg", "dek",
- "dgh", "dgk", "dkl", "efg", "efk", "egh", "egk", "ekl",
- "fgh", "fgk", "fkl", "ghi", "ghk", "gkl", "hij", "hik",
- "hkl", "ijk", "ikl", "jkl", "klm", "ab$", "ad$"}));
+ trigramsAre({"a", "ab", "ad", "abc", "abd", "abg", "ade", "adg",
+ "adk", "agh", "agk", "bcd", "bcg", "bde", "bdg", "bdk",
+ "bgh", "bgk", "cde", "cdg", "cdk", "cgh", "cgk", "def",
+ "deg", "dek", "dgh", "dgk", "dkl", "efg", "efk", "egh",
+ "egk", "ekl", "fgh", "fgk", "fkl", "ghi", "ghk", "gkl",
+ "hij", "hik", "hkl", "ijk", "ikl", "jkl", "klm"}));
}
TEST(DexTrigrams, QueryTrigrams) {
- EXPECT_THAT(generateQueryTrigrams("c"), trigramsAre({"c$$"}));
- EXPECT_THAT(generateQueryTrigrams("cl"), trigramsAre({"cl$"}));
+ EXPECT_THAT(generateQueryTrigrams("c"), trigramsAre({"c"}));
+ EXPECT_THAT(generateQueryTrigrams("cl"), trigramsAre({"cl"}));
EXPECT_THAT(generateQueryTrigrams("cla"), trigramsAre({"cla"}));
- EXPECT_THAT(generateQueryTrigrams("_"), trigramsAre({"_$$"}));
- EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__$"}));
- EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({"___"}));
+ EXPECT_THAT(generateQueryTrigrams("_"), trigramsAre({"_"}));
+ EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__"}));
+ EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({}));
EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
@@ -529,6 +530,31 @@ TEST(DexTest, FuzzyMatch) {
UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
}
+// TODO(sammccall): enable after D52796 bugfix.
+#if 0
+TEST(DexTest, ShortQuery) {
+ auto I =
+ Dex::build(generateSymbols({"OneTwoThreeFour"}), RefSlab(), URISchemes);
+ FuzzyFindRequest Req;
+ bool Incomplete;
+
+ EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre("OneTwoThreeFour"));
+ EXPECT_FALSE(Incomplete) << "Empty string is not a short query";
+
+ Req.Query = "t";
+ EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre());
+ EXPECT_TRUE(Incomplete) << "Short queries have different semantics";
+
+ Req.Query = "tt";
+ EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre());
+ EXPECT_TRUE(Incomplete) << "Short queries have different semantics";
+
+ Req.Query = "ttf";
+ EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre("OneTwoThreeFour"));
+ EXPECT_FALSE(Incomplete) << "3-char string is not a short query";
+}
+#endif
+
TEST(DexTest, MatchQualifiedNamesWithoutSpecificScope) {
auto I = Dex::build(generateSymbols({"a::y1", "b::y2", "y3"}), RefSlab(),
URISchemes);
More information about the cfe-commits
mailing list