[PATCH] D44882: [clangd] Implementation of workspace/symbol request

[Ilya Biryukov via Phabricator via cfe-commits]

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