hokein updated this revision to Diff 175135.
hokein marked 2 inline comments as done.
hokein added a comment.
Polish the code, address comments.
Repository:
rCTE Clang Tools Extra
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,36 @@
}
};
+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, 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
@@ -340,6 +340,8 @@
}
void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
+ FileStatus FStatus;
+ FStatus.uri = URIForFile(FileName);
auto Task = [=]() mutable {
// Will be used to check if we can avoid rebuilding the AST.
bool InputsAreTheSame =
@@ -350,7 +352,8 @@
bool PrevDiagsWereReported = DiagsWereReported;
FileInputs = Inputs;
DiagsWereReported = false;
-
+ FStatus.kind = FileStatusKind::BuildingPreamble;
+ Callbacks.onFileUpdated(FStatus);
log("Updating file {0} with command [{1}] {2}", FileName,
Inputs.CompileCommand.Directory,
join(Inputs.CompileCommand.CommandLine, " "));
@@ -361,6 +364,8 @@
elog("Could not build CompilerInvocation for file {0}", FileName);
// Remove the old AST if it's still in cache.
IdleASTs.take(this);
+ FStatus.messages.push_back("fail to build CompilerInvocation;");
+ Callbacks.onFileUpdated(FStatus);
// Make sure anyone waiting for the preamble gets notified it could not
// be built.
PreambleWasBuilt.notify();
@@ -386,7 +391,8 @@
// to it.
OldPreamble.reset();
PreambleWasBuilt.notify();
-
+ FStatus.kind = FileStatusKind::BuildingFile;
+ Callbacks.onFileUpdated(FStatus);
if (!CanReuseAST) {
IdleASTs.take(this); // Remove the old AST if it's still in cache.
} else {
@@ -403,13 +409,20 @@
// current file at this point?
log("Skipping rebuild of the AST for {0}, inputs are the same.",
FileName);
+ FStatus.kind = FileStatusKind::Ready;
+ FStatus.messages.push_back("reuse prebuilt AST;");
+ Callbacks.onFileUpdated(FStatus);
return;
}
}
// We only need to build the AST if diagnostics were requested.
- if (WantDiags == WantDiagnostics::No)
+ if (WantDiags == WantDiagnostics::No) {
+ FStatus.messages.push_back(
+ "skip building AST as no diagnostics were required;");
+ Callbacks.onFileUpdated(FStatus);
return;
+ }
{
std::lock_guard<std::mutex> Lock(DiagsMu);
@@ -440,10 +453,13 @@
Callbacks.onMainAST(FileName, **AST);
DiagsWereReported = true;
}
+ FStatus.kind = FileStatusKind::Ready;
+ Callbacks.onFileUpdated(FStatus);
// Stash the AST in the cache for further use.
IdleASTs.put(this, std::move(*AST));
};
-
+ FStatus.kind = FileStatusKind::Queued;
+ Callbacks.onFileUpdated(FStatus);
startTask("Update", std::move(Task), WantDiags);
}
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -895,6 +895,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
@@ -621,6 +621,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,13 +36,16 @@
namespace clangd {
+// FIXME: find a better name.
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 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
@@ -744,6 +744,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
