This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Remove support for pre-standard semanticHighlighting notification
ClosedPublic

Authored by sammccall on Jan 27 2021, 4:17 PM.

Details

Summary

This is obsoleted by the standard semanticTokens request family.
As well as the protocol details, this allows us to remove a bunch of plumbing
around pushing highlights to clients.

This should not land until the new protocol has feature parity, see D77702.

Diff Detail

Event Timeline

sammccall created this revision.Jan 27 2021, 4:17 PM
sammccall requested review of this revision.Jan 27 2021, 4:17 PM
kadircet accepted this revision.Jan 28 2021, 12:16 AM

Thanks, LGTM!

there are also some comments on ASTWorker::PublishMu talking about semantic highlights publishing.

clang-tools-extra/clangd/SemanticHighlighting.h
15

not sure if this sentence will help anyone. if you choose to drop this one, i suppose the former can also be rephrased as Protocol details can be found at ...

This revision is now accepted and ready to land.Jan 28 2021, 12:16 AM

oh also in addition to This should not land until the new protocol has feature parity, see D77702., it might be better to hold on to it a little bit longer, since it is touching lots of pieces and might make cherry-picking some fixes that touch the same files harder to the release branch.

sammccall marked an inline comment as done.Jan 28 2021, 12:24 AM

oh also in addition to This should not land until the new protocol has feature parity, see D77702., it might be better to hold on to it a little bit longer, since it is touching lots of pieces and might make cherry-picking some fixes that touch the same files harder to the release branch.

Yeah I don't want to land this for a couple of weeks in any case.
On the other hand, the release branch has sometimes been long-lived (months) and I don't think we should try to avoid invasive changes until the release has landed necessarily, it could eat a good fraction of the release cycle.

Remove some comments

Rebase, now the feature parity stuff is in

This revision was landed with ongoing or failed builds.Feb 10 2021, 1:12 PM
This revision was automatically updated to reflect the committed changes.
clang-tools-extra/clangd/TUScheduler.cpp