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

This revision was automatically updated to reflect the committed changes. Closed by commit rGe72baa76b91f: [clangd] Add semantic token for labels (authored by ckandeler). Changed prior to commit: https://reviews.llvm.org/D143260?vs=494583=529234#toc Repository: rG LLVM Github Monorepo

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

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. alright then, this patch LGTM. going forward let's try to introduce new kinds (or handling for different semantic constructs) in a more holistic manner. concerns with new language structs

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

ckandeler added a comment. In D143260#4328948 , @kadircet wrote: > Considering that we have added a bunch of token kinds, let me ask you another > question then, how many more new token kinds do we expect to have? e.g. if > label is the last one sure,

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

nridge added a comment. In D143260#4328948 , @kadircet wrote: > Considering that we have added a bunch of token kinds, let me ask you another > question then, how many more new token kinds do we expect to have? e.g. if > label is the last one sure, but

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

kadircet added a comment. (sorry for taking so long to get back to this) In D143260#4270807 , @nridge wrote: > In the case of labels (and angle brackets (D139926 > ) and operators (D136594 >

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

nridge added a comment. In 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

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

ckandeler added a comment. In my opinion, it is not possible to have a competitive client if you limit yourself to the official LSP feature set; you just need language-specific extensions in practice. And while of course not every silly idea should be blindly accepted, I think this "lowest

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

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

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

ckandeler added a comment. Hello? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143260/new/ https://reviews.llvm.org/D143260 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

ckandeler added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143260/new/ https://reviews.llvm.org/D143260 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

nridge added a comment. Another potential consideration here is the GNU "labels as values " language extension, which clang seems to support, and which allows referencing a `LabelDecl` from a context other than a goto-statement (and

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

nridge added a comment. FWIW, I agree that switch labels and goto labels are conceptually quite different: the thing before a switch label is an expression, and if it's an identifier it references an already-declared entity with its own kind (e.g. enumerator), whereas a goto label basically

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

ckandeler added a comment. In D143260#4122523 , @kadircet wrote: > at hindsight i can't see why `goto X;` and `X:` is not enough for clients to > implement this without any need for semantic analysis. are there contexts > where this kind of syntactical

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

kadircet added a comment. thanks for the patch, but could you elaborate a little bit on "why this is useful for clients". in theory semantic highlighting tries to provide annotations for tokens that are hard to disambiguate by a syntax highlighter due to need for context. at hindsight i can't

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

ckandeler created this revision. ckandeler added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman. Herald added a project: All. ckandeler requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.