kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/TUScheduler.cpp:623
std::string TaskName = llvm::formatv("Update ({0})", Inputs.Version);
auto Task = [=]() mutable {
auto RunPublish = [&](llvm::function_ref<void()> Publish) {
----------------
sammccall wrote:
> sammccall wrote:
> > this function is getting too long/monolithic, we should find a way to split
> > it up.
> > ASTTask should be a member function, I think.
> I also lost track of the control flow halfway through the function, and can't
> work it out.
> I don't know really what to concretely advise, but it needs to be a lot
> simpler, maybe by finding some better abstractions or just better
> understanding conceptually what this is doing.
> ASTTask should be a member function, I think.
Moved it into a meber named `publishDiagnostics`
================
Comment at: clang-tools-extra/clangd/TUScheduler.cpp:693
+ std::lock_guard<std::mutex> Lock(Mutex);
+ // LatestPreamble might be the last reference to old preamble, do not
+ // trigger destructor while holding the lock.
----------------
sammccall wrote:
> Didn't we decide to make this available only after building the golden AST
> and publishing diagnostics?
as discussed offline, this is harmless since it is being executed on ASTWorker
thread and makes new preamble available to code completion/signature help
faster.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76725/new/
https://reviews.llvm.org/D76725
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
