[PATCH] D43518: [clangd] Allow embedders some control over when diagnostics are generated.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/ClangdServer.h:282
   scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
+                          WantDiagnostics,
                           Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS);
----------------
Maybe add a parameter name here?
It's mostly a personal preference, I tend to copy-paste parameter lists between declaration/definition site if I change them. Missing parameter names totally break this workflow for me :-)


================
Comment at: clangd/TUScheduler.cpp:298
+      while (shouldSkipHeadLocked())
+        Requests.pop_front();
+      assert(!Requests.empty() && "skipped the whole queue");
----------------
Instead of skipping requests here we could try removing them from back of the queue in `startTask` (or rewriting the last request instead of adding a new one).
It feels the code could be simpler, as we will only ever have to remove a single request from the queue. And it could also keep the queue smaller in case of many subsequent `Auto` requests.
WDYT?


================
Comment at: clangd/TUScheduler.cpp:339
+    // Used unless followed by an update that generates diagnostics.
+    for (; Next != Requests.end(); ++Next)
+      if (Next->UpdateType == WantDiagnostics::Yes ||
----------------
Maybe skip updates directly in this function and make it return void?
Calling a function in a loop that loops through elements itself is a little confusing.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43518