hokein updated this revision to Diff 175277.
hokein marked 2 inline comments as done.
hokein added a comment.
Herald added a subscriber: jfb.
stop emitting file status when ASTworker is shutting down.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54796/new/
https://reviews.llvm.org/D54796
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/Protocol.cpp
clangd/Protocol.h
clangd/TUScheduler.cpp
clangd/TUScheduler.h
unittests/clangd/ClangdTests.cpp
Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -44,6 +44,8 @@
namespace {
+MATCHER_P(FileState, State, "") { return arg.kind == State; }
+
bool diagsContainErrors(const std::vector<Diag> &Diagnostics) {
for (auto D : Diagnostics) {
if (D.Severity == DiagnosticsEngine::Error ||
@@ -154,6 +156,64 @@
}
};
+TEST_F(ClangdVFSTest, FileStatus) {
+ class CaptureFileStatus : public DiagnosticsConsumer {
+ public:
+ void onDiagnosticsReady(PathRef File,
+ std::vector<Diag> Diagnostics) override {}
+
+ void onFileUpdated(const FileStatus &FStatus) override {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ AllStatus.push_back(FStatus);
+ }
+
+ std::vector<FileStatus> AllStatus;
+
+ private:
+ std::mutex Mutex;
+ } CaptureFStatus;
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
+ ClangdServer Server(CDB, FS, CaptureFStatus, ClangdServer::optsForTest());
+ Server.addDocument(testPath("foo.cpp"), "int main() {}",
+ WantDiagnostics::Yes);
+ ASSERT_TRUE(Server.blockUntilIdleForTest());
+
+ EXPECT_THAT(CaptureFStatus.AllStatus,
+ ElementsAre(FileState(FileStatusKind::Queued),
+ FileState(FileStatusKind::BuildingPreamble),
+ FileState(FileStatusKind::BuildingFile),
+ FileState(FileStatusKind::Ready)));
+}
+
+TEST_F(ClangdVFSTest, NoFileStatusEmittedForRemovedFile) {
+ class CaptureFileStatus : public DiagnosticsConsumer {
+ public:
+ void onDiagnosticsReady(PathRef File,
+ std::vector<Diag> Diagnostics) override {}
+
+ void onFileUpdated(const FileStatus &FStatus) override {
+ ASSERT_TRUE(FirstRequest)
+ << "Expected to see exacly one file status updated.";
+ if (FirstRequest) {
+ ASSERT_TRUE(FStatus.kind == FileStatusKind::Queued);
+ FirstRequest = false;
+ // We wait long enough that the document gets removed.
+ std::this_thread::sleep_for(std::chrono::milliseconds(50));
+ }
+ }
+ std::atomic<bool> FirstRequest = {true};
+ } CaptureFStatus;
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
+ ClangdServer Server(CDB, FS, CaptureFStatus, ClangdServer::optsForTest());
+ Server.addDocument(testPath("foo.cpp"), "int main() {}",
+ WantDiagnostics::Yes);
+ Server.removeDocument(testPath("foo.cpp"));
+ ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for finishing";
+ ASSERT_TRUE(!CaptureFStatus.FirstRequest);
+}
+
TEST_F(ClangdVFSTest, Parse) {
// FIXME: figure out a stable format for AST dumps, so that we can check the
// output of the dump itself is equal to the expected one, not just that it's
Index: clangd/TUScheduler.h
===================================================================
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -75,6 +75,9 @@
/// Called whenever the diagnostics for \p File are produced.
virtual void onDiagnostics(PathRef File, std::vector<Diag> Diags) {}
+
+ /// Called whenever the file status is updated.
+ virtual void onFileUpdated(const FileStatus &FStatus) {}
};
/// Handles running tasks for ClangdServer and managing the resources (e.g.,
Index: clangd/TUScheduler.cpp
===================================================================
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -251,13 +251,18 @@
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.
+ /// 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
// don't. When the old handle is destroyed, the old worker will stop reporting
// diagnostics.
bool ReportDiagnostics = true; /* GUARDED_BY(DiagMu) */
+ /// Guards a critical section for running the file status callbacks.
+ std::mutex FileStatusMu;
+ // Similar to ReportDiagnostics, used to prevent remove document + emitting
+ // out-of-order file status.
+ bool ReportFileStatus = true; /* GUARDED_BY(FileStatusMu) */
};
/// A smart-pointer-like class that points to an active ASTWorker.
@@ -340,7 +345,21 @@
}
void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
+ FileStatus FStatus;
+ FStatus.uri = URIForFile(FileName);
auto Task = [=]() mutable {
+ auto EmitFileStatus = [&FStatus, this](FileStatusKind State,
+ StringRef Message = "") {
+ FStatus.kind = State;
+ if (!Message.empty())
+ FStatus.messages.push_back(Message);
+ {
+ std::lock_guard<std::mutex> Lock(FileStatusMu);
+ // Stop emitting file statuses when the ASTWorker is shutting down.
+ if (ReportFileStatus)
+ Callbacks.onFileUpdated(FStatus);
+ }
+ };
// Will be used to check if we can avoid rebuilding the AST.
bool InputsAreTheSame =
std::tie(FileInputs.CompileCommand, FileInputs.Contents) ==
@@ -350,7 +369,7 @@
bool PrevDiagsWereReported = DiagsWereReported;
FileInputs = Inputs;
DiagsWereReported = false;
-
+ EmitFileStatus(FileStatusKind::BuildingPreamble);
log("Updating file {0} with command [{1}] {2}", FileName,
Inputs.CompileCommand.Directory,
join(Inputs.CompileCommand.CommandLine, " "));
@@ -361,6 +380,8 @@
elog("Could not build CompilerInvocation for file {0}", FileName);
// Remove the old AST if it's still in cache.
IdleASTs.take(this);
+ EmitFileStatus(FileStatusKind::BuildingPreamble,
+ "fail to build CompilerInvocation;");
// Make sure anyone waiting for the preamble gets notified it could not
// be built.
PreambleWasBuilt.notify();
@@ -386,7 +407,7 @@
// to it.
OldPreamble.reset();
PreambleWasBuilt.notify();
-
+ EmitFileStatus(FileStatusKind::BuildingFile);
if (!CanReuseAST) {
IdleASTs.take(this); // Remove the old AST if it's still in cache.
} else {
@@ -403,13 +424,17 @@
// current file at this point?
log("Skipping rebuild of the AST for {0}, inputs are the same.",
FileName);
+ EmitFileStatus(FileStatusKind::Ready, "reuse prebuilt AST;");
return;
}
}
// We only need to build the AST if diagnostics were requested.
- if (WantDiags == WantDiagnostics::No)
+ if (WantDiags == WantDiagnostics::No) {
+ EmitFileStatus(FileStatusKind::BuildingFile,
+ "skip building AST as no diagnostics were required;");
return;
+ }
{
std::lock_guard<std::mutex> Lock(DiagsMu);
@@ -440,10 +465,18 @@
Callbacks.onMainAST(FileName, **AST);
DiagsWereReported = true;
}
+ EmitFileStatus(FileStatusKind::Ready);
// Stash the AST in the cache for further use.
IdleASTs.put(this, std::move(*AST));
};
+ {
+ std::lock_guard<std::mutex> Lock(FileStatusMu);
+ if (ReportFileStatus) {
+ FStatus.kind = FileStatusKind::Queued;
+ Callbacks.onFileUpdated(FStatus);
+ }
+ }
startTask("Update", std::move(Task), WantDiags);
}
@@ -533,6 +566,10 @@
std::lock_guard<std::mutex> Lock(DiagsMu);
ReportDiagnostics = false;
}
+ {
+ std::lock_guard<std::mutex> Lock(FileStatusMu);
+ ReportFileStatus = false;
+ }
{
std::lock_guard<std::mutex> Lock(Mutex);
assert(!Done && "stop() called twice");
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -934,6 +934,30 @@
};
bool fromJSON(const llvm::json::Value &, ReferenceParams &);
+enum class FileStatusKind {
+ PreparingBuild = 1, // build system is preparing for building the file, e.g.
+ // getting compilation command.
+ Queued = 2, // The file is pending to be built.
+ BuildingPreamble = 3, // The preamble of the file is being built.
+ BuildingFile = 4, // The file is being built.
+ Ready = 5, // The file is ready.
+};
+
+/// Clangd extension: a file status indicates the current status of the file in
+/// clangd, sent via the `window/showMessage` notification.
+struct FileStatus {
+ /// The text document's URI.
+ URIForFile uri;
+ /// The current state of the file.
+ FileStatusKind kind;
+ /// Messages of the file status, which can be shown in the status
+ /// bar. These can be details worth surfacing to users, e.g. build system
+ /// doesn't know how to build the file (using a fallback compilation command);
+ /// compiler fails to build the AST.
+ std::vector<std::string> messages;
+};
+llvm::json::Value toJSON(const FileStatus &FStatus);
+
} // namespace clangd
} // namespace clang
Index: clangd/Protocol.cpp
===================================================================
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -647,6 +647,14 @@
};
}
+llvm::json::Value toJSON(const FileStatus &FStatus) {
+ return json::Object{
+ {"uri", FStatus.uri},
+ {"kind", static_cast<int>(FStatus.kind)},
+ {"message", join(FStatus.messages.begin(), FStatus.messages.end(), "\n")},
+ };
+}
+
bool fromJSON(const json::Value &Params, RenameParams &R) {
json::ObjectMapper O(Params);
return O && O.map("textDocument", R.textDocument) &&
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -36,6 +36,7 @@
namespace clangd {
+// FIXME: find a better name.
class DiagnosticsConsumer {
public:
virtual ~DiagnosticsConsumer() = default;
@@ -43,6 +44,8 @@
/// 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(const FileStatus &FStatus){};
};
/// 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(const FileStatus &FStatus) override {
+ DiagConsumer.onFileUpdated(FStatus);
+ }
+
private:
FileIndex *FIndex;
DiagnosticsConsumer &DiagConsumer;
@@ -135,6 +139,7 @@
void ClangdServer::addDocument(PathRef File, StringRef Contents,
WantDiagnostics WantDiags) {
+ // FIXME: emit PreparingBuild file status.
WorkScheduler.update(File,
ParseInputs{getCompileCommand(File),
FSProvider.getFileSystem(), Contents.str()},
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -52,6 +52,7 @@
private:
// Implement DiagnosticsConsumer.
void onDiagnosticsReady(PathRef File, std::vector<Diag> Diagnostics) override;
+ void onFileUpdated(const FileStatus &FStatus) override;
// LSP methods. Notifications have signature void(const Params&).
// Calls have signature void(const Params&, Callback<Response>).
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -783,6 +783,10 @@
});
}
+void ClangdLSPServer::onFileUpdated(const FileStatus &FStatus) {
+ notify("window/showMessage", FStatus);
+}
+
void ClangdLSPServer::reparseOpenedFiles() {
for (const Path &FilePath : DraftMgr.getActiveFiles())
Server->addDocument(FilePath, *DraftMgr.getDraft(FilePath),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
