[PATCH] D64492: [clangd] Added highlightings for namespace specifiers.
This revision was automatically updated to reflect the committed changes. Closed by commit rL365745: [clangd] Added highlightings for namespace specifiers. (authored by jvikstrom, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D64492?vs=209139&id=209146#toc Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64492/new/ https://reviews.llvm.org/D64492 Files: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp clang-tools-extra/trunk/clangd/SemanticHighlighting.h clang-tools-extra/trunk/clangd/test/semantic-highlighting.test clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp === --- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp +++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp @@ -11,8 +11,6 @@ #include "Protocol.h" #include "SourceCode.h" #include "clang/AST/ASTContext.h" -#include "clang/AST/Decl.h" -#include "clang/AST/DeclarationName.h" #include "clang/AST/RecursiveASTVisitor.h" namespace clang { @@ -36,7 +34,21 @@ return Tokens; } + bool VisitNamespaceAliasDecl(NamespaceAliasDecl *NAD) { +// The target namespace of an alias can not be found in any other way. +addToken(NAD->getTargetNameLoc(), HighlightingKind::Namespace); +return true; + } + bool VisitNamedDecl(NamedDecl *ND) { +// UsingDirectiveDecl's namespaces do not show up anywhere else in the +// Visit/Traverse mehods. But they should also be highlighted as a +// namespace. +if (const auto *UD = dyn_cast(ND)) { + addToken(UD->getIdentLocation(), HighlightingKind::Namespace); + return true; +} + // Constructors' TypeLoc has a TypePtr that is a FunctionProtoType. It has // no tag decl and therefore constructors must be gotten as NamedDecls // instead. @@ -65,17 +77,28 @@ bool VisitTypeLoc(TypeLoc &TL) { // 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. +// structs. It also makes us not highlight certain namespace qualifiers +// twice. For elaborated types the actual type is highlighted as an inner +// TypeLoc. if (TL.getTypeLocClass() == TypeLoc::TypeLocClass::Elaborated) return true; if (const Type *TP = TL.getTypePtr()) if (const TagDecl *TD = TP->getAsTagDecl()) - addToken(TL.getBeginLoc(), TD); +addToken(TL.getBeginLoc(), TD); return true; } + bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLoc) { +if (NestedNameSpecifier *NNS = NNSLoc.getNestedNameSpecifier()) + if (NNS->getKind() == NestedNameSpecifier::Namespace || + NNS->getKind() == NestedNameSpecifier::NamespaceAlias) +addToken(NNSLoc.getLocalBeginLoc(), HighlightingKind::Namespace); + +return RecursiveASTVisitor< +HighlightingTokenCollector>::TraverseNestedNameSpecifierLoc(NNSLoc); + } + private: void addToken(SourceLocation Loc, const NamedDecl *D) { if (D->getDeclName().isIdentifier() && D->getName().empty()) @@ -104,6 +127,14 @@ addToken(Loc, HighlightingKind::Function); return; } +if (isa(D)) { + addToken(Loc, HighlightingKind::Namespace); + return; +} +if (isa(D)) { + addToken(Loc, HighlightingKind::Namespace); + return; +} } void addToken(SourceLocation Loc, HighlightingKind Kind) { @@ -218,6 +249,8 @@ return "entity.name.type.class.cpp"; case HighlightingKind::Enum: return "entity.name.type.enum.cpp"; + case HighlightingKind::Namespace: +return "entity.name.namespace.cpp"; case HighlightingKind::NumKinds: llvm_unreachable("must not pass NumKinds to the function"); } Index: clang-tools-extra/trunk/clangd/test/semantic-highlighting.test === --- clang-tools-extra/trunk/clangd/test/semantic-highlighting.test +++ clang-tools-extra/trunk/clangd/test/semantic-highlighting.test @@ -9,12 +9,15 @@ # CHECK-NEXT: ], # CHECK-NEXT: [ # CHECK-NEXT:"entity.name.function.cpp" -# CHECK-NEXT: ] +# CHECK-NEXT: ], # CHECK-NEXT: [ # CHECK-NEXT:"entity.name.type.class.cpp" # CHECK-NEXT: ], # CHECK-NEXT: [ # CHECK-NEXT:"entity.name.type.enum.cpp" +# CHECK-NEXT: ], +# CHECK-NEXT: [ +# CHECK-NEXT:"entity.name.namespace.cpp" # CHECK-NEXT: ] # CHECK-NEXT:] # CHECK-NEXT: }, Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.h === --- clang-tools-extra
[PATCH] D64492: [clangd] Added highlightings for namespace specifiers.
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
[PATCH] D64492: [clangd] Added highlightings for namespace specifiers.
jvikstrom added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:87 + bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLoc) { +if (NestedNameSpecifier *NNS = NNSLoc.getNestedNameSpecifier()) + if (NNS->getKind() == NestedNameSpecifier::Namespace || sammccall wrote: > if you're just doing something and then calling base, can you make this Visit > instead of Traverse? NestedNameSpecifierLoc does not have a visit method for some reason. It only has a traverse method and this seems to be the way people get nested namespaces from what I can tell. (Maybe a Visit method is something that should be added to the RecursiveASTVisitor, but I wouldn't do that in this patch). Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:86 template - struct $Class[[C]] : abc::A { + struct $Class[[C]] : $Namespace[[abc]]::A { typename T::A* D; sammccall wrote: > can you add a case where we spell with leading colons e.g. `::abc::A` Added it in the case that focuses on namespaces further down. 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
[PATCH] D64492: [clangd] Added highlightings for namespace specifiers.
jvikstrom updated this revision to Diff 209139. jvikstrom marked 4 inline comments as done. jvikstrom added a comment. Moved alias target namespace add token to another function and added testcase for global namespace specifier. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64492/new/ https://reviews.llvm.org/D64492 Files: clang-tools-extra/clangd/SemanticHighlighting.cpp clang-tools-extra/clangd/SemanticHighlighting.h clang-tools-extra/clangd/test/semantic-highlighting.test clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp === --- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -36,7 +36,8 @@ {HighlightingKind::Variable, "Variable"}, {HighlightingKind::Function, "Function"}, {HighlightingKind::Class, "Class"}, - {HighlightingKind::Enum, "Enum"}}; + {HighlightingKind::Enum, "Enum"}, + {HighlightingKind::Namespace, "Namespace"}}; std::vector ExpectedTokens; for (const auto &KindString : KindToString) { std::vector Toks = makeHighlightingTokens( @@ -75,18 +76,18 @@ }; )cpp", R"cpp( - namespace abc { + namespace $Namespace[[abc]] { template struct $Class[[A]] { T t; }; } template - struct $Class[[C]] : abc::A { + struct $Class[[C]] : $Namespace[[abc]]::A { typename T::A* D; }; - abc::$Class[[A]] $Variable[[AA]]; - typedef abc::$Class[[A]] AAA; + $Namespace[[abc]]::$Class[[A]] $Variable[[AA]]; + typedef $Namespace[[abc]]::$Class[[A]] AAA; struct $Class[[B]] { $Class[[B]](); ~$Class[[B]](); @@ -108,6 +109,29 @@ $Enum[[E]] EEE; $Enum[[EE]] ; }; +)cpp", +R"cpp( + namespace $Namespace[[abc]] { +namespace {} +namespace $Namespace[[bcd]] { + struct $Class[[A]] {}; + namespace $Namespace[[cde]] { +struct $Class[[A]] { + enum class $Enum[[B]] { +Hi, + }; +}; + } +} + } + using namespace $Namespace[[abc]]::$Namespace[[bcd]]; + namespace $Namespace[[vwz]] = +$Namespace[[abc]]::$Namespace[[bcd]]::$Namespace[[cde]]; + $Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[AA]]; + $Namespace[[vwz]]::$Class[[A]]::$Enum[[B]] $Variable[[AAA]] = +$Namespace[[vwz]]::$Class[[A]]::$Enum[[B]]::Hi; + ::$Namespace[[vwz]]::$Class[[A]] $Variable[[B]]; + ::$Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[BB]]; )cpp"}; for (const auto &TestCase : TestCases) { checkHighlightings(TestCase); Index: clang-tools-extra/clangd/test/semantic-highlighting.test === --- clang-tools-extra/clangd/test/semantic-highlighting.test +++ clang-tools-extra/clangd/test/semantic-highlighting.test @@ -9,12 +9,15 @@ # CHECK-NEXT: ], # CHECK-NEXT: [ # CHECK-NEXT:"entity.name.function.cpp" -# CHECK-NEXT: ] +# CHECK-NEXT: ], # CHECK-NEXT: [ # CHECK-NEXT:"entity.name.type.class.cpp" # CHECK-NEXT: ], # CHECK-NEXT: [ # CHECK-NEXT:"entity.name.type.enum.cpp" +# CHECK-NEXT: ], +# CHECK-NEXT: [ +# CHECK-NEXT:"entity.name.type.namespace.cpp" # CHECK-NEXT: ] # CHECK-NEXT:] # CHECK-NEXT: }, Index: clang-tools-extra/clangd/SemanticHighlighting.h === --- clang-tools-extra/clangd/SemanticHighlighting.h +++ clang-tools-extra/clangd/SemanticHighlighting.h @@ -28,6 +28,7 @@ Function, Class, Enum, + Namespace, NumKinds, }; Index: clang-tools-extra/clangd/SemanticHighlighting.cpp === --- clang-tools-extra/clangd/SemanticHighlighting.cpp +++ clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -11,8 +11,6 @@ #include "Protocol.h" #include "SourceCode.h" #include "clang/AST/ASTContext.h" -#include "clang/AST/Decl.h" -#include "clang/AST/DeclarationName.h" #include "clang/AST/RecursiveASTVisitor.h" namespace clang { @@ -36,7 +34,21 @@ return Tokens; } + bool VisitNamespaceAliasDecl(NamespaceAliasDecl *NAD) { +// The target namespace of an alias can not be found in any other way. +addToken(NAD->getTargetNameLoc(), HighlightingKind::Namespace); +return true; + } + bool VisitNamedDecl(NamedDecl *ND) { +// UsingDirectiveDecl's namespaces do not show up anywhere else in the +// Visit/Traverse mehods. But they should also be highlighted as a +// n
[PATCH] D64492: [clangd] Added highlightings for namespace specifiers.
sammccall added inline comments. 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. 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) Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:87 + bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLoc) { +if (NestedNameSpecifier *NNS = NNSLoc.getNestedNameSpecifier()) + if (NNS->getKind() == NestedNameSpecifier::Namespace || if you're just doing something and then calling base, can you make this Visit instead of Traverse? Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:131 + // The target namespace can not be found in any other way. + addToken(NAD->getTargetNameLoc(), HighlightingKind::Namespace); + return; this doesn't fit with the contract of this addToken() function, which should only highlight the provided token. Instead, can you call this from VisitNamespaceAliasDecl? Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:86 template - struct $Class[[C]] : abc::A { + struct $Class[[C]] : $Namespace[[abc]]::A { typename T::A* D; can you add a case where we spell with leading colons e.g. `::abc::A` 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
[PATCH] D64492: [clangd] Added highlightings for namespace specifiers.
jvikstrom added inline comments. 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. 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`. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:247 + case HighlightingKind::Namespace: +return "entity.name.type.namespace.cpp"; case HighlightingKind::NumKinds: 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. 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
[PATCH] D64492: [clangd] Added highlightings for namespace specifiers.
jvikstrom updated this revision to Diff 208998. jvikstrom marked 6 inline comments as done. jvikstrom added a comment. Addressed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64492/new/ https://reviews.llvm.org/D64492 Files: clang-tools-extra/clangd/SemanticHighlighting.cpp clang-tools-extra/clangd/SemanticHighlighting.h clang-tools-extra/clangd/test/semantic-highlighting.test clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp === --- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -36,7 +36,8 @@ {HighlightingKind::Variable, "Variable"}, {HighlightingKind::Function, "Function"}, {HighlightingKind::Class, "Class"}, - {HighlightingKind::Enum, "Enum"}}; + {HighlightingKind::Enum, "Enum"}, + {HighlightingKind::Namespace, "Namespace"}}; std::vector ExpectedTokens; for (const auto &KindString : KindToString) { std::vector Toks = makeHighlightingTokens( @@ -75,18 +76,18 @@ }; )cpp", R"cpp( - namespace abc { + namespace $Namespace[[abc]] { template struct $Class[[A]] { T t; }; } template - struct $Class[[C]] : abc::A { + struct $Class[[C]] : $Namespace[[abc]]::A { typename T::A* D; }; - abc::$Class[[A]] $Variable[[AA]]; - typedef abc::$Class[[A]] AAA; + $Namespace[[abc]]::$Class[[A]] $Variable[[AA]]; + typedef $Namespace[[abc]]::$Class[[A]] AAA; struct $Class[[B]] { $Class[[B]](); ~$Class[[B]](); @@ -108,6 +109,27 @@ $Enum[[E]] EEE; $Enum[[EE]] ; }; +)cpp", +R"cpp( + namespace $Namespace[[abc]] { +namespace {} +namespace $Namespace[[bcd]] { + struct $Class[[A]] {}; + namespace $Namespace[[cde]] { +struct $Class[[A]] { + enum class $Enum[[B]] { +Hi, + }; +}; + } +} + } + using namespace $Namespace[[abc]]::$Namespace[[bcd]]; + namespace $Namespace[[vwz]] = +$Namespace[[abc]]::$Namespace[[bcd]]::$Namespace[[cde]]; + $Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[AA]]; + $Namespace[[vwz]]::$Class[[A]]::$Enum[[B]] $Variable[[AAA]] = +$Namespace[[vwz]]::$Class[[A]]::$Enum[[B]]::Hi; )cpp"}; for (const auto &TestCase : TestCases) { checkHighlightings(TestCase); Index: clang-tools-extra/clangd/test/semantic-highlighting.test === --- clang-tools-extra/clangd/test/semantic-highlighting.test +++ clang-tools-extra/clangd/test/semantic-highlighting.test @@ -9,12 +9,15 @@ # CHECK-NEXT: ], # CHECK-NEXT: [ # CHECK-NEXT:"entity.name.function.cpp" -# CHECK-NEXT: ] +# CHECK-NEXT: ], # CHECK-NEXT: [ # CHECK-NEXT:"entity.name.type.class.cpp" # CHECK-NEXT: ], # CHECK-NEXT: [ # CHECK-NEXT:"entity.name.type.enum.cpp" +# CHECK-NEXT: ], +# CHECK-NEXT: [ +# CHECK-NEXT:"entity.name.type.namespace.cpp" # CHECK-NEXT: ] # CHECK-NEXT:] # CHECK-NEXT: }, Index: clang-tools-extra/clangd/SemanticHighlighting.h === --- clang-tools-extra/clangd/SemanticHighlighting.h +++ clang-tools-extra/clangd/SemanticHighlighting.h @@ -28,6 +28,7 @@ Function, Class, Enum, + Namespace, NumKinds, }; Index: clang-tools-extra/clangd/SemanticHighlighting.cpp === --- clang-tools-extra/clangd/SemanticHighlighting.cpp +++ clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -11,8 +11,6 @@ #include "Protocol.h" #include "SourceCode.h" #include "clang/AST/ASTContext.h" -#include "clang/AST/Decl.h" -#include "clang/AST/DeclarationName.h" #include "clang/AST/RecursiveASTVisitor.h" namespace clang { @@ -37,6 +35,14 @@ } bool VisitNamedDecl(NamedDecl *ND) { +// UsingDirectiveDecl's namespaces do not show up anywhere else in the +// Visit/Traverse mehods. But they should also be highlighted as a +// namespace. +if (const auto *UD = dyn_cast(ND)) { + addToken(UD->getIdentLocation(), HighlightingKind::Namespace); + return true; +} + // Constructors' TypeLoc has a TypePtr that is a FunctionProtoType. It has // no tag decl and therefore constructors must be gotten as NamedDecls // instead. @@ -65,17 +71,28 @@ bool VisitTypeLoc(TypeLoc &TL) { // This check is for not getting two entries when there are anonymous -// structs
[PATCH] D64492: [clangd] Added highlightings for namespace specifiers.
hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:41 +// namespace. +if (const auto UD = dyn_cast(ND)) { + addToken(UD->getIdentLocation(), HighlightingKind::Namespace); nit: const auto * Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:42 +if (const auto UD = dyn_cast(ND)) { + addToken(UD->getIdentLocation(), HighlightingKind::Namespace); +} nit: you miss a `return` 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. 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`). Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:247 + case HighlightingKind::Namespace: +return "entity.name.type.namespace.cpp"; case HighlightingKind::NumKinds: I think here should be `entity.name.namespace.cpp`, vscode uses this TX scope for namespace. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:114 +R"cpp( + namespace $Namespace[[abc]] { +namespace $Namespace[[bcd]] { could you add a testcase for anonymous namespace? `namespace {}` Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:119 +struct $Class[[A]] { + static enum class $Enum[[B]] { +Hi, nit: the `static` is not needed for a type. 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
[PATCH] D64492: [clangd] Added highlightings for namespace specifiers.
jvikstrom created this revision. jvikstrom added reviewers: hokein, sammccall, ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Added highlightings for namespace specifiers. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D64492 Files: clang-tools-extra/clangd/SemanticHighlighting.cpp clang-tools-extra/clangd/SemanticHighlighting.h clang-tools-extra/clangd/test/semantic-highlighting.test clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp === --- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -36,7 +36,8 @@ {HighlightingKind::Variable, "Variable"}, {HighlightingKind::Function, "Function"}, {HighlightingKind::Class, "Class"}, - {HighlightingKind::Enum, "Enum"}}; + {HighlightingKind::Enum, "Enum"}, + {HighlightingKind::Namespace, "Namespace"}}; std::vector ExpectedTokens; for (const auto &KindString : KindToString) { std::vector Toks = makeHighlightingTokens( @@ -75,18 +76,18 @@ }; )cpp", R"cpp( - namespace abc { + namespace $Namespace[[abc]] { template struct $Class[[A]] { T t; }; } template - struct $Class[[C]] : abc::A { + struct $Class[[C]] : $Namespace[[abc]]::A { typename T::A* D; }; - abc::$Class[[A]] $Variable[[AA]]; - typedef abc::$Class[[A]] AAA; + $Namespace[[abc]]::$Class[[A]] $Variable[[AA]]; + typedef $Namespace[[abc]]::$Class[[A]] AAA; struct $Class[[B]] { $Class[[B]](); ~$Class[[B]](); @@ -108,6 +109,26 @@ $Enum[[E]] EEE; $Enum[[EE]] ; }; +)cpp", +R"cpp( + namespace $Namespace[[abc]] { +namespace $Namespace[[bcd]] { + struct $Class[[A]] {}; + namespace $Namespace[[cde]] { +struct $Class[[A]] { + static enum class $Enum[[B]] { +Hi, + }; +}; + } +} + } + using namespace $Namespace[[abc]]::$Namespace[[bcd]]; + namespace $Namespace[[vwz]] = +$Namespace[[abc]]::$Namespace[[bcd]]::$Namespace[[cde]]; + $Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[AA]]; + $Namespace[[vwz]]::$Class[[A]]::$Enum[[B]] $Variable[[AAA]] = +$Namespace[[vwz]]::$Class[[A]]::$Enum[[B]]::Hi; )cpp"}; for (const auto &TestCase : TestCases) { checkHighlightings(TestCase); Index: clang-tools-extra/clangd/test/semantic-highlighting.test === --- clang-tools-extra/clangd/test/semantic-highlighting.test +++ clang-tools-extra/clangd/test/semantic-highlighting.test @@ -9,12 +9,15 @@ # CHECK-NEXT: ], # CHECK-NEXT: [ # CHECK-NEXT:"entity.name.function.cpp" -# CHECK-NEXT: ] +# CHECK-NEXT: ], # CHECK-NEXT: [ # CHECK-NEXT:"entity.name.type.class.cpp" # CHECK-NEXT: ], # CHECK-NEXT: [ # CHECK-NEXT:"entity.name.type.enum.cpp" +# CHECK-NEXT: ], +# CHECK-NEXT: [ +# CHECK-NEXT:"entity.name.type.namespace.cpp" # CHECK-NEXT: ] # CHECK-NEXT:] # CHECK-NEXT: }, Index: clang-tools-extra/clangd/SemanticHighlighting.h === --- clang-tools-extra/clangd/SemanticHighlighting.h +++ clang-tools-extra/clangd/SemanticHighlighting.h @@ -28,6 +28,7 @@ Function, Class, Enum, + Namespace, NumKinds, }; Index: clang-tools-extra/clangd/SemanticHighlighting.cpp === --- clang-tools-extra/clangd/SemanticHighlighting.cpp +++ clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -11,8 +11,6 @@ #include "Protocol.h" #include "SourceCode.h" #include "clang/AST/ASTContext.h" -#include "clang/AST/Decl.h" -#include "clang/AST/DeclarationName.h" #include "clang/AST/RecursiveASTVisitor.h" namespace clang { @@ -37,6 +35,13 @@ } bool VisitNamedDecl(NamedDecl *ND) { +// UsingDirectiveDecl's namespaces do not show up anywhere else in the +// Visit/Traverse mehods. But they should also be highlighted as a +// namespace. +if (const auto UD = dyn_cast(ND)) { + addToken(UD->getIdentLocation(), HighlightingKind::Namespace); +} + // Constructors' TypeLoc has a TypePtr that is a FunctionProtoType. It has // no tag decl and therefore constructors must be gotten as NamedDecls // instead. @@ -76,6 +81,16 @@ return true; } + bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLoc) { +if (NestedNameSpecif