[PATCH] D143260: [clangd] Add semantic token for labels

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

sorry for long silence.

> But access specifiers are a completely different thing semantically, that's the point: The user does not tell the client: "I want everything  that is followed by a single colon in this color"; that would be silly. They say "I want goto labels in this color", exactly because then they immediately stand out compared to access specifiers.

I am not sure if access specifiers and label declarations occur in the same context often enough for this mixing to actually cause trouble TBH.

---

I was mostly being cautious for new semantic highlighting token. I believe today we've too low of a bar in clangd's semantic highlighting functionality for introducing "custom" token types, which are probably not used by any people apart from the ones involved in the introduction of the token (we don't have custom themes/scopes even for vscode, we don't even mention them in our user-facing documentation).
So I feel like these custom semantic token types are getting us into "death by thousand cuts" situation. Each of these changes are small enough on their own, but we'll slowly get into a state in which we're spending time both calculating and emitting all of those token types that are just dropped on the floor (and also sometimes the extra cost to maintain them as language semantics change).

Hence my stance here is still towards "we don't need it", unless we have some big editor(s) that can already recognize "label" as a semantic token type to provide the functionality for a set of users. that way we can justify of maintaining the code and the runtime costs. (or finding a way to make this more closely aligned with an existing token type/modifier)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143260