[clangd] C++ API for emitting file status

[hokein]

Summary

Introduce clangd C++ API to emit the current status of file

[hokein]

The code is not polished yet, I'd like to get some high-level comments before moving further. Here is a quick vim demo.

[ilya-biryukov]

Have you considered moving all status updates into the TUScheduler?
TUScheduler has full information about the file status changes, so it seems to most natural place to provide this kind of information.

[sammccall]

clangd/ClangdLSPServer.cpp

787

we should have a protocol model object (in Protocol.h) for the file status updates.

I think it should *likely* it should be distinct from the FileStatus in TUScheduler - optimizing for easy update/comparison and programmatic checks vs optimizing for matching the desired protocol.

Naming is hard, I'd be tempted to take FileStatus for the protocol object and and make the TUScheduler one something more obscure like TUScheduler::TUState.

Unclear to me which one ClangdServer should expose, my *guess* is we should start by exposing the protocol object, and switch if it doesn't capture enough of the semantics that embedders need.

clangd/TUScheduler.cpp

338

So I'm slightly nervous about just emitting objects directly. The problem is if you have independent properties (e.g. "is using a fallback command" vs "are we computing diagnostics"), then it's a burden on the code here to pass them all every time and a burden on the callback code if you don't.

I think one solution (which *may* be what Ilya's suggesting) is to put a mutable FileStatus object in the ASTWorker, and mutate it in place and then invoke the callback, passing the const FileStatus&.

[hokein]

Polish the code, address comments.

[hokein]

Thanks for the comment, the patch should be ready for review.

There is one thing I'm not certain: should we stop emitting the file status when the file is removed (similar to the behavior of diagnostics)? For example, the file is removed while the AST is building. The current behavior of the patch will emit them.

clangd/ClangdLSPServer.cpp

787

SG. Moved FileStatus to Protocol.h.

clangd/TUScheduler.cpp

338

ASTWorker seems the right place for FileStatus, given that interesting statuses are in ASTWorker::update, I think a local FileStatus should be good enough.

ASTWorker is not sufficient to emit all interesting statuses -- at least for PreparingBuild, we invoke getCompileCommand in ClangdServer::addDocument...

[jkorous]

Hi @hokein, I am just keeping up to date with changes.

clangd/ClangdServer.h

39

It would be unfortunate to have this name clashing with clang::DiagnosticsConsumer indeed.
How about something like FileEventConsumer?

clangd/TUScheduler.cpp

359

Wouldn't some different failure status like FileStatusKind::FailedBuild be appropriate here?

[ilya-biryukov]

In D54796#1306960, @hokein wrote:

There is one thing I'm not certain: should we stop emitting the file status when the file is removed (similar to the behavior of diagnostics)? For example, the file is removed while the AST is building. The current behavior of the patch will emit them.

It would be reasonable to be consistent with diagnostics: stop emitting the statuses when ASTWorker was put into shutdown mode.
However, the clients will still need to handle statuses for "removed" files somehow: the notifications in the LSP are inherently racy, IIUC there's no way to serialize removeFile and status updates if we're processing the requests in a (truly) asynchronous manner.

[hokein]

stop emitting file status when ASTworker is shutting down.

[hokein]

It would be reasonable to be consistent with diagnostics: stop emitting the statuses when ASTWorker was put into shutdown mode.

Thanks, sounds fair.

clangd/ClangdServer.h

39

yeah, I plan to rename it later (when the patch gets stable enough). FileEventConsumer is a candidate.

clangd/TUScheduler.cpp

359

The failure status is grouped into FileStatus::messages. I think messages provide enough information to users (if there are something wrong when building the file).

[sammccall]

clangd/ClangdLSPServer.cpp

785

notifying via showMessage looks sensible at first glance as a fallback.
Later we may want to expose a richer API guarded by a client capability, and fallback to showMessage if the capability isn't advertised by the client.

Can you verify that showMessage is a statusbar-type message and not a dialog in VSCode?

785

At the moment you've got FileStatus seralizing into the wire format of ShowMessageParams which seems at least odd.

We should define and pass ShowMessageParams here, and the transformation from FileStatus -> ShowMessageParams should be explicit (a named function or maybe better: inlined here) as it has a different semantic meaning than just toJSON.

If the plan is to expose the FileStatus object but you're not going to do so in this patch, I think it's OK to leave it in Protocol.h - though we should review its structure as if we were going to expose it.

clangd/ClangdServer.cpp

137

Can you explain how this should be implemented?
It seems like not being able to track PreparingBuild is the main weakness of putting the FileStatus in TUScheduler.

If we can't implement it, fine, but this FIXME suggests it just isn't done yet.

clangd/ClangdServer.h

39

The name/problem isn't new; can we do the rename in a separate patch? (before or after). It may have a blast radius in tests etc.

(I'm not sure about FileEventConsumer as it's easily confused with something that watches for file changes on disk. My suggestion would be CompilationWatcher or TUWatcher or something)

47

we should have PathRef File here too, for consistency.

It's redundant with FileStatus::uri but I think that field is actually a layering violation: DiagnosticsConsumer knows about file paths, not URIs.

The more I think about this, the more I think we should move FileStatus to the ClangdServer or TUScheduler layer and exclude URI. If we later want to expose the info via LSP we should define a separate struct in Protocol. WDYT?

clangd/Protocol.h

937 ↗

(On Diff #175277)

"StatusKind" doesn't carry much meaning.
More subtly, it implies a hierarchy of information in FileStatus. I don't think this reflects reality: if kind = BuildingPreamble and messages = {"has fallback compile command"} then these are peers, not primary/secondary.

I'd suggest calling this FileAction instead. In this case you may consider renaming Ready -> Idle (as "ready" doesn't really tell you what the action is/isn't).

943 ↗

(On Diff #175277)

"ready" also seems a little strong: if a file failed to build, it will be in this state but isn't really "ready" for anything.

947 ↗

(On Diff #175277)

(I think this comment doesn't quite make sense: window/showMessage is only revelant to clients that receive JSON, and the JSON doesn't follow this structure)

957 ↗

(On Diff #175277)

Some of the most important messages we need to show are warnings (failed compilation, fallback command). These deserve some visual indicator in the client.
You're also sending messages that are unimportant to users ("reusing AST").

I'd suggest either splitting these up into vector<string> messages, vector<string> warnings, or adding a severity.

957 ↗

(On Diff #175277)

in fact, I'm not sure vector<string> is the right model here at all (for the implementation in TUScheduler) - what happens when you have multiple messages produced at different places?

clangd/TUScheduler.cpp

254

Why a new mutex rather than reusing ReportDiagnostics?

336

this seems like it could be a method on ASTWorker instead?

401

This looks like the wrong layer to me. Suppose you have a queue like this:

[update] [go to def] [update] [go to def]

As implemented, your status is going to be
building ready building ready
which seems misleading.

This makes me think:

• the "ready"/"idle" state is a property of the worker thread, not any particular action
• maybe we need another status (running action) and then a string field to tell us which action it was.

[hokein]

• address review comments
• drop the LSP change, only focus on TUScheduler in this patch

[hokein]

@ilya-biryukov, I hope the current patch is not too big for you to review, happy to chat offline if you want (sam and I had a lot of discussions before he is OOO).

clangd/ClangdLSPServer.cpp

785

On the second thought, solving the LSP is not the main goal of this patch, so I drop this part out (will send out a follow-up patch on the LSP part).

clangd/ClangdServer.cpp

137

Given the current code, we can't implement it, but I do think this status is valuable (especially for some build system that might be slow to prepare the building environment). Rephased the FIXME, and removed this state in the struct definition (to avoid misleading).

clangd/ClangdServer.h

47

Done, use File in the interface for consistency.

The more I think about this, the more I think we should move FileStatus to the ClangdServer or TUScheduler layer and exclude URI. If we later want to expose the info via LSP we should define a separate struct in Protocol. WDYT?

That sounds good. The FileStatus aims to be the LSP structure in this patch. I agree we should have a dedicate structure which represents the internal status of TU/file.

clangd/TUScheduler.cpp

401

A sensible status for [update] [go to def] would be queued buildingPreamble buildingFile RunningAction(GoToDef)

[ilya-biryukov]

@ilya-biryukov, I hope the current patch is not too big for you to review, happy to chat offline if you want (sam and I had a lot of discussions before he is OOO).

Picking it up. Mostly NITs from my side and a few questions to better understand the design. Also happy to chat offline.

clangd/ClangdServer.h

39

ClangdServerCallbacks? xD

47

Have we thought about the way we might expose statuses via the LSP?
The TUStatus seems a bit too clangd-specific (i.e. BuildingPreamble, ReuseAST is not something that makes sense in the protocol). Which might be fine on the ClangdServer layer, but it feels like we should generalize before sending it over via the LSP

clangd/TUScheduler.cpp

209

Naming: emitTUStatus?

238

Maybe add a GUARDED_BY(DiagMu) comment?
Also maybe put closer to the mutex itself, e.g. right before ReportDiagnostics.

547

Code style: remove braces around a single-statement if branch

586

NIT: remove braces?

clangd/TUScheduler.h

60

What's the fundamental difference between BuildingFile and RunningAction?
We will often rebuild ASTs while running various actions that read the preamble.

Maybe we could squash those two together? One can view diagnostics as an action on the AST, similar to a direct LSP request like findReferences.

64

Maybe set default value to avoid unexpected undefined behavior in case someone forget to initialize the field?

73

NIT: maybe clarify: indicates whether clang failed... or similar?

74

Naming: should this be BuildFailed?

75

NIT: should this be reused?

76

ReuseAST?

unittests/clangd/TUSchedulerTests.cpp

723 ↗

(On Diff #175859)

It seems we could make this test deterministic if we:

1. create a Notification
2. make it ready after calling removeDocument.
3. wait for it to become ready at the point where we have sleep_for now.

[hokein]

Fix nits.

[hokein]

clangd/ClangdServer.h

47

The TUStatus seems a bit too clangd-specific (i.e. BuildingPreamble, ReuseAST is not something that makes sense in the protocol).

Yes, this is by design. TUStatus presents the internal states of clangd, and would not be exposed via LSP.

Some ways of exposing status via the LSP

• define a separate structure e.g. FileStatus in LSP, render TUStatus to it, and sending it to clients (probably we might want to add a new extension textDocument/filestatus)
• reuse some existing LSP methods (window/showMessage, window/logMessage), render TUStatus to one of these methods' params.

clangd/TUScheduler.h

60

They are two different states, you can think RunningAction means the AST worker starts processing a task (for example Update, GoToDefinition) and BuildingFile means building the AST (which is one possible step when processing the task).

In the current implementation, BuildingPreamble and BuildingFile are only emitted when AST worker processes the Update task (as we are mostly interested in TU states of ASTWorker::update); for other AST tasks, we just emit RunningAction which should be enough.

Given the following requests in the worker queue:
[Update]
[GoToDef]
[Update]
[CodeComplete]

statuses we emitted are

RunningAction(Update) BuildingPreamble BuildingFile
RunningAction(GoToDef)
RunningAction(Update) BuildingPreamble BuildingFile
RunningAction(GetCurrentPreamble)
Idle

64

Done.

[ilya-biryukov]

clangd/ClangdServer.h

47

LG, I believe the BuildingPreamble status in particular might be useful on the C++ API level for us.
Not sure showMessage or logMessage is a good idea, the first one will definitely be annoying if it'll be popping up all the time. Having a new method in the LSP LG.

clangd/TUScheduler.cpp

254

The comment duplicates the code, maybe remove it?

255

Is Status actually ever read from multiple threads?
I assume it's only accessed on the worker thread, right? I think we can leave out the locks around it and put beside other fields only accessed by the worker thread, i.e. DiagsWereReported, etc.

PS Sorry go going back and forth, I didn't think about it in the previous review iteration.

544

NIT: Maybe change Stop emitting to Do not emit?
The current wording seems to suggest we're gonna signal some other code to stop doing that...

586

Maybe do it outside the lock? The less dependencies between the locks we have, the better and this one seems unnecessary.

clangd/TUScheduler.h

57

Maybe remove this value and leave out the default constructor too?
That would ensure we never fill have State field with undefined value without having this enumerator.
And FWIW, putting any value as the default seems fine even if we want to keep the default ctor, all the users have to set the State anyway.

60

That's the confusing part: clangd might be building the ASTs inside RunningAction too, but we only choose to report BuildingFile during updates.
I would get rid of BuildingFile and change it to RunningAction(Diagnostics) instead, it feels like that would allow avoiding some confusion in the future.

66

Ah, the annoying part about member initializers :-(
(Just grumbling, no need to do anything about it)

unittests/clangd/TUSchedulerTests.cpp

702 ↗

(On Diff #176098)

Using a single matcher with two params could improve readability, e.g.

ElementsAre(TUState(TUAction::Queued, ""), TUState(TUAction::RunningAction, "Update"), ...)

Feel free to leave as is if you like the current one better, of course. Just a suggestion.

[hokein]

Address comments and fix a bug in the test.

[hokein]

clangd/ClangdServer.h

47

showMessage seems work well via vim YCM, but it is annoying in vscode (as it pops up diags) -- we need to customize this behavior in our own extensions (showing messages in status bar). We need to revisit different ways.

clangd/TUScheduler.cpp

255

Unfortunately not, most statuses are emitted via worker thread, but we emit Queued status in the main thread...

586

Is it safe to read the Requests out of the lock here?

clangd/TUScheduler.h

57

We need the default constructor... When ASTWorker is constructed, we don't know the any state, I'm nervous about putting any value as default value (it will hurt the code readability, future readers might be confused -- why the status is Queued when constructing).

Adding an Unknown seems ok to me, it is a common practice...

60

BuildingFile might be more natural (understandable) than Diagnostics. Two possible ways here:

• we can also report BuildingFile for other AST tasks, does it reduce the confusing part?
• or if you think Diagnostic is an important state of Update, how about adding a new state RunningDiagnostic? RunningAction is a higher level information IMO, clangd is running diagnostic inside RunningAction(Update).

WDYT?

66

Yeap, if we use C++14, we can get rid of it.

unittests/clangd/TUSchedulerTests.cpp

723 ↗

(On Diff #175859)

Good suggestion, indeed it found a bug in the previous code...

[ilya-biryukov]

Sorry for the delays. Not sure about emitting Queued on the main thread, see the corresponding comment. Otherwise looks very good.

clangd/ClangdServer.h

47

I'd err on the side of getting a new LSP method/extension that's supposed to show messages in the status bar.
It seems showMessage was specifically designed for user interactions and not status bar updates. But that's out of the scope of this patch, so definitely not blocking it.

clangd/TUScheduler.cpp

255

Hm, that sounds surprising.
What if the rebuild is in progress on the worker thread and we emit the queued status on the main thread at the same time? We might get weird sequences of statuses, right?
E.g. Queued → BuildingPreamble → [new update on the main thread] Queued → [processing request on a worker thread] RunningAction('XRefs')

The Queued request status between BuildingPreamble and RunningAction looks really weird.
Maybe we could avoid that by setting emitting the Queued status on the worker thread whenever we're about to sleep on the debounce timeout?

586

No, but we could store the value of .empty() in a local var under the lock. The code looks a bit weirder, but that's a small price to pay for less locks being held at the same time.

clangd/TUScheduler.h

57

Idle would be the valid state for an ASTWorker with an empty queue. Why not use it instead?

Can't disagree using Unknown is a common practice, but is it a good one? It forces every caller to handle this value, even though it's semantics are always unclear.
Why not use Optional<EnumWithoutUnkown> if I really want to indicate the lack of value in those cases? Otherwise every enum type implements its own version of Optional.

60

If you look at the TUScheduler as a black box, diagnostics are not that much different from other actions reading the AST.
I'd err on the side of not sending updates unless something interesting is actually going on.
What we're currently doing makes sense (sending a single update per action), I think it doesn't really matter if we call it BuildingFile or RunningAction("Diagnostics") or RunningDiagnostics. Pick the one you like.

Let's put a comment on BuildingFile that it's only emitted when building the AST for diagnostics and the read actions emit RunningAction instead

[hokein]

Address review comments

• remove Unknown enum type
• make TUState only accessed by the worker thread.

[hokein]

clangd/TUScheduler.cpp

255

This sounds fair enough, and can simplify the code.

Maybe we could avoid that by setting emitting the Queued status on the worker thread whenever we're about to sleep on the debounce timeout?

The Queued state is originally designed for the state that the worker thread is blocked by the max-concurrent-threads semaphore.
Emitting it when we're about to sleep on the debounce timeout doesn't seem a right place to me. I think a reasonable place is before requiring the Barrier.

586

SG.

clangd/TUScheduler.h

57

Idle seems a compromising state for initialization.

60

Done. added comment.

[ilya-biryukov]

LGTM, still have a few NITs and suggestions (see the comment about emitting "queued" on waiting for diagnostics and not emitting queued if the lock was acquired from the first try), but they don't look like something that should block this change.

clangd/TUScheduler.cpp

255

Ah, good point, waiting on a barrier can definitely take some time and is a form of queuing that we might want to notify the clients about.
It would be nice to notify only when the Barrier couldn't be acquired right away though, this probably be achieved adding try_lock() on the semaphore and only emitting a Queued notification if it fails. That way we'll avoid spamming the clients with non-useful statuses.

Wrt to deciding on whether we should notify when waiting on a debounce timer or on acquiring a semaphore... Why not both?

403

Should we also emit this status if we reuse the AST, but still report the diagnostics?

576

See the other suggestion about emitting this status only when locking the barrier failed.
This might be a good fit for a different patch, though, maybe add a FIXME here?

clangd/TUScheduler.h

75

NIT: start with a capital letter to be consistent with the rest of the codebase.

77

NIT: start with a capital letter to be consistent with the rest of the codebase.

[hokein]

Address comments.

[hokein]

clangd/TUScheduler.cpp

255

Sounds good.

403

Yes, I missed that. And I think we also should emit a status if buildAST fails.

576

Agree, added a fixme, will do it in a follow-up patch.

[hokein]

Forgot to associate this patch to the actual commit, committed in rL348475.

clangd/ClangdLSPServer.h

public:
   /// constructor. This method must not be executed more than once.
   ///
   /// \return Whether we shut down cleanly with a 'shutdown' -> 'exit' sequence.
   bool run();
 
 private:
   // Implement DiagnosticsConsumer.
   void onDiagnosticsReady(PathRef File, std::vector<Diag> Diagnostics) override;
+  void onFileUpdated(PathRef File, FileStatus FS) override;
 
   // LSP methods. Notifications have signature void(const Params&).
   // Calls have signature void(const Params&, Callback<Response>).
   void onInitialize(const InitializeParams &, Callback<llvm::json::Value>);
   void onShutdown(const ShutdownParams &, Callback<std::nullptr_t>);
   void onDocumentDidOpen(const DidOpenTextDocumentParams &);
   void onDocumentDidChange(const DidChangeTextDocumentParams &);
   void onDocumentDidClose(const DidCloseTextDocumentParams &);

clangd/ClangdLSPServer.cpp

void ClangdLSPServer::onDiagnosticsReady(PathRef File,
   // Publish diagnostics.
   notify("textDocument/publishDiagnostics",
          json::Object{
              {"uri", URI},
              {"diagnostics", std::move(LSPDiagnostics)},
          });
 }
 
+ void ClangdLSPServer::onFileUpdated(PathRef File, FileStatus FS) {
+  URIForFile URI(File);
+  notify("window/showMessage",
+         json::Object {
+           {"uri", URI},
+           {"message", FS.Message},
+         });
+ }
+
 void ClangdLSPServer::reparseOpenedFiles() {
   for (const Path &FilePath : DraftMgr.getActiveFiles())
     Server->addDocument(FilePath, *DraftMgr.getDraft(FilePath),
                         WantDiagnostics::Auto);
 }
 
 } // namespace clangd
 } // namespace clang

clangd/ClangdServer.h

 #include <type_traits>
 #include <utility>
 
 namespace clang {
 class PCHContainerOperations;
 
 namespace clangd {
 
 class DiagnosticsConsumer {
 public:
   virtual ~DiagnosticsConsumer() = default;
 
   /// Called by ClangdServer when \p Diagnostics for \p File are ready.
   virtual void onDiagnosticsReady(PathRef File,
                                   std::vector<Diag> Diagnostics) = 0;
+
+  /// Called by ClangdServer when the status of file is updated.
+  virtual void onFileUpdated(PathRef File, FileStatus Status) = 0;
 };
 
 /// Manages a collection of source files and derived data (ASTs, indexes),
 /// and provides language-aware features such as code completion.
 ///
 /// The primary client is ClangdLSPServer which exposes these features via
 /// the Language Server protocol. ClangdServer may also be embedded directly,
 /// though its API is not stable over time.
private:
   /// slow. Think of a way to cache this.
   llvm::Expected<tooling::Replacements>
   formatCode(llvm::StringRef Code, PathRef File,
              ArrayRef<tooling::Range> Ranges);
 
   typedef uint64_t DocVersion;
 
   void consumeDiagnostics(PathRef File, DocVersion Version,
+                          FileStatus FS,
                           std::vector<Diag> Diags);
 
   tooling::CompileCommand getCompileCommand(PathRef File);
 
   const GlobalCompilationDatabase &CDB;
   DiagnosticsConsumer &DiagConsumer;
   const FileSystemProvider &FSProvider;
 

clangd/ClangdServer.cpp

ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
   else if (Opts.StaticIndex)
     Index = Opts.StaticIndex;
   else
     Index = nullptr;
 }
 
 void ClangdServer::addDocument(PathRef File, StringRef Contents,
                                WantDiagnostics WantDiags) {
   DocVersion Version = ++InternalVersion[File];
+  // FIXME: notify clients if we are using fallback compilation command.
+  FileStatus FS{FileStatus::State::Preparing,
+                "Getting compilation command..."};
+  consumeDiagnostics(File, Version, FS, {});
   ParseInputs Inputs = {getCompileCommand(File), FSProvider.getFileSystem(),
                         Contents.str()};
 
   Path FileStr = File.str();
-  WorkScheduler.update(File, std::move(Inputs), WantDiags,
-                       [this, FileStr, Version](std::vector<Diag> Diags) {
-                         consumeDiagnostics(FileStr, Version, std::move(Diags));
-                       });
+  WorkScheduler.update(
+      File, std::move(Inputs), WantDiags,
+      [this, FileStr, Version](FileStatus FS, std::vector<Diag> Diags) {
+        consumeDiagnostics(FileStr, Version, FS, std::move(Diags));
 }
 
 void ClangdServer::removeDocument(PathRef File) {
   ++InternalVersion[File];
   WorkScheduler.remove(File);
 }
 
 void ClangdServer::codeComplete(PathRef File, Position Pos,
auto Action = [Pos](Callback<Optional<Hover>> CB,
     if (!InpAST)
       return CB(InpAST.takeError());
     CB(clangd::getHover(InpAST->AST, Pos));
   };
 
   WorkScheduler.runWithAST("Hover", File, Bind(Action, std::move(CB)));
 }
 
+// FIXME: this is a **hacky** way to emit FileStatus to client, revisit it.
 void ClangdServer::consumeDiagnostics(PathRef File, DocVersion Version,
-                                      std::vector<Diag> Diags) {
+                                      FileStatus FS, std::vector<Diag> Diags) {
   // We need to serialize access to resulting diagnostics to avoid calling
   // `onDiagnosticsReady` in the wrong order.
   std::lock_guard<std::mutex> DiagsLock(DiagnosticsMutex);
   DocVersion &LastReportedDiagsVersion = ReportedDiagnosticVersions[File];
 
   // FIXME(ibiryukov): get rid of '<' comparison here. In the current
   // implementation diagnostics will not be reported after version counters'
   // overflow. This should not happen in practice, since DocVersion is a
   // 64-bit unsigned integer.
   if (Version < LastReportedDiagsVersion)
     return;
   LastReportedDiagsVersion = Version;
 
+  if (FS.S == FileStatus::State::Ready)
     DiagConsumer.onDiagnosticsReady(File, std::move(Diags));
+
+  DiagConsumer.onFileUpdated(File, FS);
 }
 
 tooling::CompileCommand ClangdServer::getCompileCommand(PathRef File) {
   trace::Span Span("GetCompileCommand");
   Optional<tooling::CompileCommand> C = CDB.getCompileCommand(File);
   if (!C) // FIXME: Suppress diagnostics? Let the user know?
     C = CDB.getFallbackCommand(File);
 

clangd/TUScheduler.h

 /// *idle* ASTs. If queue has operations requiring the AST, they might be
 /// kept in memory.
 struct ASTRetentionPolicy {
   /// Maximum number of ASTs to be retained in memory when there are no pending
   /// requests for them.
   unsigned MaxRetainedASTs = 3;
 };
 
+/// Indicates the current status of the file in ClangdServer.
+/// FIXME: should we add error code (e.g. severity)?
+struct FileStatus {
+  enum class State {
+    Unknown,
+    Preparing, // Build system is preparing for the file.
+    Wait,      // The file is waiting in the queue to be processed.
+    Building,  // The file is being built.
+    Ready,     // The file is ready.
+
+    // FIXME: others?
+  };
+
+
+  State S;
+  // A detailed message of the status, which can be shown in the status bar.
+  // This can contain details, e.g. the compile command used to build the file;
+  // whether the preamble/AST is reused.
+  std::string Message;
+};
+
 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, ASTContext &Ctx,
                              std::shared_ptr<clang::Preprocessor> PP) {}
   /// Called on the AST built for the file itself. Note that preamble AST nodes
public:
   /// Returns a list of files with ASTs currently stored in memory. This method
   /// is not very reliable and is only used for test. E.g., the results will not
   /// contain files that currently run something over their AST.
   std::vector<Path> getFilesWithCachedAST() const;
 
   /// Schedule an update for \p File. Adds \p File to a list of tracked files if
   /// \p File was not part of it before.
   /// FIXME(ibiryukov): remove the callback from this function.
-  void update(PathRef File, ParseInputs Inputs, WantDiagnostics WD,
-              llvm::unique_function<void(std::vector<Diag>)> OnUpdated);
+  void update(
+      PathRef File, ParseInputs Inputs, WantDiagnostics WD,
+      llvm::unique_function<void(FileStatus, std::vector<Diag>)> OnUpdated);
 
   /// Remove \p File from the list of tracked files and schedule removal of its
   /// resources.
   void remove(PathRef File);
 
   /// Schedule an async task with no dependencies.
   void run(llvm::StringRef Name, llvm::unique_function<void()> Action);
 

clangd/TUScheduler.cpp

static ASTWorkerHandle create(PathRef FileName,
                                 AsyncTaskRunner *Tasks, Semaphore &Barrier,
                                 steady_clock::duration UpdateDebounce,
                                 std::shared_ptr<PCHContainerOperations> PCHs,
                                 bool StorePreamblesInMemory,
                                 ParsingCallbacks &Callbacks);
   ~ASTWorker();
 
   void update(ParseInputs Inputs, WantDiagnostics,
-              llvm::unique_function<void(std::vector<Diag>)> OnUpdated);
+              llvm::unique_function<void(FileStatus, std::vector<Diag>)> OnUpdated);
   void runWithAST(StringRef Name,
                   unique_function<void(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(
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(StringRef Name, unique_function<void()> Task,
                  Optional<WantDiagnostics> UpdateType);
   /// 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 {
     unique_function<void()> Action;
private:
   const Path FileName;
   /// 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;
   /// Helper class required to build the ASTs.
   const std::shared_ptr<PCHContainerOperations> PCHs;
 
   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;
   /// Size of the last AST
   /// 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;
 };
 
 /// 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 {
   friend class ASTWorker;
   ASTWorkerHandle(std::shared_ptr<ASTWorker> Worker)
       : Worker(std::move(Worker)) {

 #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,
-                       unique_function<void(std::vector<Diag>)> OnUpdated) {
+                       unique_function<void(FileStatus, std::vector<Diag>)> OnUpdated) {
   auto Task = [=](decltype(OnUpdated) OnUpdated) mutable {
+    OnUpdated(FileStatus{FileStatus::State::Building, "Building"}, {});
     // 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);
     bool PrevDiagsWereReported = DiagsWereReported;
     FileInputs = Inputs;
     DiagsWereReported = false;
 
     log("Updating file {0} with command [{1}] {2}", FileName,
         Inputs.CompileCommand.Directory,
         join(Inputs.CompileCommand.CommandLine, " "));
     // Rebuild the preamble and the AST.
     std::unique_ptr<CompilerInvocation> Invocation =
         buildCompilerInvocation(Inputs);
     if (!Invocation) {
       elog("Could not build CompilerInvocation for file {0}", FileName);
       // Remove the old AST if it's still in cache.
       IdleASTs.take(this);
       // Make sure anyone waiting for the preamble gets notified it could not
       // be built.
       PreambleWasBuilt.notify();
       return;
     }
 
     std::shared_ptr<const PreambleData> OldPreamble =
         getPossiblyStalePreamble();
     std::shared_ptr<const PreambleData> NewPreamble = buildPreamble(
if (!CanReuseAST) {
         // 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);
+        OnUpdated({FileStatus::State::Ready, "Ready"}, {});
         return;
       }
     }
 
     // We only need to build the AST if diagnostics were requested.
     if (WantDiags == WantDiagnostics::No)
       return;
 
     // Get the AST for diagnostics.
     Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
     if (!AST) {
       Optional<ParsedAST> NewAST =
           buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs);
       AST = NewAST ? llvm::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
     }
     // 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) {
-      OnUpdated((*AST)->getDiagnostics());
+      OnUpdated({FileStatus::State::Ready, "Ready"}, (*AST)->getDiagnostics());
       trace::Span Span("Running main AST callback");
       Callbacks.onMainAST(FileName, **AST);
       DiagsWereReported = true;
     }
     // Stash the AST in the cache for further use.
     IdleASTs.put(this, std::move(*AST));
   };
-
+  OnUpdated({FileStatus::State::Wait, "Wait"}, {});
   startTask("Update", Bind(Task, std::move(OnUpdated)), WantDiags);
 }
 
 void ASTWorker::runWithAST(
     StringRef Name, unique_function<void(Expected<InputsAndAST>)> Action) {
   auto Task = [=](decltype(Action) Action) {
     Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
     if (!AST) {
Requests.push_back(
          Context::current().derive(kFileBeingProcessed, FileName), UpdateType});
   }
   RequestsCV.notify_all();
 }
 
 void ASTWorker::run() {
   while (true) {
     Request Req;
     {
       std::unique_lock<std::mutex> Lock(Mutex);
       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.
Request Req;
 
         wait(Lock, RequestsCV, Wait);
       }
       Req = std::move(Requests.front());
       // Leave it on the queue for now, so waiters don't see an empty queue.
     } // unlock Mutex
 
     {
       std::lock_guard<Semaphore> BarrierLock(Barrier);
       WithContext Guard(std::move(Req.Ctx));
       trace::Span Tracer(Req.Name);
       Req.Action();
     }
 
     {
       std::lock_guard<std::mutex> Lock(Mutex);
       Requests.pop_front();
     }
     RequestsCV.notify_all();
   }
 }
 
 Deadline ASTWorker::scheduleLocked() {
   if (Requests.empty())
     return Deadline::infinity(); // Wait for new requests.
   while (shouldSkipHeadLocked())
     Requests.pop_front();
bool TUScheduler::blockUntilIdle(Deadline D) const {
   if (PreambleTasks)
     if (!PreambleTasks->wait(D))
       return false;
   return true;
 }
 
 void TUScheduler::update(PathRef File, ParseInputs Inputs,
                          WantDiagnostics WantDiags,
-                         unique_function<void(std::vector<Diag>)> OnUpdated) {
+                         unique_function<void(FileStatus, std::vector<Diag>)> OnUpdated) {
   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, PCHOps, StorePreamblesInMemory, *Callbacks);
     FD = std::unique_ptr<FileData>(new FileData{
         Inputs.Contents, Inputs.CompileCommand, std::move(Worker)});