[clang-tools-extra] r348475 - [clangd] C++ API for emitting file status.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 6 01:41:05 PST 2018
Author: hokein
Date: Thu Dec 6 01:41:04 2018
New Revision: 348475
URL: http://llvm.org/viewvc/llvm-project?rev=348475&view=rev
Log:
[clangd] C++ API for emitting file status.
Introduce clangd C++ API to emit the current status of file.
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=348475&r1=348474&r2=348475&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Dec 6 01:41:04 2018
@@ -86,6 +86,10 @@ struct UpdateIndexCallbacks : public Par
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 @@ ClangdServer::ClangdServer(const GlobalC
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()},
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=348475&r1=348474&r2=348475&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Thu Dec 6 01:41:04 2018
@@ -37,6 +37,7 @@ class PCHContainerOperations;
namespace clangd {
+// FIXME: find a better name.
class DiagnosticsConsumer {
public:
virtual ~DiagnosticsConsumer() = default;
@@ -44,6 +45,8 @@ public:
/// Called by ClangdServer when \p Diagnostics for \p File are ready.
virtual void onDiagnosticsReady(PathRef File,
std::vector<Diag> 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),
Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=348475&r1=348474&r2=348475&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Thu Dec 6 01:41:04 2018
@@ -205,6 +205,10 @@ private:
/// Adds a new task to the end of the request queue.
void startTask(StringRef Name, unique_function<void()> Task,
Optional<WantDiagnostics> 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 @@ private:
ParsingCallbacks &Callbacks;
/// Helper class required to build the ASTs.
const std::shared_ptr<PCHContainerOperations> PCHs;
+ /// Only accessed by the worker thread.
+ TUStatus Status;
Semaphore &Barrier;
/// Inputs, corresponding to the current state of AST.
@@ -251,7 +257,9 @@ private:
bool Done; /* GUARDED_BY(Mutex) */
std::deque<Request> 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,9 @@ ASTWorker::ASTWorker(PathRef FileName, T
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 +349,7 @@ ASTWorker::~ASTWorker() {
}
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 +360,7 @@ void ASTWorker::update(ParseInputs Input
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 +371,9 @@ void ASTWorker::update(ParseInputs Input
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 +399,7 @@ void ASTWorker::update(ParseInputs Input
// 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 +416,9 @@ void ASTWorker::update(ParseInputs Input
// 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;
}
}
@@ -425,6 +441,16 @@ void ASTWorker::update(ParseInputs Input
Optional<ParsedAST> NewAST =
buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs);
AST = NewAST ? llvm::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);
}
// We want to report the diagnostics even if this update was cancelled.
// It seems more useful than making the clients wait indefinitely if they
@@ -443,8 +469,7 @@ void ASTWorker::update(ParseInputs Input
// 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 +585,18 @@ void ASTWorker::startTask(StringRef Name
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(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;
@@ -581,11 +618,13 @@ void ASTWorker::run() {
Ctx.emplace(Requests.front().Ctx.clone());
Tracer.emplace("Debounce");
SPAN_ATTACH(*Tracer, "next_request", Requests.front().Name);
- if (!(Wait == Deadline::infinity()))
+ if (!(Wait == Deadline::infinity())) {
+ emitTUStatus({TUAction::Queued, Req.Name});
SPAN_ATTACH(*Tracer, "sleep_ms",
std::chrono::duration_cast<std::chrono::milliseconds>(
Wait.time() - steady_clock::now())
.count());
+ }
}
wait(Lock, RequestsCV, Wait);
@@ -595,16 +634,23 @@ void ASTWorker::run() {
} // unlock Mutex
{
+ // FIXME: only emit this status when the Barrier couldn't be acquired.
+ emitTUStatus({TUAction::Queued, Req.Name});
std::lock_guard<Semaphore> 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<std::mutex> Lock(Mutex);
Requests.pop_front();
+ IsEmpty = Requests.empty();
}
+ if (IsEmpty)
+ emitTUStatus({TUAction::Idle, /*Name*/ ""});
RequestsCV.notify_all();
}
}
Modified: clang-tools-extra/trunk/clangd/TUScheduler.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.h?rev=348475&r1=348474&r2=348475&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.h (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.h Thu Dec 6 01:41:04 2018
@@ -52,6 +52,36 @@ struct ASTRetentionPolicy {
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 @@ public:
/// Called whenever the diagnostics for \p File are produced.
virtual void onDiagnostics(PathRef File, std::vector<Diag> 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.,
Modified: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp?rev=348475&r1=348474&r2=348475&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp Thu Dec 6 01:41:04 2018
@@ -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 <algorithm>
#include <utility>
@@ -23,6 +24,7 @@ namespace clangd {
namespace {
using ::testing::_;
+using ::testing::AllOf;
using ::testing::AnyOf;
using ::testing::Each;
using ::testing::ElementsAre;
@@ -30,6 +32,10 @@ using ::testing::Pair;
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 @@ TEST_F(TUSchedulerTests, Run) {
EXPECT_EQ(Counter.load(), 3);
}
+TEST_F(TUSchedulerTests, TUStatus) {
+ class CaptureTUStatus : public DiagnosticsConsumer {
+ public:
+ void onDiagnosticsReady(PathRef File,
+ std::vector<Diag> Diagnostics) override {}
+
+ void onFileUpdated(PathRef File, const TUStatus &Status) override {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ AllStatus.push_back(Status);
+ }
+
+ std::vector<TUStatus> 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<std::vector<Location>> 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<Diag> 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
More information about the cfe-commits
mailing list