[PATCH] D33415: [clangd] Replaced WorkerRequest with std::function...

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.


================
Comment at: clangd/ClangdServer.cpp:85
+        Request = std::move(RequestQueue.front());
+        RequestQueue.pop_front();
       } // unlock Mutex
----------------
Why are we taking it from the front and not from the back? Maybe at least add a comment about this behavior.


================
Comment at: clangd/ClangdServer.cpp:102
   } // unlock Mutex
+  RequestCV.notify_one();
   Worker.join();
----------------
Why did this get out?


================
Comment at: clangd/ClangdServer.cpp:230
+        Unit.dumpAST(ResultOS);
+      }
+      DumpPromise.set_value(std::move(Result));
----------------
Maybe an explicit flush and no block?


================
Comment at: clangd/ClangdUnitStore.h:56
+  template <class Func>
+  void runOnExistingUnit(PathRef File, Func Action) {
+    std::lock_guard<std::mutex> Lock(Mutex);
----------------
Maybe make it less generic and put the implementation in the source file? It uses std::function anyways.


https://reviews.llvm.org/D33415