Page MenuHomePhabricator

[clangd] Get rid of WantDiagnostics::Yes
Changes PlannedPublic

Authored by kadircet on Jun 9 2020, 4:29 AM.

Details

Reviewers
sammccall
Summary

This is an extension clangd provides in LSP layer and to embedders of
ClangdServer. Currently there are no users of WantDiagnostics::Yes apart from
our tests, as from a practical point of view it is quite similar to
WantDiagnostics::Auto.

This patch gets rid of WantDiagnostics::Yes to get rid of some logic in
TUScheduler that handles it.

Tests are updated using the facts that an update is executed if:

  • there's a read following it, or
  • it is the last update in the request queue.

Diff Detail

Unit TestsFailed

TimeTest
80 mslinux > Clang-Unit.StaticAnalyzer/_/StaticAnalysisTests::Unknown Unit Message ("")
Note: Google Test filter = TestReturnValueUnderConstructionChecker.ReturnValueUnderConstructionChecker [==========] Running 1 test from 1 test case. [----------] Global test environment set-up.
440 mslinux > LLVM.CodeGen/AMDGPU::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/premerge-debian-5d5d95fcbb-cs8pv-2/llvm-project/premerge-checks/build/bin/llc -march=amdgcn -mcpu=gfx1010 -mattr=-nsa-encoding -verify-machineinstrs -show-mc-encoding < /mnt/disks/ssd0/agent/premerge-debian-5d5d95fcbb-cs8pv-2/llvm-project/premerge-checks/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.image.nsa.ll | /mnt/disks/ssd0/agent/premerge-debian-5d5d95fcbb-cs8pv-2/llvm-project/premerge-checks/build/bin/FileCheck -check-prefixes=GCN,NONSA /mnt/disks/ssd0/agent/premerge-debian-5d5d95fcbb-cs8pv-2/llvm-project/premerge-checks/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.image.nsa.ll
20 mslinux > LLVM.MC/AArch64::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/premerge-debian-5d5d95fcbb-cs8pv-2/llvm-project/premerge-checks/build/bin/llvm-mc -triple aarch64-none-linux-gnu /mnt/disks/ssd0/agent/premerge-debian-5d5d95fcbb-cs8pv-2/llvm-project/premerge-checks/llvm/test/MC/AArch64/mov-expression-as-immediate.s -filetype=obj -o /mnt/disks/ssd0/agent/premerge-debian-5d5d95fcbb-cs8pv-2/llvm-project/premerge-checks/build/test/MC/AArch64/Output/mov-expression-as-immediate.s.tmp | /mnt/disks/ssd0/agent/premerge-debian-5d5d95fcbb-cs8pv-2/llvm-project/premerge-checks/build/bin/llvm-objdump -d /mnt/disks/ssd0/agent/premerge-debian-5d5d95fcbb-cs8pv-2/llvm-project/premerge-checks/build/test/MC/AArch64/Output/mov-expression-as-immediate.s.tmp | /mnt/disks/ssd0/agent/premerge-debian-5d5d95fcbb-cs8pv-2/llvm-project/premerge-checks/build/bin/FileCheck /mnt/disks/ssd0/agent/premerge-debian-5d5d95fcbb-cs8pv-2/llvm-project/premerge-checks/llvm/test/MC/AArch64/mov-expression-as-immediate.s

Event Timeline

kadircet created this revision.Jun 9 2020, 4:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 9 2020, 4:29 AM

Sorry for the long delay here...

clang-tools-extra/clangd/ClangdLSPServer.cpp
670

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?

kadircet planned changes to this revision.Jul 2 2020, 4:05 AM
kadircet marked an inline comment as done.
kadircet added inline comments.
clang-tools-extra/clangd/ClangdLSPServer.cpp
670

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.

sammccall added inline comments.Jul 2 2020, 4:22 AM
clang-tools-extra/clangd/ClangdLSPServer.cpp
670

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.