[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: clangd/index/IndexAction.h:33
+    std::function<void(RefSlab)> RefsCallback,
+    std::function<void(FileDigests)> FileDigestsCallback);
 
----------------
sammccall wrote:
> thinking about what we eventually want here:
>  - the index action needs to tell the auto-indexer what the file content or digests are
>  - the auto-indexer needs to tell the index action which files to index symbols from
>  - this decision is in turn based on the digests
> 
> What about this design:
>  - `createStaticIndexingAction` accepts a filter-function: "should we index symbols from file X"?
>  - `SymbolCollector` invokes this once per file and caches the result.
>  - the signature is something like `bool(SourceManager&, FileEntry*)`
>  - auto-index passes a function that reads the contents from the FileEntry, checks whether the hash is up-to-date to return true/false, and **stores the resulting hashes** for later
> 
> That way the caching/hash stuff never actually leaks outside auto-index that needs it, and we get the interface we need to precisely avoid generating Symbols we don't want.
> 
> (That function could just always return true in this patch, if the filtering isn't trivial).
> 
> It would mean that auto-index would need to redundantly compute the URI once per file, I don't think that matters.
This indeed sounds like a better design for the long term use. Thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53433