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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to