ilya-biryukov added reviewers: sammccall, ilya-biryukov.
ilya-biryukov added a subscriber: sammccall.
ilya-biryukov added a comment.
Adding @sammccall, he will definitely want to take a look at index-related
changes.
On a high level, the approach seems just right.
A few questions immedieately that came to mind:
- Unconditionally storing much more symbols in the index might have subtle
performance implications, especially for our internal use (the codebase is
huge). I bet that internally we wouldn't want to store the symbols not needed
for completion, so we'll probably need a switch to disable storing them in the
indexing implementation. But let's wait for Sam to take a look, he certainly
has a better perspective on the issues there.
- Current `fuzzyFind` implementation can only match qualifiers strictly (i.e.
st::vector won't match std::vector). Should we look into allowing fuzzy matches
for this feature? (not in this patch, rather in the long term).
- Have you considered also allowing `'.'` and `' '` (space) as separators in
the request? Having additional separators doesn't really hurt complexity of the
implementation, but allows to switch between tools for different languages
easier.
E.g., `std.vector.push_back` and `std vector push_back` could produce the same
matches as `std::vector::push_back`.
================
Comment at: clangd/ClangdServer.h:163
+ StringRef Query, const clangd::WorkspaceSymbolOptions &Opts,
+ UniqueFunction<void(llvm::Expected<std::vector<SymbolInformation>>)>
+ Callback);
----------------
NIT: use `Callback` typedef.
================
Comment at: clangd/WorkspaceSymbols.cpp:165
+ [](const SymbolInformation &A, const SymbolInformation &B) {
+ return A.name < B.name;
+ });
----------------
We have `FuzzyMatcher`, it can produce decent match scores and is already used
in completion.
Any reason not to use it here?
================
Comment at: clangd/index/Index.h:141
unsigned References = 0;
+ // Whether or not the symbol should be considered for completion.
----------------
NIT: remove empty lines to be consistent with the rest of the struct.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44882
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
