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

Sat Apr 15 01:06:50 PDT 2023

nridge added a comment.

In D143260#4267883 <https://reviews.llvm.org/D143260#4267883>, @kadircet wrote:

> 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 think the focus on access specifiers is a bit of a distraction, especially given the mentioned "labels as values" language extension, which makes possible usages of the label of the form `&&labelname` in any expression context (producing a pointer value).

In my mind, having a semantic token for labels is a matter of completeness: all other identifier tokens get some sort of semantic token depending on what kind of `Decl` they resolve to. `LabelDecl` is a kind of `Decl` that can be declared as `label:`, and referenced as `goto label` or `&&label` where `label` is lexically an identifier token -- so it makes sense for those identifiers to be covered by a semantic token as well.

> 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).

In the case of labels (and angle brackets (D139926 <https://reviews.llvm.org/D139926>) and operators (D136594 <https://reviews.llvm.org/D136594>)), the set of users benefiting from them is presumably "all QtCreator users" (assuming QtCreator assigns a default color to these).

You're right that vscode users don't benefit from them unless they configure `semanticTokenColorCustomizations`; in my mind, this is a reason for us to write some vscode themes, rather than to avoid adding new tokens. (I volunteer to write vscode themes if that helps change your mind.)

> 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

I agree that the volume of semantic tokens is potentially a concern. I think it's more of a concern in the case of tokens that occur fairly frequently, like operator tokens. (This is why, in the review of D136594 <https://reviews.llvm.org/D136594>, I suggested only emitting semantic tokens for overloaded operators, since it's distinguishing those from built-in operators that's the more interesting use case in highlighting operators.) By comparison, label tokens would be relatively rare and thus their overhead would be quite small.

> 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

I'm not sure whether you mean LSP-based editors, but if we're counting editors that had "label" as a semantic token type pre-LSP, and are now looking to maintain that through their switch to LSP, then both Eclipse CDT and (I assume) Qt Creator fall into that category.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143260