This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rCTE343775: [cland] Dex: fix/simplify short-trigram generation
(authored by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52808?
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
> The problem is LSP clients are free to assume that the result list is
> complete (unless marked as incomplete) and therefore will never retrieve the
> better symbols.
Good point. Thanks for
sammccall updated this revision to Diff 168280.
sammccall added a comment.
Use similar but better-defined rules for short trigram matches.
Modify Dex to account for the matches not being exhaustive.
Unfortunately the test needs https://reviews.llvm.org/D52796, which depends on
this patch.
Repo
sammccall marked an inline comment as done.
sammccall added a comment.
**TL;DR**: i'm no longer convinced of my conclusions for short-query, look at
the proposal below.
In https://reviews.llvm.org/D52808#1254899, @ioeric wrote:
> > Generate more short-query trigrams, e.g. "AbcDefGhi" now yields
ioeric added a comment.
> Generate more short-query trigrams, e.g. "AbcDefGhi" now yields "d" and "ag".
I am concerned about the impact on the size of posting lists (we can measure)
and retrieval quality by adding more incomplete trigrams.
> This is effectively required by LSP, having "ag" not
sammccall updated this revision to Diff 168049.
sammccall added a comment.
Update comment, revert unintended change
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52808
Files:
clangd/index/dex/Trigram.cpp
clangd/index/dex/Trigram.h
unittests/clangd/DexTests.cpp
Index: un
sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, mgrang, jkorous,
ilya-biryukov.
1. Instead of a$$ for a short-query trigram, just use a
2. Generate more short-query trigrams, e.g. "AbcDefGhi" now yields "d" and
"ag".
