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

Marc-Andre Laperle via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 26 07:16:40 PDT 2018


malaperle added a comment.

In https://reviews.llvm.org/D44882#1048043, @ilya-biryukov wrote:

> - 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.


I'm a bit surprised that internally you do not want symbols beyond the ones for completion. We have a lot more features in mind that will make the index much bigger, like adding all occurrences (maybe not in the current YAML though). But having options to control the amount of information indexed sounds good to me as there can be a lot of different project sizes and there can be different tradeoffs. I had in mind to an option for including/excluding the occurrences because without them, you lose workspace/references, call hierarchy, etc, but you still have code completion and workspace/symbol, document/symbol, etc while making the index much smaller. But more options sounds good.

> - 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).

I'm not sure, I'm thinking there should be a balance between how fuzzy it it and how much noise it generates. Right now I find it a bit too fuzzy since when I type "Draft" (to find DraftMgr), it picks up things like DocumentRangeFormattingParams. Adding fuzziness to the namespace would make this worse. Maybe with improved scoring it won't matter too much? I'll try FuzzyMatcher and see.

> - 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`.

I haven't thought of that and I think it's a good idea. Sounds like a good, isolated thing to do in a separate patch.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882





More information about the cfe-commits mailing list