[clangd] Wait for compile command in ASTWorker instead of ClangdServer

[ioeric]

Summary

This makes addDocument non-blocking and would also allow code completion
(in fallback mode) to run when worker waits for the compile command.

[ilya-biryukov]

This is long overdue... Thanks!

clangd/TUScheduler.cpp

338

The command is filled with a fallback by ClangdServer, right? Why do we need to repeat this here?

358

Could you document that we initially start with a fallback, so update here?

359

Maybe update the heuristic field in the current compile command if its empty to indicate we're using an older command?
That might happen if a previous call to getCompileCommand succeeded (so the stored command does not have a heuristic set).

559

This is racy, FileInputs is only accessed from a worker thread now.
I'm afraid we'll need a separate variable with a lock around it (could reuse one lock for preamble and compile command, probably)

843

We should add a comment that compile command in inputs is ignored.
IMO even better is to accept an FS and Contents instead of ParseInputs.

[ioeric]

• address review comments

clangd/TUScheduler.cpp

338

ASTWorker::FileInputs is not set until ASTWorker runs update and gets compile command. Before that, FileInputs contains empty compile command that can be used by runWithPreamble/runWithAST (afaict). Initializing it to fallback command seems to be a better alternative.

359

If CDB.getCompileCommand failed, we would use Inputs.CompileCommand which should be the fallback command set in ClangdServer. It might be a good idea to use the old command, but it doesn't seem to be in the scope of this patch. Added a FIXME.

559

It looks like we could also guard FileInputs? Adding another variable seems to make the state more complicated.

843

Added commen. Unfortunately, ParseInputs contains other things like Index and Opts, so we couldn't replace it with FS and Contents.

[ioeric]

• Add missing comment to TUScheduler.h

[ilya-biryukov]

clangd/TUScheduler.cpp

338

That makes sense. Could we remove the getFallbackCommand call from the ClangdServer::addDocument?
Let's make the TUScheduler fully responsible for the compile command.

359

Oh, I mixed Inputs and FileInputs in my head.
It feels reasonable to completely move the handling of compile command to TUScheduler, see the other comment at the constructor of ASTWorker.

559

FileInputs are currently accessed without locks in a bunch of places and it'd be nice to keep it that way (the alternative is more complicated code).
Doing the same thing we do for preamble looks like the best alternative here.

[ioeric]

• address comments

clangd/TUScheduler.cpp

338

Sounds good.

559

The reason I think it would be easier to guard FileInputs with mutex instead of pulling a new variable is that CompileCommand is usually used along with other fields in FileInputs. I think we could manage this with relatively few changes. Please take a look at the new changes.

[ilya-biryukov]

clangd/TUScheduler.cpp

559

Unfortunately we can't share Inputs.FS safely as the vfs implementations are not thread-safe.

[ioeric]

clangd/TUScheduler.cpp

559

It seems to me that the behavior for FS hasn't change in this patch. And FileInputs (incl. Inputs.FS) has always been available/shared across worker threads. We don't seem to have systematic way to prevent raciness before this patch, and it would be good to have it somehow, but it's probably out of the scope.

Maybe I'm missing something or misinterpreting. Could you elaborate?

[ilya-biryukov]

clangd/TUScheduler.cpp

559

And FileInputs (incl. Inputs.FS) has always been available/shared across worker threads

I don't think that's the case, we did not a public API to get the hands on ASTWorker::FileInputs.FS and we're getting one now.
Even though the current patch does not add such racy accesses, it certainly makes it much easier to do it accidentally from the clients of ASTWorker in the future (one just needs to access getCurrentInputs().FS).

The (not very) systematic way to prevent raciness that we use now is to not protect the members which are potentially racy with locks and document they are accessed only from a worker thread.
Having a separate copy of the compile command is a small price to pay (both in terms of performance and complexity) to avoid exposing non-thread-safe members of ASTWorker in its public interface.

[ioeric]

clangd/TUScheduler.cpp

559

I don't think that makes a fundamental difference as FileInputs.FS is already racy. But if "public" API is the concern, we could simply restrict the scope of the API and make return CompileCommand only.

[ioeric]

clangd/TUScheduler.cpp

559

Inside ASTWorker, FileInputs can still be guarded with Mutex as a whole.

[ioeric]

• Only return compile command (instead of FileInputs) from AST worker.

[ilya-biryukov]

Thanks, the change LG now. Only nits from my side!

clangd/Compiler.h

19

NIT: this looks unrelated to the actual change. Maybe revert?

clangd/TUScheduler.cpp

222

Could you add a comment that this is private because Inputs.FS is not thread-safe and the client code should take care to not expose it via a public interface.

253

This comment is not true anymore, the FileInputs might be out-of-sync with the AST for short spans of time.
Maybe something like:

/// File inputs, currently being used by the worker.
/// Inputs are written and read by the worker thread, compile command can also be consumed by clients of ASTWorker.

339

NIT: maybe add a comment explaining why only CompileCommand is set and not the other fields?
Thinking of something like:

// Other fields are never read outside worker thread and the worker thread will initialize them before first use.

354

IIUC, The comment does not correspond to the latest version.
s/contains fallback command/does not have a command.

371

NIT: maybe avoid using new?

FileInputs = std::make_shared<ParseInputs>(Inputs)

Feel free to keep as is too, I find the proposed style simpler to follow, but you could reasonably disagree.

unittests/clangd/ClangdTests.cpp

1148

Not strictly related to this patch, but maybe we could add a flag to completion results to indicate if the completion happened via a fallback mode or not?

Would make the test code more straightforward and the tests like these would not rely on a particular implementation of the fallback mode (e.g. I can imagine the fallback mode learning about the scopes later on)

unittests/clangd/CodeCompleteTests.cpp

1390

Could you expand why we need this?

[ioeric]

• address comments

unittests/clangd/ClangdTests.cpp

1148

We are setting the context to Recovery and make fallback as part of Recovery. Do you we should distinguish fallback mode from Recovery?

unittests/clangd/CodeCompleteTests.cpp

1390

Added a comment.

[ilya-biryukov]

LGTM! See the NITs, specifically about runAddDocument - those definitely look worth fixing.

unittests/clangd/ClangdTests.cpp

1113

Ah, we should really have a wait-with-timeout for these use-cases.
It's sad that we'll block indefinitely in the old implementation at this point. Having a failing test is much better than the one that never finishes.

No need to do anything in this patch, though, that requires a change to Threading.h that is best done separately anyway.

1148

Ah, Recovery looks good enough if we check the same location twice and second should be non-recovery.

Maybe keep only the Context == Recovery check? Checking for particular results only seems to make test less focused.

unittests/clangd/CodeCompleteTests.cpp

1390

Ah, thanks! Could we instead use a sync helper here to keep the code a bit simpler (won't need a comment too)?

runAddDocument(Server, ...);

1392

Same here: could you use runAddDocument?

[ioeric]

• use sync runAddDocument in test

clangd/ClangdServer.h

 
 private:
   /// FIXME: This stats several files to find a .clang-format file. I/O can be
   /// slow. Think of a way to cache this.
   llvm::Expected<tooling::Replacements>
   formatCode(llvm::StringRef Code, PathRef File,
              ArrayRef<tooling::Range> Ranges);
 
-  tooling::CompileCommand getCompileCommand(PathRef File);
-
   const GlobalCompilationDatabase &CDB;
   const FileSystemProvider &FSProvider;
 
   Path ResourceDir;
   // The index used to look up symbols. This could be:
   //   - null (all index functionality is optional)
   //   - the dynamic index owned by ClangdServer (DynamicIdx)
   //   - the static index passed to the constructor

clangd/ClangdServer.cpp

: CDB(CDB), FSProvider(FSProvider),
       ClangTidyOptProvider(Opts.ClangTidyOptProvider),
       SuggestMissingIncludes(Opts.SuggestMissingIncludes),
       WorkspaceRoot(Opts.WorkspaceRoot),
       // Pass a callback into `WorkScheduler` to extract symbols from a newly
       // parsed file and rebuild the file index synchronously each time an AST
       // is parsed.
       // FIXME(ioeric): this can be slow and we may be able to index on less
       // critical paths.
-      WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
+      WorkScheduler(CDB, Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
                     llvm::make_unique<UpdateIndexCallbacks>(DynamicIdx.get(),
                                                             DiagConsumer),
                     Opts.UpdateDebounce, Opts.RetentionPolicy) {
   // Adds an index to the stack, at higher priority than existing indexes.
   auto AddIndex = [&](SymbolIndex *Idx) {
     if (this->Index != nullptr) {
       MergedIdx.push_back(llvm::make_unique<MergedIndex>(Idx, this->Index));
       this->Index = MergedIdx.back().get();

 void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
                                WantDiagnostics WantDiags) {
   ParseOptions Opts;
   Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
   if (ClangTidyOptProvider)
     Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File);
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
 
-  // FIXME: some build systems like Bazel will take time to preparing
-  // environment to build the file, it would be nice if we could emit a
-  // "PreparingBuild" status to inform users, it is non-trivial given the
-  // current implementation.
   ParseInputs Inputs;
-  Inputs.CompileCommand = getCompileCommand(File);
+  // To avoid blocking the call, let ASTWorker wait for the actual compile
+  // command during update.
+  Inputs.CompileCommand = CDB.getFallbackCommand(File);
   Inputs.FS = FSProvider.getFileSystem();
   Inputs.Contents = Contents;
   Inputs.Opts = std::move(Opts);
   Inputs.Index = Index;
   WorkScheduler.update(File, Inputs, WantDiags);
 }
 
 void ClangdServer::removeDocument(PathRef File) { WorkScheduler.remove(File); }
auto Action = [Pos, Resolve, Direction](decltype(CB) CB,
     if (!InpAST)
       return CB(InpAST.takeError());
     CB(clangd::getTypeHierarchy(InpAST->AST, Pos, Resolve, Direction));
   };
 
   WorkScheduler.runWithAST("Type Hierarchy", File, Bind(Action, std::move(CB)));
 }
 
-tooling::CompileCommand ClangdServer::getCompileCommand(PathRef File) {
-  trace::Span Span("GetCompileCommand");
-  llvm::Optional<tooling::CompileCommand> C = CDB.getCompileCommand(File);
-  if (!C) // FIXME: Suppress diagnostics? Let the user know?
-    C = CDB.getFallbackCommand(File);
-  return std::move(*C);
-}
-
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.
 }
 
 void ClangdServer::workspaceSymbols(
     llvm::StringRef Query, int Limit,
     Callback<std::vector<SymbolInformation>> CB) {

clangd/Compiler.h

 // run their own compile actions that share logic.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H
 
 #include "../clang-tidy/ClangTidyOptions.h"
+#include "GlobalCompilationDatabase.h"
 #include "index/Index.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Tooling/CompilationDatabase.h"
 
 namespace clang {
 namespace clangd {

clangd/GlobalCompilationDatabase.cpp

 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
   std::vector<std::string> Argv = {getFallbackClangPath()};
   // Clang treats .h files as C by default, resulting in unhelpful diagnostics.
   // Parsing as Objective C++ is friendly to more cases.
   if (llvm::sys::path::extension(File) == ".h")
     Argv.push_back("-xobjective-c++-header");
   Argv.push_back(File);
-  return tooling::CompileCommand(llvm::sys::path::parent_path(File),
-                                 llvm::sys::path::filename(File),
-                                 std::move(Argv),
-                                 /*Output=*/"");
+  tooling::CompileCommand Cmd(llvm::sys::path::parent_path(File),
+                              llvm::sys::path::filename(File), std::move(Argv),
+  Cmd.Heuristic = "clangd fallback";
+  return Cmd;
 }
 
 DirectoryBasedGlobalCompilationDatabase::
     DirectoryBasedGlobalCompilationDatabase(
         llvm::Optional<Path> CompileCommandsDir)
     : CompileCommandsDir(std::move(CompileCommandsDir)) {}
 
 DirectoryBasedGlobalCompilationDatabase::

clangd/TUScheduler.h

 //===--- TUScheduler.h -------------------------------------------*-C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TUSCHEDULER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TUSCHEDULER_H
 
 #include "ClangdUnit.h"
 #include "Function.h"
+#include "GlobalCompilationDatabase.h"
 #include "Threading.h"
 #include "index/CanonicalIncludes.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include <future>
 
 namespace clang {

 /// preambles and ASTs) for opened files.
 /// TUScheduler is not thread-safe, only one thread should be providing updates
 /// and scheduling tasks.
 /// Callbacks are run on a threadpool and it's appropriate to do slow work in
 /// them. Each task has a name, used for tracing (should be UpperCamelCase).
 /// FIXME(sammccall): pull out a scheduler options struct.
 class TUScheduler {
 public:
-  TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
+  TUScheduler(const GlobalCompilationDatabase &CDB, unsigned AsyncThreadsCount,
+              bool StorePreamblesInMemory,
               std::unique_ptr<ParsingCallbacks> ASTCallbacks,
               std::chrono::steady_clock::duration UpdateDebounce,
               ASTRetentionPolicy RetentionPolicy);
   ~TUScheduler();
 
   /// Returns estimated memory usage for each of the currently open files.
   /// The order of results is unspecified.
   std::vector<std::pair<Path, std::size_t>> getUsedBytesPerFile() const;
public:
   // The file being built/processed in the current thread. This is a hack in
   // order to get the file name into the index implementations. Do not depend on
   // this inside clangd.
   // FIXME: remove this when there is proper index support via build system
   // integration.
   static llvm::Optional<llvm::StringRef> getFileBeingProcessedInContext();
 
 private:
+  const GlobalCompilationDatabase &CDB;
   const bool StorePreamblesInMemory;
   std::unique_ptr<ParsingCallbacks> Callbacks; // not nullptr
   Semaphore Barrier;
   llvm::StringMap<std::unique_ptr<FileData>> Files;
   std::unique_ptr<ASTCache> IdleASTs;
   // None when running tasks synchronously and non-None when running tasks
   // asynchronously.
   llvm::Optional<AsyncTaskRunner> PreambleTasks;

clangd/TUScheduler.cpp

 //   AST for some timeout.
 // - If subsequent updates come before rebuild was started, we replace the
 //   pending inputs and reset the timer.
 // - If any reads of the AST are scheduled, we start building the AST
 //   immediately.
 
 #include "TUScheduler.h"
 #include "Cancellation.h"
+#include "GlobalCompilationDatabase.h"
 #include "Logger.h"
 #include "Trace.h"
 #include "index/CanonicalIncludes.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Path.h"
 #include <algorithm>
 #include <memory>
 #include <queue>
 #include <thread>

 /// The ASTWorker that manages the AST is shared by both the processing thread
 /// and the TUScheduler. The TUScheduler should discard an ASTWorker when
 /// remove() is called, but its thread may be busy and we don't want to block.
 /// So the workers are accessed via an ASTWorkerHandle. Destroying the handle
 /// signals the worker to exit its run loop and gives up shared ownership of the
 /// worker.
 class ASTWorker {
   friend class ASTWorkerHandle;
-  ASTWorker(PathRef FileName, TUScheduler::ASTCache &LRUCache,
-            Semaphore &Barrier, bool RunSync,
-            steady_clock::duration UpdateDebounce,
-            bool StorePreamblesInMemory, ParsingCallbacks &Callbacks);
+  ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB,
+            TUScheduler::ASTCache &LRUCache, Semaphore &Barrier, bool RunSync,
+            steady_clock::duration UpdateDebounce, bool StorePreamblesInMemory,
+            ParsingCallbacks &Callbacks);
 
 public:
   /// Create a new ASTWorker and return a handle to it.
   /// The processing thread is spawned using \p Tasks. However, when \p Tasks
   /// is null, all requests will be processed on the calling thread
   /// synchronously instead. \p Barrier is acquired when processing each
   /// request, it is used to limit the number of actively running threads.
-  static ASTWorkerHandle create(PathRef FileName,
-                                TUScheduler::ASTCache &IdleASTs,
-                                AsyncTaskRunner *Tasks, Semaphore &Barrier,
-                                steady_clock::duration UpdateDebounce,
-                                bool StorePreamblesInMemory,
-                                ParsingCallbacks &Callbacks);
+  static ASTWorkerHandle
+  create(PathRef FileName, const GlobalCompilationDatabase &CDB,
+         TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks,
+         Semaphore &Barrier, steady_clock::duration UpdateDebounce,
+         bool StorePreamblesInMemory, ParsingCallbacks &Callbacks);
   ~ASTWorker();
 
   void update(ParseInputs Inputs, WantDiagnostics);
   void
   runWithAST(llvm::StringRef Name,
              llvm::unique_function<void(llvm::Expected<InputsAndAST>)> Action);
   bool blockUntilIdle(Deadline Timeout) const;
 
   std::shared_ptr<const PreambleData> getPossiblyStalePreamble() const;
 
   /// Obtain a preamble reflecting all updates so far. Threadsafe.
   /// It may be delivered immediately, or later on the worker thread.
   void getCurrentPreamble(
       llvm::unique_function<void(std::shared_ptr<const PreambleData>)>);
   /// Wait for the first build of preamble to finish. Preamble itself can be
   /// accessed via getPossiblyStalePreamble(). Note that this function will
   /// return after an unsuccessful build of the preamble too, i.e. result of
   /// getPossiblyStalePreamble() can be null even after this function returns.
   void waitForFirstPreamble() const;
 
+  const tooling::CompileCommand &getCurrentCompileCommand() const;
+
   std::size_t getUsedBytes() const;
   bool isASTCached() const;
 
 private:
   // Must be called exactly once on processing thread. Will return after
   // stop() is called on a separate thread and all pending requests are
   // processed.
   void run();
private:
   /// Determines the next action to perform.
   /// All actions that should never run are discarded.
   /// Returns a deadline for the next action. If it's expired, run now.
   /// scheduleLocked() is called again at the deadline, or if requests arrive.
   Deadline scheduleLocked();
   /// Should the first task in the queue be skipped instead of run?
   bool shouldSkipHeadLocked() const;
 
   struct Request {
     llvm::unique_function<void()> Action;
     std::string Name;
     steady_clock::time_point AddTime;
     Context Ctx;
     llvm::Optional<WantDiagnostics> UpdateType;
   };
 
   /// Handles retention of ASTs.
   TUScheduler::ASTCache &IdleASTs;
   const bool RunSync;
   /// Time to wait after an update to see whether another update obsoletes it.
   const steady_clock::duration UpdateDebounce;
   /// File that ASTWorker is responsible for.
   const Path FileName;
+  const GlobalCompilationDatabase &CDB;
   /// Whether to keep the built preambles in memory or on disk.
   const bool StorePreambleInMemory;
   /// Callback invoked when preamble or main file AST is built.
   ParsingCallbacks &Callbacks;
   /// Only accessed by the worker thread.
   TUStatus Status;
 
   Semaphore &Barrier;
   /// Inputs, corresponding to the current state of AST.
   ParseInputs FileInputs;
   /// Whether the diagnostics for the current FileInputs were reported to the
   /// users before.
   bool DiagsWereReported = false;
   /// Guards members used by both TUScheduler and the worker thread.
   mutable std::mutex Mutex;
   std::shared_ptr<const PreambleData> LastBuiltPreamble; /* GUARDED_BY(Mutex) */
   /// Becomes ready when the first preamble build finishes.
   Notification PreambleWasBuilt;
   /// Set to true to signal run() to finish processing.
   bool Done;                    /* GUARDED_BY(Mutex) */
   std::deque<Request> Requests; /* GUARDED_BY(Mutex) */
   mutable std::condition_variable RequestsCV;
   // FIXME: rename it to better fix the current usage, we also use it to guard
   // emitting TUStatus.
public:
   /// be schedule via the returned reference, i.e. only reads of the preamble
   /// are possible.
   std::shared_ptr<const ASTWorker> lock() { return Worker; }
 
 private:
   std::shared_ptr<ASTWorker> Worker;
 };
 
-ASTWorkerHandle ASTWorker::create(PathRef FileName,
-                                  TUScheduler::ASTCache &IdleASTs,
-                                  AsyncTaskRunner *Tasks, Semaphore &Barrier,
-                                  steady_clock::duration UpdateDebounce,
-                                  bool StorePreamblesInMemory,
-                                  ParsingCallbacks &Callbacks) {
+ASTWorkerHandle
+ASTWorker::create(PathRef FileName, const GlobalCompilationDatabase &CDB,
+                  TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks,
+                  Semaphore &Barrier, steady_clock::duration UpdateDebounce,
+                  bool StorePreamblesInMemory, ParsingCallbacks &Callbacks) {
   std::shared_ptr<ASTWorker> Worker(
-      new ASTWorker(FileName, IdleASTs, Barrier, /*RunSync=*/!Tasks,
+      new ASTWorker(FileName, CDB, IdleASTs, Barrier, /*RunSync=*/!Tasks,
                     UpdateDebounce, StorePreamblesInMemory, Callbacks));
   if (Tasks)
     Tasks->runAsync("worker:" + llvm::sys::path::filename(FileName),
                     [Worker]() { Worker->run(); });
 
   return ASTWorkerHandle(std::move(Worker));
 }
 
-ASTWorker::ASTWorker(PathRef FileName, TUScheduler::ASTCache &LRUCache,
-                     Semaphore &Barrier, bool RunSync,
-                     steady_clock::duration UpdateDebounce,
+ASTWorker::ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB,
+                     TUScheduler::ASTCache &LRUCache, Semaphore &Barrier,
+                     bool RunSync, steady_clock::duration UpdateDebounce,
                      bool StorePreamblesInMemory, ParsingCallbacks &Callbacks)
     : IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
-      FileName(FileName), StorePreambleInMemory(StorePreamblesInMemory),
+      FileName(FileName), CDB(CDB),
+      StorePreambleInMemory(StorePreamblesInMemory),
       Callbacks(Callbacks), Status{TUAction(TUAction::Idle, ""),
                                    TUStatus::BuildDetails()},
-      Barrier(Barrier), Done(false) {}
+      Barrier(Barrier), Done(false) {
+  FileInputs.CompileCommand = CDB.getFallbackCommand(FileName);
+}
 
 ASTWorker::~ASTWorker() {
   // Make sure we remove the cached AST, if any.
   IdleASTs.take(this);
 #ifndef NDEBUG
   std::lock_guard<std::mutex> Lock(Mutex);
   assert(Done && "handle was not destroyed");
   assert(Requests.empty() && "unprocessed requests when destroying ASTWorker");
 #endif
 }
 
 void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
   llvm::StringRef TaskName = "Update";
   auto Task = [=]() mutable {
+    // Get the actual command as `Inputs` contains fallback command.
+    // FIXME: some build systems like Bazel will take time to preparing
+    // environment to build the file, it would be nice if we could emit a
+    // "PreparingBuild" status to inform users, it is non-trivial given the
+    // current implementation.
+    // FIXME: consider using old command if it's not a fallback one.
+    if (auto Cmd = CDB.getCompileCommand(FileName))
+      Inputs.CompileCommand = *Cmd;
     // Will be used to check if we can avoid rebuilding the AST.
     bool InputsAreTheSame =
         std::tie(FileInputs.CompileCommand, FileInputs.Contents) ==
         std::tie(Inputs.CompileCommand, Inputs.Contents);
 
-    tooling::CompileCommand OldCommand = std::move(FileInputs.CompileCommand);
+    tooling::CompileCommand OldCommand;
     bool PrevDiagsWereReported = DiagsWereReported;
+
+    {
+      std::lock_guard<std::mutex> Lock(Mutex);
+      OldCommand = std::move(FileInputs.CompileCommand);
       FileInputs = Inputs;
+    }
     DiagsWereReported = false;
     emitTUStatus({TUAction::BuildingPreamble, TaskName});
     log("Updating file {0} with command {1}\n[{2}]\n{3}", FileName,
         Inputs.CompileCommand.Heuristic,
         Inputs.CompileCommand.Directory,
         llvm::join(Inputs.CompileCommand.CommandLine, " "));
     // Rebuild the preamble and the AST.
     std::unique_ptr<CompilerInvocation> Invocation =
Requests.insert(LastUpdate.base(),
                           Context::current().clone(),
                           /*UpdateType=*/None});
   Lock.unlock();
   RequestsCV.notify_all();
 }
 
 void ASTWorker::waitForFirstPreamble() const { PreambleWasBuilt.wait(); }
 
+const tooling::CompileCommand &ASTWorker::getCurrentCompileCommand() const {
+  std::unique_lock<std::mutex> Lock(Mutex);
+  return FileInputs.CompileCommand;
+}
+
 std::size_t ASTWorker::getUsedBytes() const {
   // Note that we don't report the size of ASTs currently used for processing
   // the in-flight requests. We used this information for debugging purposes
   // only, so this should be fine.
   std::size_t Result = IdleASTs.getUsedBytes(this);
   if (auto Preamble = getPossiblyStalePreamble())
     Result += Preamble->Preamble.getSize();
   return Result;
FileStatus TUStatus::render(PathRef File) const {
   FStatus.uri = URIForFile::canonicalize(File, /*TUPath=*/File);
   FStatus.state = renderTUAction(Action);
   return FStatus;
 }
 
 struct TUScheduler::FileData {
   /// Latest inputs, passed to TUScheduler::update().
   std::string Contents;
-  tooling::CompileCommand Command;
   ASTWorkerHandle Worker;
 };
 
-TUScheduler::TUScheduler(unsigned AsyncThreadsCount,
+TUScheduler::TUScheduler(const GlobalCompilationDatabase &CDB,
+                         unsigned AsyncThreadsCount,
                          bool StorePreamblesInMemory,
                          std::unique_ptr<ParsingCallbacks> Callbacks,
                          std::chrono::steady_clock::duration UpdateDebounce,
                          ASTRetentionPolicy RetentionPolicy)
-    : StorePreamblesInMemory(StorePreamblesInMemory),
+    : CDB(CDB), StorePreamblesInMemory(StorePreamblesInMemory),
       Callbacks(Callbacks ? move(Callbacks)
                           : llvm::make_unique<ParsingCallbacks>()),
       Barrier(AsyncThreadsCount),
       IdleASTs(llvm::make_unique<ASTCache>(RetentionPolicy.MaxRetainedASTs)),
       UpdateDebounce(UpdateDebounce) {
   if (0 < AsyncThreadsCount) {
     PreambleTasks.emplace();
     WorkerThreads.emplace();
for (auto &File : Files)
     if (!File.getValue()->Worker->blockUntilIdle(D))
       return false;
   if (PreambleTasks)
     if (!PreambleTasks->wait(D))
       return false;
   return true;
 }
 
 void TUScheduler::update(PathRef File, ParseInputs Inputs,
                          WantDiagnostics WantDiags) {
   std::unique_ptr<FileData> &FD = Files[File];
   if (!FD) {
     // Create a new worker to process the AST-related tasks.
     ASTWorkerHandle Worker = ASTWorker::create(
-        File, *IdleASTs, WorkerThreads ? WorkerThreads.getPointer() : nullptr,
-        Barrier, UpdateDebounce, StorePreamblesInMemory, *Callbacks);
-    FD = std::unique_ptr<FileData>(new FileData{
-        Inputs.Contents, Inputs.CompileCommand, std::move(Worker)});
+        File, CDB, *IdleASTs,
+        WorkerThreads ? WorkerThreads.getPointer() : nullptr, Barrier,
+        UpdateDebounce, StorePreamblesInMemory, *Callbacks);
+    FD = std::unique_ptr<FileData>(
+        new FileData{Inputs.Contents, std::move(Worker)});
   } else {
     FD->Contents = Inputs.Contents;
-    FD->Command = Inputs.CompileCommand;
   }
   FD->Worker->update(std::move(Inputs), WantDiags);
 }
 
 void TUScheduler::remove(PathRef File) {
   bool Removed = Files.erase(File);
   if (!Removed)
     elog("Trying to remove file from TUScheduler that is not tracked: {0}",
if (It == Files.end()) {
     return;
   }
 
   if (!PreambleTasks) {
     trace::Span Tracer(Name);
     SPAN_ATTACH(Tracer, "file", File);
     std::shared_ptr<const PreambleData> Preamble =
         It->second->Worker->getPossiblyStalePreamble();
-    Action(InputsAndPreamble{It->second->Contents, It->second->Command,
+    Action(InputsAndPreamble{It->second->Contents,
+                             It->second->Worker->getCurrentCompileCommand(),
                              Preamble.get()});
     return;
   }
 
   // Future is populated if the task needs a specific preamble.
   std::future<std::shared_ptr<const PreambleData>> ConsistentPreamble;
   if (Consistency == Consistent) {
     std::promise<std::shared_ptr<const PreambleData>> Promise;
auto Task = [Worker, Consistency,
     trace::Span Tracer(Name);
     SPAN_ATTACH(Tracer, "file", File);
     Action(InputsAndPreamble{Contents, Command, Preamble.get()});
   };
 
   PreambleTasks->runAsync(
       "task:" + llvm::sys::path::filename(File),
       Bind(Task, std::string(Name), std::string(File), It->second->Contents,
-           It->second->Command,
+           Worker->getCurrentCompileCommand(),
            Context::current().derive(kFileBeingProcessed, File),
            std::move(ConsistentPreamble), std::move(Action)));
 }
 
 std::vector<std::pair<Path, std::size_t>>
 TUScheduler::getUsedBytesPerFile() const {
   std::vector<std::pair<Path, std::size_t>> Result;
   Result.reserve(Files.size());

unittests/clangd/ClangdTests.cpp

TEST_F(ClangdVFSTest, FallbackWhenPreambleIsNotReady) {
   ASSERT_TRUE(Server.blockUntilIdleForTest());
   EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, Code.point(),
                                        clangd::CodeCompleteOptions()))
                   .Completions,
               ElementsAre(AllOf(Field(&CodeCompletion::Name, "xyz"),
                                 Field(&CodeCompletion::Scope, "ns::"))));
 }
 
+TEST_F(ClangdVFSTest, FallbackWhenWaitingForCompileCommand) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  // Returns compile command only when notified.
+  class DelayedCompilationDatabase : public GlobalCompilationDatabase {
+  public:
+    DelayedCompilationDatabase(Notification &CanReturnCommand)
+        : CanReturnCommand(CanReturnCommand) {}
+
+    llvm::Optional<tooling::CompileCommand>
+    getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override {
+      CanReturnCommand.wait();
+      auto FileName = llvm::sys::path::filename(File);
+      std::vector<std::string> CommandLine = {"clangd", "-ffreestanding", File};
+      return {tooling::CompileCommand(llvm::sys::path::parent_path(File),
+                                      FileName, std::move(CommandLine), "")};
+    }
+
+    std::vector<std::string> ExtraClangFlags;
+
+  private:
+    Notification &CanReturnCommand;
+  };
+
+  Notification CanReturnCommand;
+  DelayedCompilationDatabase CDB(CanReturnCommand);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto FooCpp = testPath("foo.cpp");
+  Annotations Code(R"cpp(
+    namespace ns { int xyz; }
+    using namespace ns;
+    int main() {
+       xy^
+    })cpp");
+  FS.Files[FooCpp] = FooCpp;
+  Server.addDocument(FooCpp, Code.code());
+
+  // Sleep for some time to make sure code completion is not run because update
+  // hasn't been scheduled.
+  std::this_thread::sleep_for(std::chrono::milliseconds(10));
+  auto Opts = clangd::CodeCompleteOptions();
+  Opts.AllowFallback = true;
+
+  auto Res = cantFail(runCodeComplete(Server, FooCpp, Code.point(), Opts));
+  EXPECT_EQ(Res.Context, CodeCompletionContext::CCC_Recovery);
+  // Identifier-based fallback completion doesn't know about "symbol" scope.
+  EXPECT_THAT(Res.Completions,
+              ElementsAre(AllOf(Field(&CodeCompletion::Name, "xyz"),
+                                Field(&CodeCompletion::Scope, ""))));
+
+  CanReturnCommand.notify();
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
+  EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, Code.point(),
+                                       clangd::CodeCompleteOptions()))
+                  .Completions,
+              ElementsAre(AllOf(Field(&CodeCompletion::Name, "xyz"),
+                                Field(&CodeCompletion::Scope, "ns::"))));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

unittests/clangd/CodeCompleteTests.cpp

Annotations Source(R"cpp(
     int Struct<T>::comments_quux() {
       int a = comments^;
     }
   )cpp");
   // FIXME: Auto-completion in a template requires disabling delayed template
   // parsing.
   CDB.ExtraClangFlags.push_back("-fno-delayed-template-parsing");
   Server.addDocument(FooCpp, Source.code(), WantDiagnostics::Yes);
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
   CodeCompleteResult Completions = cantFail(runCodeComplete(
       Server, FooCpp, Source.point(), clangd::CodeCompleteOptions()));
 
   // We should not get any of those comments in completion.
   EXPECT_THAT(
       Completions.Completions,
       UnorderedElementsAre(AllOf(Not(IsDocumented()), Named("comments_foo")),
                            AllOf(IsDocumented(), Named("comments_baz")),
                            AllOf(IsDocumented(), Named("comments_quux")),
                            AllOf(Not(IsDocumented()), Named("comments_ns")),

unittests/clangd/TUSchedulerTests.cpp

 #include "gtest/gtest.h"
 #include <algorithm>
 #include <utility>
 
 namespace clang {
 namespace clangd {
 namespace {
 
-using ::testing::_;
-using ::testing::AllOf;
 using ::testing::AnyOf;
 using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
 MATCHER_P2(TUState, State, ActionName, "") {
   return arg.Action.S == State && arg.Action.Name == ActionName;
protected:
   llvm::StringMap<time_t> Timestamps;
   MockCompilationDatabase CDB;
 };
 
 Key<llvm::unique_function<void(PathRef File, std::vector<Diag>)>>
     TUSchedulerTests::DiagsCallbackKey;
 
 TEST_F(TUSchedulerTests, MissingFiles) {
-  TUScheduler S(getDefaultAsyncThreadsCount(),
+  TUScheduler S(CDB, getDefaultAsyncThreadsCount(),
                 /*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
                 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
                 ASTRetentionPolicy());
 
   auto Added = testPath("added.cpp");
   Files[Added] = "";
 
   auto Missing = testPath("missing.cpp");

 
 TEST_F(TUSchedulerTests, WantDiagnostics) {
   std::atomic<int> CallbackCount(0);
   {
     // To avoid a racy test, don't allow tasks to actualy run on the worker
     // thread until we've scheduled them all.
     Notification Ready;
     TUScheduler S(
-        getDefaultAsyncThreadsCount(),
+        CDB, getDefaultAsyncThreadsCount(),
         /*StorePreamblesInMemory=*/true, captureDiags(),
         /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
         ASTRetentionPolicy());
     auto Path = testPath("foo.cpp");
     updateWithDiags(S, Path, "", WantDiagnostics::Yes,
                     [&](std::vector<Diag>) { Ready.wait(); });
     updateWithDiags(S, Path, "request diags", WantDiagnostics::Yes,
                     [&](std::vector<Diag>) { ++CallbackCount; });
std::atomic<int> CallbackCount(0);
     ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   }
   EXPECT_EQ(2, CallbackCount);
 }
 
 TEST_F(TUSchedulerTests, Debounce) {
   std::atomic<int> CallbackCount(0);
   {
-    TUScheduler S(getDefaultAsyncThreadsCount(),
+    TUScheduler S(CDB, getDefaultAsyncThreadsCount(),
                   /*StorePreamblesInMemory=*/true, captureDiags(),
                   /*UpdateDebounce=*/std::chrono::seconds(1),
                   ASTRetentionPolicy());
     // FIXME: we could probably use timeouts lower than 1 second here.
     auto Path = testPath("foo.cpp");
     updateWithDiags(S, Path, "auto (debounced)", WantDiagnostics::Auto,
                     [&](std::vector<Diag>) {
                       ADD_FAILURE()
static std::vector<std::string> includes(const PreambleData *Preamble) {
   return Result;
 }
 
 TEST_F(TUSchedulerTests, PreambleConsistency) {
   std::atomic<int> CallbackCount(0);
   {
     Notification InconsistentReadDone; // Must live longest.
     TUScheduler S(
-        getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true,
+        CDB, getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true,
         /*ASTCallbacks=*/nullptr,
         /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
         ASTRetentionPolicy());
     auto Path = testPath("foo.cpp");
     // Schedule two updates (A, B) and two preamble reads (stale, consistent).
     // The stale read should see A, and the consistent read should see B.
     // (We recognize the preambles by their included files).
     updateWithCallback(S, Path, "#include <A>", WantDiagnostics::Yes, [&]() {
TEST_F(TUSchedulerTests, Cancellation) {
   //    R2A              <-- cancelled
   //    R2B
   //   U3(WantDiags=Yes)
   //    R3               <-- cancelled
   std::vector<std::string> DiagsSeen, ReadsSeen, ReadsCanceled;
   {
     Notification Proceed; // Ensure we schedule everything.
     TUScheduler S(
-        getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true,
+        CDB, getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true,
         /*ASTCallbacks=*/captureDiags(),
         /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
         ASTRetentionPolicy());
     auto Path = testPath("foo.cpp");
     // Helper to schedule a named update and return a function to cancel it.
     auto Update = [&](std::string ID) -> Canceler {
       auto T = cancelableTask();
       WithContext C(std::move(T.first));
TEST_F(TUSchedulerTests, ManyUpdates) {
 
   std::mutex Mut;
   int TotalASTReads = 0;
   int TotalPreambleReads = 0;
   int TotalUpdates = 0;
 
   // Run TUScheduler and collect some stats.
   {
-    TUScheduler S(getDefaultAsyncThreadsCount(),
+    TUScheduler S(CDB, getDefaultAsyncThreadsCount(),
                   /*StorePreamblesInMemory=*/true, captureDiags(),
                   /*UpdateDebounce=*/std::chrono::milliseconds(50),
                   ASTRetentionPolicy());
 
     std::vector<std::string> Files;
     for (int I = 0; I < FilesCount; ++I) {
       std::string Name = "foo" + std::to_string(I) + ".cpp";
       Files.push_back(testPath(Name));
TEST_F(TUSchedulerTests, ManyUpdates) {
   EXPECT_EQ(TotalASTReads, FilesCount * UpdatesPerFile);
   EXPECT_EQ(TotalPreambleReads, FilesCount * UpdatesPerFile);
 }
 
 TEST_F(TUSchedulerTests, EvictedAST) {
   std::atomic<int> BuiltASTCounter(0);
   ASTRetentionPolicy Policy;
   Policy.MaxRetainedASTs = 2;
-  TUScheduler S(
+  TUScheduler S(CDB,
                 /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true,
                 /*ASTCallbacks=*/nullptr,
-      /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy);
+                /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+                Policy);
 
   llvm::StringLiteral SourceContents = R"cpp(
     int* a;
     double* b = a;
   )cpp";
   llvm::StringLiteral OtherSourceContents = R"cpp(
     int* a;
     double* b = a + 0;
TEST_F(TUSchedulerTests, EvictedAST) {
 
   // Check the AST for foo.cpp is retained now and one of the others got
   // evicted.
   EXPECT_THAT(S.getFilesWithCachedAST(),
               UnorderedElementsAre(Foo, AnyOf(Bar, Baz)));
 }
 
 TEST_F(TUSchedulerTests, EmptyPreamble) {
-  TUScheduler S(
+  TUScheduler S(CDB,
                 /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
                 /*ASTCallbacks=*/nullptr,
                 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
                 ASTRetentionPolicy());
 
   auto Foo = testPath("foo.cpp");
   auto Header = testPath("foo.h");
 
   Files[Header] = "void foo()";
   Timestamps[Header] = time_t(0);
   auto WithPreamble = R"cpp(
     #include "foo.h"
S.runWithPreamble(
             cantFail(std::move(Preamble)).Preamble->Preamble.getBounds().Size,
             0u);
       });
 }
 
 TEST_F(TUSchedulerTests, RunWaitsForPreamble) {
   // Testing strategy: we update the file and schedule a few preamble reads at
   // the same time. All reads should get the same non-null preamble.
-  TUScheduler S(
+  TUScheduler S(CDB,
                 /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
                 /*ASTCallbacks=*/nullptr,
                 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
                 ASTRetentionPolicy());
   auto Foo = testPath("foo.cpp");
   auto NonEmptyPreamble = R"cpp(
     #define FOO 1
     #define BAR 2
 
     int main() {}
   )cpp";
   constexpr int ReadsToSchedule = 10;
TEST_F(TUSchedulerTests, RunWaitsForPreamble) {
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   // Check all actions got the same non-null preamble.
   std::lock_guard<std::mutex> Lock(PreamblesMut);
   ASSERT_NE(Preambles[0], nullptr);
   ASSERT_THAT(Preambles, Each(Preambles[0]));
 }
 
 TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
-  TUScheduler S(
+  TUScheduler S(CDB,
                 /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
                 /*StorePreambleInMemory=*/true, captureDiags(),
                 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
                 ASTRetentionPolicy());
 
   auto Source = testPath("foo.cpp");
   auto Header = testPath("foo.h");
 
   Files[Header] = "int a;";
   Timestamps[Header] = time_t(0);
 
   auto SourceContents = R"cpp(
TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
 
   // Update to the compile commands should also cause a rebuild.
   CDB.ExtraClangFlags.push_back("-DSOMETHING");
   ASSERT_TRUE(DoUpdate(OtherSourceContents));
   ASSERT_FALSE(DoUpdate(OtherSourceContents));
 }
 
 TEST_F(TUSchedulerTests, NoChangeDiags) {
-  TUScheduler S(
+  TUScheduler S(CDB,
                 /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
                 /*StorePreambleInMemory=*/true, captureDiags(),
                 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
                 ASTRetentionPolicy());
 
   auto FooCpp = testPath("foo.cpp");
   auto Contents = "int a; int b;";
 
   updateWithDiags(
       S, FooCpp, Contents, WantDiagnostics::No,
       [](std::vector<Diag>) { ADD_FAILURE() << "Should not be called."; });
   S.runWithAST("touchAST", FooCpp, [](Expected<InputsAndAST> IA) {
TEST_F(TUSchedulerTests, NoChangeDiags) {
   // diags have previously been reported and the inputs didn't change.
   updateWithDiags(
       S, FooCpp, Contents, WantDiagnostics::Auto,
       [&](std::vector<Diag>) { ADD_FAILURE() << "Should not be called."; });
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
 }
 
 TEST_F(TUSchedulerTests, Run) {
-  TUScheduler S(/*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
+  TUScheduler S(CDB, /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
                 /*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr,
                 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
                 ASTRetentionPolicy());
   std::atomic<int> Counter(0);
   S.run("add 1", [&] { ++Counter; });
   S.run("add 2", [&] { Counter += 2; });
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   EXPECT_EQ(Counter.load(), 3);