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

2019-07-11 Thread Johan Vikström via Phabricator via cfe-commits
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=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 ) {
 // 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
===
--- 

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

2019-07-11 Thread 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



___
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.

2019-07-11 Thread Johan Vikström via Phabricator via cfe-commits
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.

2019-07-11 Thread Johan Vikström via Phabricator via cfe-commits
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  : 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  : 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
+// namespace.
+if 

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

2019-07-10 Thread Sam McCall via Phabricator via cfe-commits
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.

2019-07-10 Thread Johan Vikström via Phabricator via cfe-commits
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.

2019-07-10 Thread Johan Vikström via Phabricator via cfe-commits
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  : 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  : 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 ) {
 // This check is for not getting two entries when there are anonymous
-// structs. It also makes us not 

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

2019-07-10 Thread Haojian Wu via Phabricator via cfe-commits
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.

2019-07-10 Thread Johan Vikström via Phabricator via cfe-commits
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  : 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  : 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 (NestedNameSpecifier *NNS =