[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

[Kugan Vivekanandarajah via Phabricator via cfe-commits]

kuganv added a comment.

In D148088#4316352 <https://reviews.llvm.org/D148088#4316352>, @kadircet wrote:

> Sorry I was trying to give some brief idea about what it might look like in `Implementation Concerns` section above, but let me give some more details;
>
> I think we can just change the signature for PreambleParsedCallback <https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/Preamble.h#L83> to pass along refcounted objects. forgot to mention in the first comment, but we should also change the CanonicalIncludes to be a shared_ptr so that it can outlive the PreambleData. We should invoke the callback inside buildPreamble <https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/Preamble.cpp#L669> after a successful build. Afterwards we should also change the signature for onPreambleAST <https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/TUScheduler.h#L164> to take AST, PP and CanonicalIncludes as ref-counted objects again and `PreambleThread::build` should just forward objects received from `PreambleParsedCallback`. Afterwards inside the UpdateIndexCallbacks::onPreambleAST <https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/ClangdServer.cpp#L70> we can just invoke indexing on `Tasks` if it's present or synchronously in the absence of it.

Thanks  @kadircet for the details.

> Does that make sense?

It does make sense. Let me work on this and get back to you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148088/new/

https://reviews.llvm.org/D148088