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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
