kbobyrev added a comment.
Wow, nice work, thank you!
================
Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:51
+public:
+ // Returns the tokens which are given symbol's characteristics. Currently,
the
+ // generated tokens only contain fuzzy matching trigrams and symbol's scope,
----------------
No longer "returns": "adds/generates"?
================
Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:83
+ Result.clear();
+ if (Identifier.size() < 14) {
+ identifierTrigrams(Identifier, [&](Trigram T) {
----------------
It isn't immediately obvious why processing different id lengths is different.
I'm assuming it's less expensive to find duplicates in the small vector as we
go than to find the unique elements in the end in case of the short identifier,
but I'd be happy to have a comment here.
Also, `14` is a magic number:
* Move it to `Trigram::SMALL_IDENTIFIER_SIZE` or some other constant?
* How did you find this number? Did you try running benchmark with different
values and figured this one is the best for the LLVM index? If not, it would
make sense to do that.
================
Comment at: clang-tools-extra/clangd/index/dex/Trigram.h:49
+ std::string str() const { return std::string(Data.data(), Data[3]); }
+ uint32_t uint() const { return llvm::bit_cast<uint32_t>(Data); }
+ friend struct ::llvm::DenseMapInfo<Trigram>;
----------------
nit: maybe call it `UID` or something similar, "unsigned int" is somewhat
surprising in this context
================
Comment at: clang-tools-extra/clangd/index/dex/Trigram.h:55
+
/// Returns list of unique fuzzy-search trigrams from unqualified symbol.
/// The trigrams give the 3-character query substrings this symbol can match.
----------------
Also doesn't "return" anymore
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79918/new/
https://reviews.llvm.org/D79918
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
