[clangd] Update TUStatus api to accommodate preamble thread

[kadircet]

Summary

TUStatus api had a single thread in mind. This introudces a section
action to represent state of the preamble thread. In the file status extension,
we keep old behavior almost the same. We only prepend current task with a
parsing includes if preamble thread is working. We omit the idle thread in the
output unless both threads are idle.

[sammccall]

clang-tools-extra/clangd/TUScheduler.cpp

162

nit: "manager" doesn't really explain what this is, and it's used both as the class name and the main description in the comment.

Maybe TUStatusTracker or something - "Threadsafe holder for TUStatus..."

167

you could consider a slightly more general version:

StatusMgr.update([&](TUStatus &S) {
  S.PreambleAction = Idle;
});

it's a bit more wordy at the callsite but it makes the correspondence much more direct.

199

why mutable with no const functions?

264

Seems clearer to do this immediately before blocking?

at the top:

if (!NextReq) {
  Lock.unlock();
  StatusManager.setPreambleAction(Idle);
  Lock.lock();
  ReqCV.wait(NextReq || Done);
}
if (Done)
  break;

272

why this change?

clang-tools-extra/clangd/TUScheduler.h

113

What's the purpose of this change? It seems to make the build details harder to maintain/extend.

(In particular, say we were going to add a "fallback command was used" bit, where would it go after this change?)

121

why TUAction rather than a PreambleAction enum { Building, Idle }?
It seems Name will never be used, RunningAction/Queued are not possible etc.

[kadircet]

• Address comments

clang-tools-extra/clangd/TUScheduler.cpp

162

NAMIIIING !!! :(((

199

I had a readable version at some point, just leftover from those days :P

264

i agree, but I wanted to keep the current semantics. we only start emitting tustatuses after thread starts executing the first request.

happy to change *both* preamblethread and astworker to emit before blocking though. wdyt?

272

this needs to happen after resetting CurrentReq and previously this was part of build. It makes more sense to hanlde this resetting as part of the worker thread, rather than as part of the preamble build logic.

but I suppose it doesn't belong into this patch. moving it to the https://reviews.llvm.org/D76125

clang-tools-extra/clangd/TUScheduler.h

113

this was to narrow down the interface. Currently builddetails is only being used to report buildstatus, and those cases are mutually exclusive(a build can't be both a reuse and failure). Therefore i wanted to make that more explicit.

if we decide to add more details, I would say we should rather get BuildDetails struct back, with a Status enum and a couple more state to signal any other information.

[sammccall]

clang-tools-extra/clangd/TUScheduler.cpp

264

I think the difference is moot - we never create either the AST or preamble worker until there's something to do.

The code around scheduling/sleeping in the AST worker thread is way more complicated, and I'm not confident moving the status broadcast to the top of the loop would be clearer there.

Up to you: if you think both are clearer, move both. If you think the preamble is clearer at the top and AST worker at the bottom, then you can choose between consistency and clarity :-)

360

StatusManager -> Status

493

StatusManager -> Status

707

(might be clearer to call once and make the logic conditional)

1063

nit: write this as a switch for consistency plus the covered warning?

clang-tools-extra/clangd/TUScheduler.h

113

OK, I understand but don't really agree - the fact that these are exclusive is a coincidence that we don't need to model APIs around. This doesn't seem related to the rest of this patch, either.

I think we're going around in circles on this though, will leave this up to you.

121

comment just repeats the declaration, drop it?

122

I think these variables are still best described at a high level actions, not states. (Well, the state of the thread is that it is running this action, but that's a little roundabout. TUAction::State probably should have been called Kind instead).

Unfortunate that we need both a type name and a variable name though. ASTAction ASTActivity; PreambleAction PreambleActivity;?

125

If keeping the enum, this variable needs a real name.
I'd suggest LastBuild and dropping the comment.

[kadircet]

• Address comments

[kadircet]

clang-tools-extra/clangd/TUScheduler.cpp

264

as discussed offline, this might make tests flaky due to a possible race between this thread and the thread issuing the write. so leaving it as it is.

[kadircet]

• Rebase

clang-tools-extra/clangd/ClangdLSPServer.cpp

 #include "Context.h"
 #include "Diagnostics.h"
 #include "DraftStore.h"
 #include "FormattedString.h"
 #include "GlobalCompilationDatabase.h"
 #include "Protocol.h"
 #include "SemanticHighlighting.h"
 #include "SourceCode.h"
+#include "TUScheduler.h"
 #include "Trace.h"
 #include "URI.h"
 #include "refactor/Tweak.h"
 #include "clang/Basic/Version.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
ClangdLSPServer::ClangdLSPServer(
   MsgHandler->bind("textDocument/symbolInfo", &ClangdLSPServer::onSymbolInfo);
   MsgHandler->bind("textDocument/typeHierarchy", &ClangdLSPServer::onTypeHierarchy);
   MsgHandler->bind("typeHierarchy/resolve", &ClangdLSPServer::onResolveTypeHierarchy);
   MsgHandler->bind("textDocument/selectionRange", &ClangdLSPServer::onSelectionRange);
   MsgHandler->bind("textDocument/documentLink", &ClangdLSPServer::onDocumentLink);
   // clang-format on
 }
 
-ClangdLSPServer::~ClangdLSPServer() { IsBeingDestroyed = true;
+ClangdLSPServer::~ClangdLSPServer() {
+  IsBeingDestroyed = true;
   // Explicitly destroy ClangdServer first, blocking on threads it owns.
   // This ensures they don't access any other members.
   Server.reset();
 }
 
 bool ClangdLSPServer::run() {
   // Run the Language Server loop.
   bool CleanExit = true;

 
 void ClangdLSPServer::onFileUpdated(PathRef File, const TUStatus &Status) {
   if (!SupportFileStatus)
     return;
   // FIXME: we don't emit "BuildingFile" and `RunningAction`, as these
   // two statuses are running faster in practice, which leads the UI constantly
   // changing, and doesn't provide much value. We may want to emit status at a
   // reasonable time interval (e.g. 0.5s).
-  if (Status.Action.S == TUAction::BuildingFile ||
-      Status.Action.S == TUAction::RunningAction)
+  if (Status.PreambleActivity == PreambleAction::Idle &&
+      (Status.ASTActivity.K == ASTAction::Building ||
+       Status.ASTActivity.K == ASTAction::RunningAction))
     return;
   notify("textDocument/clangd.fileStatus", Status.render(File));
 }
 
 void ClangdLSPServer::reparseOpenedFiles(
     const llvm::StringSet<> &ModifiedFiles) {
   if (ModifiedFiles.empty())
     return;

clang-tools-extra/clangd/TUScheduler.h

struct DebouncePolicy {
   float RebuildRatio = 1;
 
   /// Compute the time to debounce based on this policy and recent build times.
   clock::duration compute(llvm::ArrayRef<clock::duration> History) const;
   /// A policy that always returns the same duration, useful for tests.
   static DebouncePolicy fixed(clock::duration);
 };
 
-struct TUAction {
-  enum State {
-    Queued,           // The TU is pending in the thread task queue to be built.
-    RunningAction,    // Starting running actions on the TU.
-    BuildingPreamble, // The preamble of the TU is being built.
-    BuildingFile,     // The TU is being built. It is only emitted when building
-                      // the AST for diagnostics in write action (update).
+enum class PreambleAction {
+  Idle,
+  Building,
+};
+
+struct ASTAction {
+  enum Kind {
+    Queued,        // The action is pending in the thread task queue to be run.
+    RunningAction, // Started running actions on the TU.
+    Building,      // The AST is being built.
     Idle, // Indicates the worker thread is idle, and ready to run any upcoming
           // actions.
   };
-  TUAction(State S, llvm::StringRef Name) : S(S), Name(Name) {}
-  State S;
+  ASTAction() = default;
+  ASTAction(Kind K, llvm::StringRef Name) : K(K), Name(Name) {}
+  Kind K = ASTAction::Idle;
   /// The name of the action currently running, e.g. Update, GoToDef, Hover.
   /// Empty if we are in the idle state.
   std::string Name;
 };
 
 // Internal status of the TU in TUScheduler.
 struct TUStatus {
   struct BuildDetails {
     /// Indicates whether clang failed to build the TU.
     bool BuildFailed = false;
     /// Indicates whether we reused the prebuilt AST.
     bool ReuseAST = false;
   };
   /// Serialize this to an LSP file status item.
   FileStatus render(PathRef File) const;
 
-  TUAction Action;
+  PreambleAction PreambleActivity = PreambleAction::Idle;
+  ASTAction ASTActivity;
+  /// Stores status of the last build for the translation unit.
   BuildDetails Details;
 };
 
 class ParsingCallbacks {
 public:
   virtual ~ParsingCallbacks() = default;
 
   /// Called on the AST that was built for emitting the preamble. The built AST
   /// contains only AST nodes from the #include directives at the start of the
   /// file. AST node in the current file should be observed on onMainAST call.
   virtual void onPreambleAST(PathRef Path, llvm::StringRef Version,
                              ASTContext &Ctx,
                              std::shared_ptr<clang::Preprocessor> PP,

clang-tools-extra/clangd/TUScheduler.cpp

 #include "Path.h"
 #include "Preamble.h"
 #include "Threading.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/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Errc.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Threading.h"
 #include <algorithm>
 #include <memory>
 #include <mutex>
 #include <queue>
+#include <string>
 #include <thread>
 
 namespace clang {
 namespace clangd {
 using std::chrono::steady_clock;
 
 namespace {
 class ASTWorker;
private:
   std::mutex Mut;
   unsigned MaxRetainedASTs;
   /// Items sorted in LRU order, i.e. first item is the most recently accessed
   /// one.
   std::vector<KVPair> LRU; /* GUARDED_BY(Mut) */
 };
 
 namespace {
+/// Threadsafe manager for updating a TUStatus and emitting it after each
+/// update.
+class SynchronizedTUStatus {
+public:
+  SynchronizedTUStatus(PathRef FileName, ParsingCallbacks &Callbacks)
+      : FileName(FileName), Callbacks(Callbacks) {}
+
+  void update(llvm::function_ref<void(TUStatus &)> Mutator) {
+    std::lock_guard<std::mutex> Lock(StatusMu);
+    Mutator(Status);
+    emitStatusLocked();
+  }
+
+  /// Prevents emitting of further updates.
+  void stop() {
+    std::lock_guard<std::mutex> Lock(StatusMu);
+    CanPublish = false;
+  }
+
+private:
+  void emitStatusLocked() {
+    if (CanPublish)
+      Callbacks.onFileUpdated(FileName, Status);
+  }
+
+  const Path FileName;
+
+  std::mutex StatusMu;
+  TUStatus Status;
+  bool CanPublish = true;
+  ParsingCallbacks &Callbacks;
+};
+
 /// Responsible for building and providing access to the preamble of a TU.
 /// Whenever the thread is idle and the preamble is outdated, it starts to build
 /// a fresh preamble from the latest inputs. If RunSync is true, preambles are
 /// built synchronously in update() instead.
 class PreambleThread {
 public:
   PreambleThread(llvm::StringRef FileName, ParsingCallbacks &Callbacks,
-                 bool StorePreambleInMemory, bool RunSync)
+                 bool StorePreambleInMemory, bool RunSync,
+                 SynchronizedTUStatus &Status)
       : FileName(FileName), Callbacks(Callbacks),
-        StoreInMemory(StorePreambleInMemory), RunSync(RunSync) {}
+        StoreInMemory(StorePreambleInMemory), RunSync(RunSync), Status(Status) {
+  }
 
   size_t getUsedBytes() const {
     auto Preamble = latest();
     return Preamble ? Preamble->Preamble.getSize() : 0;
   }
 
   /// It isn't guaranteed that each requested version will be built. If there
   /// are multiple update requests while building a preamble, only the last one
   /// will be built.
   void update(CompilerInvocation *CI, ParseInputs PI) {
     // If compiler invocation was broken, just fail out early.
     if (!CI) {
-      TUStatus::BuildDetails Details;
-      Details.BuildFailed = true;
-      std::string TaskName = llvm::formatv("Update ({0})", PI.Version);
-      emitTUStatus({TUAction::BuildingPreamble, std::move(TaskName)}, &Details);
       // Make sure anyone waiting for the preamble gets notified it could not be
       // built.
       BuiltFirst.notify();
       return;
     }
     // Make possibly expensive copy while not holding the lock.
     Request Req = {std::make_unique<CompilerInvocation>(*CI), std::move(PI)};
     if (RunSync) {
while (true) {
         ReqCV.wait(Lock, [this] { return NextReq || Done; });
         if (Done)
           break;
         CurrentReq = std::move(*NextReq);
         NextReq.reset();
       }
       // Build the preamble and let the waiters know about it.
       build(std::move(*CurrentReq));
+      bool IsEmpty = false;
       {
         std::lock_guard<std::mutex> Lock(Mutex);
         CurrentReq.reset();
+        IsEmpty = !NextReq.hasValue();
+      }
+      if (IsEmpty) {
+        // We don't perform this above, before waiting for a request to make
+        // tests more deterministic. As there can be a race between this thread
+        // and client thread(clangdserver).
+        Status.update([](TUStatus &Status) {
+          Status.PreambleActivity = PreambleAction::Idle;
+        });
       }
       BuiltFirst.notify();
       ReqCV.notify_all();
     }
     // We are no longer going to build any preambles, let the waiters know that.
     BuiltFirst.notify();
     dlog("Preamble worker for {0} finished", FileName);
   }
struct Request {
     ParseInputs Inputs;
   };
 
   bool isDone() {
     std::lock_guard<std::mutex> Lock(Mutex);
     return Done;
   }
 
-  /// Updates the TUStatus and emits it. Only called in the worker thread.
-  void emitTUStatus(TUAction Action,
-                    const TUStatus::BuildDetails *Details = nullptr) {
-    // Do not emit TU statuses when the worker is shutting down.
-    if (isDone())
-      return;
-    TUStatus Status({std::move(Action), {}});
-    if (Details)
-      Status.Details = *Details;
-    Callbacks.onFileUpdated(FileName, Status);
-  }
-
   /// Builds a preamble for Req and caches it. Might re-use the latest built
   /// preamble if it is valid for Req. Also signals waiters about the build.
   /// FIXME: We shouldn't cache failed preambles, if we've got a successful
   /// build before.
   void build(Request Req) {
     assert(Req.CI && "Got preamble request with null compiler invocation");
     const ParseInputs &Inputs = Req.Inputs;
     std::shared_ptr<const PreambleData> OldPreamble =
         Inputs.ForceRebuild ? nullptr : latest();
 
-    std::string TaskName = llvm::formatv("Update ({0})", Inputs.Version);
-    emitTUStatus({TUAction::BuildingPreamble, std::move(TaskName)});
+    Status.update([&](TUStatus &Status) {
+      Status.PreambleActivity = PreambleAction::Building;
+    });
 
     auto Preamble = clang::clangd::buildPreamble(
         FileName, std::move(*Req.CI), OldPreamble, Inputs, StoreInMemory,
         [this, Version(Inputs.Version)](
             ASTContext &Ctx, std::shared_ptr<clang::Preprocessor> PP,
             const CanonicalIncludes &CanonIncludes) {
           Callbacks.onPreambleAST(FileName, Version, Ctx, std::move(PP),
                                   CanonIncludes);
private:
   mutable std::condition_variable ReqCV;           /* GUARDED_BY(Mutex) */
   std::shared_ptr<const PreambleData> LatestBuild; /* GUARDED_BY(Mutex) */
 
   Notification BuiltFirst;
   const Path FileName;
   ParsingCallbacks &Callbacks;
   const bool StoreInMemory;
   const bool RunSync;
+
+  SynchronizedTUStatus &Status;
 };
 
 class ASTWorkerHandle;
 
 /// Owns one instance of the AST, schedules updates and reads of it.
 /// Also responsible for building and providing access to the preamble.
 /// Each ASTWorker processes the async requests sent to it on a separate
 /// dedicated thread.
private:
   // processed.
   void run();
   /// Signal that run() should finish processing pending requests and exit.
   void stop();
   /// Adds a new task to the end of the request queue.
   void startTask(llvm::StringRef Name, llvm::unique_function<void()> Task,
                  llvm::Optional<WantDiagnostics> UpdateType,
                  TUScheduler::ASTActionInvalidation);
-  /// Updates the TUStatus and emits it. Only called in the worker thread.
-  void emitTUStatus(TUAction FAction,
-                    const TUStatus::BuildDetails *Detail = nullptr);
 
   /// 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;
private:
   const bool RunSync;
   /// Time to wait after an update to see whether another update obsoletes it.
   const DebouncePolicy UpdateDebounce;
   /// File that ASTWorker is responsible for.
   const Path FileName;
   const GlobalCompilationDatabase &CDB;
   /// Callback invoked when preamble or main file AST is built.
   ParsingCallbacks &Callbacks;
-  /// Only accessed by the worker thread.
-  TUStatus Status;
 
   Semaphore &Barrier;
   /// Whether the 'onMainAST' callback ran for the current FileInputs.
   bool RanASTCallback = false;
   /// Guards members used by both TUScheduler and the worker thread.
   mutable std::mutex Mutex;
   /// File inputs, currently being used by the worker.
   /// Inputs are written and read by the worker thread, compile command can also
private:
   // Used to prevent remove document + add document races that lead to
   // out-of-order callbacks for publishing results of onMainAST callback.
   //
   // The lifetime of the old/new ASTWorkers will overlap, but their handles
   // don't. When the old handle is destroyed, the old worker will stop reporting
   // any results to the user.
   bool CanPublishResults = true; /* GUARDED_BY(PublishMu) */
 
+  SynchronizedTUStatus Status;
   PreambleThread PW;
 };
 
 /// A smart-pointer-like class that points to an active ASTWorker.
 /// In destructor, signals to the underlying ASTWorker that no new requests will
 /// be sent and the processing loop may exit (after running all pending
 /// requests).
 class ASTWorkerHandle {
ASTWorker::create(PathRef FileName, const GlobalCompilationDatabase &CDB,
   return ASTWorkerHandle(std::move(Worker));
 }
 
 ASTWorker::ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB,
                      TUScheduler::ASTCache &LRUCache, Semaphore &Barrier,
                      bool RunSync, DebouncePolicy UpdateDebounce,
                      bool StorePreamblesInMemory, ParsingCallbacks &Callbacks)
     : IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
-      FileName(FileName), CDB(CDB),
-      Callbacks(Callbacks), Status{TUAction(TUAction::Idle, ""),
-                                   TUStatus::BuildDetails()},
-      Barrier(Barrier), Done(false),
-      PW(FileName, Callbacks, StorePreamblesInMemory, RunSync) {
+      FileName(FileName), CDB(CDB), Callbacks(Callbacks), Barrier(Barrier),
+      Done(false), Status(FileName, Callbacks),
+      PW(FileName, Callbacks, StorePreamblesInMemory, RunSync, Status) {
   auto Inputs = std::make_shared<ParseInputs>();
   // Set a fallback command because compile command can be accessed before
   // `Inputs` is initialized. Other fields are only used after initialization
   // from client inputs.
   Inputs->CompileCommand = CDB.getFallbackCommand(FileName);
   FileInputs = std::move(Inputs);
 }
 
if (!Invocation) {
       return;
     }
 
     bool CanReuseAST = InputsAreTheSame && (OldPreamble == NewPreamble);
     // Before doing the expensive AST reparse, we want to release our reference
     // to the old preamble, so it can be freed if there are no other references
     // to it.
     OldPreamble.reset();
-    emitTUStatus({TUAction::BuildingFile, TaskName});
+    Status.update([&](TUStatus &Status) {
+      Status.ASTActivity.K = ASTAction::Building;
+      Status.ASTActivity.Name = std::move(TaskName);
+    });
     if (!CanReuseAST) {
       IdleASTs.take(this); // Remove the old AST if it's still in cache.
     } else {
       // We don't need to rebuild the AST, check if we need to run the callback.
       if (RanCallbackForPrevInputs) {
         RanASTCallback = true;
         // Take a shortcut and don't report the diagnostics, since they should
         // not changed. All the clients should handle the lack of OnUpdated()
         // call anyway to handle empty result from buildAST.
         // FIXME(ibiryukov): the AST could actually change if non-preamble
         // includes changed, but we choose to ignore it.
         // FIXME(ibiryukov): should we refresh the cache in IdleASTs for the
         // current file at this point?
         log("Skipping rebuild of the AST for {0}, inputs are the same.",
             FileName);
-        TUStatus::BuildDetails Details;
-        Details.ReuseAST = true;
-        emitTUStatus({TUAction::BuildingFile, TaskName}, &Details);
+
+        Status.update([](TUStatus &Status) {
+          Status.Details.ReuseAST = true;
+          Status.Details.BuildFailed = false;
+        });
         return;
       }
     }
 
     // We only need to build the AST if diagnostics were requested.
     if (WantDiags == WantDiagnostics::No)
       return;
 
     {
       std::lock_guard<std::mutex> Lock(PublishMu);
       // No need to rebuild the AST if we won't send the diagnostics. However,
       // note that we don't prevent preamble rebuilds.
       if (!CanPublishResults)
         return;
     }
 
     // Get the AST for diagnostics.
     llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
     auto RebuildStartTime = DebouncePolicy::clock::now();
     if (!AST) {
       llvm::Optional<ParsedAST> NewAST =
           buildAST(FileName, std::move(Invocation), CompilerInvocationDiags,
                    Inputs, NewPreamble);
+      // buildAST fails.
+      Status.update([&](TUStatus &Status) {
+        Status.Details.ReuseAST = false;
+        Status.Details.BuildFailed = !NewAST.hasValue();
+      });
       AST = NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
-      if (!(*AST)) { // buildAST fails.
-        TUStatus::BuildDetails Details;
-        Details.BuildFailed = true;
-        emitTUStatus({TUAction::BuildingFile, TaskName}, &Details);
-      }
     } else {
       // We are reusing the AST.
-      TUStatus::BuildDetails Details;
-      Details.ReuseAST = true;
-      emitTUStatus({TUAction::BuildingFile, TaskName}, &Details);
+      Status.update([](TUStatus &Status) {
+        Status.Details.ReuseAST = true;
+        Status.Details.BuildFailed = false;
+      });
     }
 
     // We want to report the diagnostics even if this update was cancelled.
     // It seems more useful than making the clients wait indefinitely if they
     // spam us with updates.
     // Note *AST can still be null if buildAST fails.
     if (*AST) {
       {
void ASTWorker::stop() {
     std::lock_guard<std::mutex> Lock(PublishMu);
     CanPublishResults = false;
   }
   {
     std::lock_guard<std::mutex> Lock(Mutex);
     assert(!Done && "stop() called twice");
     Done = true;
   }
+  Status.stop();
   RequestsCV.notify_all();
 }
 
 void ASTWorker::startTask(llvm::StringRef Name,
                           llvm::unique_function<void()> Task,
                           llvm::Optional<WantDiagnostics> UpdateType,
                           TUScheduler::ASTActionInvalidation Invalidation) {
   if (RunSync) {
if (RunSync) {
     }
     Requests.push_back({std::move(Task), std::string(Name), steady_clock::now(),
                         std::move(Ctx), UpdateType, Invalidation,
                         std::move(Invalidate)});
   }
   RequestsCV.notify_all();
 }
 
-void ASTWorker::emitTUStatus(TUAction Action,
-                             const TUStatus::BuildDetails *Details) {
-  Status.Action = std::move(Action);
-  if (Details)
-    Status.Details = *Details;
-  std::lock_guard<std::mutex> Lock(PublishMu);
-  // Do not emit TU statuses when the ASTWorker is shutting down.
-  if (CanPublishResults) {
-    Callbacks.onFileUpdated(FileName, Status);
-  }
-}
-
 void ASTWorker::run() {
   auto _ = llvm::make_scope_exit([this] { PW.stop(); });
   while (true) {
     {
       std::unique_lock<std::mutex> Lock(Mutex);
       assert(!CurrentRequest && "A task is already running, multiple workers?");
       for (auto Wait = scheduleLocked(); !Wait.expired();
            Wait = scheduleLocked()) {
         if (Done) {
           if (Requests.empty())
             return;
           else     // Even though Done is set, finish pending requests.
             break; // However, skip delays to shutdown fast.
         }
 
         // Tracing: we have a next request, attribute this sleep to it.
         llvm::Optional<WithContext> Ctx;
         llvm::Optional<trace::Span> Tracer;
         if (!Requests.empty()) {
           Ctx.emplace(Requests.front().Ctx.clone());
           Tracer.emplace("Debounce");
           SPAN_ATTACH(*Tracer, "next_request", Requests.front().Name);
           if (!(Wait == Deadline::infinity())) {
-            emitTUStatus({TUAction::Queued, Requests.front().Name});
+            Status.update([&](TUStatus &Status) {
+              Status.ASTActivity.K = ASTAction::Queued;
+              Status.ASTActivity.Name = Requests.front().Name;
+            });
             SPAN_ATTACH(*Tracer, "sleep_ms",
                         std::chrono::duration_cast<std::chrono::milliseconds>(
                             Wait.time() - steady_clock::now())
                             .count());
           }
         }
 
         wait(Lock, RequestsCV, Wait);
       }
       CurrentRequest = std::move(Requests.front());
       Requests.pop_front();
     } // unlock Mutex
 
     // It is safe to perform reads to CurrentRequest without holding the lock as
     // only writer is also this thread.
     {
       std::unique_lock<Semaphore> Lock(Barrier, std::try_to_lock);
       if (!Lock.owns_lock()) {
-        emitTUStatus({TUAction::Queued, CurrentRequest->Name});
+        Status.update([&](TUStatus &Status) {
+          Status.ASTActivity.K = ASTAction::Queued;
+          Status.ASTActivity.Name = CurrentRequest->Name;
+        });
         Lock.lock();
       }
       WithContext Guard(std::move(CurrentRequest->Ctx));
       trace::Span Tracer(CurrentRequest->Name);
-      emitTUStatus({TUAction::RunningAction, CurrentRequest->Name});
+      Status.update([&](TUStatus &Status) {
+        Status.ASTActivity.K = ASTAction::RunningAction;
+        Status.ASTActivity.Name = CurrentRequest->Name;
+      });
       CurrentRequest->Action();
     }
 
     bool IsEmpty = false;
     {
       std::lock_guard<std::mutex> Lock(Mutex);
       CurrentRequest.reset();
       IsEmpty = Requests.empty();
     }
-    if (IsEmpty)
-      emitTUStatus({TUAction::Idle, /*Name*/ ""});
+    if (IsEmpty) {
+      Status.update([&](TUStatus &Status) {
+        Status.ASTActivity.K = ASTAction::Idle;
+        Status.ASTActivity.Name = "";
+      });
+    }
     RequestsCV.notify_all();
   }
 }
 
 Deadline ASTWorker::scheduleLocked() {
   if (Requests.empty())
     return Deadline::infinity(); // Wait for new requests.
   // Handle cancelled requests first so the rest of the scheduler doesn't.
bool ASTWorker::blockUntilIdle(Deadline Timeout) const {
   return wait(Lock, RequestsCV, Timeout,
               [&] { return Requests.empty() && !CurrentRequest; });
 }
 
 // Render a TUAction to a user-facing string representation.
 // TUAction represents clangd-internal states, we don't intend to expose them
 // to users (say C++ programmers) directly to avoid confusion, we use terms that
 // are familiar by C++ programmers.
-std::string renderTUAction(const TUAction &Action) {
-  std::string Result;
-  llvm::raw_string_ostream OS(Result);
-  switch (Action.S) {
-  case TUAction::Queued:
-    OS << "file is queued";
+std::string renderTUAction(const PreambleAction PA, const ASTAction &AA) {
+  llvm::SmallVector<std::string, 2> Result;
+  switch (PA) {
+  case PreambleAction::Building:
+    Result.push_back("parsing includes");
     break;
-  case TUAction::RunningAction:
-    OS << "running " << Action.Name;
+  case PreambleAction::Idle:
+    // We handle idle specially below.
+    break;
+  }
+  switch (AA.K) {
+  case ASTAction::Queued:
+    Result.push_back("file is queued");
     break;
-  case TUAction::BuildingPreamble:
-    OS << "parsing includes";
+  case ASTAction::RunningAction:
+    Result.push_back("running " + AA.Name);
     break;
-  case TUAction::BuildingFile:
-    OS << "parsing main file";
+  case ASTAction::Building:
+    Result.push_back("parsing main file");
     break;
-  case TUAction::Idle:
-    OS << "idle";
+  case ASTAction::Idle:
+    // We handle idle specially below.
     break;
   }
-  return OS.str();
+  if (Result.empty())
+    return "idle";
+  return llvm::join(Result, ",");
 }
 
 } // namespace
 
 unsigned getDefaultAsyncThreadsCount() {
   return llvm::heavyweight_hardware_concurrency().compute_thread_count();
 }
 
 FileStatus TUStatus::render(PathRef File) const {
   FileStatus FStatus;
   FStatus.uri = URIForFile::canonicalize(File, /*TUPath=*/File);
-  FStatus.state = renderTUAction(Action);
+  FStatus.state = renderTUAction(PreambleActivity, ASTActivity);
   return FStatus;
 }
 
 struct TUScheduler::FileData {
   /// Latest inputs, passed to TUScheduler::update().
   std::string Contents;
   ASTWorkerHandle Worker;
 };

clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

 #include "Threading.h"
 #include "clang/Basic/DiagnosticDriver.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <algorithm>
+#include <bits/stdint-uintn.h>
 #include <chrono>
 #include <utility>
 
 namespace clang {
 namespace clangd {
 namespace {
 
 using ::testing::AnyOf;
 using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::Field;
 using ::testing::IsEmpty;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
-MATCHER_P2(TUState, State, ActionName, "") {
-  if (arg.Action.S != State) {
-    *result_listener << "state is " << arg.Action.S;
+MATCHER_P2(TUState, PreambleActivity, ASTActivity, "") {
+  if (arg.PreambleActivity != PreambleActivity) {
+    *result_listener << "preamblestate is "
+                     << static_cast<uint8_t>(arg.PreambleActivity);
     return false;
   }
-  if (arg.Action.Name != ActionName) {
-    *result_listener << "name is " << arg.Action.Name;
+  if (arg.ASTActivity.K != ASTActivity) {
+    *result_listener << "aststate is " << arg.ASTActivity.K;
     return false;
   }
   return true;
 }
 
 TUScheduler::Options optsForTest() {
   return TUScheduler::Options(ClangdServer::optsForTest());
 }
auto SourceContents = R"cpp(
       #include "foo.h"
       int b = a;
     )cpp";
 
   ParseInputs Inputs = getInputs(Source, SourceContents);
 
   // Update the source contents, which should trigger an initial build with
   // the header file missing.
-  updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
-                  [](std::vector<Diag> Diags) {
-    EXPECT_THAT(
-        Diags,
-        ElementsAre(
-            Field(&Diag::Message, "'foo.h' file not found"),
-            Field(&Diag::Message, "use of undeclared identifier 'a'")));
+  updateWithDiags(
+      S, Source, Inputs, WantDiagnostics::Yes, [](std::vector<Diag> Diags) {
+        EXPECT_THAT(Diags,
+                    ElementsAre(Field(&Diag::Message, "'foo.h' file not found"),
+                                Field(&Diag::Message,
+                                      "use of undeclared identifier 'a'")));
       });
 
   // Add the header file. We need to recreate the inputs since we changed a
   // file from underneath the test FS.
   Files[Header] = "int a;";
   Timestamps[Header] = time_t(1);
   Inputs = getInputs(Source, SourceContents);
 
   // The addition of the missing header file shouldn't trigger a rebuild since
   // we don't track missing files.
-  updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
-                  [](std::vector<Diag> Diags) {
+  updateWithDiags(
+      S, Source, Inputs, WantDiagnostics::Yes, [](std::vector<Diag> Diags) {
         ADD_FAILURE() << "Did not expect diagnostics for missing header update";
       });
 
   // Forcing the reload should should cause a rebuild which no longer has any
   // errors.
   Inputs.ForceRebuild = true;
-  updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
-                  [](std::vector<Diag> Diags) {
-    EXPECT_THAT(Diags, IsEmpty());
-  });
+  updateWithDiags(
+      S, Source, Inputs, WantDiagnostics::Yes,
+      [](std::vector<Diag> Diags) { EXPECT_THAT(Diags, IsEmpty()); });
 
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
 }
 TEST_F(TUSchedulerTests, NoChangeDiags) {
   TUScheduler S(CDB, optsForTest(), captureDiags());
 
   auto FooCpp = testPath("foo.cpp");
   auto Contents = "int a; int b;";
Server.locateSymbolAt(testPath("foo.cpp"), Code.point(),
                         [](Expected<std::vector<LocatedSymbol>> Result) {
                           ASSERT_TRUE((bool)Result);
                         });
 
   ASSERT_TRUE(Server.blockUntilIdleForTest());
 
   EXPECT_THAT(CaptureTUStatus.allStatus(),
               ElementsAre(
-                  // Statuses of "Update" action.
-                  TUState(TUAction::RunningAction, "Update (1)"),
-                  TUState(TUAction::BuildingPreamble, "Update (1)"),
-                  TUState(TUAction::BuildingFile, "Update (1)"),
-
-                  // Statuses of "Definitions" action
-                  TUState(TUAction::RunningAction, "Definitions"),
-                  TUState(TUAction::Idle, /*No action*/ "")));
+                  // Everything starts with ASTWorker starting to execute an
+                  // update
+                  TUState(PreambleAction::Idle, ASTAction::RunningAction),
+                  // We build the preamble
+                  TUState(PreambleAction::Building, ASTAction::RunningAction),
+                  // Preamble worker goes idle
+                  TUState(PreambleAction::Idle, ASTAction::RunningAction),
+                  // We start building the ast
+                  TUState(PreambleAction::Idle, ASTAction::Building),
+                  // Built finished succesffully
+                  TUState(PreambleAction::Idle, ASTAction::Building),
+                  // Rnning go to def
+                  TUState(PreambleAction::Idle, ASTAction::RunningAction),
+                  // both workers go idle
+                  TUState(PreambleAction::Idle, ASTAction::Idle)));
 }
 
 TEST_F(TUSchedulerTests, CommandLineErrors) {
   // We should see errors from command-line parsing inside the main file.
   CDB.ExtraClangFlags = {"-fsome-unknown-flag"};
 
   // (!) 'Ready' must live longer than TUScheduler.
   Notification Ready;
 
   TUScheduler S(CDB, optsForTest(), captureDiags());
   std::vector<Diag> Diagnostics;
   updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
-                  WantDiagnostics::Yes,
-                  [&](std::vector<Diag> D) {
+                  WantDiagnostics::Yes, [&](std::vector<Diag> D) {
                     Diagnostics = std::move(D);
                     Ready.notify();
                   });
   Ready.wait();
 
   EXPECT_THAT(
       Diagnostics,
       ElementsAre(AllOf(
           Field(&Diag::ID, Eq(diag::err_drv_unknown_argument)),
           Field(&Diag::Name, Eq("drv_unknown_argument")),
           Field(&Diag::Message, "unknown argument: '-fsome-unknown-flag'"))));
 }
 
 TEST_F(TUSchedulerTests, CommandLineWarnings) {
   // We should not see warnings from command-line parsing.
   CDB.ExtraClangFlags = {"-Wsome-unknown-warning"};
 
   // (!) 'Ready' must live longer than TUScheduler.
   Notification Ready;
 
   TUScheduler S(CDB, optsForTest(), captureDiags());
   std::vector<Diag> Diagnostics;
   updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
-                  WantDiagnostics::Yes,
-                  [&](std::vector<Diag> D) {
+                  WantDiagnostics::Yes, [&](std::vector<Diag> D) {
                     Diagnostics = std::move(D);
                     Ready.notify();
                   });
   Ready.wait();
 
   EXPECT_THAT(Diagnostics, IsEmpty());
 }