[PATCH] D95706: [clangd] Expose more dependent-name detail via semanticTokens

[Sam McCall via Phabricator via cfe-commits]

sammccall marked an inline comment as done.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:239
+  auto Result = Priority1 > Priority2 ? Tokens[0] : Tokens[1];
+  Result.Modifiers = Tokens[0].Modifiers | Tokens[1].Modifiers;
+  return Result;
----------------
nridge wrote:
> I like that this ends up with a concrete kind + dependent modifier, to give a hint that the concrete kind was determined heuristically :)
> 
> Maybe we could adjust the comment at the top of the function to call this behaviour out.
Right! I like this too but forgot do doc it. Done.


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:78
   Abstract,
+  DependentName,
 
----------------
nridge wrote:
> Just `Dependent` might be enough
AFAICS the current implementation is closer to "dependent name" than "dependent", I think.

For example, in the RHS of `template <typename T> using Val = T::value_type`, both "T" and "value_type" certainly refer to dependent types. But only `value_type` is a dependent name, and only `value_type` gets the old kinds/new modifier.

---

Or are you proposing we change the implementation too?

It's not intuitively which version would be more useful. FWIW I have the current DependentName kind highlighted in bold bright orange, and I find it really helpful :-) When there are errors and RecoveryExpr kicks in, it colors the resulting unresolved names, which I like surprisingly much.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95706