[PATCH] D139926: [clangd] Add semantic token for angle brackets

This revision was automatically updated to reflect the committed changes. Closed by commit rGbbddbe580bca: [clangd] Add semantic token for angle brackets (authored by ckandeler). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139926/new/ https://revi

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler updated this revision to Diff 493610. ckandeler added a comment. Incorporated review comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139926/new/ https://reviews.llvm.org/D139926 Files: clang-tools-extra/clangd/SemanticHighlighti

[PATCH] D139926: [clangd] Add semantic token for angle brackets

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. In D139926#4091990 , @nridge wrote: > but it sounds like you've looked at that well i did now :P --- thanks a lot to you both, let's ship it!

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:382 + +// For the inner element of a nested template instantiation with no space +// between the '>' characters, TemplateSpecializationLocInfo::RAngleLoc has ka

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler updated this revision to Diff 493594. ckandeler marked an inline comment as done. ckandeler added a comment. Improved implementation as per review comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139926/new/ https://reviews.llvm.org

[PATCH] D139926: [clangd] Add semantic token for angle brackets

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:382 + +// For the inner element of a nested template instantiation with no space +// between the '>' characters, TemplateSpecializationLocInfo::RAngleLoc has i a

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added a comment. Thanks for the updates! In D139926#4064769 , @kadircet wrote: > LMK if you're going to take a look at the implementation @nridge, otherwise I > am happy to do that as well. The implementation looks good to me now. (I admit I don

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler updated this revision to Diff 493247. ckandeler added a comment. Support more cases as per review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139926/new/ https://reviews.llvm.org/D139926 Files: clang-tools-extra/clangd/Sema

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added a comment. I did find one other edge case (last one, I promise!) template void foo(T); template class A { friend void foo<>(T); // <-- }; This one is `FunctionDecl::getDependentSpecializationInfo()->getLAngleLoc()` Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added a comment. In D139926#4084846 , @nridge wrote: > template > class A { > template void foo(U a) { } > template<> void foo(int a) { } // <-- > }; > > This one is `ClassScopeFunctionSpecializationDecl::getTemplateArgsAsWritten

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:1026 +concept $Concept_decl[[C2]] = true; +template $Bracket[[<]]C2$Bracket[[<]]int$Bracket[[>]] $TemplateParameter_def[[A]]$Bracket[[>]] +class $C

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added a comment. In D139926#4084970 , @ckandeler wrote: > Adding AutoTypeLoc broke tons of tests, so I left it out for now. Poked at this a bit, it looks like `AutoTypeLoc.getLAngleLoc()` is only safe to access if it `isConstrained()`. This is po

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:1026 +concept $Concept_decl[[C2]] = true; +template $Bracket[[<]]C2$Bracket[[<]]int$Bracket[[>]] $TemplateParameter_def[[A]]$Bracket[[>]] +class

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler added a comment. In D139926#4084868 , @nridge wrote: > I figured I might as well look through the AST API for classes with > getLAngleLoc/getRAngleLoc methods. > > It looks like we've got almost all of them (including the ones mentioned in > r

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler updated this revision to Diff 492670. ckandeler marked 3 inline comments as done. ckandeler added a comment. Handled more cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139926/new/ https://reviews.llvm.org/D139926 Files: clang-to

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added a comment. I figured I might as well look through the AST API for classes with getLAngleLoc/getRAngleLoc methods. It looks like we've got almost all of them (including the ones mentioned in recent comments) except: - OverloadExpr - DependentTemplateSpecializationTypeLoc - AutoType

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added a comment. In D139926#4084856 , @nridge wrote: > In D139926#4084854 , @ckandeler > wrote: > >> I would have expected these to be handled by VisitDeclRefExpr(), but they >> aren't. Any idea? > > Ther

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added a comment. In D139926#4084854 , @ckandeler wrote: > I would have expected these to be handled by VisitDeclRefExpr(), but they > aren't. Any idea? There are two other expression types, `DependentScopeDeclRefExpr` and `CXXDependentScopeMembe

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler added a comment. In D139926#4084801 , @nridge wrote: > Function calls are still missing some cases: > > I would have expected these to be handled by VisitDeclRefExpr(), but they aren't. Any idea? Repository: rG LLVM Github Monorepo CHA

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:641 + VisitClassTemplateSpecializationDecl(ClassTemplateSpecializationDecl *D) { +for (unsigned i = 0; i < D->getNumTemplateParameterLists(); ++i) { + if (auto *TPL = D->getTempl

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added a comment. Two more cases I found: template concept C = true; template A> // <-- (inner pair) class B {}; This is a `TypeConstraint`, but RecursiveASTVisitor is lacking a Visit() method for it, so I think you'll need to override `TraverseTypeConstraint()` instead (ex

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added a comment. Function calls are still missing some cases: template void free(); template struct A { template void mem(); }; void foo() { A a; a.mem(); // <-- } template void bar() { free(); // <-- A a; a.mem(); // <--

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added a comment. In D139926#4076652 , @ckandeler wrote: > Thanks for the test cases! > All fixed, except: > >> // variable template specialization >> // parameter and argument lists are missing semantic tokens >> template <> >> constexpr int V = 5;

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler added a comment. In D139926#4076057 , @nridge wrote: > One other thought that has occurred to me is that as we add more semantic > tolens for punctuation, the test cases in `GetsCorrectTokens` become harder > to read. > > What would you think

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler added a comment. Thanks for the test cases! All fixed, except: > // variable template specialization > // parameter and argument lists are missing semantic tokens > template <> > constexpr int V = 5; Argument list fixed. I didn't manage to make the parameter list work. Repository:

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler updated this revision to Diff 491718. ckandeler marked an inline comment as done. ckandeler added a comment. Added test cases from review comment and made them pass. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139926/new/ https://review

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added a comment. One other thought that has occurred to me is that as we add more semantic tolens for punctuation, the test cases in `GetsCorrectTokens` become harder to read. What would you think about omitting punctuation tokens in the `GetsCorrectTokens` test cases (both `Operator` a

[PATCH] D139926: [clangd] Add semantic token for angle brackets

nridge added a comment. I played around with the patch a little, and found some cases where semantic tokens should be present but aren't: template class S { public: template class Nested; }; // explicit specialization // parameter list is missing semantic tokens template <>

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler added a comment. In D139926#4064769 , @kadircet wrote: >> We can easily check the actual character at the given position in the >> client, so I could just merge the two highlighting kinds. > > Thanks! Note that it might not be as easy at the fa

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler updated this revision to Diff 490457. ckandeler marked 2 inline comments as done. ckandeler added a comment. Made token name more generic, added test cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139926/new/ https://reviews.llvm.or

[PATCH] D139926: [clangd] Add semantic token for angle brackets

kadircet added a comment. > We can easily check the actual character at the given position in the client, > so I could just merge the two highlighting kinds. Thanks! Note that it might not be as easy at the face of templates, eg: #define LESS < template LESS typename T > class A {}; > No

[PATCH] D139926: [clangd] Add semantic token for angle brackets

ckandeler updated this revision to Diff 489722. ckandeler marked 2 inline comments as done. ckandeler retitled this revision from "[clangd] Add semantic tokens for angle brackets" to "[clangd] Add semantic token for angle brackets". ckandeler added a comment. Added test cases, merged the two High