ilya-biryukov added inline comments.

Comment at: clangd/TUScheduler.cpp:140
+  // Time to wait after an update to see whether another update obsoletes it.
+  steady_clock::duration UpdateDebounce;
Maybe make it `const` and put beside `RunSync`? Both are scheduling options, so 
it probably makes sense to group them together.

Comment at: clangd/TUScheduler.cpp:324
+        if (*Deadline)
+          RequestsCV.wait_until(Lock, **Deadline);
+        else
It looks like if we unwrap `Optional<Deadline>` to `Deadline`, we could replace 
this code with `wait` helper from `Threading.h`.
The tracing code (e.g. `if (!Requests.empty) { /*...*/}`) could be changed to 
log only when `*Deadline - steady_clock::now()` is positive.
Will probably make the code simpler. WDYT?

Comment at: clangd/TUScheduler.cpp:358
+  for (const auto &R : Requests)
+    if (R.UpdateType == None || R.UpdateType == WantDiagnostics::Yes)
+      return None;
NIT: I tend to find multi-level nested statements easier to read with braces, 
for (const auto &R : Requests) {
  if (Cond)
    return None
But this is up to you.

Comment at: clangd/TUScheduler.h:55
+              ASTParsedCallback ASTCallback,
+              std::chrono::steady_clock::duration UpdateDebounce =
+                  std::chrono::milliseconds(500));
As discussed offline, we want to expose the debounce parameter in 
`ClangdServer`, as there are existing clients of the code that already send 
updates with low frequency and it would be wasteful for them to do any extra 

  rCTE Clang Tools Extra

cfe-commits mailing list

Reply via email to