[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