Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -37,6 +37,7 @@ namespace clangd { +// FIXME: find a better name. class DiagnosticsConsumer { public: virtual ~DiagnosticsConsumer() = default; @@ -44,6 +45,8 @@ /// Called by ClangdServer when \p Diagnostics for \p File are ready. virtual void onDiagnosticsReady(PathRef File, std::vector Diagnostics) = 0; + /// Called whenever the file status is updated. + virtual void onFileUpdated(PathRef File, const TUStatus &Status){}; }; /// Manages a collection of source files and derived data (ASTs, indexes), Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -86,6 +86,10 @@ DiagConsumer.onDiagnosticsReady(File, std::move(Diags)); } + void onFileUpdated(PathRef File, const TUStatus &Status) override { + DiagConsumer.onFileUpdated(File, Status); + } + private: FileIndex *FIndex; DiagnosticsConsumer &DiagConsumer; @@ -144,6 +148,10 @@ void ClangdServer::addDocument(PathRef File, StringRef Contents, WantDiagnostics WantDiags) { + // 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. WorkScheduler.update(File, ParseInputs{getCompileCommand(File), FSProvider.getFileSystem(), Contents.str()}, Index: clangd/TUScheduler.h =================================================================== --- clangd/TUScheduler.h +++ clangd/TUScheduler.h @@ -52,6 +52,36 @@ unsigned MaxRetainedASTs = 3; }; +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). + 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; + /// 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; + }; + + TUAction Action; + BuildDetails Details; +}; + class ParsingCallbacks { public: virtual ~ParsingCallbacks() = default; @@ -75,6 +105,9 @@ /// Called whenever the diagnostics for \p File are produced. virtual void onDiagnostics(PathRef File, std::vector Diags) {} + + /// Called whenever the TU status is updated. + virtual void onFileUpdated(PathRef File, const TUStatus &Status) {} }; /// Handles running tasks for ClangdServer and managing the resources (e.g., Index: clangd/TUScheduler.cpp =================================================================== --- clangd/TUScheduler.cpp +++ clangd/TUScheduler.cpp @@ -205,6 +205,10 @@ /// Adds a new task to the end of the request queue. void startTask(StringRef Name, unique_function Task, Optional UpdateType); + /// 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. @@ -234,6 +238,8 @@ ParsingCallbacks &Callbacks; /// Helper class required to build the ASTs. const std::shared_ptr PCHs; + /// Only accessed by the worker thread. + TUStatus Status; Semaphore &Barrier; /// Inputs, corresponding to the current state of AST. @@ -251,7 +257,9 @@ bool Done; /* GUARDED_BY(Mutex) */ std::deque Requests; /* GUARDED_BY(Mutex) */ mutable std::condition_variable RequestsCV; - /// Guards a critical section for running the diagnostics callbacks. + // FIXME: rename it to better fix the current usage, we also use it to guard + // emitting TUStatus. + /// Guards a critical section for running the diagnostics callbacks. std::mutex DiagsMu; // Used to prevent remove document + leading to out-of-order diagnostics: // The lifetime of the old/new ASTWorkers will overlap, but their handles @@ -326,8 +334,10 @@ bool StorePreamblesInMemory, ParsingCallbacks &Callbacks) : IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce), FileName(FileName), StorePreambleInMemory(StorePreamblesInMemory), - Callbacks(Callbacks), PCHs(std::move(PCHs)), Barrier(Barrier), - Done(false) {} + Callbacks(Callbacks), + PCHs(std::move(PCHs)), Status{TUAction(TUAction::Idle, ""), + TUStatus::BuildDetails()}, + Barrier(Barrier), Done(false) {} ASTWorker::~ASTWorker() { // Make sure we remove the cached AST, if any. @@ -340,6 +350,7 @@ } void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) { + StringRef TaskName = "Update"; auto Task = [=]() mutable { // Will be used to check if we can avoid rebuilding the AST. bool InputsAreTheSame = @@ -350,7 +361,7 @@ bool PrevDiagsWereReported = DiagsWereReported; FileInputs = Inputs; DiagsWereReported = false; - + emitTUStatus({TUAction::BuildingPreamble, TaskName}); log("Updating file {0} with command [{1}] {2}", FileName, Inputs.CompileCommand.Directory, join(Inputs.CompileCommand.CommandLine, " ")); @@ -361,6 +372,9 @@ elog("Could not build CompilerInvocation for file {0}", FileName); // Remove the old AST if it's still in cache. IdleASTs.take(this); + TUStatus::BuildDetails Details; + Details.BuildFailed = true; + emitTUStatus({TUAction::BuildingPreamble, TaskName}, &Details); // Make sure anyone waiting for the preamble gets notified it could not // be built. PreambleWasBuilt.notify(); @@ -386,7 +400,7 @@ // to it. OldPreamble.reset(); PreambleWasBuilt.notify(); - + emitTUStatus({TUAction::BuildingFile, TaskName}); if (!CanReuseAST) { IdleASTs.take(this); // Remove the old AST if it's still in cache. } else { @@ -403,6 +417,9 @@ // 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); return; } } @@ -443,8 +460,7 @@ // Stash the AST in the cache for further use. IdleASTs.put(this, std::move(*AST)); }; - - startTask("Update", std::move(Task), WantDiags); + startTask(TaskName, std::move(Task), WantDiags); } void ASTWorker::runWithAST( @@ -560,6 +576,18 @@ 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 Lock(DiagsMu); + // Do not emit TU statuses when the ASTWorker is shutting down. + if (ReportDiagnostics) { + Callbacks.onFileUpdated(FileName, Status); + } +} + void ASTWorker::run() { while (true) { Request Req; @@ -595,16 +623,22 @@ } // unlock Mutex { + emitTUStatus({TUAction::Queued, Req.Name}); std::lock_guard BarrierLock(Barrier); WithContext Guard(std::move(Req.Ctx)); trace::Span Tracer(Req.Name); + emitTUStatus({TUAction::RunningAction, Req.Name}); Req.Action(); } + bool IsEmpty = false; { std::lock_guard Lock(Mutex); Requests.pop_front(); + IsEmpty = Requests.empty(); } + if (IsEmpty) + emitTUStatus({TUAction::Idle, /*Name*/ ""}); RequestsCV.notify_all(); } } Index: unittests/clangd/TUSchedulerTests.cpp =================================================================== --- unittests/clangd/TUSchedulerTests.cpp +++ unittests/clangd/TUSchedulerTests.cpp @@ -7,12 +7,13 @@ // //===----------------------------------------------------------------------===// +#include "Annotations.h" #include "Context.h" #include "Matchers.h" #include "TUScheduler.h" #include "TestFS.h" -#include "llvm/ADT/ScopeExit.h" #include "gmock/gmock.h" +#include "llvm/ADT/ScopeExit.h" #include "gtest/gtest.h" #include #include @@ -23,6 +24,7 @@ namespace { using ::testing::_; +using ::testing::AllOf; using ::testing::AnyOf; using ::testing::Each; using ::testing::ElementsAre; @@ -30,6 +32,10 @@ using ::testing::Pointee; using ::testing::UnorderedElementsAre; +MATCHER_P2(TUState, State, ActionName, "") { + return arg.Action.S == State && arg.Action.Name == ActionName; +} + class TUSchedulerTests : public ::testing::Test { protected: ParseInputs getInputs(PathRef File, std::string Contents) { @@ -658,6 +664,78 @@ EXPECT_EQ(Counter.load(), 3); } +TEST_F(TUSchedulerTests, TUStatus) { + class CaptureTUStatus : public DiagnosticsConsumer { + public: + void onDiagnosticsReady(PathRef File, + std::vector Diagnostics) override {} + + void onFileUpdated(PathRef File, const TUStatus &Status) override { + std::lock_guard Lock(Mutex); + AllStatus.push_back(Status); + } + + std::vector AllStatus; + + private: + std::mutex Mutex; + } CaptureTUStatus; + MockFSProvider FS; + MockCompilationDatabase CDB; + ClangdServer Server(CDB, FS, CaptureTUStatus, ClangdServer::optsForTest()); + Annotations Code("int m^ain () {}"); + + // We schedule the following tasks in the queue: + // [Update] [GoToDefinition] + Server.addDocument(testPath("foo.cpp"), Code.code(), WantDiagnostics::Yes); + Server.findDefinitions(testPath("foo.cpp"), Code.point(), + [](Expected> Result) { + ASSERT_TRUE((bool)Result); + }); + + ASSERT_TRUE(Server.blockUntilIdleForTest()); + + EXPECT_THAT(CaptureTUStatus.AllStatus, + ElementsAre( + // Statuses of "Update" action. + TUState(TUAction::Queued, "Update"), + TUState(TUAction::RunningAction, "Update"), + TUState(TUAction::BuildingPreamble, "Update"), + TUState(TUAction::BuildingFile, "Update"), + + // Statuses of "Definitions" action + TUState(TUAction::Queued, "Definitions"), + TUState(TUAction::RunningAction, "Definitions"), + TUState(TUAction::Idle, /*No action*/ ""))); +} + +TEST_F(TUSchedulerTests, NoTUStatusEmittedForRemovedFile) { + class CaptureTUStatus : public DiagnosticsConsumer { + public: + void onDiagnosticsReady(PathRef File, + std::vector Diagnostics) override {} + + void onFileUpdated(PathRef File, const TUStatus &Status) override { + // Queued is emitted by the main thread, we don't block it. + if (Status.Action.S == TUAction::Queued) + return; + // Block the worker thread until the document is removed. + ASSERT_TRUE(Status.Action.S == TUAction::RunningAction); + Removed.wait(); + } + Notification Removed; + } CaptureTUStatus; + MockFSProvider FS; + MockCompilationDatabase CDB; + ClangdServer Server(CDB, FS, CaptureTUStatus, ClangdServer::optsForTest()); + + Server.addDocument(testPath("foo.cpp"), "int main() {}", + WantDiagnostics::Yes); + Server.removeDocument(testPath("foo.cpp")); + CaptureTUStatus.Removed.notify(); + ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for finishing"; +} + } // namespace } // namespace clangd } // namespace clang