[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-05 Thread Johan Vikström via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
jvikstrom marked an inline comment as done.
Closed by commit rL365205: [clangd] Added highlighting for variable references 
(declrefs) (authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64199?vs=208143=208165#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64199

Files:
  clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
  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
@@ -34,26 +34,46 @@
 return Tokens;
   }
 
-  bool VisitVarDecl(VarDecl *Var) {
-addToken(Var, HighlightingKind::Variable);
+  bool VisitNamedDecl(NamedDecl *ND) {
+// FIXME: (De)Constructors/operator need to be highlighted some other way.
+if (ND->getDeclName().getNameKind() != DeclarationName::Identifier)
+  return true;
+
+if (ND->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.
+  return true;
+addToken(ND->getLocation(), ND);
 return true;
   }
-  bool VisitFunctionDecl(FunctionDecl *Func) {
-addToken(Func, HighlightingKind::Function);
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() !=
+DeclarationName::Identifier)
+  // Only want to highlight identifiers.
+  return true;
+
+addToken(Ref->getLocation(), Ref->getDecl());
 return true;
   }
 
 private:
-  void addToken(const NamedDecl *D, HighlightingKind Kind) {
-if (D->getLocation().isMacroID())
-  // FIXME: skip tokens inside macros for now.
+  void addToken(SourceLocation Loc, const Decl *D) {
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Variable);
   return;
+}
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Function);
+  return;
+}
+  }
 
-if (D->getDeclName().isEmpty())
-  // Don't add symbols that don't have any length.
+  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+if (Loc.isMacroID())
+  // FIXME: skip tokens inside macros for now.
   return;
 
-auto R = getTokenRange(SM, Ctx.getLangOpts(), D->getLocation());
+auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");
Index: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
@@ -48,20 +48,35 @@
 
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
-  R"cpp(
-struct A {
-  double SomeMember;
-};
-struct {
-}   $Variable[[HStruct]];
-void $Function[[foo]](int $Variable[[a]]) {
-  auto $Variable[[VeryLongVariableName]] = 12312;
-  A $Variable[[aa]];
-}
-  )cpp",
-  R"cpp(
-void $Function[[foo]](int);
-  )cpp"};
+R"cpp(
+  struct AS {
+double SomeMember;
+  };
+  struct {
+  } $Variable[[S]];
+  void $Function[[foo]](int $Variable[[A]]) {
+auto $Variable[[VeryLongVariableName]] = 12312;
+AS $Variable[[AA]];
+auto $Variable[[L]] = $Variable[[AA]].SomeMember + $Variable[[A]];
+auto $Variable[[FN]] = [ $Variable[[AA]]](int $Variable[[A]]) -> void {};
+$Variable[[FN]](12312);
+  }
+)cpp",
+R"cpp(
+  void $Function[[foo]](int);
+  void $Function[[Gah]]();
+  void $Function[[foo]]() {
+auto $Variable[[Bou]] = $Function[[Gah]];
+  }
+)cpp",
+R"cpp(
+  struct A {
+A();
+~A();
+void $Function[[abc]]();
+void operator<<(int);
+  };
+)cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.

thanks, looks good.




Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:52
   R"cpp(
-struct A {
-  double SomeMember;
-};
-struct {
-}   $Variable[[HStruct]];
-void $Function[[foo]](int $Variable[[a]]) {
-  auto $Variable[[VeryLongVariableName]] = 12312;
-  A $Variable[[aa]];
-}
-  )cpp",
+  struct AS {
+double SomeMember;

nit: I'd add 2-space to the code, like: 

```
R"cpp(
struct AS {}
...
)cpp, 
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-05 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom marked 4 inline comments as done.
jvikstrom added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:61
+}
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::Function);

hokein wrote:
> sammccall wrote:
> > jvikstrom wrote:
> > > sammccall wrote:
> > > > note that methods, constructors, and destructors inherit from 
> > > > functiondecl, so if you want to exclude/distinguish those, order 
> > > > matters here
> > > I'm aware of that, but thanks for the heads up. Although should I add it 
> > > in a comment somewhere in the method? Also added an additional testcase 
> > > for classes and FIXMEs to the skip if statement in VisitNamedDecl.
> > I don't think it needs a comment, especially if you're not actually 
> > highlighting them (because they have weird DeclarationNames)
> > 
> > > FIXMEs to the skip if statement in VisitNamedDecl
> > I'm not actually sure there's anything to fix here - it's a bit hard to 
> > talk about constructor/destructor highlighting as distinct from type name 
> > highlighting in C++. If you want them highlighted as classes, then that 
> > should just start working when you start handling TypeLocs.
> I think constructor/destructor can be categorized in the `function` group, 
> like `entity.name.function.constructor`, `entity.name.function.destructor`
I'll have a look at constructors/destructors at the same time as I look at types



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:69
+void $Function[[foo]]() {
+  int $Variable[[b]];
+  auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};

hokein wrote:
> nit: even for the test code, could we make the code style consistent (like 
> follow the LLVM code style) here?
I think this should be consistent with LLVM code style. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-05 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 208143.
jvikstrom added a comment.

Separated into three testcases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  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
@@ -49,19 +49,34 @@
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
   R"cpp(
-struct A {
-  double SomeMember;
-};
-struct {
-}   $Variable[[HStruct]];
-void $Function[[foo]](int $Variable[[a]]) {
-  auto $Variable[[VeryLongVariableName]] = 12312;
-  A $Variable[[aa]];
-}
-  )cpp",
+  struct AS {
+double SomeMember;
+  };
+  struct {
+  } $Variable[[S]];
+  void $Function[[foo]](int $Variable[[A]]) {
+auto $Variable[[VeryLongVariableName]] = 12312;
+AS $Variable[[AA]];
+auto $Variable[[L]] = $Variable[[AA]].SomeMember + $Variable[[A]];
+auto $Variable[[FN]] = [ $Variable[[AA]]](int $Variable[[A]]) -> void {};
+$Variable[[FN]](12312);
+  }
+)cpp",
+  R"cpp(
+  void $Function[[foo]](int);
+  void $Function[[Gah]]();
+  void $Function[[foo]]() {
+auto $Variable[[Bou]] = $Function[[Gah]];
+  }
+)cpp",
   R"cpp(
-void $Function[[foo]](int);
-  )cpp"};
+  struct A {
+A();
+~A();
+void $Function[[abc]]();
+void operator<<(int);
+  };
+)cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
   }
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -34,26 +34,46 @@
 return Tokens;
   }
 
-  bool VisitVarDecl(VarDecl *Var) {
-addToken(Var, HighlightingKind::Variable);
+  bool VisitNamedDecl(NamedDecl *ND) {
+// FIXME: (De)Constructors/operator need to be highlighted some other way.
+if (ND->getDeclName().getNameKind() != DeclarationName::Identifier)
+  return true;
+
+if (ND->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.
+  return true;
+addToken(ND->getLocation(), ND);
 return true;
   }
-  bool VisitFunctionDecl(FunctionDecl *Func) {
-addToken(Func, HighlightingKind::Function);
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() !=
+DeclarationName::Identifier)
+  // Only want to highlight identifiers.
+  return true;
+
+addToken(Ref->getLocation(), Ref->getDecl());
 return true;
   }
 
 private:
-  void addToken(const NamedDecl *D, HighlightingKind Kind) {
-if (D->getLocation().isMacroID())
-  // FIXME: skip tokens inside macros for now.
+  void addToken(SourceLocation Loc, const Decl *D) {
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Variable);
   return;
+}
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Function);
+  return;
+}
+  }
 
-if (D->getDeclName().isEmpty())
-  // Don't add symbols that don't have any length.
+  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+if (Loc.isMacroID())
+  // FIXME: skip tokens inside macros for now.
   return;
 
-auto R = getTokenRange(SM, Ctx.getLangOpts(), D->getLocation());
+auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-05 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 208140.
jvikstrom added a comment.

Made tests more readable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  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
@@ -49,19 +49,43 @@
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
   R"cpp(
-struct A {
-  double SomeMember;
-};
-struct {
-}   $Variable[[HStruct]];
-void $Function[[foo]](int $Variable[[a]]) {
-  auto $Variable[[VeryLongVariableName]] = 12312;
-  A $Variable[[aa]];
-}
-  )cpp",
+  struct AS {
+double SomeMember;
+  };
+  void $Function[[foo]](int $Variable[[A]]) {
+auto $Variable[[VeryLongVariableName]] = 12312;
+AS $Variable[[AA]];
+auto $Variable[[L]] = $Variable[[AA]].SomeMember + $Variable[[A]];
+  }
+)cpp",
+  R"cpp(
+  struct {
+  } $Variable[[S]];
+)cpp",
+  R"cpp(
+  void $Function[[foo]](int);
+)cpp",
+  R"cpp(
+  void $Function[[Gah]]();
+  void $Function[[foo]]() {
+auto $Variable[[Bou]] = $Function[[Gah]];
+  }
+)cpp",
+  R"cpp(
+  void $Function[[foo]]() {
+int $Variable[[B]];
+auto $Variable[[FN]] = [ $Variable[[B]]](int $Variable[[A]]) -> void {};
+$Variable[[FN]](12312);
+  }
+)cpp",
   R"cpp(
-void $Function[[foo]](int);
-  )cpp"};
+  struct A {
+A();
+~A();
+void $Function[[abc]]();
+void operator<<(int);
+  };
+)cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
   }
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -34,26 +34,46 @@
 return Tokens;
   }
 
-  bool VisitVarDecl(VarDecl *Var) {
-addToken(Var, HighlightingKind::Variable);
+  bool VisitNamedDecl(NamedDecl *ND) {
+// FIXME: (De)Constructors/operator need to be highlighted some other way.
+if (ND->getDeclName().getNameKind() != DeclarationName::Identifier)
+  return true;
+
+if (ND->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.
+  return true;
+addToken(ND->getLocation(), ND);
 return true;
   }
-  bool VisitFunctionDecl(FunctionDecl *Func) {
-addToken(Func, HighlightingKind::Function);
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() !=
+DeclarationName::Identifier)
+  // Only want to highlight identifiers.
+  return true;
+
+addToken(Ref->getLocation(), Ref->getDecl());
 return true;
   }
 
 private:
-  void addToken(const NamedDecl *D, HighlightingKind Kind) {
-if (D->getLocation().isMacroID())
-  // FIXME: skip tokens inside macros for now.
+  void addToken(SourceLocation Loc, const Decl *D) {
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Variable);
   return;
+}
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Function);
+  return;
+}
+  }
 
-if (D->getDeclName().isEmpty())
-  // Don't add symbols that don't have any length.
+  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+if (Loc.isMacroID())
+  // FIXME: skip tokens inside macros for now.
   return;
 
-auto R = getTokenRange(SM, Ctx.getLangOpts(), D->getLocation());
+auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

the implementation looks good, a few comments on the test.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:61
+}
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::Function);

sammccall wrote:
> jvikstrom wrote:
> > sammccall wrote:
> > > note that methods, constructors, and destructors inherit from 
> > > functiondecl, so if you want to exclude/distinguish those, order matters 
> > > here
> > I'm aware of that, but thanks for the heads up. Although should I add it in 
> > a comment somewhere in the method? Also added an additional testcase for 
> > classes and FIXMEs to the skip if statement in VisitNamedDecl.
> I don't think it needs a comment, especially if you're not actually 
> highlighting them (because they have weird DeclarationNames)
> 
> > FIXMEs to the skip if statement in VisitNamedDecl
> I'm not actually sure there's anything to fix here - it's a bit hard to talk 
> about constructor/destructor highlighting as distinct from type name 
> highlighting in C++. If you want them highlighted as classes, then that 
> should just start working when you start handling TypeLocs.
I think constructor/destructor can be categorized in the `function` group, like 
`entity.name.function.constructor`, `entity.name.function.destructor`



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:69
+void $Function[[foo]]() {
+  int $Variable[[b]];
+  auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};

nit: even for the test code, could we make the code style consistent (like 
follow the LLVM code style) here?



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:76
+  R"cpp(
+struct A {
+  A();

now we have 4 test cases, the purpose of each testcase is unclear to me (some 
of them are testing duplicated things), could we narrow the testcase so the 
each testcase just test one particular thing (e.g. one test for `Variable` and 
its references; one test for `Function` and its references;)?

I think the testcase here is to verify we don't highlighting the 
constructor/destructor, and operator, just 

```
R"cpp(
struct Foo {
  Foo();
  ~Foo();
  void $Function[[foo]]();
}
)cpp"
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-05 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 208123.
jvikstrom added a comment.

Added additional testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  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
@@ -57,10 +57,32 @@
 void $Function[[foo]](int $Variable[[a]]) {
   auto $Variable[[VeryLongVariableName]] = 12312;
   A $Variable[[aa]];
+  auto $Variable[[l]] = $Variable[[aa]].SomeMember + $Variable[[a]];
 }
   )cpp",
   R"cpp(
 void $Function[[foo]](int);
+  )cpp",
+  R"cpp(
+void $Function[[gah]]();
+void $Function[[foo]]() {
+  int $Variable[[b]];
+  auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};
+  $Variable[[FN]](12312);
+  auto $Variable[[bou]] = $Function[[gah]];
+}
+  )cpp",
+  R"cpp(
+struct A {
+  A();
+  ~A();
+  void $Function[[abc]]();
+  void operator<<(int);
+};
+void $Function[[foo]]() {
+  A $Variable[[a]];
+  $Variable[[a]].abc();
+}
   )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -34,26 +34,46 @@
 return Tokens;
   }
 
-  bool VisitVarDecl(VarDecl *Var) {
-addToken(Var, HighlightingKind::Variable);
+  bool VisitNamedDecl(NamedDecl *ND) {
+// FIXME: (De)Constructors/operator need to be highlighted some other way.
+if (ND->getDeclName().getNameKind() != DeclarationName::Identifier)
+  return true;
+
+if (ND->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.
+  return true;
+addToken(ND->getLocation(), ND);
 return true;
   }
-  bool VisitFunctionDecl(FunctionDecl *Func) {
-addToken(Func, HighlightingKind::Function);
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() !=
+DeclarationName::Identifier)
+  // Only want to highlight identifiers.
+  return true;
+
+addToken(Ref->getLocation(), Ref->getDecl());
 return true;
   }
 
 private:
-  void addToken(const NamedDecl *D, HighlightingKind Kind) {
-if (D->getLocation().isMacroID())
-  // FIXME: skip tokens inside macros for now.
+  void addToken(SourceLocation Loc, const Decl *D) {
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Variable);
   return;
+}
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Function);
+  return;
+}
+  }
 
-if (D->getDeclName().isEmpty())
-  // Don't add symbols that don't have any length.
+  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+if (Loc.isMacroID())
+  // FIXME: skip tokens inside macros for now.
   return;
 
-auto R = getTokenRange(SM, Ctx.getLangOpts(), D->getLocation());
+auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -57,10 +57,32 @@
 void $Function[[foo]](int $Variable[[a]]) {
   auto $Variable[[VeryLongVariableName]] = 12312;
   A $Variable[[aa]];
+  auto $Variable[[l]] = $Variable[[aa]].SomeMember + $Variable[[a]];
 }
   )cpp",
   R"cpp(
 void $Function[[foo]](int);
+  )cpp",
+  R"cpp(
+void $Function[[gah]]();
+void $Function[[foo]]() {
+  int $Variable[[b]];
+  auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};
+  $Variable[[FN]](12312);
+  auto $Variable[[bou]] = $Function[[gah]];
+}
+  )cpp",
+  R"cpp(
+struct A {
+  A();
+  ~A();
+  void $Function[[abc]]();
+  void operator<<(int);
+};
+void $Function[[foo]]() {
+  A $Variable[[a]];
+  $Variable[[a]].abc();
+}
   )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp

[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-04 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:38
+  bool VisitNamedDecl(NamedDecl *ND) {
+if (ND->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.

jvikstrom wrote:
> sammccall wrote:
> > I think you might want to bail out (both here and in VisitDeclRefExpr) if 
> > the name kind isn't identifier.
> > 
> > Reason is you're only coloring the token at location, and most of the other 
> > name kinds can span multiple tokens or otherwise need special consideration.
> I must have missed the Identifier NameKind because I was first-hand looking 
> for something like that. 
> Thanks.
> 
> Are you aware of any testcase I could add for this by the way?
Such a testcase would ensure you're not coloring any part of `struct F { ~F(); 
}` as a method, or `operator <<` etc.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:61
+}
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::Function);

jvikstrom wrote:
> sammccall wrote:
> > note that methods, constructors, and destructors inherit from functiondecl, 
> > so if you want to exclude/distinguish those, order matters here
> I'm aware of that, but thanks for the heads up. Although should I add it in a 
> comment somewhere in the method? Also added an additional testcase for 
> classes and FIXMEs to the skip if statement in VisitNamedDecl.
I don't think it needs a comment, especially if you're not actually 
highlighting them (because they have weird DeclarationNames)

> FIXMEs to the skip if statement in VisitNamedDecl
I'm not actually sure there's anything to fix here - it's a bit hard to talk 
about constructor/destructor highlighting as distinct from type name 
highlighting in C++. If you want them highlighted as classes, then that should 
just start working when you start handling TypeLocs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-04 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

(LG from my side)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-04 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:38
+  bool VisitNamedDecl(NamedDecl *ND) {
+if (ND->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.

sammccall wrote:
> I think you might want to bail out (both here and in VisitDeclRefExpr) if the 
> name kind isn't identifier.
> 
> Reason is you're only coloring the token at location, and most of the other 
> name kinds can span multiple tokens or otherwise need special consideration.
I must have missed the Identifier NameKind because I was first-hand looking for 
something like that. 
Thanks.

Are you aware of any testcase I could add for this by the way?



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:61
+}
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::Function);

sammccall wrote:
> note that methods, constructors, and destructors inherit from functiondecl, 
> so if you want to exclude/distinguish those, order matters here
I'm aware of that, but thanks for the heads up. Although should I add it in a 
comment somewhere in the method? Also added an additional testcase for classes 
and FIXMEs to the skip if statement in VisitNamedDecl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-04 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 208041.
jvikstrom marked 2 inline comments as done.
jvikstrom added a comment.

Added testcae. Added another bailout from VisitNamedDecl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  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
@@ -57,10 +57,31 @@
 void $Function[[foo]](int $Variable[[a]]) {
   auto $Variable[[VeryLongVariableName]] = 12312;
   A $Variable[[aa]];
+  auto $Variable[[l]] = $Variable[[aa]].SomeMember + $Variable[[a]];
 }
   )cpp",
   R"cpp(
 void $Function[[foo]](int);
+  )cpp",
+  R"cpp(
+void $Function[[gah]]();
+void $Function[[foo]]() {
+  int $Variable[[b]];
+  auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};
+  $Variable[[FN]](12312);
+  auto $Variable[[bou]] = $Function[[gah]];
+}
+  )cpp",
+  R"cpp(
+struct A {
+  A();
+  ~A();
+  void $Function[[abc]]();
+};
+void $Function[[foo]]() {
+  A $Variable[[a]];
+  $Variable[[a]].abc();
+}
   )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -34,26 +34,46 @@
 return Tokens;
   }
 
-  bool VisitVarDecl(VarDecl *Var) {
-addToken(Var, HighlightingKind::Variable);
+  bool VisitNamedDecl(NamedDecl *ND) {
+// FIXME: This also skips constructors and destructors.
+if (ND->getDeclName().getNameKind() != DeclarationName::Identifier)
+  return true;
+
+if (ND->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.
+  return true;
+addToken(ND->getLocation(), ND);
 return true;
   }
-  bool VisitFunctionDecl(FunctionDecl *Func) {
-addToken(Func, HighlightingKind::Function);
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() !=
+DeclarationName::Identifier)
+  // Only want to highlight identifiers.
+  return true;
+
+addToken(Ref->getLocation(), Ref->getDecl());
 return true;
   }
 
 private:
-  void addToken(const NamedDecl *D, HighlightingKind Kind) {
-if (D->getLocation().isMacroID())
-  // FIXME: skip tokens inside macros for now.
+  void addToken(SourceLocation Loc, const Decl *D) {
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Variable);
   return;
+}
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Function);
+  return;
+}
+  }
 
-if (D->getDeclName().isEmpty())
-  // Don't add symbols that don't have any length.
+  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+if (Loc.isMacroID())
+  // FIXME: skip tokens inside macros for now.
   return;
 
-auto R = getTokenRange(SM, Ctx.getLangOpts(), D->getLocation());
+auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -57,10 +57,31 @@
 void $Function[[foo]](int $Variable[[a]]) {
   auto $Variable[[VeryLongVariableName]] = 12312;
   A $Variable[[aa]];
+  auto $Variable[[l]] = $Variable[[aa]].SomeMember + $Variable[[a]];
 }
   )cpp",
   R"cpp(
 void $Function[[foo]](int);
+  )cpp",
+  R"cpp(
+void $Function[[gah]]();
+void $Function[[foo]]() {
+  int $Variable[[b]];
+  auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};
+  $Variable[[FN]](12312);
+  auto $Variable[[bou]] = $Function[[gah]];
+}
+  )cpp",
+  R"cpp(
+struct A {
+  A();
+  ~A();
+  void $Function[[abc]]();
+};
+void $Function[[foo]]() {
+  A $Variable[[a]];
+  $Variable[[a]].abc();
+}
   )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ 

[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-04 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:38
+  bool VisitNamedDecl(NamedDecl *ND) {
+if (ND->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.

I think you might want to bail out (both here and in VisitDeclRefExpr) if the 
name kind isn't identifier.

Reason is you're only coloring the token at location, and most of the other 
name kinds can span multiple tokens or otherwise need special consideration.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:61
+}
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::Function);

note that methods, constructors, and destructors inherit from functiondecl, so 
if you want to exclude/distinguish those, order matters here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-04 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 208030.
jvikstrom added a comment.

Removed debug prints from test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  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
@@ -57,10 +57,20 @@
 void $Function[[foo]](int $Variable[[a]]) {
   auto $Variable[[VeryLongVariableName]] = 12312;
   A $Variable[[aa]];
+  auto $Variable[[l]] = $Variable[[aa]].SomeMember + $Variable[[a]];
 }
   )cpp",
   R"cpp(
 void $Function[[foo]](int);
+  )cpp",
+  R"cpp(
+void $Function[[gah]]();
+void $Function[[foo]]() {
+  int $Variable[[b]];
+  auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};
+  $Variable[[FN]](12312);
+  auto $Variable[[bou]] = $Function[[gah]];
+}
   )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -34,26 +34,42 @@
 return Tokens;
   }
 
-  bool VisitVarDecl(VarDecl *Var) {
-addToken(Var, HighlightingKind::Variable);
+  bool VisitNamedDecl(NamedDecl *ND) {
+if (ND->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.
+  return true;
+addToken(ND->getLocation(), ND);
 return true;
   }
-  bool VisitFunctionDecl(FunctionDecl *Func) {
-addToken(Func, HighlightingKind::Function);
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() ==
+DeclarationName::CXXOperatorName)
+  // Don't want to highlight operator usages.
+  return true;
+
+addToken(Ref->getLocation(), Ref->getDecl());
 return true;
   }
 
 private:
-  void addToken(const NamedDecl *D, HighlightingKind Kind) {
-if (D->getLocation().isMacroID())
-  // FIXME: skip tokens inside macros for now.
+  void addToken(SourceLocation Loc, const Decl* D) {
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::Variable);
+  return;
+}
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::Function);
   return;
+}
+  }
 
-if (D->getDeclName().isEmpty())
-  // Don't add symbols that don't have any length.
+  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+if (Loc.isMacroID())
+  // FIXME: skip tokens inside macros for now.
   return;
 
-auto R = getTokenRange(SM, Ctx.getLangOpts(), D->getLocation());
+auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -57,10 +57,20 @@
 void $Function[[foo]](int $Variable[[a]]) {
   auto $Variable[[VeryLongVariableName]] = 12312;
   A $Variable[[aa]];
+  auto $Variable[[l]] = $Variable[[aa]].SomeMember + $Variable[[a]];
 }
   )cpp",
   R"cpp(
 void $Function[[foo]](int);
+  )cpp",
+  R"cpp(
+void $Function[[gah]]();
+void $Function[[foo]]() {
+  int $Variable[[b]];
+  auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};
+  $Variable[[FN]](12312);
+  auto $Variable[[bou]] = $Function[[gah]];
+}
   )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -34,26 +34,42 @@
 return Tokens;
   }
 
-  bool VisitVarDecl(VarDecl *Var) {
-addToken(Var, HighlightingKind::Variable);
+  bool VisitNamedDecl(NamedDecl *ND) {
+if (ND->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.
+  return true;
+addToken(ND->getLocation(), ND);
 return true;
   }
-  bool VisitFunctionDecl(FunctionDecl *Func) {
-addToken(Func, HighlightingKind::Function);
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() ==
+DeclarationName::CXXOperatorName)
+  // Don't want to 

[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-04 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 208029.
jvikstrom added a comment.

Removed VisitVarDecl and VisitFuncDecl in favor of VisitNamedDecl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  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
@@ -28,7 +28,7 @@
 
   return Tokens;
 }
-
+#include 
 void checkHighlightings(llvm::StringRef Code) {
   Annotations Test(Code);
   auto AST = TestTU::withCode(Test.code()).build();
@@ -43,6 +43,14 @@
   }
 
   auto ActualTokens = getSemanticHighlightings(AST);
+  std::cout << "\n\n";
+  for(auto Tok : ExpectedTokens) {
+std::cout << "E:: " << Tok.R.start.line << ":" << Tok.R.start.character << std::endl;
+  }
+  for(auto Tok : ActualTokens) {
+std::cout << "A:: " << Tok.R.start.line << ":" << Tok.R.start.character << std::endl;
+  }
+
   EXPECT_THAT(ActualTokens, testing::UnorderedElementsAreArray(ExpectedTokens));
 }
 
@@ -57,10 +65,20 @@
 void $Function[[foo]](int $Variable[[a]]) {
   auto $Variable[[VeryLongVariableName]] = 12312;
   A $Variable[[aa]];
+  auto $Variable[[l]] = $Variable[[aa]].SomeMember + $Variable[[a]];
 }
   )cpp",
   R"cpp(
 void $Function[[foo]](int);
+  )cpp",
+  R"cpp(
+void $Function[[gah]]();
+void $Function[[foo]]() {
+  int $Variable[[b]];
+  auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};
+  $Variable[[FN]](12312);
+  auto $Variable[[bou]] = $Function[[gah]];
+}
   )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -34,26 +34,42 @@
 return Tokens;
   }
 
-  bool VisitVarDecl(VarDecl *Var) {
-addToken(Var, HighlightingKind::Variable);
+  bool VisitNamedDecl(NamedDecl *ND) {
+if (ND->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.
+  return true;
+addToken(ND->getLocation(), ND);
 return true;
   }
-  bool VisitFunctionDecl(FunctionDecl *Func) {
-addToken(Func, HighlightingKind::Function);
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() ==
+DeclarationName::CXXOperatorName)
+  // Don't want to highlight operator usages.
+  return true;
+
+addToken(Ref->getLocation(), Ref->getDecl());
 return true;
   }
 
 private:
-  void addToken(const NamedDecl *D, HighlightingKind Kind) {
-if (D->getLocation().isMacroID())
-  // FIXME: skip tokens inside macros for now.
+  void addToken(SourceLocation Loc, const Decl* D) {
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::Variable);
+  return;
+}
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::Function);
   return;
+}
+  }
 
-if (D->getDeclName().isEmpty())
-  // Don't add symbols that don't have any length.
+  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+if (Loc.isMacroID())
+  // FIXME: skip tokens inside macros for now.
   return;
 
-auto R = getTokenRange(SM, Ctx.getLangOpts(), D->getLocation());
+auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-04 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 208028.
jvikstrom added a comment.

Added overload for addToken and added more code to the test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  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
@@ -28,7 +28,7 @@
 
   return Tokens;
 }
-
+#include 
 void checkHighlightings(llvm::StringRef Code) {
   Annotations Test(Code);
   auto AST = TestTU::withCode(Test.code()).build();
@@ -43,6 +43,14 @@
   }
 
   auto ActualTokens = getSemanticHighlightings(AST);
+  std::cout << "\n\n";
+  for(auto Tok : ExpectedTokens) {
+std::cout << "E:: " << Tok.R.start.line << ":" << Tok.R.start.character << std::endl;
+  }
+  for(auto Tok : ActualTokens) {
+std::cout << "A:: " << Tok.R.start.line << ":" << Tok.R.start.character << std::endl;
+  }
+
   EXPECT_THAT(ActualTokens, testing::UnorderedElementsAreArray(ExpectedTokens));
 }
 
@@ -57,10 +65,20 @@
 void $Function[[foo]](int $Variable[[a]]) {
   auto $Variable[[VeryLongVariableName]] = 12312;
   A $Variable[[aa]];
+  auto $Variable[[l]] = $Variable[[aa]].SomeMember + $Variable[[a]];
 }
   )cpp",
   R"cpp(
 void $Function[[foo]](int);
+  )cpp",
+  R"cpp(
+void $Function[[gah]]();
+void $Function[[foo]]() {
+  int $Variable[[b]];
+  auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};
+  $Variable[[FN]](12312);
+  auto $Variable[[bou]] = $Function[[gah]];
+}
   )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -35,25 +35,42 @@
   }
 
   bool VisitVarDecl(VarDecl *Var) {
-addToken(Var, HighlightingKind::Variable);
+addNamedDecl(Var);
 return true;
   }
   bool VisitFunctionDecl(FunctionDecl *Func) {
-addToken(Func, HighlightingKind::Function);
+addNamedDecl(Func);
+return true;
+  }
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() ==
+DeclarationName::CXXOperatorName)
+  // Don't want to highlight operator usages.
+  return true;
+
+addToken(Ref->getLocation(), Ref->getDecl());
 return true;
   }
 
 private:
-  void addToken(const NamedDecl *D, HighlightingKind Kind) {
-if (D->getLocation().isMacroID())
-  // FIXME: skip tokens inside macros for now.
+  void addToken(SourceLocation Loc, const Decl* D) {
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::Variable);
   return;
+}
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::Function);
+  return;
+}
+  }
 
-if (D->getDeclName().isEmpty())
-  // Don't add symbols that don't have any length.
+  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+if (Loc.isMacroID())
+  // FIXME: skip tokens inside macros for now.
   return;
 
-auto R = getTokenRange(SM, Ctx.getLangOpts(), D->getLocation());
+auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");
@@ -62,6 +79,13 @@
 
 Tokens.push_back({Kind, R.getValue()});
   }
+
+  void addNamedDecl(const NamedDecl *D) {
+if (D->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.
+  return;
+addToken(D->getLocation(), D);
+  }
 };
 
 // Encode binary data into base64.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-04 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:46
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() ==

hokein wrote:
> The `DeclRefExpr` is a very general expression, which can reference a 
> variable, a function, an enum, etc. I think we want to distinguish with them 
> (rather than putting all into `Variable` type).
> 
>  And we are missing large majority of entities now, I think we could start 
> collecting more entities (class, method, enum, etc).
IMO the way to ensure consistency is to make the highlight a function of the 
decl, regardless of whether you found it in the decl itself, a declrefexpr, etc.

so something like:

```
// (overloaded)
addToken(SourceLocation Loc, NamedDecl *D) {
  if (isa(D))
return addToken(Loc, HighlightingKind::Variable);
  // ... etc
}

bool VisitDeclRefExpr(DeclRefExpr *Ref) { 
  // bail out for operators here (if it's only *usages* you don't want to 
highlight)
  addToken(Ref->getLocation(), Ref->getDecl());
}

bool VisitNamedDecl(NamedDecl *ND) {
  addToken(ND->getLocation(), ND);
}
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:46
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() ==

The `DeclRefExpr` is a very general expression, which can reference a variable, 
a function, an enum, etc. I think we want to distinguish with them (rather than 
putting all into `Variable` type).

 And we are missing large majority of entities now, I think we could start 
collecting more entities (class, method, enum, etc).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64199: [clangd] Added highlighting for variable references (declrefs)

2019-07-04 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom created this revision.
jvikstrom added reviewers: hokein, sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Added highlighting for variable references using VisitDeclRefExpr.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64199

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  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
@@ -57,10 +57,18 @@
 void $Function[[foo]](int $Variable[[a]]) {
   auto $Variable[[VeryLongVariableName]] = 12312;
   A $Variable[[aa]];
+  auto $Variable[[l]] = $Variable[[aa]].SomeMember + $Variable[[a]];
 }
   )cpp",
   R"cpp(
 void $Function[[foo]](int);
+  )cpp",
+  R"cpp(
+void $Function[[foo]]() {
+  int $Variable[[b]];
+  auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};
+  $Variable[[FN]](12312);
+}
   )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -35,25 +35,31 @@
   }
 
   bool VisitVarDecl(VarDecl *Var) {
-addToken(Var, HighlightingKind::Variable);
+addNamedDecl(Var, HighlightingKind::Variable);
 return true;
   }
   bool VisitFunctionDecl(FunctionDecl *Func) {
-addToken(Func, HighlightingKind::Function);
+addNamedDecl(Func, HighlightingKind::Function);
+return true;
+  }
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() ==
+DeclarationName::CXXOperatorName)
+  // Don't want to highlight operators.
+  return true;
+
+addToken(Ref->getLocation(), HighlightingKind::Variable);
 return true;
   }
 
 private:
-  void addToken(const NamedDecl *D, HighlightingKind Kind) {
-if (D->getLocation().isMacroID())
+  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+if (Loc.isMacroID())
   // FIXME: skip tokens inside macros for now.
   return;
 
-if (D->getDeclName().isEmpty())
-  // Don't add symbols that don't have any length.
-  return;
-
-auto R = getTokenRange(SM, Ctx.getLangOpts(), D->getLocation());
+auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");
@@ -62,6 +68,13 @@
 
 Tokens.push_back({Kind, R.getValue()});
   }
+
+  void addNamedDecl(const NamedDecl *D, HighlightingKind Kind) {
+if (D->getDeclName().isEmpty())
+  // Don't add symbols that don't have any length.
+  return;
+addToken(D->getLocation(), Kind);
+  }
 };
 
 // Encode binary data into base64.


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -57,10 +57,18 @@
 void $Function[[foo]](int $Variable[[a]]) {
   auto $Variable[[VeryLongVariableName]] = 12312;
   A $Variable[[aa]];
+  auto $Variable[[l]] = $Variable[[aa]].SomeMember + $Variable[[a]];
 }
   )cpp",
   R"cpp(
 void $Function[[foo]](int);
+  )cpp",
+  R"cpp(
+void $Function[[foo]]() {
+  int $Variable[[b]];
+  auto $Variable[[FN]] = [ $Variable[[b]]](int $Variable[[a]]) -> void {};
+  $Variable[[FN]](12312);
+}
   )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -35,25 +35,31 @@
   }
 
   bool VisitVarDecl(VarDecl *Var) {
-addToken(Var, HighlightingKind::Variable);
+addNamedDecl(Var, HighlightingKind::Variable);
 return true;
   }
   bool VisitFunctionDecl(FunctionDecl *Func) {
-addToken(Func, HighlightingKind::Function);
+addNamedDecl(Func, HighlightingKind::Function);
+return true;
+  }
+
+  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
+if (Ref->getNameInfo().getName().getNameKind() ==
+DeclarationName::CXXOperatorName)
+  // Don't want to highlight operators.
+  return true;
+
+addToken(Ref->getLocation(), HighlightingKind::Variable);
 return true;
   }
 
 private:
-  void addToken(const