ilya-biryukov added inline comments.

Comment at: clangd/Threading.h:60
+/// A point in time we may wait for, or None to wait forever.
+/// (We use Optional because buggy implementations of std::chrono overflow...)
+using Deadline = llvm::Optional<std::chrono::steady_clock::time_point>;
Maybe remove the comment or add more context (i.e. add references) on why the 
overflow is buggy?

Comment at: clangd/Threading.h:66
+template <typename Func>
+LLVM_NODISCARD bool wait(std::mutex &Mutex, std::condition_variable &CV,
+                         Deadline D, Func F) {
Maybe move this helper to .cpp file?

Comment at: clangd/Threading.h:68
+                         Deadline D, Func F) {
+  std::unique_lock<std::mutex> Lock(Mutex);
+  if (D)
Maybe keep the locking part out of this helper? It's often desirable to hold 
the lock after `wait()`. This will model how `std::condition_variable::wait` is 

Comment at: clangd/Threading.h:83
-  void waitForAll();
+  bool waitForAll(Deadline D = llvm::None) const;
   void runAsync(UniqueFunction<void()> Action);
For that particular method maybe we could have two overload: with and without 
the deadline, i.e.
  void waitForAll() const;
  LLVM_NODISCARD bool waitForAll(Deadline D) const;

There are places (like the destructor of this class) where the first overload 
is used and consuming the return value is just adding noise, but clients 
passing the deadline (e.g. `blockUntilIdle()`) should definitely consume the 
return value.

Comment at: unittests/clangd/ClangdTests.cpp:784
   class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer {
-  public:
-    NoConcurrentAccessDiagConsumer(std::promise<void> StartSecondReparse)
-        : StartSecondReparse(std::move(StartSecondReparse)) {}
-    void onDiagnosticsReady(
-        PathRef File,
-        Tagged<std::vector<DiagWithFixIts>> Diagnostics) override {
+    std::atomic<bool> InCallback = {false};
Do we need to change this test?
It was specifically designed to keep the second request from overriding the 
first one before it was processed.

  rCTE Clang Tools Extra

cfe-commits mailing list

Reply via email to