[PATCH] D41730: [clangd] Use ToolExecutor to write the global-symbol-builder tool.
Eric Liu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 9 04:46:50 PST 2018
ioeric added a comment.
In https://reviews.llvm.org/D41730#970704, @sammccall wrote:
> This makes `SymbolCollector` (production code) depend on YAML (explicitly experimental), as well as on Tooling interfaces.
> At least we should invert this by making the thing passed to SymbolCollector a `function<void(const Symbol&)>` or so.
Thanks for the catch! I completely missed these perspectives...
> Moreover, it's unfortunate we'd need to change SymbolCollector at all. Currently it's a nice abstraction that does one thing (AST -> `SymbolSlab`), now we're making it do two things. It can't be for scalability reasons - there's no problem fitting all the symbols from a TU in memory. So why do we need to do this?
> I don't really understand the `ToolExecutor` interfaces well, but ISTM overriding `SymbolIndexActionFactory::runInvocation` to flush the symbols in the slab out to the `ExecutionContext` would allow you to use `SymbolCollector` unchanged?
`WrapperFrontendAction` is helpful here. All changes have been moved into the tool now. PTAL
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41730
More information about the cfe-commits
mailing list