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

Krasimir Georgiev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 23 02:48:38 PDT 2017

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();
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.


More information about the cfe-commits mailing list