[PATCH] D40548: [clangd] Symbol index interfaces and an in-memory index implementation.

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.


================
Comment at: clangd/index/Index.h:134
+  virtual bool
+  fuzzyFind(Context &Ctx, const FuzzyFindRequest &Req,
+            std::function<void(const Symbol &)> Callback) const = 0;
----------------
ioeric wrote:
> malaperle wrote:
> > I think a more generic std::function would be useful, similar to the indexstore's filter
> > ```
> > bool searchSymbols(llvm::function_ref<bool(IndexRecordSymbol, bool &stop)> filter,
> >                      llvm::function_ref<void(IndexRecordSymbol)> receiver)
> > ```
> Do you have an use case in mind which requires different filtering? This could probably be a separate interface. 
> 
> I think the behavior of fuzzy finding is well-defined, and how filtering is done is an implementation detail. User-specified filter might make implementation less flexible, especially for large indexes that are not managed in memory. 
For example
- Searching by USR
- By non-qualified name (useful also for Open Workspace Symbol)
- Most likely other filters, hmm, by Symbol kind, language?
- No filter, i.e. retrieve *all* symbols. Useful mainly for development and to get index statistics

There could also be a boolean in the callback return value (or the filter callback) to limit the number of returned values.
I haven't tried locally but it looks like it would be pretty doable to change the FuzzyFindRequest into a std::function.
A big disadvantage of having just one method though is that is would be difficult for an implementation to optimize for a specific type of query. For example, if you search by non-qualified name, an implementation could use a mapping of non-qualified-name-to-USR but it would be difficult to know what to do given only the std::function/filter passed by argument.
In that sense, perhaps it is better to have multiple methods for each use cases. Or perhaps some enum for each kind of query. What do you think?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40548