[PATCH] D64492: [clangd] Added highlightings for namespace specifiers.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

looks good from my side, just a comment for the TM scope name.



================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:73
     // This check is for not getting two entries when there are anonymous
     // structs. It also makes us not highlight namespace qualifiers. For
     // elaborated types the actual type is highlighted as an inner TypeLoc.
----------------
sammccall wrote:
> jvikstrom wrote:
> > hokein wrote:
> > > this comment is stale with this patch, now we are highlighting namespace, it seems more natural to do it here, we can get a NestedNameSpecifierLoc from an `ElaboratedTypeLoc ` (so that we don't need the `TraverseNestedNameSpecifierLoc`).
> > Doing it in VisitTypeLoc means that we fail on this testcase:
> > 
> > ```
> >       namespace $Namespace[[aa]] {
> >         namespace $Namespace[[bb]] {
> >           struct $Class[[A]] {};
> >         }
> >       }
> >       $Namespace[[aa]]::$Namespace[[bb]]::$Class[[A]] $Variable[[a]];
> > ```
> > 
> > It can't detect the `bb` namespace qualifier in `aa::bb::A a;`.
> > 
> > Maybe there is some way of solving this without `TraverseNestedNameSpecifierLoc` that I am not aware of?
> > Also don't know how I'd get a `NestedNameSpecifierLoc` from an `ElaboratedTypeLoc`.
> TraverseNestedNameSpecifierLoc is the right way to deal with namespaces-as-prefixes, I think it's best to follow that consistently.
> 
> (For namespaces not-as-prefixes, you've got namespacealiasdecl, namespacedecl, using-directives... I think that's everything)
thanks for the explanation, fair enough.


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:247
+  case HighlightingKind::Namespace:
+    return "entity.name.type.namespace.cpp";
   case HighlightingKind::NumKinds:
----------------
jvikstrom wrote:
> hokein wrote:
> > I think here should be `entity.name.namespace.cpp`, vscode uses this TX scope for namespace.
> Really? Because I see `entity.name.type.namespace.cpp` when I inspect the TM scopes for namespaces.
that's weird,  TM inspector told me it was `entity.name.namespace.cpp`, I can also find it in https://github.com/microsoft/vscode/blob/master/extensions/cpp/syntaxes/cpp.tmLanguage.json#L1709


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64492