[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

The callback will be invoked in clangd when we receive a reply from the client.

unfortunately, we don't have any unittest for ClangdLSPServer .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65387

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h


Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -146,7 +146,16 @@
   std::unique_ptr<MessageHandler> MsgHandler;
   std::atomic<int> NextCallID = {0};
   std::mutex TranspWriter;
-  void call(StringRef Method, llvm::json::Value Params);
+
+  // A callback that will be invoked when receiving a client reply for a server
+  // request.
+  using ReplyCallback = std::function<void(llvm::Expected<llvm::json::Value>)>;
+  std::mutex ReplyCallbacksMutex;
+  // Request ID => reply Callbacks
+  llvm::StringMap<ReplyCallback> ReplyCallbacks;
+
+  void call(StringRef Method, llvm::json::Value Params, ReplyCallback CB);
+  void onReply(llvm::json::Value ID, llvm::Expected<llvm::json::Value> Result);
   void notify(StringRef Method, llvm::json::Value Params);
 
   const FileSystemProvider &FSProvider;
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -146,11 +146,9 @@
   bool onReply(llvm::json::Value ID,
                llvm::Expected<llvm::json::Value> Result) override {
     WithContext HandlerContext(handlerContext());
-    // We ignore replies, just log them.
-    if (Result)
-      log("<-- reply({0})", ID);
-    else
-      log("<-- reply({0}) error: {1}", ID, llvm::toString(Result.takeError()));
+    // Log the reply.
+    log("<-- reply({0})", ID);
+    Server.onReply(std::move(ID), std::move(Result));
     return true;
   }
 
@@ -310,14 +308,40 @@
 };
 
 // call(), notify(), and reply() wrap the Transport, adding logging and locking.
-void ClangdLSPServer::call(llvm::StringRef Method, llvm::json::Value Params) {
+void ClangdLSPServer::call(llvm::StringRef Method, llvm::json::Value Params,
+                           ReplyCallback CB) {
   auto ID = NextCallID++;
+  auto StrID = llvm::to_string(ID);
+  {
+    std::lock_guard<std::mutex> Lock(ReplyCallbacksMutex);
+    ReplyCallbacks[StrID] = std::move(CB);
+  }
   log("--> {0}({1})", Method, ID);
-  // We currently don't handle responses, so no need to store ID anywhere.
   std::lock_guard<std::mutex> Lock(TranspWriter);
   Transp.call(Method, std::move(Params), ID);
 }
 
+void ClangdLSPServer::onReply(llvm::json::Value ID,
+                              llvm::Expected<llvm::json::Value> Result) {
+  auto StrID = llvm::to_string(ID);
+  ReplyCallback CB = nullptr;
+  {
+    std::lock_guard<std::mutex> Lock(ReplyCallbacksMutex);
+    CB = ReplyCallbacks.lookup(StrID);
+    ReplyCallbacks.erase(StrID);
+  }
+  // No callback for the client reply, just log them.
+  if (!CB) {
+    if (Result)
+      log("no corresponding callback for the reply ({0})", ID);
+    else
+      log("no corresponding callback for the reply ({0}) error: {1}", ID,
+          llvm::toString(Result.takeError()));
+    return;
+  }
+  CB(std::move(Result));
+}
+
 void ClangdLSPServer::notify(llvm::StringRef Method, llvm::json::Value Params) {
   log("--> {0}", Method);
   std::lock_guard<std::mutex> Lock(TranspWriter);
@@ -501,7 +525,7 @@
     Edit.edit = std::move(WE);
     // Ideally, we would wait for the response and if there is no error, we
     // would reply success/failure to the original RPC.
-    call("workspace/applyEdit", Edit);
+    call("workspace/applyEdit", Edit, /*ReplyCallback=*/nullptr);
   };
   if (Params.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND &&
       Params.workspaceEdit) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65387.212140.patch
Type: text/x-patch
Size: 3629 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190729/0a0d4de1/attachment.bin>