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

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 19 01:33:52 PST 2023


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 {};



> Note that << and >> are not the only (or even the primary, in my mind) issue: there is an ambiguity between </> as template argument list delimiters vs. comparison operators.

Right, I was trying to imply all operator uses (e.g. including comparisons).

---

Currently my only high level comment is renaming the new kind to just bracket, apart from that i think the idea LG. LMK if you're going to take a look at the implementation @nridge, otherwise I am happy to do that as well.



================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:54
   Operator,
+  AngleBracket,
 
----------------
actually let's name these as `Bracket` rather than `AngleBracket`, as we might want to increase the coverage further in the future (and a use case such as yours can still look at the textual tokens to match relevant brackets).


================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:939
+            $Class[[S]]$AngleBracket[[<]]$Class[[S]]$AngleBracket[[<]]int$AngleBracket[[>]]$AngleBracket[[>]] $LocalVariable_def[[s1]];
+            $Class[[S]]<$Class[[S]]$AngleBracket[[<]]int$AngleBracket[[>]]\
+> $LocalVariable_def[[s2]];
----------------
i don't think this is enough of a test case, we probably need a bunch other like:
```
foo >\
>>

foo >\
 >>
```

i think the firstone is going to highlight `\` for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926



More information about the cfe-commits mailing list