[PATCH] D136594: [clangd] Add support for semantic token type "operator"

Christian Kandeler via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 21 00:37:10 PST 2022


ckandeler added a comment.

In D136594#3940143 <https://reviews.llvm.org/D136594#3940143>, @nridge wrote:

> A couple of high-level thoughts on this:
>
> 1. Based on the discussion in https://github.com/clangd/clangd/issues/1115, I believe highlighting of **built-in** operators should be out of scope for semantic highlighting, at least in the default mode; client-side highlighting should be sufficient for these, similar to strings and literals.
> 2. An alternative to assigning (user-provided) operators a new token kind would be to assign them the same token kind as the entity they invoke (i.e. function or method). Both approaches have their advantages:
>   - If we use the function/method kinds, then uses of user-provided operators will be highlighted differently from built-in operators even when using a default / standard theme that doesn't know about clangd-specific token types.
>   - If we use a dedicated operator kind, users can configure different styles for operators vs. function/methods (and they may want different styles given that syntactically the two look quite different).
>
>     One way to get the best of both worlds could be to use the function/method kinds in combination with an `operator` **modifier**. That would color overloaded operators out of the box while also allowing users to customize the style based on the presence of the modifier. What do you think about this approach?

The problem with "operator" as a modifier is that if and when the augmentsSyntaxTokens feature is implemented, we'll need an operator token kind anyway (and having both seems weird).
The modifier semantics could also be switched around, btw, so that instead of "userProvided" we could have "builtIn" (which might also apply to other token kinds). I don't have a strong opinion on this.
Should I perhaps add the augmentsSyntaxTokens option and rebase this patch to make it its first user?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136594



More information about the cfe-commits mailing list