[PATCH] D81456: [clangd] Get rid of WantDiagnostics::Yes

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 2 04:48:11 PDT 2020


sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:670
   auto Version = DraftMgr.addDraft(File, Params.textDocument.version, Contents);
-  Server->addDocument(File, Contents, encodeVersion(Version),
-                      WantDiagnostics::Yes);
+  Server->addDocument(File, Contents, encodeVersion(Version));
 }
----------------
kadircet wrote:
> sammccall wrote:
> > This cuts against the assertion in the description that there are no uses!
> > 
> > And I believe the debounce test demonstrates that that we will in fact debounce an initial request that's `Auto`, right?
> > So isn't this going to add 0.5sec to time-before-first-diagnostics in all cases?
> ah right, let's reconsider this after having a special initial AST build (one without preamble serialization/deserialization and clang-tidy) to reduce first diagnostic latency.
Yeah, 0.5s isn't a killer so we *could* sequence it the other way.
But it'd be a bit unfortunate as the temporary regression would probably make it into the 11 release, so since this is "just" tech debt cleanup it'd be nice to hold off.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81456/new/

https://reviews.llvm.org/D81456





More information about the cfe-commits mailing list