[PATCH] D80279: [libclang] Extend clang_Cursor_Evaluate().

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: nik.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.

Let this function (try to) evaluate expressions, in addition to
declarations and compound statements.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80279

Files:
  clang/include/clang-c/Index.h
  clang/test/Index/evaluate-cursor.cpp
  clang/tools/libclang/CIndex.cpp


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -4041,10 +4041,14 @@
 }
 
 CXEvalResult clang_Cursor_Evaluate(CXCursor C) {
-  if (const Expr *E =
-  clang_getCursorKind(C) == CXCursor_CompoundStmt
-  ? evaluateCompoundStmtExpr(cast(getCursorStmt(C)))
-  : evaluateDeclExpr(getCursorDecl(C)))
+  const Expr *E = nullptr;
+  if (clang_getCursorKind(C) == CXCursor_CompoundStmt)
+E = evaluateCompoundStmtExpr(cast(getCursorStmt(C)));
+  else if (clang_isDeclaration(C.kind))
+E = evaluateDeclExpr(getCursorDecl(C));
+  else if (clang_isExpression(C.kind))
+E = getCursorExpr(C);
+  if (E)
 return const_cast(
 reinterpret_cast(evaluateExpr(const_cast(E), 
C)));
   return nullptr;
Index: clang/test/Index/evaluate-cursor.cpp
===
--- clang/test/Index/evaluate-cursor.cpp
+++ clang/test/Index/evaluate-cursor.cpp
@@ -26,6 +26,9 @@
   static const auto g = alignof(f);
 };
 
+constexpr static int calc_val() { return 1 + 2; }
+const auto the_value = calc_val() + sizeof(char);
+
 // RUN: c-index-test -evaluate-cursor-at=%s:4:7 \
 // RUN:-evaluate-cursor-at=%s:8:7 \
 // RUN:-evaluate-cursor-at=%s:8:11 -std=c++11 %s | FileCheck %s
@@ -53,3 +56,12 @@
 // RUN:-evaluate-cursor-at=%s:26:21 \
 // RUN:-std=c++11 %s | FileCheck -check-prefix=CHECK-DOES-NOT-CRASH %s
 // CHECK-DOES-NOT-CRASH: Not Evaluatable
+
+// RUN: c-index-test -evaluate-cursor-at=%s:30:1 \
+// RUN:-evaluate-cursor-at=%s:30:32 \
+// RUN:-evaluate-cursor-at=%s:30:35 \
+// RUN:-evaluate-cursor-at=%s:30:37 -std=c++11 %s | FileCheck %s 
-check-prefix=CHECK-EXPR
+// CHECK-EXPR: unsigned, Value: 4
+// CHECK-EXPR: Value: 3
+// CHECK-EXPR: unsigned, Value: 4
+// CHECK-EXPR: unsigned, Value: 1
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -5933,6 +5933,7 @@
  * If cursor is a statement declaration tries to evaluate the
  * statement and if its variable, tries to evaluate its initializer,
  * into its corresponding type.
+ * If it's an expression, tries to evaluate the expression.
  */
 CINDEX_LINKAGE CXEvalResult clang_Cursor_Evaluate(CXCursor C);
 


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -4041,10 +4041,14 @@
 }
 
 CXEvalResult clang_Cursor_Evaluate(CXCursor C) {
-  if (const Expr *E =
-  clang_getCursorKind(C) == CXCursor_CompoundStmt
-  ? evaluateCompoundStmtExpr(cast(getCursorStmt(C)))
-  : evaluateDeclExpr(getCursorDecl(C)))
+  const Expr *E = nullptr;
+  if (clang_getCursorKind(C) == CXCursor_CompoundStmt)
+E = evaluateCompoundStmtExpr(cast(getCursorStmt(C)));
+  else if (clang_isDeclaration(C.kind))
+E = evaluateDeclExpr(getCursorDecl(C));
+  else if (clang_isExpression(C.kind))
+E = getCursorExpr(C);
+  if (E)
 return const_cast(
 reinterpret_cast(evaluateExpr(const_cast(E), C)));
   return nullptr;
Index: clang/test/Index/evaluate-cursor.cpp
===
--- clang/test/Index/evaluate-cursor.cpp
+++ clang/test/Index/evaluate-cursor.cpp
@@ -26,6 +26,9 @@
   static const auto g = alignof(f);
 };
 
+constexpr static int calc_val() { return 1 + 2; }
+const auto the_value = calc_val() + sizeof(char);
+
 // RUN: c-index-test -evaluate-cursor-at=%s:4:7 \
 // RUN:-evaluate-cursor-at=%s:8:7 \
 // RUN:-evaluate-cursor-at=%s:8:11 -std=c++11 %s | FileCheck %s
@@ -53,3 +56,12 @@
 // RUN:-evaluate-cursor-at=%s:26:21 \
 // RUN:-std=c++11 %s | FileCheck -check-prefix=CHECK-DOES-NOT-CRASH %s
 // CHECK-DOES-NOT-CRASH: Not Evaluatable
+
+// RUN: c-index-test -evaluate-cursor-at=%s:30:1 \
+// RUN:-evaluate-cursor-at=%s:30:32 \
+// RUN:-evaluate-cursor-at=%s:30:35 \
+// RUN:-evaluate-cursor-at=%s:30:37 -std=c++11 %s | FileCheck %s -check-prefix=CHECK-EXPR
+// CHECK-EXPR: unsigned, Value: 4
+// CHECK-EXPR: Value: 3
+// CHECK-EXPR: unsigned, Value: 4
+// CHECK-EXPR: unsigned, Value: 1
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -5933,6 +5933,7 @@
  * 

[PATCH] D80279: [libclang] Extend clang_Cursor_Evaluate().

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Thanks for the quick review. I don't have commit access, so can someone please 
commit it for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80279



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

[PATCH] D80279: [libclang] Extend clang_Cursor_Evaluate().

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

A kind reminder that I need someone with commit rights to merge this for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80279



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

[PATCH] D82629: [libclang] Fix crash when visiting a captured VLA.

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

I suppose it boils down to whether the visit() function is supposed to except 
null arguments or not. Since I don't know the answer to that, I added the check 
at the caller. From the description of the linked patch, it appears that other 
places are also calling the function with a null pointer, but that could just 
mean that these should be fixed independently. Someone with better knowledge of 
the code would have to make that decision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82629



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

[PATCH] D82629: [libclang] Fix crash when visiting a captured VLA.

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D82629#2134521 , @jkorous wrote:

> @ckandeler do you have commit access or do you want me to land the patch?


I do not, so it'd be great if you could do it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82629



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

[PATCH] D82629: [libclang] Fix crash when visiting a captured VLA.

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: milianw.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.

When a variable-length array is being captured in a lambda, the AST
contains two captures, the first one having a null init expression.
Visiting that one triggers an assertion, so skip it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82629

Files:
  clang/test/Index/evaluate-cursor.cpp
  clang/tools/libclang/CIndex.cpp


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3272,7 +3272,7 @@
   }
   // Visit init captures
   for (auto InitExpr : E->capture_inits()) {
-if (Visit(InitExpr))
+if (InitExpr && Visit(InitExpr))
   return true;
   }
 
Index: clang/test/Index/evaluate-cursor.cpp
===
--- clang/test/Index/evaluate-cursor.cpp
+++ clang/test/Index/evaluate-cursor.cpp
@@ -29,6 +29,13 @@
 constexpr static int calc_val() { return 1 + 2; }
 const auto the_value = calc_val() + sizeof(char);
 
+void vlaTest()
+{
+  int msize = 4;
+  float arr[msize];
+  [&arr]{};
+}
+
 // RUN: c-index-test -evaluate-cursor-at=%s:4:7 \
 // RUN:-evaluate-cursor-at=%s:8:7 \
 // RUN:-evaluate-cursor-at=%s:8:11 -std=c++11 %s | FileCheck %s
@@ -65,3 +72,7 @@
 // CHECK-EXPR: Value: 3
 // CHECK-EXPR: unsigned, Value: 4
 // CHECK-EXPR: unsigned, Value: 1
+
+// RUN: c-index-test -evaluate-cursor-at=%s:36:5 \
+// RUN:-std=c++11 %s | FileCheck -check-prefix=VLA %s
+// VLA: Not Evaluatable


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3272,7 +3272,7 @@
   }
   // Visit init captures
   for (auto InitExpr : E->capture_inits()) {
-if (Visit(InitExpr))
+if (InitExpr && Visit(InitExpr))
   return true;
   }
 
Index: clang/test/Index/evaluate-cursor.cpp
===
--- clang/test/Index/evaluate-cursor.cpp
+++ clang/test/Index/evaluate-cursor.cpp
@@ -29,6 +29,13 @@
 constexpr static int calc_val() { return 1 + 2; }
 const auto the_value = calc_val() + sizeof(char);
 
+void vlaTest()
+{
+  int msize = 4;
+  float arr[msize];
+  [&arr]{};
+}
+
 // RUN: c-index-test -evaluate-cursor-at=%s:4:7 \
 // RUN:-evaluate-cursor-at=%s:8:7 \
 // RUN:-evaluate-cursor-at=%s:8:11 -std=c++11 %s | FileCheck %s
@@ -65,3 +72,7 @@
 // CHECK-EXPR: Value: 3
 // CHECK-EXPR: unsigned, Value: 4
 // CHECK-EXPR: unsigned, Value: 1
+
+// RUN: c-index-test -evaluate-cursor-at=%s:36:5 \
+// RUN:-std=c++11 %s | FileCheck -check-prefix=VLA %s
+// VLA: Not Evaluatable
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82629: [libclang] Fix crash when visiting a captured VLA.

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 274071.
ckandeler added a comment.

Fixed formatting issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82629

Files:
  clang/test/Index/evaluate-cursor.cpp
  clang/tools/libclang/CIndex.cpp


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3272,7 +3272,7 @@
   }
   // Visit init captures
   for (auto InitExpr : E->capture_inits()) {
-if (Visit(InitExpr))
+if (InitExpr && Visit(InitExpr))
   return true;
   }
 
Index: clang/test/Index/evaluate-cursor.cpp
===
--- clang/test/Index/evaluate-cursor.cpp
+++ clang/test/Index/evaluate-cursor.cpp
@@ -29,6 +29,12 @@
 constexpr static int calc_val() { return 1 + 2; }
 const auto the_value = calc_val() + sizeof(char);
 
+void vlaTest() {
+  int msize = 4;
+  float arr[msize];
+  [&arr] {};
+}
+
 // RUN: c-index-test -evaluate-cursor-at=%s:4:7 \
 // RUN:-evaluate-cursor-at=%s:8:7 \
 // RUN:-evaluate-cursor-at=%s:8:11 -std=c++11 %s | FileCheck %s
@@ -65,3 +71,7 @@
 // CHECK-EXPR: Value: 3
 // CHECK-EXPR: unsigned, Value: 4
 // CHECK-EXPR: unsigned, Value: 1
+
+// RUN: c-index-test -evaluate-cursor-at=%s:35:5 \
+// RUN:-std=c++11 %s | FileCheck -check-prefix=VLA %s
+// VLA: Not Evaluatable


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3272,7 +3272,7 @@
   }
   // Visit init captures
   for (auto InitExpr : E->capture_inits()) {
-if (Visit(InitExpr))
+if (InitExpr && Visit(InitExpr))
   return true;
   }
 
Index: clang/test/Index/evaluate-cursor.cpp
===
--- clang/test/Index/evaluate-cursor.cpp
+++ clang/test/Index/evaluate-cursor.cpp
@@ -29,6 +29,12 @@
 constexpr static int calc_val() { return 1 + 2; }
 const auto the_value = calc_val() + sizeof(char);
 
+void vlaTest() {
+  int msize = 4;
+  float arr[msize];
+  [&arr] {};
+}
+
 // RUN: c-index-test -evaluate-cursor-at=%s:4:7 \
 // RUN:-evaluate-cursor-at=%s:8:7 \
 // RUN:-evaluate-cursor-at=%s:8:11 -std=c++11 %s | FileCheck %s
@@ -65,3 +71,7 @@
 // CHECK-EXPR: Value: 3
 // CHECK-EXPR: unsigned, Value: 4
 // CHECK-EXPR: unsigned, Value: 1
+
+// RUN: c-index-test -evaluate-cursor-at=%s:35:5 \
+// RUN:-std=c++11 %s | FileCheck -check-prefix=VLA %s
+// VLA: Not Evaluatable
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:863
+  Tok.addModifier(HighlightingModifier::Declaration);
+if (const auto Func = dyn_cast(Decl)) {
+  if (Func->isThisDeclarationADefinition())

dgoldman wrote:
> Instead of doing it like this, I wonder if would make more sense to call 
> getDefinition from 
> https://cs.github.com/llvm/llvm-project/blob/ae071a59bc6cc09e0e0043e0ef1d9725853f1681/clang-tools-extra/clangd/XRefs.cpp#L76
>  and if it matches Decl, add the Definition modifier?
Won't that incur an additional look-up or some other type of work? I'm not 
deeply familiar with the implementation, but a cursory glance seems to suggests 
that isThisDeclarationADefinition() is just an accessor, while getDefinition() 
"does things".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

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

[PATCH] D128329: [clangd] Also mark output arguments of operator call expressions

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 440650.
ckandeler marked 2 inline comments as done.
ckandeler added a comment.

Improvements as per review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128329

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
@@ -738,6 +738,11 @@
   )cpp",
   // Modifier for variables passed as non-const references
   R"cpp(
+struct $Class_decl[[ClassWithOp]] {
+void operator()(int);
+void operator()(int, int &);
+void operator()(int, int, const int &);
+};
 void $Function_decl[[fun]](int, const int,
int*, const int*,
int&, const int&,
@@ -759,6 +764,13 @@
$LocalVariable[[array]], 
$LocalVariable_usedAsMutableReference[[array]], 
$LocalVariable[[array]]
);
+  [](int){}($LocalVariable[[val]]);
+  [](int&){}($LocalVariable_usedAsMutableReference[[val]]);
+  [](const int&){}($LocalVariable[[val]]);
+  $Class[[ClassWithOp]] $LocalVariable_decl[[c]];
+  $LocalVariable[[c]]($LocalVariable[[val]]);
+  $LocalVariable[[c]](0, $LocalVariable_usedAsMutableReference[[val]]);
+  $LocalVariable[[c]](0, 0, $LocalVariable[[val]]);
 }
 struct $Class_decl[[S]] {
   $Class_decl[[S]](int&) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -537,13 +537,17 @@
 if (isa(E))
   return true;
 
-// FIXME ...here it would make sense though.
-if (isa(E))
-  return true;
+// FIXME: consider highlighting parameters of some other overloaded
+// operators as well
+llvm::ArrayRef Args = {E->getArgs(), E->getNumArgs()};
+if (const auto callOp = dyn_cast(E)) {
+  if (callOp->getOperator() != OO_Call)
+return true;
+  Args = Args.drop_front(); // Drop object parameter
+}
 
 highlightMutableReferenceArguments(
-dyn_cast_or_null(E->getCalleeDecl()),
-{E->getArgs(), E->getNumArgs()});
+dyn_cast_or_null(E->getCalleeDecl()), Args);
 
 return true;
   }


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -738,6 +738,11 @@
   )cpp",
   // Modifier for variables passed as non-const references
   R"cpp(
+struct $Class_decl[[ClassWithOp]] {
+void operator()(int);
+void operator()(int, int &);
+void operator()(int, int, const int &);
+};
 void $Function_decl[[fun]](int, const int,
int*, const int*,
int&, const int&,
@@ -759,6 +764,13 @@
$LocalVariable[[array]], $LocalVariable_usedAsMutableReference[[array]], 
$LocalVariable[[array]]
);
+  [](int){}($LocalVariable[[val]]);
+  [](int&){}($LocalVariable_usedAsMutableReference[[val]]);
+  [](const int&){}($LocalVariable[[val]]);
+  $Class[[ClassWithOp]] $LocalVariable_decl[[c]];
+  $LocalVariable[[c]]($LocalVariable[[val]]);
+  $LocalVariable[[c]](0, $LocalVariable_usedAsMutableReference[[val]]);
+  $LocalVariable[[c]](0, 0, $LocalVariable[[val]]);
 }
 struct $Class_decl[[S]] {
   $Class_decl[[S]](int&) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -537,13 +537,17 @@
 if (isa(E))
   return true;
 
-// FIXME ...here it would make sense though.
-if (isa(E))
-  return true;
+// FIXME: consider highlighting parameters of some other overloaded
+// operators as well
+llvm::ArrayRef Args = {E->getArgs(), E->getNumArgs()};
+if (const auto callOp = dyn_cast(E)) {
+  if (callOp->getOperator() != OO_Call)
+return true;
+  Args = Args.drop_front(); // Drop object parameter
+}
 
 highlightMutableRefere

[PATCH] D128329: [clangd] Also mark output arguments of operator call expressions

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D128329#3617172 , @nridge wrote:

> I'm going to take the liberty of approving this, as it seems straightforward 
> and unlikely to be contentious in any way.

Thanks!

> Please let me know if you need me to commit it.

Yes, please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128329

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

[PATCH] D128892: [clangd] Also mark output arguments of array subscript expressions

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

... with the "usedAsMutableReference" semantic token modifier.
It's quite unusual to declare the index parameter of a subscript
operator as a non-const reference type, but arguably that makes it even
more helpful to be aware of it when working with such code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128892

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
@@ -742,6 +742,8 @@
 void operator()(int);
 void operator()(int, int &);
 void operator()(int, int, const int &);
+int &operator[](int &);
+int operator[](int) const;
 };
 void $Function_decl[[fun]](int, const int,
int*, const int*,
@@ -768,9 +770,12 @@
   [](int&){}($LocalVariable_usedAsMutableReference[[val]]);
   [](const int&){}($LocalVariable[[val]]);
   $Class[[ClassWithOp]] $LocalVariable_decl[[c]];
+  const $Class[[ClassWithOp]] $LocalVariable_decl_readonly[[c2]];
   $LocalVariable[[c]]($LocalVariable[[val]]);
   $LocalVariable[[c]](0, $LocalVariable_usedAsMutableReference[[val]]);
   $LocalVariable[[c]](0, 0, $LocalVariable[[val]]);
+  $LocalVariable[[c]][$LocalVariable_usedAsMutableReference[[val]]];
+  $LocalVariable_readonly[[c2]][$LocalVariable[[val]]];
 }
 struct $Class_decl[[S]] {
   $Class_decl[[S]](int&) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -543,9 +543,14 @@
 // operators as well
 llvm::ArrayRef Args = {E->getArgs(), E->getNumArgs()};
 if (const auto callOp = dyn_cast(E)) {
-  if (callOp->getOperator() != OO_Call)
+  switch (callOp->getOperator()) {
+  case OO_Call:
+  case OO_Subscript:
+Args = Args.drop_front(); // Drop object parameter
+break;
+  default:
 return true;
-  Args = Args.drop_front(); // Drop object parameter
+  }
 }
 
 highlightMutableReferenceArguments(


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -742,6 +742,8 @@
 void operator()(int);
 void operator()(int, int &);
 void operator()(int, int, const int &);
+int &operator[](int &);
+int operator[](int) const;
 };
 void $Function_decl[[fun]](int, const int,
int*, const int*,
@@ -768,9 +770,12 @@
   [](int&){}($LocalVariable_usedAsMutableReference[[val]]);
   [](const int&){}($LocalVariable[[val]]);
   $Class[[ClassWithOp]] $LocalVariable_decl[[c]];
+  const $Class[[ClassWithOp]] $LocalVariable_decl_readonly[[c2]];
   $LocalVariable[[c]]($LocalVariable[[val]]);
   $LocalVariable[[c]](0, $LocalVariable_usedAsMutableReference[[val]]);
   $LocalVariable[[c]](0, 0, $LocalVariable[[val]]);
+  $LocalVariable[[c]][$LocalVariable_usedAsMutableReference[[val]]];
+  $LocalVariable_readonly[[c2]][$LocalVariable[[val]]];
 }
 struct $Class_decl[[S]] {
   $Class_decl[[S]](int&) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -543,9 +543,14 @@
 // operators as well
 llvm::ArrayRef Args = {E->getArgs(), E->getNumArgs()};
 if (const auto callOp = dyn_cast(E)) {
-  if (callOp->getOperator() != OO_Call)
+  switch (callOp->getOperator()) {
+  case OO_Call:
+  case OO_Subscript:
+Args = Args.drop_front(); // Drop object parameter
+break;
+  default:
 return true;
-  Args = Args.drop_front(); // Drop object parameter
+  }
 }
 
 highlightMutableReferenceArguments(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-

[PATCH] D128977: [clangd] Support "usedAsMutableReference" in member initializations

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

That is, mark constructor parameters being used to initialize
non-const reference members.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128977

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
@@ -745,6 +745,15 @@
 int &operator[](int &);
 int operator[](int) const;
 };
+struct $Class_decl[[ClassWithRefMembers]] {
+  $Class_decl[[ClassWithRefMembers]](int $Parameter_decl[[i]])
+: $Field[[i1]]($Parameter[[i]]),
+  $Field_readonly[[i2]]($Parameter[[i]]),
+  $Field[[i3]]($Parameter_usedAsMutableReference[[i]]) {}
+  int $Field_decl[[i1]];
+  const int &$Field_decl_readonly[[i2]];
+  int &$Field_decl[[i3]];
+};
 void $Function_decl[[fun]](int, const int,
int*, const int*,
int&, const int&,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -523,6 +523,8 @@
 /// e.g. highlights dependent names and 'auto' as the underlying type.
 class CollectExtraHighlightings
 : public RecursiveASTVisitor {
+  using Base = RecursiveASTVisitor;
+
 public:
   CollectExtraHighlightings(HighlightingsBuilder &H) : H(H) {}
 
@@ -533,6 +535,22 @@
 return true;
   }
 
+  bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
+if (!Init->isMemberInitializer())
+  return Base::TraverseConstructorInitializer(Init);
+if (const auto Member = Init->getMember()) {
+  const auto MemberType = Member->getType();
+  if (MemberType->isLValueReferenceType() &&
+  !MemberType->getPointeeType().isConstQualified()) {
+if (const auto Param = dyn_cast(Init->getInit())) {
+  H.addExtraModifier(Param->getLocation(),
+ HighlightingModifier::UsedAsMutableReference);
+}
+  }
+}
+return Base::TraverseConstructorInitializer(Init);
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 // Highlighting parameters passed by non-const reference does not really
 // make sense for literals...


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -745,6 +745,15 @@
 int &operator[](int &);
 int operator[](int) const;
 };
+struct $Class_decl[[ClassWithRefMembers]] {
+  $Class_decl[[ClassWithRefMembers]](int $Parameter_decl[[i]])
+: $Field[[i1]]($Parameter[[i]]),
+  $Field_readonly[[i2]]($Parameter[[i]]),
+  $Field[[i3]]($Parameter_usedAsMutableReference[[i]]) {}
+  int $Field_decl[[i1]];
+  const int &$Field_decl_readonly[[i2]];
+  int &$Field_decl[[i3]];
+};
 void $Function_decl[[fun]](int, const int,
int*, const int*,
int&, const int&,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -523,6 +523,8 @@
 /// e.g. highlights dependent names and 'auto' as the underlying type.
 class CollectExtraHighlightings
 : public RecursiveASTVisitor {
+  using Base = RecursiveASTVisitor;
+
 public:
   CollectExtraHighlightings(HighlightingsBuilder &H) : H(H) {}
 
@@ -533,6 +535,22 @@
 return true;
   }
 
+  bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
+if (!Init->isMemberInitializer())
+  return Base::TraverseConstructorInitializer(Init);
+if (const auto Member = Init->getMember()) {
+  const auto MemberType = Member->getType();
+  if (MemberType->isLValueReferenceType() &&
+  !MemberType->getPointeeType().isConstQualified()) {
+if (const auto Param = dyn_cast(Init->getInit())) {
+  H.addExtraModifier(Param->getLocation(),
+ HighlightingModifier::UsedAsMutableReference)

[PATCH] D128977: [clangd] Support "usedAsMutableReference" in member initializations

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 442257.
ckandeler marked 4 inline comments as done.
ckandeler added a comment.

Implemented reviewer suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128977

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
@@ -745,6 +745,23 @@
 int &operator[](int &);
 int operator[](int) const;
 };
+struct $Class_decl[[ClassWithStaticMember]] {
+static inline int $StaticField_decl_static[[j]] = 0;
+};
+struct $Class_decl[[ClassWithRefMembers]] {
+  $Class_decl[[ClassWithRefMembers]](int $Parameter_decl[[i]])
+: $Field[[i1]]($Parameter[[i]]),
+  $Field_readonly[[i2]]($Parameter[[i]]),
+  $Field[[i3]]($Parameter_usedAsMutableReference[[i]]),
+  $Field_readonly[[i4]]($Class[[ClassWithStaticMember]]::$StaticField_static[[j]]),
+  $Field[[i5]]($Class[[ClassWithStaticMember]]::$StaticField_static_usedAsMutableReference[[j]])
+  {}
+  int $Field_decl[[i1]];
+  const int &$Field_decl_readonly[[i2]];
+  int &$Field_decl[[i3]];
+  const int &$Field_decl_readonly[[i4]];
+  int &$Field_decl[[i5]];
+};
 void $Function_decl[[fun]](int, const int,
int*, const int*,
int&, const int&,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -523,6 +523,8 @@
 /// e.g. highlights dependent names and 'auto' as the underlying type.
 class CollectExtraHighlightings
 : public RecursiveASTVisitor {
+  using Base = RecursiveASTVisitor;
+
 public:
   CollectExtraHighlightings(HighlightingsBuilder &H) : H(H) {}
 
@@ -533,6 +535,13 @@
 return true;
   }
 
+  bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
+if (Init->isMemberInitializer())
+  if (auto *Member = Init->getMember())
+highlightMutableReferenceArgument(Member->getType(), Init->getInit());
+return Base::TraverseConstructorInitializer(Init);
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 // Highlighting parameters passed by non-const reference does not really
 // make sense for literals...
@@ -542,8 +551,8 @@
 // FIXME: consider highlighting parameters of some other overloaded
 // operators as well
 llvm::ArrayRef Args = {E->getArgs(), E->getNumArgs()};
-if (const auto callOp = dyn_cast(E)) {
-  switch (callOp->getOperator()) {
+if (auto *CallOp = dyn_cast(E)) {
+  switch (CallOp->getOperator()) {
   case OO_Call:
   case OO_Subscript:
 Args = Args.drop_front(); // Drop object parameter
@@ -559,6 +568,33 @@
 return true;
   }
 
+  void highlightMutableReferenceArgument(QualType T, const Expr *Arg) {
+if (!Arg)
+  return;
+
+// Is this parameter passed by non-const reference?
+// FIXME The condition T->idDependentType() could be relaxed a bit,
+// e.g. std::vector& is dependent but we would want to highlight it
+if (!T->isLValueReferenceType() ||
+T.getNonReferenceType().isConstQualified() || T->isDependentType()) {
+  return;
+}
+
+llvm::Optional Location;
+
+// FIXME Add "unwrapping" for ArraySubscriptExpr and UnaryOperator,
+//  e.g. highlight `a` in `a[i]`
+// FIXME Handle dependent expression types
+if (auto *DR = dyn_cast(Arg))
+  Location = DR->getLocation();
+else if (auto *M = dyn_cast(Arg))
+  Location = M->getMemberLoc();
+
+if (Location)
+  H.addExtraModifier(*Location,
+ HighlightingModifier::UsedAsMutableReference);
+  }
+
   void
   highlightMutableReferenceArguments(const FunctionDecl *FD,
  llvm::ArrayRef Args) {
@@ -571,31 +607,7 @@
   // highlighting modifier to the corresponding expression
   for (size_t I = 0;
I < std::min(size_t(ProtoType->getNumParams()), Args.size()); ++I) {
-auto T = ProtoType->getParamType(I);
-
-// Is this parameter passed by non-const reference?
-// FIXME The condition !T->idDependentType() could be relaxed a bit,
-// e.g. std::vector& is dependent but we would want to highlight it
-if (T->isLValueReferenceType() &&
-!T.getNonReferenceType().isConstQualified() &&
-!T->isDependentType()) {
-  if (

[PATCH] D128977: [clangd] Support "usedAsMutableReference" in member initializations

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:541
+  return Base::TraverseConstructorInitializer(Init);
+if (const auto Member = Init->getMember()) {
+  const auto MemberType = Member->getType();

nridge wrote:
> Use `auto *Member` to make clang-tidy happy (`const auto *` is also fine)
> 
> Likewise in a few places below
Ewww... But okay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128977

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

[PATCH] D128977: [clangd] Support "usedAsMutableReference" in member initializations

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Note that the build failure seems unrelated, as it's due to a failing 
clang-tidy test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128977

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

[PATCH] D128977: [clangd] Support "usedAsMutableReference" in member initializations

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Can you please merge it as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128977

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

[PATCH] D128977: [clangd] Support "usedAsMutableReference" in member initializations

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D128977#3638198 , @nridge wrote:

> Yes, I plan to. I'm just building it locally to verify the tests are passing. 
> (And sometimes, for reasons I don't understand, LLVM's build system triggers 
> a full rebuild even though I've only applied a patch that touches a couple of 
> files...)

I see, thanks.
Regarding the scope of rebuilds, it seems to me that the build system pulls in 
repository meta data, as there's lots of files rebuilt after simply amending a 
commit with no sources having changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128977

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

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 448655.
ckandeler added a comment.

Addressed remaining Objective-C issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
@@ -18,18 +18,18 @@
 TEST_F(AnnotateHighlightingsTest, Test) {
   EXPECT_AVAILABLE("^vo^id^ ^f(^) {^}^"); // available everywhere.
   EXPECT_AVAILABLE("[[int a; int b;]]");
-  EXPECT_EQ("void /* Function [decl] [globalScope] */f() {}",
+  EXPECT_EQ("void /* Function [decl] [def] [globalScope] */f() {}",
 apply("void ^f() {}"));
 
   EXPECT_EQ(apply("[[int f1(); const int x = f1();]]"),
 "int /* Function [decl] [globalScope] */f1(); "
-"const int /* Variable [decl] [readonly] [fileScope] */x = "
+"const int /* Variable [decl] [def] [readonly] [fileScope] */x = "
 "/* Function [globalScope] */f1();");
 
   // Only the targeted range is annotated.
   EXPECT_EQ(apply("void f1(); void f2() {^}"),
 "void f1(); "
-"void /* Function [decl] [globalScope] */f2() {}");
+"void /* Function [decl] [def] [globalScope] */f2() {}");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -97,52 +97,52 @@
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
   R"cpp(
-  struct $Class_decl[[AS]] {
+  struct $Class_decl_def[[AS]] {
 double $Field_decl[[SomeMember]];
   };
   struct {
-  } $Variable_decl[[S]];
-  void $Function_decl[[foo]](int $Parameter_decl[[A]], $Class[[AS]] $Parameter_decl[[As]]) {
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[VeryLongVariableName]] = 12312;
-$Class[[AS]] $LocalVariable_decl[[AA]];
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
-auto $LocalVariable_decl[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_decl[[A]]) -> void {};
+  } $Variable_decl_def[[S]];
+  void $Function_decl_def[[foo]](int $Parameter_decl[[A]], $Class[[AS]] $Parameter_decl[[As]]) {
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl_def[[VeryLongVariableName]] = 12312;
+$Class[[AS]] $LocalVariable_decl_def[[AA]];
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl_def[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
+auto $LocalVariable_decl_def[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_decl[[A]]) -> void {};
 $LocalVariable[[FN]](12312);
   }
 )cpp",
   R"cpp(
   void $Function_decl[[foo]](int);
   void $Function_decl[[Gah]]();
-  void $Function_decl[[foo]]() {
-auto $LocalVariable_decl[[Bou]] = $Function[[Gah]];
+  void $Function_decl_def[[foo]]() {
+auto $LocalVariable_decl_def[[Bou]] = $Function[[Gah]];
   }
-  struct $Class_decl[[A]] {
+  struct $Class_decl_def[[A]] {
 void $Method_decl[[abc]]();
   };
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
 template
-struct $Class_decl[[A]] {
+struct $Class_decl_def[[A]] {
   $TemplateParameter[[T]] $Field_decl[[t]];
 };
   }
   template
-  struct $Class_decl[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
+  struct $Class_decl_def[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
 typename $TemplateParameter[[T]]::$Type_dependentName[[A]]* $Field_decl[[D]];
   };
-  $Namespace[[abc]]::$Class[[A]] $Variable_decl[[AA]];
+  $Namespace[[abc]]::$Class[[A]] $Variable_decl_def[[AA]];
   typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]];
-  struct $Class_decl[[B]] {
+  struct $Class_decl_def[[B]] {
 $Class_decl[[B]]();
 ~$Class[[B]](); // FIXME: inconsistent with constructor
 void operator<<($Class[[B]]);
 $Class[[AAA]] $Field_decl[[AA]];
   };
-  $Class[[B]]::$Class_decl[[B]]() {}
+  $Class[[B]]::

[PATCH] D130015: [clangd] Add "usedAsMutablePointer" highlighting modifier

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130015

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

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added reviewers: sammccall, nridge.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This matches user expectations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131175

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
@@ -841,6 +841,12 @@
 $Function_deprecated[[Foo]]($Parameter[[x]]); 
 }
   )cpp",
+  // Predefined identifiers
+  R"cpp(
+void $Function_decl[[Foo]]() {
+const char *$LocalVariable_decl_readonly[[s]] = $Macro[[__func__]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -542,6 +542,11 @@
 return Base::TraverseConstructorInitializer(Init);
   }
 
+  bool VisitPredefinedExpr(PredefinedExpr *E) {
+H.addToken(E->getLocation(), HighlightingKind::Macro);
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 // Highlighting parameters passed by non-const reference does not really
 // make sense for literals...


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -841,6 +841,12 @@
 $Function_deprecated[[Foo]]($Parameter[[x]]); 
 }
   )cpp",
+  // Predefined identifiers
+  R"cpp(
+void $Function_decl[[Foo]]() {
+const char *$LocalVariable_decl_readonly[[s]] = $Macro[[__func__]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -542,6 +542,11 @@
 return Base::TraverseConstructorInitializer(Init);
   }
 
+  bool VisitPredefinedExpr(PredefinedExpr *E) {
+H.addToken(E->getLocation(), HighlightingKind::Macro);
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 // Highlighting parameters passed by non-const reference does not really
 // make sense for literals...
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130015: [clangd] Add "usedAsMutablePointer" highlighting modifier

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

IMO the relevant point is the (non-)const-ness of the corresponding parameter, 
i.e. can the passed object get mutated or not. The fact that there's an 
ampersand at the call site (which is not even the case if the variable is 
itself a pointer) does not tell me that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130015

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

[PATCH] D130015: [clangd] Add "usedAsMutablePointer" highlighting modifier

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 451420.
ckandeler marked an inline comment as done.
ckandeler added a comment.

Fixed style issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130015

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.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
@@ -373,7 +373,7 @@
   void $Function_decl[[foo]]() {
 $Class[[F]] $LocalVariable_decl[[FF]];
 $Class[[G]]<$Class[[F]], &$Class[[F]]::$Method[[f]]> $LocalVariable_decl[[GG]];
-$LocalVariable[[GG]].$Method[[foo]](&$LocalVariable[[FF]]);
+$LocalVariable[[GG]].$Method[[foo]](&$LocalVariable_usedAsMutablePointer[[FF]]);
 $Class[[A]]<$Function[[foo]]> $LocalVariable_decl[[AA]];
   }
 )cpp",
@@ -772,14 +772,14 @@
   const int* $LocalVariable_decl_readonly[[constPtr]];
   int** $LocalVariable_decl[[array]];
   $Function[[fun]]($LocalVariable[[val]], $LocalVariable[[val]], 
-   $LocalVariable[[ptr]], $LocalVariable_readonly[[constPtr]], 
+   $LocalVariable_usedAsMutablePointer[[ptr]], $LocalVariable_readonly[[constPtr]],
$LocalVariable_usedAsMutableReference[[val]], $LocalVariable[[val]], 
 
$LocalVariable_usedAsMutableReference[[ptr]],
$LocalVariable_readonly_usedAsMutableReference[[constPtr]],
$LocalVariable_readonly[[constPtr]],
 
-   $LocalVariable[[array]], $LocalVariable_usedAsMutableReference[[array]], 
+   $LocalVariable_usedAsMutablePointer[[array]], $LocalVariable_usedAsMutableReference[[array]],
$LocalVariable[[array]]
);
   [](int){}($LocalVariable[[val]]);
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- clang-tools-extra/clangd/test/semantic-tokens.test
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  8193
+# CHECK-NEXT:  16385
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "1"
 # CHECK-NEXT:  }
@@ -49,7 +49,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  8193
+# CHECK-NEXT:  16385
 # CHECK-NEXT:],
 #Inserted at position 1
 # CHECK-NEXT:"deleteCount": 0,
@@ -72,12 +72,12 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  8193,
+# CHECK-NEXT:  16385,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  8193
+# CHECK-NEXT:  16385
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "3"
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -66,6 +66,7 @@
 # CHECK-NEXT:"dependentName",
 # CHECK-NEXT:"defaultLibrary",
 # CHECK-NEXT:"usedAsMutableReference",
+# CHECK-NEXT:"usedAsMutablePointer",
 # CHECK-NEXT:"functionScope",
 # CHECK-NEXT:"classScope",
 # CHECK-NEXT:"fileScope",
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -71,6 +71,7 @@
   DependentName,
   DefaultLibrary,
   UsedAsMutableReference,
+  UsedAsMutablePointer,
 
   FunctionScope,
   ClassScope,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -572,11 +572,13 @@
 if (!Arg)
   return;
 
-// Is this parameter passed by non-const reference?
+// Is this parameter passed by non-const pointer or reference?
 // FIXME The condition T->idDependentType() could be relaxed a bit,
 // e.g. std::vector& is dependent but we would want to highlight it
-if (!T->isLValueReferenceType() ||
-T.getN

[PATCH] D130015: [clangd] Add "usedAsMutablePointer" highlighting modifier

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:603
   H.addExtraModifier(*Location,
- HighlightingModifier::UsedAsMutableReference);
+ IsRef ? HighlightingModifier::UsedAsMutableReference
+   : HighlightingModifier::UsedAsMutablePointer);

kadircet wrote:
> why do you think we should have different modifiers for these? they both 
> indicate the same thing, the parameter might get mutated by the call, and it 
> being a pointer or reference doesn't seem to matter.
Because I'm aware that there are people (like Nathan) who don't find it useful 
for pointers, and having just one modifier would force it on everybody. With 
two distinct modifiers, clients can decide for themselves, possibly even 
exposing the choice to users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130015

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

[PATCH] D118976: clangd: Set a diagnostic on a code action resulting from a tweak

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

... if there is a match.
This is needed to that clients can can make a connection between a
diagnostic and an associated quickfix-tweak.
Ideally, quickfix-kind tweak code actions would be provided inline along
with the non-tweak fixes, but this doesn't seem easily achievable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118976

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -986,7 +986,7 @@
 
   // Now enumerate the semantic code actions.
   auto ConsumeActions =
-  [Reply = std::move(Reply), File, Selection = Params.range,
+  [Diags = Params.context.diagnostics, Reply = std::move(Reply), File, 
Selection = Params.range,
FixIts = std::move(FixIts), this](
   llvm::Expected> Tweaks) mutable {
 if (!Tweaks)
@@ -994,8 +994,17 @@
 
 std::vector Actions = std::move(FixIts);
 Actions.reserve(Actions.size() + Tweaks->size());
-for (const auto &T : *Tweaks)
+for (const auto &T : *Tweaks) {
   Actions.push_back(toCodeAction(T, File, Selection));
+  if (T.Kind != CodeAction::QUICKFIX_KIND)
+continue;
+  for (const Diagnostic &D : Diags) {
+if (D.range == Selection) {
+  Actions.back().diagnostics = {D};
+  break;
+}
+  }
+}
 
 // If there's exactly one quick-fix, call it "preferred".
 // We never consider refactorings etc as preferred.


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -986,7 +986,7 @@
 
   // Now enumerate the semantic code actions.
   auto ConsumeActions =
-  [Reply = std::move(Reply), File, Selection = Params.range,
+  [Diags = Params.context.diagnostics, Reply = std::move(Reply), File, Selection = Params.range,
FixIts = std::move(FixIts), this](
   llvm::Expected> Tweaks) mutable {
 if (!Tweaks)
@@ -994,8 +994,17 @@
 
 std::vector Actions = std::move(FixIts);
 Actions.reserve(Actions.size() + Tweaks->size());
-for (const auto &T : *Tweaks)
+for (const auto &T : *Tweaks) {
   Actions.push_back(toCodeAction(T, File, Selection));
+  if (T.Kind != CodeAction::QUICKFIX_KIND)
+continue;
+  for (const Diagnostic &D : Diags) {
+if (D.range == Selection) {
+  Actions.back().diagnostics = {D};
+  break;
+}
+  }
+}
 
 // If there's exactly one quick-fix, call it "preferred".
 // We never consider refactorings etc as preferred.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118782: clangd: Add a break for every case in the PopulateSwitch tweak

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 405893.
ckandeler added a comment.

Improved code as per review comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118782

Files:
  clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
  clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
@@ -123,7 +123,7 @@
   )"",
   R""(
 enum class Enum {A,B};
-switch (Enum::A) {case Enum::A:case Enum::B:break;}
+switch (Enum::A) {case Enum::A:break;case Enum::B:break;}
   )"",
   },
   {
@@ -135,7 +135,7 @@
   )"",
   R""(
 enum Enum {A,B,C};
-switch (A) {case B:break;case A:case C:break;}
+switch (A) {case B:break;case A:break;case C:break;}
   )"",
   },
   {
@@ -148,7 +148,7 @@
   )"",
   R""(
 enum Enum {A,B=1,C};
-switch (A) {case 1:break;case A:case C:break;}
+switch (A) {case 1:break;case A:break;case C:break;}
   )"",
   },
   {
@@ -162,7 +162,7 @@
   R""(
 enum class Enum {A,B,C};
 switch (Enum::A)
-{case Enum::B:break;case Enum::A:case Enum::C:break;}
+{case Enum::B:break;case Enum::A:break;case Enum::C:break;}
   )"",
   },
   {
@@ -193,12 +193,12 @@
   // Duplicated constant names
   Function,
   R""(enum Enum {A,B,b=B}; ^switch (A) {})"",
-  R""(enum Enum {A,B,b=B}; switch (A) {case A:case B:break;})"",
+  R""(enum Enum {A,B,b=B}; switch (A) {case A:break;case B:break;})"",
   },
   {
   // Duplicated constant names all in switch
   Function,
-  R""(enum Enum {A,B,b=B}; ^switch (A) {case A:case B:break;})"",
+  R""(enum Enum {A,B,b=B}; ^switch (A) {case A:break;case B:break;})"",
   "unavailable",
   },
   {
@@ -217,7 +217,7 @@
R""(
 enum CEnum {A,B,C};
 enum CEnum val = A;
-switch (val) {case B:break;case A:case C:break;}
+switch (val) {case B:break;case A:break;case C:break;}
   )"",
"TestTU.c"},
   {// C: Only filling in missing enumerators w/ typedefs
@@ -236,7 +236,7 @@
 typedef enum ControlState ControlState;
 enum ControlState : UInteger {A,B,C};
 ControlState controlState = A;
-switch (controlState) {case A:break;case B:case C:break;}
+switch (controlState) {case A:break;case B:break;case C:break;}
   )"",
"TestTU.c"},
   };
Index: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -22,7 +22,9 @@
 //   void f(Color color) {
 // switch (color) {
 // case RED:
+//   break;
 // case GREEN:
+//   break;
 // case BLUE:
 //   break;
 // }
@@ -203,11 +205,10 @@
 Text.append({"case ", getQualification(DeclASTCtx, DeclCtx, Loc, EnumD)});
 if (EnumD->isScoped())
   Text.append({EnumD->getName(), "::"});
-Text.append({EnumConstant.second.getEnumConstant()->getName(), ":"});
+Text.append({EnumConstant.second.getEnumConstant()->getName(), ":break;"});
   }
 
   assert(!Text.empty() && "No enumerators to insert!");
-  Text += "break;";
 
   const SourceManager &SM = Ctx.getSourceManager();
   return Effect::mainFileEdit(


Index: clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
@@ -123,7 +123,7 @@
   )"",
   R""(
 enum class Enum {A,B};
-switch (Enum::A) {case Enum::A:case Enum::B:break;}
+switch (Enum::A) {case Enum::A:break;case Enum::B:break;}
   )"",
   },
   {
@@ -135,7 +135,7 @@
   )"",
   R""(
 enum Enum {A,B,C};
-switch (A) {case B:break;case A:case C:break;}
+switch (A) {case B:break;case A:break;case C:break;}
   )"",
   },
   {
@@ -148,7 +148,7 @@
   )"",
   R""(
 enum Enum {A,B=1,C};
-switch (A) {case 1:break;case A:case C:break;}
+switch (A) {case 1:break;case A:break;case C:break;}
   )"",
   },
   {
@@ -162,7 +162,7 @@
  

[PATCH] D118976: clangd: Set a diagnostic on a code action resulting from a tweak

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

> This is assuming a semantic connection that we don't know exists.
> Without any more specific reason to draw this connection, this seems like a 
> heuristic that could equally be applied by the client.

The difference being that the client would have to poke around in the 
technically opaque Command structure to find the range.
Seems possible, but presumably there are no guarantees about the properties?

> Is there a particular action/diagnostic pair you want this for?

Yes, -Wswitch/PopulateSwitch (which is the only quickfix kind of tweak at the 
moment).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118976

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

[PATCH] D118976: clangd: Set a diagnostic on a code action resulting from a tweak

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 405911.
ckandeler added a comment.

Formatting & logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118976

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -986,9 +986,9 @@
 
   // Now enumerate the semantic code actions.
   auto ConsumeActions =
-  [Reply = std::move(Reply), File, Selection = Params.range,
-   FixIts = std::move(FixIts), this](
-  llvm::Expected> Tweaks) mutable {
+  [Diags = Params.context.diagnostics, Reply = std::move(Reply), File,
+   Selection = Params.range, FixIts = std::move(FixIts), this](
+llvm::Expected> Tweaks) mutable {
 if (!Tweaks)
   return Reply(Tweaks.takeError());
 
@@ -999,17 +999,21 @@
 
 // If there's exactly one quick-fix, call it "preferred".
 // We never consider refactorings etc as preferred.
-CodeAction *OnlyFix = nullptr;
+Optional OnlyFix; // nullptr means multiple fixes
 for (auto &Action : Actions) {
   if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND) {
 if (OnlyFix) {
-  OnlyFix->isPreferred = false;
+  *OnlyFix = nullptr;
   break;
 }
-Action.isPreferred = true;
 OnlyFix = &Action;
   }
 }
+if (OnlyFix && *OnlyFix) {
+  (*OnlyFix)->isPreferred = true;
+  if (Diags.size() == 1 && Diags.front().range == Selection)
+(*OnlyFix)->diagnostics = {Diags.front()};
+}
 
 if (SupportsCodeAction)
   return Reply(llvm::json::Array(Actions));


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -986,9 +986,9 @@
 
   // Now enumerate the semantic code actions.
   auto ConsumeActions =
-  [Reply = std::move(Reply), File, Selection = Params.range,
-   FixIts = std::move(FixIts), this](
-  llvm::Expected> Tweaks) mutable {
+  [Diags = Params.context.diagnostics, Reply = std::move(Reply), File,
+   Selection = Params.range, FixIts = std::move(FixIts), this](
+llvm::Expected> Tweaks) mutable {
 if (!Tweaks)
   return Reply(Tweaks.takeError());
 
@@ -999,17 +999,21 @@
 
 // If there's exactly one quick-fix, call it "preferred".
 // We never consider refactorings etc as preferred.
-CodeAction *OnlyFix = nullptr;
+Optional OnlyFix; // nullptr means multiple fixes
 for (auto &Action : Actions) {
   if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND) {
 if (OnlyFix) {
-  OnlyFix->isPreferred = false;
+  *OnlyFix = nullptr;
   break;
 }
-Action.isPreferred = true;
 OnlyFix = &Action;
   }
 }
+if (OnlyFix && *OnlyFix) {
+  (*OnlyFix)->isPreferred = true;
+  if (Diags.size() == 1 && Diags.front().range == Selection)
+(*OnlyFix)->diagnostics = {Diags.front()};
+}
 
 if (SupportsCodeAction)
   return Reply(llvm::json::Array(Actions));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118976: clangd: Set a diagnostic on a code action resulting from a tweak

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1011
 // We never consider refactorings etc as preferred.
 CodeAction *OnlyFix = nullptr;
 for (auto &Action : Actions) {

sammccall wrote:
> This loop is closely related to what you're doing (and the one above isn't).
> 
> Maybe we can refactor/combine as something like:
> 
> ```
> Optional OnlyFix; // nullptr means multiple fixes
> // ... loop over actions and set OnlyFix ...
> if (OnlyFix && *OnlyFix) {
>   (*OnlyFix)->isPreferred = true;
>   if (Diags.size() == 1 && Diags.front().range == Selection)
> (*OnlyFix)->diagnostics = {Diags.front()];
> }
> ```
> 
> Note this adds the conditions:
>  - should be only one diagnostic in context
>  - should be only one quickfix action
> 
> I think these reduce the chance of false positives.
Done. Though it seems the code would work just as well with OnlyFix staying a 
normal pointer. Did I misunderstand something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118976

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

[PATCH] D118976: clangd: Set a diagnostic on a code action resulting from a tweak

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 405929.
ckandeler added a comment.

Simplified code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118976

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -986,9 +986,9 @@
 
   // Now enumerate the semantic code actions.
   auto ConsumeActions =
-  [Reply = std::move(Reply), File, Selection = Params.range,
-   FixIts = std::move(FixIts), this](
-  llvm::Expected> Tweaks) mutable {
+  [Diags = Params.context.diagnostics, Reply = std::move(Reply), File,
+   Selection = Params.range, FixIts = std::move(FixIts), this](
+llvm::Expected> Tweaks) mutable {
 if (!Tweaks)
   return Reply(Tweaks.takeError());
 
@@ -1003,13 +1003,17 @@
 for (auto &Action : Actions) {
   if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND) {
 if (OnlyFix) {
-  OnlyFix->isPreferred = false;
+  OnlyFix = nullptr;
   break;
 }
-Action.isPreferred = true;
 OnlyFix = &Action;
   }
 }
+if (OnlyFix) {
+  OnlyFix->isPreferred = true;
+  if (Diags.size() == 1 && Diags.front().range == Selection)
+OnlyFix->diagnostics = {Diags.front()};
+}
 
 if (SupportsCodeAction)
   return Reply(llvm::json::Array(Actions));


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -986,9 +986,9 @@
 
   // Now enumerate the semantic code actions.
   auto ConsumeActions =
-  [Reply = std::move(Reply), File, Selection = Params.range,
-   FixIts = std::move(FixIts), this](
-  llvm::Expected> Tweaks) mutable {
+  [Diags = Params.context.diagnostics, Reply = std::move(Reply), File,
+   Selection = Params.range, FixIts = std::move(FixIts), this](
+llvm::Expected> Tweaks) mutable {
 if (!Tweaks)
   return Reply(Tweaks.takeError());
 
@@ -1003,13 +1003,17 @@
 for (auto &Action : Actions) {
   if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND) {
 if (OnlyFix) {
-  OnlyFix->isPreferred = false;
+  OnlyFix = nullptr;
   break;
 }
-Action.isPreferred = true;
 OnlyFix = &Action;
   }
 }
+if (OnlyFix) {
+  OnlyFix->isPreferred = true;
+  if (Diags.size() == 1 && Diags.front().range == Selection)
+OnlyFix->diagnostics = {Diags.front()};
+}
 
 if (SupportsCodeAction)
   return Reply(llvm::json::Array(Actions));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118782: clangd: Add a break for every case in the PopulateSwitch tweak

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler abandoned this revision.
ckandeler added a comment.

Fair enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118782

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

[PATCH] D118976: clangd: Set a diagnostic on a code action resulting from a tweak

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 406359.
ckandeler added a comment.

Fixed Lint complaints


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118976

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -986,8 +986,8 @@
 
   // Now enumerate the semantic code actions.
   auto ConsumeActions =
-  [Reply = std::move(Reply), File, Selection = Params.range,
-   FixIts = std::move(FixIts), this](
+  [Diags = Params.context.diagnostics, Reply = std::move(Reply), File,
+   Selection = Params.range, FixIts = std::move(FixIts), this](
   llvm::Expected> Tweaks) mutable {
 if (!Tweaks)
   return Reply(Tweaks.takeError());
@@ -1003,13 +1003,17 @@
 for (auto &Action : Actions) {
   if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND) {
 if (OnlyFix) {
-  OnlyFix->isPreferred = false;
+  OnlyFix = nullptr;
   break;
 }
-Action.isPreferred = true;
 OnlyFix = &Action;
   }
 }
+if (OnlyFix) {
+  OnlyFix->isPreferred = true;
+  if (Diags.size() == 1 && Diags.front().range == Selection)
+OnlyFix->diagnostics = {Diags.front()};
+}
 
 if (SupportsCodeAction)
   return Reply(llvm::json::Array(Actions));


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -986,8 +986,8 @@
 
   // Now enumerate the semantic code actions.
   auto ConsumeActions =
-  [Reply = std::move(Reply), File, Selection = Params.range,
-   FixIts = std::move(FixIts), this](
+  [Diags = Params.context.diagnostics, Reply = std::move(Reply), File,
+   Selection = Params.range, FixIts = std::move(FixIts), this](
   llvm::Expected> Tweaks) mutable {
 if (!Tweaks)
   return Reply(Tweaks.takeError());
@@ -1003,13 +1003,17 @@
 for (auto &Action : Actions) {
   if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND) {
 if (OnlyFix) {
-  OnlyFix->isPreferred = false;
+  OnlyFix = nullptr;
   break;
 }
-Action.isPreferred = true;
 OnlyFix = &Action;
   }
 }
+if (OnlyFix) {
+  OnlyFix->isPreferred = true;
+  if (Diags.size() == 1 && Diags.front().range == Selection)
+OnlyFix->diagnostics = {Diags.front()};
+}
 
 if (SupportsCodeAction)
   return Reply(llvm::json::Array(Actions));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118976: clangd: Set a diagnostic on a code action resulting from a tweak

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

It'd be great if you could merge this, as I don't have the necessary privileges.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118976

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

[PATCH] D130015: [clangd] Add "usedAsMutablePointer" highlighting modifier

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Counterpart to "usedAsMutableReference". Just as for references, there
are const and non-const pointer parameters, and it's valuable to be able
to have different highlighting for the two cases at the call site.
We could have re-used the existing modifier, but having a dedicated one
maximizes client flexibility.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130015

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.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
@@ -374,7 +374,7 @@
   void $Function_decl[[foo]]() {
 $Class[[F]] $LocalVariable_decl[[FF]];
 $Class[[G]]<$Class[[F]], &$Class[[F]]::$Method[[f]]> $LocalVariable_decl[[GG]];
-$LocalVariable[[GG]].$Method[[foo]](&$LocalVariable[[FF]]);
+$LocalVariable[[GG]].$Method[[foo]](&$LocalVariable_usedAsMutablePointer[[FF]]);
 $Class[[A]]<$Function[[foo]]> $LocalVariable_decl[[AA]];
   }
 )cpp",
@@ -773,14 +773,14 @@
   const int* $LocalVariable_decl_readonly[[constPtr]];
   int** $LocalVariable_decl[[array]];
   $Function[[fun]]($LocalVariable[[val]], $LocalVariable[[val]], 
-   $LocalVariable[[ptr]], $LocalVariable_readonly[[constPtr]], 
+   $LocalVariable_usedAsMutablePointer[[ptr]], $LocalVariable_readonly[[constPtr]],
$LocalVariable_usedAsMutableReference[[val]], $LocalVariable[[val]], 
 
$LocalVariable_usedAsMutableReference[[ptr]],
$LocalVariable_readonly_usedAsMutableReference[[constPtr]],
$LocalVariable_readonly[[constPtr]],
 
-   $LocalVariable[[array]], $LocalVariable_usedAsMutableReference[[array]], 
+   $LocalVariable_usedAsMutablePointer[[array]], $LocalVariable_usedAsMutableReference[[array]],
$LocalVariable[[array]]
);
   [](int){}($LocalVariable[[val]]);
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- clang-tools-extra/clangd/test/semantic-tokens.test
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  8193
+# CHECK-NEXT:  16385
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "1"
 # CHECK-NEXT:  }
@@ -49,7 +49,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  8193
+# CHECK-NEXT:  16385
 # CHECK-NEXT:],
 #Inserted at position 1
 # CHECK-NEXT:"deleteCount": 0,
@@ -72,12 +72,12 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  8193,
+# CHECK-NEXT:  16385,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  8193
+# CHECK-NEXT:  16385
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "3"
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -66,6 +66,7 @@
 # CHECK-NEXT:"dependentName",
 # CHECK-NEXT:"defaultLibrary",
 # CHECK-NEXT:"usedAsMutableReference",
+# CHECK-NEXT:"usedAsMutablePointer",
 # CHECK-NEXT:"functionScope",
 # CHECK-NEXT:"classScope",
 # CHECK-NEXT:"fileScope",
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -71,6 +71,7 @@
   DependentName,
   DefaultLibrary,
   UsedAsMutableReference,
+  UsedAsMutablePointer,
 
   FunctionScope,
   ClassScope,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ 

[PATCH] D110324: clangd: Do not report inline overrides twice

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

... in textDocument/references.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110324

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1947,6 +1947,9 @@
   void $overridedecl[[func]]() override;
 };
 void Derived::$overridedef[[func]]() {}
+class Derived2 : public Base {
+  void $overridedef[[func]]() override {}
+};
 void test(Derived* D) {
   D->func();  // No references to the overrides.
 })cpp";
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1431,17 +1431,20 @@
 !OverriddenBy.Subjects.empty())
   Index->relations(
   OverriddenBy, [&](const SymbolID &Subject, const Symbol &Object) {
-if (auto LSPLoc =
-toLSPLocation(Object.CanonicalDeclaration, *MainFilePath)) 
{
+const auto LSPLocDecl = toLSPLocation(Object.CanonicalDeclaration,
+  *MainFilePath);
+const auto LSPLocDef = toLSPLocation(Object.Definition,
+ *MainFilePath);
+if (LSPLocDecl && LSPLocDecl != LSPLocDef) {
   ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLoc);
+  Result.Loc = std::move(*LSPLocDecl);
   Result.Attributes =
   ReferencesResult::Declaration | ReferencesResult::Override;
   Results.References.push_back(std::move(Result));
 }
-if (auto LSPLoc = toLSPLocation(Object.Definition, *MainFilePath)) 
{
+if (LSPLocDef) {
   ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLoc);
+  Result.Loc = std::move(*LSPLocDef);
   Result.Attributes = ReferencesResult::Declaration |
   ReferencesResult::Definition |
   ReferencesResult::Override;


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1947,6 +1947,9 @@
   void $overridedecl[[func]]() override;
 };
 void Derived::$overridedef[[func]]() {}
+class Derived2 : public Base {
+  void $overridedef[[func]]() override {}
+};
 void test(Derived* D) {
   D->func();  // No references to the overrides.
 })cpp";
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1431,17 +1431,20 @@
 !OverriddenBy.Subjects.empty())
   Index->relations(
   OverriddenBy, [&](const SymbolID &Subject, const Symbol &Object) {
-if (auto LSPLoc =
-toLSPLocation(Object.CanonicalDeclaration, *MainFilePath)) {
+const auto LSPLocDecl = toLSPLocation(Object.CanonicalDeclaration,
+  *MainFilePath);
+const auto LSPLocDef = toLSPLocation(Object.Definition,
+ *MainFilePath);
+if (LSPLocDecl && LSPLocDecl != LSPLocDef) {
   ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLoc);
+  Result.Loc = std::move(*LSPLocDecl);
   Result.Attributes =
   ReferencesResult::Declaration | ReferencesResult::Override;
   Results.References.push_back(std::move(Result));
 }
-if (auto LSPLoc = toLSPLocation(Object.Definition, *MainFilePath)) {
+if (LSPLocDef) {
   ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLoc);
+  Result.Loc = std::move(*LSPLocDef);
   Result.Attributes = ReferencesResult::Declaration |
   ReferencesResult::Definition |
   ReferencesResult::Override;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/ma

[PATCH] D110324: clangd: Do not report inline overrides twice

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 374520.
ckandeler added a comment.

Fixed formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110324

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1947,6 +1947,9 @@
   void $overridedecl[[func]]() override;
 };
 void Derived::$overridedef[[func]]() {}
+class Derived2 : public Base {
+  void $overridedef[[func]]() override {}
+};
 void test(Derived* D) {
   D->func();  // No references to the overrides.
 })cpp";
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1431,17 +1431,20 @@
 !OverriddenBy.Subjects.empty())
   Index->relations(
   OverriddenBy, [&](const SymbolID &Subject, const Symbol &Object) {
-if (auto LSPLoc =
-toLSPLocation(Object.CanonicalDeclaration, *MainFilePath)) 
{
+const auto LSPLocDecl =
+toLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
+const auto LSPLocDef =
+toLSPLocation(Object.Definition, *MainFilePath);
+if (LSPLocDecl && LSPLocDecl != LSPLocDef) {
   ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLoc);
+  Result.Loc = std::move(*LSPLocDecl);
   Result.Attributes =
   ReferencesResult::Declaration | ReferencesResult::Override;
   Results.References.push_back(std::move(Result));
 }
-if (auto LSPLoc = toLSPLocation(Object.Definition, *MainFilePath)) 
{
+if (LSPLocDef) {
   ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLoc);
+  Result.Loc = std::move(*LSPLocDef);
   Result.Attributes = ReferencesResult::Declaration |
   ReferencesResult::Definition |
   ReferencesResult::Override;


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1947,6 +1947,9 @@
   void $overridedecl[[func]]() override;
 };
 void Derived::$overridedef[[func]]() {}
+class Derived2 : public Base {
+  void $overridedef[[func]]() override {}
+};
 void test(Derived* D) {
   D->func();  // No references to the overrides.
 })cpp";
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1431,17 +1431,20 @@
 !OverriddenBy.Subjects.empty())
   Index->relations(
   OverriddenBy, [&](const SymbolID &Subject, const Symbol &Object) {
-if (auto LSPLoc =
-toLSPLocation(Object.CanonicalDeclaration, *MainFilePath)) {
+const auto LSPLocDecl =
+toLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
+const auto LSPLocDef =
+toLSPLocation(Object.Definition, *MainFilePath);
+if (LSPLocDecl && LSPLocDecl != LSPLocDef) {
   ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLoc);
+  Result.Loc = std::move(*LSPLocDecl);
   Result.Attributes =
   ReferencesResult::Declaration | ReferencesResult::Override;
   Results.References.push_back(std::move(Result));
 }
-if (auto LSPLoc = toLSPLocation(Object.Definition, *MainFilePath)) {
+if (LSPLocDef) {
   ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLoc);
+  Result.Loc = std::move(*LSPLocDef);
   Result.Attributes = ReferencesResult::Declaration |
   ReferencesResult::Definition |
   ReferencesResult::Override;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110324: clangd: Do not report inline overrides twice

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 374526.
ckandeler added a comment.

Re-formatting, take two.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110324

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1947,6 +1947,9 @@
   void $overridedecl[[func]]() override;
 };
 void Derived::$overridedef[[func]]() {}
+class Derived2 : public Base {
+  void $overridedef[[func]]() override {}
+};
 void test(Derived* D) {
   D->func();  // No references to the overrides.
 })cpp";
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1431,17 +1431,20 @@
 !OverriddenBy.Subjects.empty())
   Index->relations(
   OverriddenBy, [&](const SymbolID &Subject, const Symbol &Object) {
-if (auto LSPLoc =
-toLSPLocation(Object.CanonicalDeclaration, *MainFilePath)) 
{
+const auto LSPLocDecl =
+toLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
+const auto LSPLocDef =
+toLSPLocation(Object.Definition, *MainFilePath);
+if (LSPLocDecl && LSPLocDecl != LSPLocDef) {
   ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLoc);
+  Result.Loc = std::move(*LSPLocDecl);
   Result.Attributes =
   ReferencesResult::Declaration | ReferencesResult::Override;
   Results.References.push_back(std::move(Result));
 }
-if (auto LSPLoc = toLSPLocation(Object.Definition, *MainFilePath)) 
{
+if (LSPLocDef) {
   ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLoc);
+  Result.Loc = std::move(*LSPLocDef);
   Result.Attributes = ReferencesResult::Declaration |
   ReferencesResult::Definition |
   ReferencesResult::Override;


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1947,6 +1947,9 @@
   void $overridedecl[[func]]() override;
 };
 void Derived::$overridedef[[func]]() {}
+class Derived2 : public Base {
+  void $overridedef[[func]]() override {}
+};
 void test(Derived* D) {
   D->func();  // No references to the overrides.
 })cpp";
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1431,17 +1431,20 @@
 !OverriddenBy.Subjects.empty())
   Index->relations(
   OverriddenBy, [&](const SymbolID &Subject, const Symbol &Object) {
-if (auto LSPLoc =
-toLSPLocation(Object.CanonicalDeclaration, *MainFilePath)) {
+const auto LSPLocDecl =
+toLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
+const auto LSPLocDef =
+toLSPLocation(Object.Definition, *MainFilePath);
+if (LSPLocDecl && LSPLocDecl != LSPLocDef) {
   ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLoc);
+  Result.Loc = std::move(*LSPLocDecl);
   Result.Attributes =
   ReferencesResult::Declaration | ReferencesResult::Override;
   Results.References.push_back(std::move(Result));
 }
-if (auto LSPLoc = toLSPLocation(Object.Definition, *MainFilePath)) {
+if (LSPLocDef) {
   ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLoc);
+  Result.Loc = std::move(*LSPLocDef);
   Result.Attributes = ReferencesResult::Declaration |
   ReferencesResult::Definition |
   ReferencesResult::Override;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110324: clangd: Do not report inline overrides twice

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Can you please merge it, too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110324

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

[PATCH] D110324: clangd: Do not report inline overrides twice

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110324

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

[PATCH] D107137: clangd: Provide hover info for include directives

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

It's quite useful to be able to hover over an #include and see the full
path to the header file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107137

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2750,6 +2750,15 @@
 
 // In test::Bar
 int foo = 3)",
+  },
+  {
+  [](HoverInfo &HI)
+  {
+  HI.TargetFile = Location();
+  HI.TargetFile->uri
+  = URIForFile::canonicalize("/usr/include/stdio.h", "/");
+  },
+  "/usr/include/stdio.h",
   }};
 
   for (const auto &C : Cases) {
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -74,6 +74,12 @@
 const syntax::Token *findNearbyIdentifier(const SpelledWord &Word,
   const syntax::TokenBuffer &TB);
 
+// Treat #included files as symbols, to enable go-to-definition and hover
+// on them.
+llvm::Optional locateFileReferent(const Position &Pos,
+ ParsedAST &AST,
+ llvm::StringRef MainFilePath);
+
 /// Get all document links
 std::vector getDocumentLinks(ParsedAST &AST);
 
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -227,24 +227,6 @@
   return L;
 }
 
-// Treat #included files as symbols, to enable go-to-definition on them.
-llvm::Optional locateFileReferent(const Position &Pos,
- ParsedAST &AST,
- llvm::StringRef MainFilePath) {
-  for (auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
-if (!Inc.Resolved.empty() && Inc.HashLine == Pos.line) {
-  LocatedSymbol File;
-  File.Name = std::string(llvm::sys::path::filename(Inc.Resolved));
-  File.PreferredDeclaration = {
-  URIForFile::canonicalize(Inc.Resolved, MainFilePath), Range{}};
-  File.Definition = File.PreferredDeclaration;
-  // We're not going to find any further symbols on #include lines.
-  return File;
-}
-  }
-  return llvm::None;
-}
-
 // Macros are simple: there's no declaration/definition distinction.
 // As a consequence, there's no need to look them up in the index either.
 llvm::Optional
@@ -552,6 +534,23 @@
 
 } // namespace
 
+llvm::Optional locateFileReferent(const Position &Pos,
+ ParsedAST &AST,
+ llvm::StringRef MainFilePath) {
+  for (auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
+if (!Inc.Resolved.empty() && Inc.HashLine == Pos.line) {
+  LocatedSymbol File;
+  File.Name = std::string(llvm::sys::path::filename(Inc.Resolved));
+  File.PreferredDeclaration = {
+  URIForFile::canonicalize(Inc.Resolved, MainFilePath), Range{}};
+  File.Definition = File.PreferredDeclaration;
+  // We're not going to find any further symbols on #include lines.
+  return File;
+}
+  }
+  return llvm::None;
+}
+
 std::vector
 locateSymbolTextually(const SpelledWord &Word, ParsedAST &AST,
   const SymbolIndex *Index, const std::string &MainFilePath,
@@ -1987,4 +1986,4 @@
 }
 
 } // namespace clangd
-} // namespace clang
\ No newline at end of file
+} // namespace clang
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -58,7 +58,8 @@
   std::string Documentation;
   /// Source code containing the definition of the symbol.
   std::string Definition;
-
+  /// For #include directives, the pointed-to file.
+  llvm::Optional TargetFile;
   /// Access specifier for declarations inside class/struct/unions, empty for
   /// others.
   std::string AccessSpecifier;
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -14,6 +14,7 @@
 #include "ParsedAST.h"
 #include "Selection.h"

[PATCH] D107145: clangd: Add new semantic token modifier "virtual"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This is needed for clients that want to highlight virtual functions
differently.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107145

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  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
@@ -642,8 +642,8 @@
 )cpp",
   R"cpp(
   class $Class_decl_abstract[[Abstract]] {
-virtual void $Method_decl_abstract[[pure]]() = 0;
-virtual void $Method_decl[[impl]]();
+virtual void $Method_decl_abstract_virtual[[pure]]() = 0;
+virtual void $Method_decl_virtual[[impl]]();
   };
   )cpp",
   R"cpp(
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -66,6 +66,7 @@
   Readonly,
   Static,
   Abstract,
+  Virtual,
   DependentName,
   DefaultLibrary,
 
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -240,6 +240,12 @@
   return false;
 }
 
+bool isVirtual(const Decl *D) {
+  if (const auto *CMD = llvm::dyn_cast(D))
+return CMD->isVirtual();
+  return false;
+}
+
 bool isDependent(const Decl *D) {
   if (isa(D))
 return true;
@@ -712,6 +718,8 @@
 Tok.addModifier(HighlightingModifier::Static);
   if (isAbstract(Decl))
 Tok.addModifier(HighlightingModifier::Abstract);
+  if (isVirtual(Decl))
+Tok.addModifier(HighlightingModifier::Virtual);
   if (isDependent(Decl))
 Tok.addModifier(HighlightingModifier::DependentName);
   if (isDefaultLibrary(Decl))
@@ -898,6 +906,8 @@
 return "deduced"; // nonstandard
   case HighlightingModifier::Abstract:
 return "abstract";
+  case HighlightingModifier::Virtual:
+return "virtual";
   case HighlightingModifier::DependentName:
 return "dependentName"; // nonstandard
   case HighlightingModifier::DefaultLibrary:


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -642,8 +642,8 @@
 )cpp",
   R"cpp(
   class $Class_decl_abstract[[Abstract]] {
-virtual void $Method_decl_abstract[[pure]]() = 0;
-virtual void $Method_decl[[impl]]();
+virtual void $Method_decl_abstract_virtual[[pure]]() = 0;
+virtual void $Method_decl_virtual[[impl]]();
   };
   )cpp",
   R"cpp(
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -66,6 +66,7 @@
   Readonly,
   Static,
   Abstract,
+  Virtual,
   DependentName,
   DefaultLibrary,
 
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -240,6 +240,12 @@
   return false;
 }
 
+bool isVirtual(const Decl *D) {
+  if (const auto *CMD = llvm::dyn_cast(D))
+return CMD->isVirtual();
+  return false;
+}
+
 bool isDependent(const Decl *D) {
   if (isa(D))
 return true;
@@ -712,6 +718,8 @@
 Tok.addModifier(HighlightingModifier::Static);
   if (isAbstract(Decl))
 Tok.addModifier(HighlightingModifier::Abstract);
+  if (isVirtual(Decl))
+Tok.addModifier(HighlightingModifier::Virtual);
   if (isDependent(Decl))
 Tok.addModifier(HighlightingModifier::DependentName);
   if (isDefaultLibrary(Decl))
@@ -898,6 +906,8 @@
 return "deduced"; // nonstandard
   case HighlightingModifier::Abstract:
 return "abstract";
+  case HighlightingModifier::Virtual:
+return "virtual";
   case HighlightingModifier::DependentName:
 return "dependentName"; // nonstandard
   case HighlightingModifier::DefaultLibrary:

[PATCH] D107145: clangd: Add new semantic token modifier "virtual"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 363031.
ckandeler added a comment.

Extended test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107145

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  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
@@ -642,9 +642,14 @@
 )cpp",
   R"cpp(
   class $Class_decl_abstract[[Abstract]] {
-virtual void $Method_decl_abstract[[pure]]() = 0;
-virtual void $Method_decl[[impl]]();
+  public:
+virtual void $Method_decl_abstract_virtual[[pure]]() = 0;
+virtual void $Method_decl_virtual[[impl]]();
   };
+  void $Function_decl[[foo]]($Class_abstract[[Abstract]]* 
$Parameter_decl[[A]]) {
+  $Parameter[[A]]->$Method_abstract_virtual[[pure]]();
+  $Parameter[[A]]->$Method_virtual[[impl]]();
+  }
   )cpp",
   R"cpp(
   <:[deprecated]:> int $Variable_decl_deprecated[[x]];
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -66,6 +66,7 @@
   Readonly,
   Static,
   Abstract,
+  Virtual,
   DependentName,
   DefaultLibrary,
 
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -240,6 +240,12 @@
   return false;
 }
 
+bool isVirtual(const Decl *D) {
+  if (const auto *CMD = llvm::dyn_cast(D))
+return CMD->isVirtual();
+  return false;
+}
+
 bool isDependent(const Decl *D) {
   if (isa(D))
 return true;
@@ -712,6 +718,8 @@
 Tok.addModifier(HighlightingModifier::Static);
   if (isAbstract(Decl))
 Tok.addModifier(HighlightingModifier::Abstract);
+  if (isVirtual(Decl))
+Tok.addModifier(HighlightingModifier::Virtual);
   if (isDependent(Decl))
 Tok.addModifier(HighlightingModifier::DependentName);
   if (isDefaultLibrary(Decl))
@@ -898,6 +906,8 @@
 return "deduced"; // nonstandard
   case HighlightingModifier::Abstract:
 return "abstract";
+  case HighlightingModifier::Virtual:
+return "virtual";
   case HighlightingModifier::DependentName:
 return "dependentName"; // nonstandard
   case HighlightingModifier::DefaultLibrary:


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -642,9 +642,14 @@
 )cpp",
   R"cpp(
   class $Class_decl_abstract[[Abstract]] {
-virtual void $Method_decl_abstract[[pure]]() = 0;
-virtual void $Method_decl[[impl]]();
+  public:
+virtual void $Method_decl_abstract_virtual[[pure]]() = 0;
+virtual void $Method_decl_virtual[[impl]]();
   };
+  void $Function_decl[[foo]]($Class_abstract[[Abstract]]* $Parameter_decl[[A]]) {
+  $Parameter[[A]]->$Method_abstract_virtual[[pure]]();
+  $Parameter[[A]]->$Method_virtual[[impl]]();
+  }
   )cpp",
   R"cpp(
   <:[deprecated]:> int $Variable_decl_deprecated[[x]];
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -66,6 +66,7 @@
   Readonly,
   Static,
   Abstract,
+  Virtual,
   DependentName,
   DefaultLibrary,
 
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -240,6 +240,12 @@
   return false;
 }
 
+bool isVirtual(const Decl *D) {
+  if (const auto *CMD = llvm::dyn_cast(D))
+return CMD->isVirtual();
+  return false;
+}
+
 bool isDependent(const Decl *D) {
   if (isa(D))
 return true;
@@ -712,6 +718,8 @@
 Tok.addModifier(HighlightingModifier::Static);
   if (isAbstract(Decl))
 Tok.addModifier(HighlightingModifier::Abstract);
+  if (isVirtual(Decl))
+Tok.addModifier(HighlightingModifier::Virtual);
   if (isDependent(Decl))
 Tok.addModifier(HighlightingModifier::DependentName);
   if (isDe

[PATCH] D107137: clangd: Provide hover info for include directives

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 363083.
ckandeler added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107137

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2750,6 +2750,15 @@
 
 // In test::Bar
 int foo = 3)",
+  },
+  {
+  [](HoverInfo &HI) {
+HI.Name = "stdio.h";
+HI.Definition = "/usr/include/stdio.h", "/";
+  },
+  R"(stdio.h
+
+/usr/include/stdio.h)",
   }};
 
   for (const auto &C : Cases) {
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -58,7 +58,7 @@
   std::string Documentation;
   /// Source code containing the definition of the symbol.
   std::string Definition;
-
+  const char *DefinitionLanguage = "cpp";
   /// Access specifier for declarations inside class/struct/unions, empty for
   /// others.
   std::string AccessSpecifier;
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -905,6 +905,22 @@
   if (TokensTouchingCursor.empty())
 return llvm::None;
 
+  // Show full header file path if cursor is on include directive.
+  if (const auto MainFilePath
+  = getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM)) {
+for (const auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
+  if (Inc.Resolved.empty() || Inc.HashLine != Pos.line)
+continue;
+  HoverInfo HI;
+  HI.Name = std::string(llvm::sys::path::filename(Inc.Resolved));
+  // FIXME: We don't have a fitting value for Kind.
+  HI.Definition = URIForFile::canonicalize(Inc.Resolved, *MainFilePath)
+  .file().str();
+  HI.DefinitionLanguage = "";
+  return HI;
+}
+  }
+
   // To be used as a backup for highlighting the selected token, we use back as
   // it aligns better with biases elsewhere (editors tend to send the position
   // for the left of the hovered token).
@@ -981,6 +997,7 @@
 
 markup::Document HoverInfo::present() const {
   markup::Document Output;
+
   // Header contains a text of the form:
   // variable `var`
   //
@@ -1081,7 +1098,8 @@
: Definition;
 // Note that we don't print anything for global namespace, to not annoy
 // non-c++ projects or projects that are not making use of namespaces.
-Output.addCodeBlock(ScopeComment + DefinitionWithAccess);
+Output.addCodeBlock(ScopeComment + DefinitionWithAccess,
+DefinitionLanguage);
   }
 
   return Output;


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2750,6 +2750,15 @@
 
 // In test::Bar
 int foo = 3)",
+  },
+  {
+  [](HoverInfo &HI) {
+HI.Name = "stdio.h";
+HI.Definition = "/usr/include/stdio.h", "/";
+  },
+  R"(stdio.h
+
+/usr/include/stdio.h)",
   }};
 
   for (const auto &C : Cases) {
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -58,7 +58,7 @@
   std::string Documentation;
   /// Source code containing the definition of the symbol.
   std::string Definition;
-
+  const char *DefinitionLanguage = "cpp";
   /// Access specifier for declarations inside class/struct/unions, empty for
   /// others.
   std::string AccessSpecifier;
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -905,6 +905,22 @@
   if (TokensTouchingCursor.empty())
 return llvm::None;
 
+  // Show full header file path if cursor is on include directive.
+  if (const auto MainFilePath
+  = getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM)) {
+for (const auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
+  if (Inc.Resolved.empty() || Inc.HashLine != Pos.line)
+continue;
+  HoverInfo HI;
+  HI.Name = std::string(llvm::sys::path::filename(Inc.Resolved));
+  // FIXME: We don't have a fitting value for Kind.
+  HI.Definition = URIForFile::canonicalize(Inc.Resolved, *MainFilePath)
+

[PATCH] D107145: clangd: Add new semantic token modifier "virtual"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 363106.
ckandeler added a comment.

Fixed lit tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107145

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.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
@@ -642,9 +642,14 @@
 )cpp",
   R"cpp(
   class $Class_decl_abstract[[Abstract]] {
-virtual void $Method_decl_abstract[[pure]]() = 0;
-virtual void $Method_decl[[impl]]();
+  public:
+virtual void $Method_decl_abstract_virtual[[pure]]() = 0;
+virtual void $Method_decl_virtual[[impl]]();
   };
+  void $Function_decl[[foo]]($Class_abstract[[Abstract]]* $Parameter_decl[[A]]) {
+  $Parameter[[A]]->$Method_abstract_virtual[[pure]]();
+  $Parameter[[A]]->$Method_virtual[[impl]]();
+  }
   )cpp",
   R"cpp(
   <:[deprecated]:> int $Variable_decl_deprecated[[x]];
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- clang-tools-extra/clangd/test/semantic-tokens.test
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  2049
+# CHECK-NEXT:  4097
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "1"
 # CHECK-NEXT:  }
@@ -49,7 +49,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  2049
+# CHECK-NEXT:  4097
 # CHECK-NEXT:],
 #Inserted at position 1
 # CHECK-NEXT:"deleteCount": 0,
@@ -72,12 +72,12 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  2049,
+# CHECK-NEXT:  4097,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  2049
+# CHECK-NEXT:  4097
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "3"
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -88,6 +88,7 @@
 # CHECK-NEXT:"readonly",
 # CHECK-NEXT:"static",
 # CHECK-NEXT:"abstract",
+# CHECK-NEXT:"virtual",
 # CHECK-NEXT:"dependentName",
 # CHECK-NEXT:"defaultLibrary",
 # CHECK-NEXT:"functionScope",
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -66,6 +66,7 @@
   Readonly,
   Static,
   Abstract,
+  Virtual,
   DependentName,
   DefaultLibrary,
 
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -240,6 +240,12 @@
   return false;
 }
 
+bool isVirtual(const Decl *D) {
+  if (const auto *CMD = llvm::dyn_cast(D))
+return CMD->isVirtual();
+  return false;
+}
+
 bool isDependent(const Decl *D) {
   if (isa(D))
 return true;
@@ -712,6 +718,8 @@
 Tok.addModifier(HighlightingModifier::Static);
   if (isAbstract(Decl))
 Tok.addModifier(HighlightingModifier::Abstract);
+  if (isVirtual(Decl))
+Tok.addModifier(HighlightingModifier::Virtual);
   if (isDependent(Decl))
 Tok.addModifier(HighlightingModifier::DependentName);
   if (isDefaultLibrary(Decl))
@@ -898,6 +906,8 @@
 return "deduced"; // nonstandard
   case HighlightingModifier::Abstract:
 return "abstract";
+  case HighlightingModifier::Virtual:
+return "virtual";
   case HighlightingModifier::DependentName:
 return "dependentName"; // nonstandard
   case HighlightingModifier::DefaultLibrary:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107137: clangd: Provide hover info for include directives

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 363442.
ckandeler added a comment.

Addressed lint comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107137

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2750,6 +2750,15 @@
 
 // In test::Bar
 int foo = 3)",
+  },
+  {
+  [](HoverInfo &HI) {
+HI.Name = "stdio.h";
+HI.Definition = "/usr/include/stdio.h";
+  },
+  R"(stdio.h
+
+/usr/include/stdio.h)",
   }};
 
   for (const auto &C : Cases) {
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -58,7 +58,7 @@
   std::string Documentation;
   /// Source code containing the definition of the symbol.
   std::string Definition;
-
+  const char *DefinitionLanguage = "cpp";
   /// Access specifier for declarations inside class/struct/unions, empty for
   /// others.
   std::string AccessSpecifier;
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -905,6 +905,22 @@
   if (TokensTouchingCursor.empty())
 return llvm::None;
 
+  // Show full header file path if cursor is on include directive.
+  if (const auto MainFilePath =
+  getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM)) {
+for (const auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
+  if (Inc.Resolved.empty() || Inc.HashLine != Pos.line)
+continue;
+  HoverInfo HI;
+  HI.Name = std::string(llvm::sys::path::filename(Inc.Resolved));
+  // FIXME: We don't have a fitting value for Kind.
+  HI.Definition =
+  URIForFile::canonicalize(Inc.Resolved, *MainFilePath).file().str();
+  HI.DefinitionLanguage = "";
+  return HI;
+}
+  }
+
   // To be used as a backup for highlighting the selected token, we use back as
   // it aligns better with biases elsewhere (editors tend to send the position
   // for the left of the hovered token).
@@ -981,6 +997,7 @@
 
 markup::Document HoverInfo::present() const {
   markup::Document Output;
+
   // Header contains a text of the form:
   // variable `var`
   //
@@ -1081,7 +1098,8 @@
: Definition;
 // Note that we don't print anything for global namespace, to not annoy
 // non-c++ projects or projects that are not making use of namespaces.
-Output.addCodeBlock(ScopeComment + DefinitionWithAccess);
+Output.addCodeBlock(ScopeComment + DefinitionWithAccess,
+DefinitionLanguage);
   }
 
   return Output;


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2750,6 +2750,15 @@
 
 // In test::Bar
 int foo = 3)",
+  },
+  {
+  [](HoverInfo &HI) {
+HI.Name = "stdio.h";
+HI.Definition = "/usr/include/stdio.h";
+  },
+  R"(stdio.h
+
+/usr/include/stdio.h)",
   }};
 
   for (const auto &C : Cases) {
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -58,7 +58,7 @@
   std::string Documentation;
   /// Source code containing the definition of the symbol.
   std::string Definition;
-
+  const char *DefinitionLanguage = "cpp";
   /// Access specifier for declarations inside class/struct/unions, empty for
   /// others.
   std::string AccessSpecifier;
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -905,6 +905,22 @@
   if (TokensTouchingCursor.empty())
 return llvm::None;
 
+  // Show full header file path if cursor is on include directive.
+  if (const auto MainFilePath =
+  getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM)) {
+for (const auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
+  if (Inc.Resolved.empty() || Inc.HashLine != Pos.line)
+continue;
+  HoverInfo HI;
+  HI.Name = std::string(llvm::sys::path::filename(Inc.Resolved));
+  // FIXME: We don't have a fitting value for Kind.
+  HI.Definition =
+  URIForFile::canonicalize(Inc.Resolved, *MainFilePath).file().st

[PATCH] D107145: clangd: Add new semantic token modifier "virtual"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Can someone please merge this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107145

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

[PATCH] D107365: clangd: Make documentation property of completion items more similar

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

... to the one of signature hints.
In particular, completion items now also carry annotations, which client
code might be interested in.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107365

Files:
  clang-tools-extra/clangd/CodeComplete.cpp


Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -405,8 +405,9 @@
   if (C.IndexResult) {
 SetDoc(C.IndexResult->Documentation);
   } else if (C.SemaResult) {
-SetDoc(getDocComment(*ASTCtx, *C.SemaResult,
- /*CommentsFromHeader=*/false));
+const auto docComment = getDocComment(*ASTCtx, *C.SemaResult,
+  /*CommentsFromHeader=*/false);
+SetDoc(formatDocumentation(*SemaCCS, docComment));
   }
 }
   }


Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -405,8 +405,9 @@
   if (C.IndexResult) {
 SetDoc(C.IndexResult->Documentation);
   } else if (C.SemaResult) {
-SetDoc(getDocComment(*ASTCtx, *C.SemaResult,
- /*CommentsFromHeader=*/false));
+const auto docComment = getDocComment(*ASTCtx, *C.SemaResult,
+  /*CommentsFromHeader=*/false);
+SetDoc(formatDocumentation(*SemaCCS, docComment));
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107145: clangd: Add new semantic token modifier "virtual"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107145

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

[PATCH] D107365: clangd: Make documentation property of completion items more similar

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 364412.
ckandeler added a comment.

Addressed lint complaints.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107365

Files:
  clang-tools-extra/clangd/CodeComplete.cpp


Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -405,8 +405,9 @@
   if (C.IndexResult) {
 SetDoc(C.IndexResult->Documentation);
   } else if (C.SemaResult) {
-SetDoc(getDocComment(*ASTCtx, *C.SemaResult,
- /*CommentsFromHeader=*/false));
+const auto DocComment = getDocComment(*ASTCtx, *C.SemaResult,
+  /*CommentsFromHeader=*/false);
+SetDoc(formatDocumentation(*SemaCCS, DocComment));
   }
 }
   }


Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -405,8 +405,9 @@
   if (C.IndexResult) {
 SetDoc(C.IndexResult->Documentation);
   } else if (C.SemaResult) {
-SetDoc(getDocComment(*ASTCtx, *C.SemaResult,
- /*CommentsFromHeader=*/false));
+const auto DocComment = getDocComment(*ASTCtx, *C.SemaResult,
+  /*CommentsFromHeader=*/false);
+SetDoc(formatDocumentation(*SemaCCS, DocComment));
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107365: clangd: Make documentation property of completion items more similar

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 364790.
ckandeler added a comment.

Added test coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107365

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -907,7 +907,7 @@
   auto Results = completions(
   R"cpp(
   // Non-doxygen comment.
-  int foo();
+  __attribute__((annotate("custom_annotation"))) int foo();
   /// Doxygen comment.
   /// \param int a
   int bar(int a);
@@ -919,7 +919,8 @@
   int x = ^
  )cpp");
   EXPECT_THAT(Results.Completions,
-  Contains(AllOf(Named("foo"), Doc("Non-doxygen comment.";
+  Contains(AllOf(Named("foo"),
+  Doc("Annotation: custom_annotation\nNon-doxygen comment.";
   EXPECT_THAT(
   Results.Completions,
   Contains(AllOf(Named("bar"), Doc("Doxygen comment.\n\\param int a";
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -405,8 +405,9 @@
   if (C.IndexResult) {
 SetDoc(C.IndexResult->Documentation);
   } else if (C.SemaResult) {
-SetDoc(getDocComment(*ASTCtx, *C.SemaResult,
- /*CommentsFromHeader=*/false));
+const auto DocComment = getDocComment(*ASTCtx, *C.SemaResult,
+  /*CommentsFromHeader=*/false);
+SetDoc(formatDocumentation(*SemaCCS, DocComment));
   }
 }
   }


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -907,7 +907,7 @@
   auto Results = completions(
   R"cpp(
   // Non-doxygen comment.
-  int foo();
+  __attribute__((annotate("custom_annotation"))) int foo();
   /// Doxygen comment.
   /// \param int a
   int bar(int a);
@@ -919,7 +919,8 @@
   int x = ^
  )cpp");
   EXPECT_THAT(Results.Completions,
-  Contains(AllOf(Named("foo"), Doc("Non-doxygen comment.";
+  Contains(AllOf(Named("foo"),
+  Doc("Annotation: custom_annotation\nNon-doxygen comment.";
   EXPECT_THAT(
   Results.Completions,
   Contains(AllOf(Named("bar"), Doc("Doxygen comment.\n\\param int a";
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -405,8 +405,9 @@
   if (C.IndexResult) {
 SetDoc(C.IndexResult->Documentation);
   } else if (C.SemaResult) {
-SetDoc(getDocComment(*ASTCtx, *C.SemaResult,
- /*CommentsFromHeader=*/false));
+const auto DocComment = getDocComment(*ASTCtx, *C.SemaResult,
+  /*CommentsFromHeader=*/false);
+SetDoc(formatDocumentation(*SemaCCS, DocComment));
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107137: clangd: Provide hover info for include directives

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Thanks for the review. Can you please merge it? I don't have the permissions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107137

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: sammccall.
ckandeler requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Real-world use case: The Qt framework's headers have the same name
as the respective class defined in them, and Qt's traditional qmake
build tool uses -I (rather than -isystem) to pull them in.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112996

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/included-files.cpp


Index: clang/test/CodeCompletion/included-files.cpp
===
--- clang/test/CodeCompletion/included-files.cpp
+++ clang/test/CodeCompletion/included-files.cpp
@@ -22,12 +22,12 @@
 // CHECK-3-NOT: foo.h>
 // CHECK-3: foosys>
 
-// With -I rather than -isystem, the header extension is required.
+// A header extension is not required.
 #include 
 // RUN: %clang -fsyntax-only -I %t/a -Xclang 
-code-completion-at=%t/main.cc:26:13 %t/main.cc | FileCheck 
-check-prefix=CHECK-4 %s
 // CHECK-4-NOT: foo.cc>
 // CHECK-4-NOT: foo.h>
-// CHECK-4-NOT: foosys>
+// CHECK-4: foosys>
 
 // Backslash handling.
 #include "a\foosys"
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9648,7 +9648,8 @@
   if (!(Filename.endswith_insensitive(".h") ||
 Filename.endswith_insensitive(".hh") ||
 Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
+Filename.endswith_insensitive(".inc") ||
+!Filename.contains('.')))
 break;
 }
 AddCompletion(Filename, /*IsDirectory=*/false);


Index: clang/test/CodeCompletion/included-files.cpp
===
--- clang/test/CodeCompletion/included-files.cpp
+++ clang/test/CodeCompletion/included-files.cpp
@@ -22,12 +22,12 @@
 // CHECK-3-NOT: foo.h>
 // CHECK-3: foosys>
 
-// With -I rather than -isystem, the header extension is required.
+// A header extension is not required.
 #include 
 // RUN: %clang -fsyntax-only -I %t/a -Xclang -code-completion-at=%t/main.cc:26:13 %t/main.cc | FileCheck -check-prefix=CHECK-4 %s
 // CHECK-4-NOT: foo.cc>
 // CHECK-4-NOT: foo.h>
-// CHECK-4-NOT: foosys>
+// CHECK-4: foosys>
 
 // Backslash handling.
 #include "a\foosys"
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9648,7 +9648,8 @@
   if (!(Filename.endswith_insensitive(".h") ||
 Filename.endswith_insensitive(".hh") ||
 Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
+Filename.endswith_insensitive(".inc") ||
+!Filename.contains('.')))
 break;
 }
 AddCompletion(Filename, /*IsDirectory=*/false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

I hope this isn't too controversial. After all, the files are located in 
include directories, so there shouldn't be any random garbage getting picked up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

> WDYT about detecting QT headers specifically? It seems hacky, but I don't see 
> a way out of this that doesn't involve hardcoding some filenames. Are they in 
> a directory like `"qt-11/QFoo"` that we can recognize? Even Q followed by 
> another capital letter might be a good enough heuristic.
> (The docs suggest it's just `` but the docs also say to use angle 
> brackets so I'm not sure whether to believe them)

The headers are, as far as I can tell, always located in a directory whose name 
starts with "Qt". This parent directory is also in the include path, so e.g. to 
get access to QString, which is located under QtCore/, you'd typically just 
write:

  #include 

This is the recommended, documented way of pulling in headers.
Though you could also write:

  #include 

as the parent parent directory is also in the list of include paths.

Looking at the code, it seems we have access to the parent directory, so we 
could do that name check (which I suppose has less potential for false 
positives than checking the file name).
For framework builds, the directory would be "Headers", which also seems safe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 384407.
ckandeler added a comment.

Limited the matching to Qt headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112996

Files:
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9615,6 +9615,7 @@
   }
 }
 
+const StringRef &Dirname = llvm::sys::path::filename(Dir);
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9648,7 +9649,9 @@
   if (!(Filename.endswith_insensitive(".h") ||
 Filename.endswith_insensitive(".hh") ||
 Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
+Filename.endswith_insensitive(".inc") ||
+Dirname == "Headers" ||
+Dirname.startswith("Qt") || Dirname == "ActiveQt"))
 break;
 }
 AddCompletion(Filename, /*IsDirectory=*/false);


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9615,6 +9615,7 @@
   }
 }
 
+const StringRef &Dirname = llvm::sys::path::filename(Dir);
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9648,7 +9649,9 @@
   if (!(Filename.endswith_insensitive(".h") ||
 Filename.endswith_insensitive(".hh") ||
 Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
+Filename.endswith_insensitive(".inc") ||
+Dirname == "Headers" ||
+Dirname.startswith("Qt") || Dirname == "ActiveQt"))
 break;
 }
 AddCompletion(Filename, /*IsDirectory=*/false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

>> For framework builds, the directory would be "Headers", which also seems 
>> safe.
>
> I agree extensionless headers in frameworks seem fine to show.
> We already know which includepath entries are frameworks, so we can just use 
> that info directly (see line 9674)

These aren't technically framework includes, as in order to make prefix-less 
headers work, the include path has to point directly into the Headers directory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 384485.
ckandeler added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112996

Files:
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9613,6 +9613,10 @@
   }
 }
 
+const StringRef &Dirname = llvm::sys::path::filename(Dir);
+const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
+const bool ExtensionlessHeaders = IsSystem || isQt
+|| Dir.endswith(".framework/Headers");
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9639,15 +9643,16 @@
 
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
-  case llvm::sys::fs::file_type::regular_file:
-// Only files that really look like headers. (Except in system dirs).
-if (!IsSystem) {
-  // Header extensions from Types.def, which we can't depend on here.
-  if (!(Filename.endswith_insensitive(".h") ||
-Filename.endswith_insensitive(".hh") ||
-Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
-break;
+  case llvm::sys::fs::file_type::regular_file: {
+// Only files that really look like headers. (Except in special dirs).
+// Header extensions from Types.def, which we can't depend on here.
+const bool IsHeader = Filename.endswith_insensitive(".h") ||
+Filename.endswith_insensitive(".hh") ||
+Filename.endswith_insensitive(".hpp") ||
+Filename.endswith_insensitive(".inc") ||
+(ExtensionlessHeaders && !Filename.contains('.'));
+if (!IsHeader)
+  break;
 }
 AddCompletion(Filename, /*IsDirectory=*/false);
 break;


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9613,6 +9613,10 @@
   }
 }
 
+const StringRef &Dirname = llvm::sys::path::filename(Dir);
+const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
+const bool ExtensionlessHeaders = IsSystem || isQt
+|| Dir.endswith(".framework/Headers");
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9639,15 +9643,16 @@
 
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
-  case llvm::sys::fs::file_type::regular_file:
-// Only files that really look like headers. (Except in system dirs).
-if (!IsSystem) {
-  // Header extensions from Types.def, which we can't depend on here.
-  if (!(Filename.endswith_insensitive(".h") ||
-Filename.endswith_insensitive(".hh") ||
-Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
-break;
+  case llvm::sys::fs::file_type::regular_file: {
+// Only files that really look like headers. (Except in special dirs).
+// Header extensions from Types.def, which we can't depend on here.
+const bool IsHeader = Filename.endswith_insensitive(".h") ||
+Filename.endswith_insensitive(".hh") ||
+Filename.endswith_insensitive(".hpp") ||
+Filename.endswith_insensitive(".inc") ||
+(ExtensionlessHeaders && !Filename.contains('.'));
+if (!IsHeader)
+  break;
 }
 AddCompletion(Filename, /*IsDirectory=*/false);
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:9619
+const bool ExtensionlessHeaders = IsSystem || isQt
+|| Dir.endswith(".framework/Headers");
 std::error_code EC;

I'm just noticing that this is often a symlink into a versioned directory (e.g. 
xxx.framework/Version/5/Headers). The include paths are not canonicalized, I 
hope?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D112996#3106459 , @sammccall wrote:

> can we the tests again though?

Sorry, I don't understand what you mean by that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 385453.
ckandeler added a comment.

Addressed formatting comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112996

Files:
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9613,6 +9613,10 @@
   }
 }
 
+const StringRef &Dirname = llvm::sys::path::filename(Dir);
+const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
+const bool ExtensionlessHeaders =
+IsSystem || isQt || Dir.endswith(".framework/Headers");
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9639,18 +9643,19 @@
 
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
-  case llvm::sys::fs::file_type::regular_file:
-// Only files that really look like headers. (Except in system dirs).
-if (!IsSystem) {
-  // Header extensions from Types.def, which we can't depend on here.
-  if (!(Filename.endswith_insensitive(".h") ||
-Filename.endswith_insensitive(".hh") ||
-Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
-break;
-}
+  case llvm::sys::fs::file_type::regular_file: {
+// Only files that really look like headers. (Except in special dirs).
+// Header extensions from Types.def, which we can't depend on here.
+const bool IsHeader = Filename.endswith_insensitive(".h") ||
+  Filename.endswith_insensitive(".hh") ||
+  Filename.endswith_insensitive(".hpp") ||
+  Filename.endswith_insensitive(".inc") ||
+  (ExtensionlessHeaders && 
!Filename.contains('.'));
+if (!IsHeader)
+  break;
 AddCompletion(Filename, /*IsDirectory=*/false);
 break;
+  }
   default:
 break;
   }


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9613,6 +9613,10 @@
   }
 }
 
+const StringRef &Dirname = llvm::sys::path::filename(Dir);
+const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
+const bool ExtensionlessHeaders =
+IsSystem || isQt || Dir.endswith(".framework/Headers");
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9639,18 +9643,19 @@
 
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
-  case llvm::sys::fs::file_type::regular_file:
-// Only files that really look like headers. (Except in system dirs).
-if (!IsSystem) {
-  // Header extensions from Types.def, which we can't depend on here.
-  if (!(Filename.endswith_insensitive(".h") ||
-Filename.endswith_insensitive(".hh") ||
-Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
-break;
-}
+  case llvm::sys::fs::file_type::regular_file: {
+// Only files that really look like headers. (Except in special dirs).
+// Header extensions from Types.def, which we can't depend on here.
+const bool IsHeader = Filename.endswith_insensitive(".h") ||
+  Filename.endswith_insensitive(".hh") ||
+  Filename.endswith_insensitive(".hpp") ||
+  Filename.endswith_insensitive(".inc") ||
+  (ExtensionlessHeaders && !Filename.contains('.'));
+if (!IsHeader)
+  break;
 AddCompletion(Filename, /*IsDirectory=*/false);
 break;
+  }
   default:
 break;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Can someone please merge this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 385826.
ckandeler added a comment.

Added test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112996

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/included-files.cpp

Index: clang/test/CodeCompletion/included-files.cpp
===
--- clang/test/CodeCompletion/included-files.cpp
+++ clang/test/CodeCompletion/included-files.cpp
@@ -1,35 +1,57 @@
-// RUN: rm -rf %t && mkdir %t && cp %s %t/main.cc && mkdir %t/a
-// RUN: touch %t/foo.h && touch %t/foo.cc && touch %t/a/foosys %t/a/foosys.h
+// RUN: rm -rf %t && mkdir %t && cp %s %t/main.cc && mkdir %t/a && mkdir %t/QtCore && mkdir %t/Headers %t/Some.framework %t/Some.framework/Headers
+// RUN: touch %t/foo.h && touch %t/foo.cc && touch %t/a/foosys %t/a/foosys.h && touch %t/QtCore/foosys %t/QtCore/foo.h
+// RUN: touch %t/Headers/foosys %t/Headers/foo.h %t/Some.framework/Headers/foosys %t/Some.framework/Headers/foo.h
 
 // Quoted string shows header-ish files from CWD, and all from system.
 #include "foo.h"
-// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:5:13 %t/main.cc | FileCheck -check-prefix=CHECK-1 %s
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:6:13 %t/main.cc | FileCheck -check-prefix=CHECK-1 %s
 // CHECK-1-NOT: foo.cc"
 // CHECK-1: foo.h"
 // CHECK-1: foosys"
 
 // Quoted string with dir shows header-ish files in that subdir.
 #include "a/foosys"
-// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:12:13 %t/main.cc | FileCheck -check-prefix=CHECK-2 %s
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:13:13 %t/main.cc | FileCheck -check-prefix=CHECK-2 %s
 // CHECK-2-NOT: foo.h"
 // CHECK-2: foosys.h"
 // CHECK-2-NOT: foosys"
 
 // Angled shows headers from system dirs.
 #include 
-// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:19:13 %t/main.cc | FileCheck -check-prefix=CHECK-3 %s
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:20:13 %t/main.cc | FileCheck -check-prefix=CHECK-3 %s
 // CHECK-3-NOT: foo.cc>
 // CHECK-3-NOT: foo.h>
 // CHECK-3: foosys>
 
 // With -I rather than -isystem, the header extension is required.
 #include 
-// RUN: %clang -fsyntax-only -I %t/a -Xclang -code-completion-at=%t/main.cc:26:13 %t/main.cc | FileCheck -check-prefix=CHECK-4 %s
+// RUN: %clang -fsyntax-only -I %t/a -Xclang -code-completion-at=%t/main.cc:27:13 %t/main.cc | FileCheck -check-prefix=CHECK-4 %s
 // CHECK-4-NOT: foo.cc>
 // CHECK-4-NOT: foo.h>
 // CHECK-4-NOT: foosys>
 
 // Backslash handling.
 #include "a\foosys"
-// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:33:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-5 %s
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:34:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-5 %s
 // CHECK-5: foosys.h"
+
+// Qt headers don't necessarily have extensions.
+#include 
+// RUN: %clang -fsyntax-only -I %t/QtCore -Xclang -code-completion-at=%t/main.cc:39:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-6 %s
+// CHECK-6-NOT: foo.cc>
+// CHECK-6: foo.h>
+// CHECK-6: foosys>
+
+// If the include path directly points into a framework's Headers/ directory, we allow extension-less headers.
+#include 
+// RUN: %clang -fsyntax-only -I %t/Some.framework/Headers -Xclang -code-completion-at=%t/main.cc:46:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-7 %s
+// CHECK-7-NOT: foo.cc>
+// CHECK-7: foo.h>
+// CHECK-7: foosys>
+
+// Simply naming a directory "Headers" is not enough to allow extension-less headers.
+#include 
+// RUN: %clang -fsyntax-only -I %t/Headers -Xclang -code-completion-at=%t/main.cc:53:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-8 %s
+// CHECK-8-NOT: foo.cc>
+// CHECK-8: foo.h>
+// CHECK-8-NOT: foosys>
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9613,6 +9613,10 @@
   }
 }
 
+const StringRef &Dirname = llvm::sys::path::filename(Dir);
+const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
+const bool ExtensionlessHeaders =
+IsSystem || isQt || Dir.endswith(".framework/Headers");
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9639,18 +9643,19 @@
 
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
-  case llvm::sys::fs::file_type::regular_file:
-// Only files that really look like headers. (Except in system dirs).
-if (!IsSystem) {
-  // Header extensions from Types.def, wh

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D127403#3863713 , @nridge wrote:

> In D127403#3863686 , @thakis wrote:
>
>> In D127403#3863641 , @nridge wrote:
>>
>>> Landed https://reviews.llvm.org/rGc93430bae4fc
>>
>> Still failing with that: http://45.33.8.238/linux/89240/step_9.txt
>
> Fixed in https://reviews.llvm.org/rGd5a99bf5e134, and confirmed via a local 
> build and test run that the tests are passing. Apologies for the breakage.

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

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

[PATCH] D134728: [clangd] Add highlighting modifier "constructorOrDestructor"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 468465.
ckandeler retitled this revision from "[clangd] Add highlighting modifiers 
"constructor" and "destructor"" to "[clangd] Add highlighting modifier 
"constructorOrDestructor"".
ckandeler edited the summary of this revision.
ckandeler added a comment.

Now uses just one modifier instead of two, as discussed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134728

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.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
@@ -142,16 +142,16 @@
   $Namespace[[abc]]::$Class[[A]] $Variable_def[[AA]];
   typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]];
   struct $Class_def[[B]] {
-$Class_decl[[B]]();
-~$Class[[B]](); // FIXME: inconsistent with constructor
+$Class_decl_constrDestr[[B]]();
+~$Class_decl_constrDestr[[B]]();
 void operator<<($Class[[B]]);
 $Class[[AAA]] $Field_decl[[AA]];
   };
-  $Class[[B]]::$Class_def[[B]]() {}
-  $Class[[B]]::~$Class[[B]]() {} // FIXME: inconsistent with constructor
+  $Class[[B]]::$Class_def_constrDestr[[B]]() {}
+  $Class[[B]]::~$Class_def_constrDestr[[B]]() {}
   void $Function_def[[f]] () {
 $Class[[B]] $LocalVariable_def[[BB]] = $Class[[B]]();
-$LocalVariable[[BB]].~$Class[[B]]();
+$LocalVariable[[BB]].~$Class_constrDestr[[B]]();
 $Class[[B]]();
   }
 )cpp",
@@ -310,13 +310,13 @@
 $Class[[Foo]] $Field_decl[[Fo]];
 $Enum[[En]] $Field_decl[[E]];
 int $Field_decl[[I]];
-$Class_def[[Bar]] ($Class[[Foo]] $Parameter_def[[F]],
+$Class_def_constrDestr[[Bar]] ($Class[[Foo]] $Parameter_def[[F]],
 $Enum[[En]] $Parameter_def[[E]])
 : $Field[[Fo]] ($Parameter[[F]]), $Field[[E]] ($Parameter[[E]]),
   $Field[[I]] (123) {}
   };
   class $Class_def[[Bar2]] : public $Class[[Bar]] {
-$Class_def[[Bar2]]() : $Class[[Bar]]($Class[[Foo]](), $EnumConstant_readonly[[EC]]) {}
+$Class_def_constrDestr[[Bar2]]() : $Class[[Bar]]($Class[[Foo]](), $EnumConstant_readonly[[EC]]) {}
   };
 )cpp",
   R"cpp(
@@ -757,7 +757,7 @@
 static inline int $StaticField_def_static[[j]] = 0;
 };
 struct $Class_def[[ClassWithRefMembers]] {
-  $Class_def[[ClassWithRefMembers]](int $Parameter_def[[i]])
+  $Class_def_constrDestr[[ClassWithRefMembers]](int $Parameter_def[[i]])
 : $Field[[i1]]($Parameter[[i]]),
   $Field_readonly[[i2]]($Parameter[[i]]),
   $Field[[i3]]($Parameter_usedAsMutableReference[[i]]),
@@ -803,7 +803,7 @@
   $LocalVariable_readonly[[c2]][$LocalVariable[[val]]];
 }
 struct $Class_def[[S]] {
-  $Class_def[[S]](int&) {
+  $Class_def_constrDestr[[S]](int&) {
 $Class[[S]] $LocalVariable_def[[s1]]($Field_usedAsMutableReference[[field]]);
 $Class[[S]] $LocalVariable_def[[s2]]($LocalVariable[[s1]].$Field_usedAsMutableReference[[field]]);
 
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- clang-tools-extra/clangd/test/semantic-tokens.test
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  16387
+# CHECK-NEXT:  32771
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "1"
 # CHECK-NEXT:  }
@@ -49,7 +49,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  16387
+# CHECK-NEXT:  32771
 # CHECK-NEXT:],
 #Inserted at position 1
 # CHECK-NEXT:"deleteCount": 0,
@@ -72,12 +72,12 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  16387,
+# CHECK-NEXT:  32771,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  16387
+# CHECK-NEXT:  32771
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "3"
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -68,6 +68,7 @@
 # CHECK-NEXT:"dependentName",
 # CHECK-NEXT:

[PATCH] D134728: [clangd] Add highlighting modifier "constructorOrDestructor"

[Christian Kandeler via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8b3668754c88: [clangd] Add highlighting modifier 
"constructorOrDestructor" (authored by ckandeler).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134728

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.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
@@ -142,16 +142,16 @@
   $Namespace[[abc]]::$Class[[A]] $Variable_def[[AA]];
   typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]];
   struct $Class_def[[B]] {
-$Class_decl[[B]]();
-~$Class[[B]](); // FIXME: inconsistent with constructor
+$Class_decl_constrDestr[[B]]();
+~$Class_decl_constrDestr[[B]]();
 void operator<<($Class[[B]]);
 $Class[[AAA]] $Field_decl[[AA]];
   };
-  $Class[[B]]::$Class_def[[B]]() {}
-  $Class[[B]]::~$Class[[B]]() {} // FIXME: inconsistent with constructor
+  $Class[[B]]::$Class_def_constrDestr[[B]]() {}
+  $Class[[B]]::~$Class_def_constrDestr[[B]]() {}
   void $Function_def[[f]] () {
 $Class[[B]] $LocalVariable_def[[BB]] = $Class[[B]]();
-$LocalVariable[[BB]].~$Class[[B]]();
+$LocalVariable[[BB]].~$Class_constrDestr[[B]]();
 $Class[[B]]();
   }
 )cpp",
@@ -310,13 +310,13 @@
 $Class[[Foo]] $Field_decl[[Fo]];
 $Enum[[En]] $Field_decl[[E]];
 int $Field_decl[[I]];
-$Class_def[[Bar]] ($Class[[Foo]] $Parameter_def[[F]],
+$Class_def_constrDestr[[Bar]] ($Class[[Foo]] $Parameter_def[[F]],
 $Enum[[En]] $Parameter_def[[E]])
 : $Field[[Fo]] ($Parameter[[F]]), $Field[[E]] ($Parameter[[E]]),
   $Field[[I]] (123) {}
   };
   class $Class_def[[Bar2]] : public $Class[[Bar]] {
-$Class_def[[Bar2]]() : $Class[[Bar]]($Class[[Foo]](), $EnumConstant_readonly[[EC]]) {}
+$Class_def_constrDestr[[Bar2]]() : $Class[[Bar]]($Class[[Foo]](), $EnumConstant_readonly[[EC]]) {}
   };
 )cpp",
   R"cpp(
@@ -757,7 +757,7 @@
 static inline int $StaticField_def_static[[j]] = 0;
 };
 struct $Class_def[[ClassWithRefMembers]] {
-  $Class_def[[ClassWithRefMembers]](int $Parameter_def[[i]])
+  $Class_def_constrDestr[[ClassWithRefMembers]](int $Parameter_def[[i]])
 : $Field[[i1]]($Parameter[[i]]),
   $Field_readonly[[i2]]($Parameter[[i]]),
   $Field[[i3]]($Parameter_usedAsMutableReference[[i]]),
@@ -803,7 +803,7 @@
   $LocalVariable_readonly[[c2]][$LocalVariable[[val]]];
 }
 struct $Class_def[[S]] {
-  $Class_def[[S]](int&) {
+  $Class_def_constrDestr[[S]](int&) {
 $Class[[S]] $LocalVariable_def[[s1]]($Field_usedAsMutableReference[[field]]);
 $Class[[S]] $LocalVariable_def[[s2]]($LocalVariable[[s1]].$Field_usedAsMutableReference[[field]]);
 
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- clang-tools-extra/clangd/test/semantic-tokens.test
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  16387
+# CHECK-NEXT:  32771
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "1"
 # CHECK-NEXT:  }
@@ -49,7 +49,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  16387
+# CHECK-NEXT:  32771
 # CHECK-NEXT:],
 #Inserted at position 1
 # CHECK-NEXT:"deleteCount": 0,
@@ -72,12 +72,12 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  16387,
+# CHECK-NEXT:  32771,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  16387
+# CHECK-NEXT:  32771
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "3"
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -68,6 +68,7 @@
 # CHECK-NEXT:"dependentName",
 # CHECK-NEXT:"defaultLibrary",
 # CHECK-NEXT:"usedAsMutableReference",
+# CHECK-NEXT:"constructorOrDestructor",
 # CHECK-NEXT:"f

[PATCH] D136454: [clangd] Make file limit for textDocument/rename configurable

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Without this, clients are unable to rename often-used symbols in larger
projects.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136454

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -327,6 +327,14 @@
 init(1000),
 };
 
+opt RenameFileLimit{
+"rename-file-limit",
+cat(Features),
+desc("Limit the number of files to be affected by symbol renaming. "
+ "0 means no limit (default=50)"),
+init(50),
+};
+
 list TweakList{
 "tweaks",
 cat(Features),
@@ -891,6 +899,7 @@
   Opts.BackgroundIndex = EnableBackgroundIndex;
   Opts.BackgroundIndexPriority = BackgroundIndexPriority;
   Opts.ReferencesLimit = ReferencesLimit;
+  Opts.Rename.LimitFiles = RenameFileLimit;
   auto PAI = createProjectAwareIndex(loadExternalIndex, Sync);
   if (StaticIdx) {
 IdxStack.emplace_back(std::move(StaticIdx));


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -327,6 +327,14 @@
 init(1000),
 };
 
+opt RenameFileLimit{
+"rename-file-limit",
+cat(Features),
+desc("Limit the number of files to be affected by symbol renaming. "
+ "0 means no limit (default=50)"),
+init(50),
+};
+
 list TweakList{
 "tweaks",
 cat(Features),
@@ -891,6 +899,7 @@
   Opts.BackgroundIndex = EnableBackgroundIndex;
   Opts.BackgroundIndexPriority = BackgroundIndexPriority;
   Opts.ReferencesLimit = ReferencesLimit;
+  Opts.Rename.LimitFiles = RenameFileLimit;
   auto PAI = createProjectAwareIndex(loadExternalIndex, Sync);
   if (StaticIdx) {
 IdxStack.emplace_back(std::move(StaticIdx));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136454: [clangd] Make file limit for textDocument/rename configurable

[Christian Kandeler via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6ed4a543b8b3: [clangd] Make file limit for 
textDocument/rename configurable (authored by ckandeler).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136454

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -327,6 +327,14 @@
 init(1000),
 };
 
+opt RenameFileLimit{
+"rename-file-limit",
+cat(Features),
+desc("Limit the number of files to be affected by symbol renaming. "
+ "0 means no limit (default=50)"),
+init(50),
+};
+
 list TweakList{
 "tweaks",
 cat(Features),
@@ -891,6 +899,7 @@
   Opts.BackgroundIndex = EnableBackgroundIndex;
   Opts.BackgroundIndexPriority = BackgroundIndexPriority;
   Opts.ReferencesLimit = ReferencesLimit;
+  Opts.Rename.LimitFiles = RenameFileLimit;
   auto PAI = createProjectAwareIndex(loadExternalIndex, Sync);
   if (StaticIdx) {
 IdxStack.emplace_back(std::move(StaticIdx));


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -327,6 +327,14 @@
 init(1000),
 };
 
+opt RenameFileLimit{
+"rename-file-limit",
+cat(Features),
+desc("Limit the number of files to be affected by symbol renaming. "
+ "0 means no limit (default=50)"),
+init(50),
+};
+
 list TweakList{
 "tweaks",
 cat(Features),
@@ -891,6 +899,7 @@
   Opts.BackgroundIndex = EnableBackgroundIndex;
   Opts.BackgroundIndexPriority = BackgroundIndexPriority;
   Opts.ReferencesLimit = ReferencesLimit;
+  Opts.Rename.LimitFiles = RenameFileLimit;
   auto PAI = createProjectAwareIndex(loadExternalIndex, Sync);
   if (StaticIdx) {
 IdxStack.emplace_back(std::move(StaticIdx));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136594: [clangd] Add support for semantic token type "operator"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Also add new modifier for differentiating between built-in and user-
provided operators.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136594

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.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
@@ -113,9 +113,9 @@
   void $Function_def[[foo]](int $Parameter_def[[A]], $Class[[AS]] $Parameter_def[[As]]) {
 $Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[VeryLongVariableName]] = 12312;
 $Class[[AS]] $LocalVariable_def[[AA]];
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] $Operator[[+]] $Parameter[[A]];
 auto $LocalVariable_def[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_def[[A]]) -> void {};
-$LocalVariable[[FN]](12312);
+$LocalVariable[[FN]]$Operator_userProvided[[(]]12312$Operator_userProvided[[)]];
   }
 )cpp",
   R"cpp(
@@ -144,7 +144,7 @@
   struct $Class_def[[B]] {
 $Class_decl_constrDestr[[B]]();
 ~$Class_decl_constrDestr[[B]]();
-void operator<<($Class[[B]]);
+void operator$Operator_decl[[<<]]($Class[[B]]);
 $Class[[AAA]] $Field_decl[[AA]];
   };
   $Class[[B]]::$Class_def_constrDestr[[B]]() {}
@@ -203,20 +203,20 @@
 static double $StaticField_decl_static[[S]];
 static void $StaticMethod_def_static[[bar]]() {}
 void $Method_def[[foo]]() {
-  $Field[[B]] = 123;
-  this->$Field[[B]] = 156;
+  $Field[[B]] $Operator[[=]] 123;
+  this->$Field[[B]] $Operator[[=]] 156;
   this->$Method[[foo]]();
   $Method[[foo]]();
   $StaticMethod_static[[bar]]();
-  $StaticField_static[[S]] = 90.1;
+  $StaticField_static[[S]] $Operator[[=]] 90.1;
 }
   };
   void $Function_def[[foo]]() {
 $Class[[A]] $LocalVariable_def[[AA]];
-$LocalVariable[[AA]].$Field[[B]] += 2;
+$LocalVariable[[AA]].$Field[[B]] $Operator[[+=]] 2;
 $LocalVariable[[AA]].$Method[[foo]]();
 $LocalVariable[[AA]].$Field[[E]].$Field[[C]];
-$Class[[A]]::$StaticField_static[[S]] = 90;
+$Class[[A]]::$StaticField_static[[S]] $Operator[[=]] 90;
   }
 )cpp",
   R"cpp(
@@ -295,10 +295,10 @@
   struct $Class_def[[B]] {};
   struct $Class_def[[A]] {
 $Class[[B]] $Field_decl[[BB]];
-$Class[[A]] &operator=($Class[[A]] &&$Parameter_def[[O]]);
+$Class[[A]] &operator$Operator_decl[[=]]($Class[[A]] &&$Parameter_def[[O]]);
   };
 
-  $Class[[A]] &$Class[[A]]::operator=($Class[[A]] &&$Parameter_def[[O]]) = default;
+  $Class[[A]] &$Class[[A]]::operator$Operator_def[[=]]($Class[[A]] &&$Parameter_def[[O]]) = default;
 )cpp",
   R"cpp(
   enum $Enum_decl[[En]] {
@@ -327,9 +327,9 @@
   $Enum_deduced[[auto]] $Variable_def[[AE]] = $Enum[[E]]::$EnumConstant_readonly[[E]];
   $Class_deduced[[auto]] $Variable_def[[AF]] = $Class[[Foo]]();
   $Class_deduced[[decltype]](auto) $Variable_def[[AF2]] = $Class[[Foo]]();
-  $Class_deduced[[auto]] *$Variable_def[[AFP]] = &$Variable[[AF]];
+  $Class_deduced[[auto]] *$Variable_def[[AFP]] = $Operator[[&]]$Variable[[AF]];
   $Enum_deduced[[auto]] &$Variable_def[[AER]] = $Variable[[AE]];
-  $Primitive_deduced_defaultLibrary[[auto]] $Variable_def[[Form]] = 10.2 + 2 * 4;
+  $Primitive_deduced_defaultLibrary[[auto]] $Variable_def[[Form]] = 10.2 $Operator[[+]] 2 $Operator[[*]] 4;
   $Primitive_deduced_defaultLibrary[[decltype]]($Variable[[Form]]) $Variable_def[[F]] = 10;
   auto $Variable_def[[Fun]] = []()->void{};
 )cpp",
@@ -342,21 +342,21 @@
   template
   class $Class_def[[IP]] {
 void $Method_def[[f]]() {
-  *$TemplateParameter_readonly[[U]] += 5;
+  $Operator[[*]]$TemplateParameter_readonly[[U]] $Operator[[+=]] 5;
 }
   };
   template
   class $Class_def[[Foo]] {
 void $Method_def[[f]]() {
   for(int $LocalVariable_def[[I]] = 0;
-$LocalVariable[

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

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

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:546
+  bool VisitPredefinedExpr(PredefinedExpr *E) {
+H.addToken(E->getLocation(), HighlightingKind::Macro);
+return true;

sammccall wrote:
> actually, would HighlightingKind::LocalVariable be more appropriate?
> 
> The standard calls this a "The function-local predefined variable".
> 
> Admittedly I have a tendency to think of it as macro-like by analogy with 
> `__LINE__` etc but this model is observably wrong: it breaks down with 
> templates and in other ways.
Hm, yes, I was not aware it was specified explicitly as a local variable. Will 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

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

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 453648.
ckandeler added a comment.

Use variable instead of macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

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
@@ -841,6 +841,12 @@
 $Function_deprecated[[Foo]]($Parameter[[x]]); 
 }
   )cpp",
+  // Predefined identifiers
+  R"cpp(
+void $Function_decl[[Foo]]() {
+const char *$LocalVariable_decl_readonly[[s]] = 
$LocalVariable_readonly_static[[__func__]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -542,6 +542,14 @@
 return Base::TraverseConstructorInitializer(Init);
   }
 
+  bool VisitPredefinedExpr(PredefinedExpr *E) {
+H.addToken(E->getLocation(), HighlightingKind::LocalVariable)
+.addModifier(HighlightingModifier::Static)
+.addModifier(HighlightingModifier::Readonly)
+.addModifier(HighlightingModifier::FunctionScope);
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 // Highlighting parameters passed by non-const reference does not really
 // make sense for literals...


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -841,6 +841,12 @@
 $Function_deprecated[[Foo]]($Parameter[[x]]); 
 }
   )cpp",
+  // Predefined identifiers
+  R"cpp(
+void $Function_decl[[Foo]]() {
+const char *$LocalVariable_decl_readonly[[s]] = $LocalVariable_readonly_static[[__func__]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -542,6 +542,14 @@
 return Base::TraverseConstructorInitializer(Init);
   }
 
+  bool VisitPredefinedExpr(PredefinedExpr *E) {
+H.addToken(E->getLocation(), HighlightingKind::LocalVariable)
+.addModifier(HighlightingModifier::Static)
+.addModifier(HighlightingModifier::Readonly)
+.addModifier(HighlightingModifier::FunctionScope);
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 // Highlighting parameters passed by non-const reference does not really
 // make sense for literals...
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler marked an inline comment as not done.
ckandeler added a comment.

Thanks for the review. Can you please merge it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

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

[PATCH] D132135: [clangd] Support hover on __func__ etc (PredefinedExpr)

[Christian Kandeler via Phabricator via cfe-commits]

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

Works fine for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132135

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

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

By the way: How do I change the commit message when using arc? It seems to 
ignore all subsequent changes other than in the code itself, and now the patch 
is in the repo with a misleading commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

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

[PATCH] D134728: [clangd] Add highlighting modifiers "constructor" and "destructor"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This is useful for clients that want to highlight constructors and/or
destructors different from classes, e.g. like functions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134728

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.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
@@ -133,16 +133,16 @@
   $Namespace[[abc]]::$Class[[A]] $Variable_decl[[AA]];
   typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]];
   struct $Class_decl[[B]] {
-$Class_decl[[B]]();
-~$Class[[B]](); // FIXME: inconsistent with constructor
+$Class_decl_constructor[[B]]();
+~$Class_decl_destructor[[B]]();
 void operator<<($Class[[B]]);
 $Class[[AAA]] $Field_decl[[AA]];
   };
-  $Class[[B]]::$Class_decl[[B]]() {}
-  $Class[[B]]::~$Class[[B]]() {} // FIXME: inconsistent with constructor
+  $Class[[B]]::$Class_decl_constructor[[B]]() {}
+  $Class[[B]]::~$Class_decl_destructor[[B]]() {}
   void $Function_decl[[f]] () {
 $Class[[B]] $LocalVariable_decl[[BB]] = $Class[[B]]();
-$LocalVariable[[BB]].~$Class[[B]]();
+$LocalVariable[[BB]].~$Class_destructor[[B]]();
 $Class[[B]]();
   }
 )cpp",
@@ -301,13 +301,13 @@
 $Class[[Foo]] $Field_decl[[Fo]];
 $Enum[[En]] $Field_decl[[E]];
 int $Field_decl[[I]];
-$Class_decl[[Bar]] ($Class[[Foo]] $Parameter_decl[[F]],
+$Class_decl_constructor[[Bar]] ($Class[[Foo]] $Parameter_decl[[F]],
 $Enum[[En]] $Parameter_decl[[E]])
 : $Field[[Fo]] ($Parameter[[F]]), $Field[[E]] ($Parameter[[E]]),
   $Field[[I]] (123) {}
   };
   class $Class_decl[[Bar2]] : public $Class[[Bar]] {
-$Class_decl[[Bar2]]() : $Class[[Bar]]($Class[[Foo]](), $EnumConstant_readonly[[EC]]) {}
+$Class_decl_constructor[[Bar2]]() : $Class[[Bar]]($Class[[Foo]](), $EnumConstant_readonly[[EC]]) {}
   };
 )cpp",
   R"cpp(
@@ -748,7 +748,7 @@
 static inline int $StaticField_decl_static[[j]] = 0;
 };
 struct $Class_decl[[ClassWithRefMembers]] {
-  $Class_decl[[ClassWithRefMembers]](int $Parameter_decl[[i]])
+  $Class_decl_constructor[[ClassWithRefMembers]](int $Parameter_decl[[i]])
 : $Field[[i1]]($Parameter[[i]]),
   $Field_readonly[[i2]]($Parameter[[i]]),
   $Field[[i3]]($Parameter_usedAsMutableReference[[i]]),
@@ -794,7 +794,7 @@
   $LocalVariable_readonly[[c2]][$LocalVariable[[val]]];
 }
 struct $Class_decl[[S]] {
-  $Class_decl[[S]](int&) {
+  $Class_decl_constructor[[S]](int&) {
 $Class[[S]] $LocalVariable_decl[[s1]]($Field_usedAsMutableReference[[field]]);
 $Class[[S]] $LocalVariable_decl[[s2]]($LocalVariable[[s1]].$Field_usedAsMutableReference[[field]]);
 
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- clang-tools-extra/clangd/test/semantic-tokens.test
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  8193
+# CHECK-NEXT:  32769
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "1"
 # CHECK-NEXT:  }
@@ -49,7 +49,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  8193
+# CHECK-NEXT:  32769
 # CHECK-NEXT:],
 #Inserted at position 1
 # CHECK-NEXT:"deleteCount": 0,
@@ -72,12 +72,12 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  8193,
+# CHECK-NEXT:  32769,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  8193
+# CHECK-NEXT:  32769
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "3"
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -67,6 +67,8 @@
 # CHECK-NEXT:"dependen

[PATCH] D134728: [clangd] Add highlighting modifiers "constructor" and "destructor"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D134728#3822653 , @sammccall wrote:

> It doesn't scale very well though: we're limited to 30 modifiers in total, 
> this patch brings us up to 16, if we followed this class.constructor 
> precedent for function.operator, class.constructor.copy enum.scoped etc we 
> could end up exhausting this.

In light of this (and assuming it really has to be this way), would it make 
sense to introduce a generic modifier that changes its meaning depending on the 
main highlighting kind? It would have the drawback of not being 
self-documenting, but with the given constraints might just be a pragmatic 
solution:

  enum class HighlightingModifier {
// ...
  
// Class -> Constructor/Destructor
// Document new uses here
Generic,
  
// ...
  };


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134728

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

[PATCH] D134728: [clangd] Add highlighting modifiers "constructor" and "destructor"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D134728#3822898 , @sammccall wrote:

> If the distinction isn't important, maybe we should add a single "constructor 
> or destructor" modifier...

No, it's not. I will update the patch accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134728

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

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 464170.
ckandeler marked 3 inline comments as done.
ckandeler added a comment.

Adapted according to latest comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
@@ -18,18 +18,18 @@
 TEST_F(AnnotateHighlightingsTest, Test) {
   EXPECT_AVAILABLE("^vo^id^ ^f(^) {^}^"); // available everywhere.
   EXPECT_AVAILABLE("[[int a; int b;]]");
-  EXPECT_EQ("void /* Function [decl] [globalScope] */f() {}",
+  EXPECT_EQ("void /* Function [decl] [def] [globalScope] */f() {}",
 apply("void ^f() {}"));
 
   EXPECT_EQ(apply("[[int f1(); const int x = f1();]]"),
 "int /* Function [decl] [globalScope] */f1(); "
-"const int /* Variable [decl] [readonly] [fileScope] */x = "
+"const int /* Variable [decl] [def] [readonly] [fileScope] */x = "
 "/* Function [globalScope] */f1();");
 
   // Only the targeted range is annotated.
   EXPECT_EQ(apply("void f1(); void f2() {^}"),
 "void f1(); "
-"void /* Function [decl] [globalScope] */f2() {}");
+"void /* Function [decl] [def] [globalScope] */f2() {}");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -48,12 +48,20 @@
 assert(StartOffset <= EndOffset);
 assert(NextChar <= StartOffset);
 
+bool hasDef =
+T.Modifiers & (1 << uint32_t(HighlightingModifier::Definition));
+bool hasDecl =
+T.Modifiers & (1 << uint32_t(HighlightingModifier::Declaration));
+EXPECT_TRUE(!hasDef || hasDecl);
+
 OS << Input.substr(NextChar, StartOffset - NextChar);
 OS << '$' << T.Kind;
 for (unsigned I = 0;
  I <= static_cast(HighlightingModifier::LastModifier); ++I) {
-  if (T.Modifiers & (1 << I))
-OS << '_' << static_cast(I);
+  if (T.Modifiers & (1 << I)) {
+if (I != uint32_t(HighlightingModifier::Declaration) || !hasDef)
+  OS << '_' << static_cast(I);
+  }
 }
 OS << "[[" << Input.substr(StartOffset, EndOffset - StartOffset) << "]]";
 NextChar = EndOffset;
@@ -96,52 +104,52 @@
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
   R"cpp(
-  struct $Class_decl[[AS]] {
+  struct $Class_def[[AS]] {
 double $Field_decl[[SomeMember]];
   };
   struct {
-  } $Variable_decl[[S]];
-  void $Function_decl[[foo]](int $Parameter_decl[[A]], $Class[[AS]] $Parameter_decl[[As]]) {
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[VeryLongVariableName]] = 12312;
-$Class[[AS]] $LocalVariable_decl[[AA]];
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
-auto $LocalVariable_decl[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_decl[[A]]) -> void {};
+  } $Variable_def[[S]];
+  void $Function_def[[foo]](int $Parameter_decl[[A]], $Class[[AS]] $Parameter_decl[[As]]) {
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[VeryLongVariableName]] = 12312;
+$Class[[AS]] $LocalVariable_def[[AA]];
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
+auto $LocalVariable_def[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_decl[[A]]) -> void {};
 $LocalVariable[[FN]](12312);
   }
 )cpp",
   R"cpp(
   void $Function_decl[[foo]](int);
   void $Function_decl[[Gah]]();
-  void $Function_decl[[foo]]() {
-auto $LocalVariable_decl[[Bou]] = $Function[[Gah]];
+  void $Function_def[[foo]]() {
+auto $LocalVariable_def[[Bou]] = $Function[[Gah]];
   }
-  struct $Class_decl[[A]] {
+  struct $Class_def[[A]] {
 void $Method_decl[[abc]]();
   };
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
 template
-struct $Class_decl[[A]] {
+struct $Class_def[[A]] {
   $TemplateParameter[[T]] $

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:79
+  if (auto *Var = dyn_cast(Decl))
+return !isa(Var) && Var->isThisDeclarationADefinition();
+  return isa(Decl) || isa(Decl);

I'm not 100% sure about this one, by the way. I've just never heard anyone talk 
about a "parameter definition".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

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

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

So, is this okay to merge now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

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

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 466510.
ckandeler marked 2 inline comments as done.
ckandeler added a comment.

Use modifier also for parameters,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
@@ -18,18 +18,18 @@
 TEST_F(AnnotateHighlightingsTest, Test) {
   EXPECT_AVAILABLE("^vo^id^ ^f(^) {^}^"); // available everywhere.
   EXPECT_AVAILABLE("[[int a; int b;]]");
-  EXPECT_EQ("void /* Function [decl] [globalScope] */f() {}",
+  EXPECT_EQ("void /* Function [decl] [def] [globalScope] */f() {}",
 apply("void ^f() {}"));
 
   EXPECT_EQ(apply("[[int f1(); const int x = f1();]]"),
 "int /* Function [decl] [globalScope] */f1(); "
-"const int /* Variable [decl] [readonly] [fileScope] */x = "
+"const int /* Variable [decl] [def] [readonly] [fileScope] */x = "
 "/* Function [globalScope] */f1();");
 
   // Only the targeted range is annotated.
   EXPECT_EQ(apply("void f1(); void f2() {^}"),
 "void f1(); "
-"void /* Function [decl] [globalScope] */f2() {}");
+"void /* Function [decl] [def] [globalScope] */f2() {}");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -48,12 +48,21 @@
 assert(StartOffset <= EndOffset);
 assert(NextChar <= StartOffset);
 
+bool hasDef =
+T.Modifiers & (1 << uint32_t(HighlightingModifier::Definition));
+bool hasDecl =
+T.Modifiers & (1 << uint32_t(HighlightingModifier::Declaration));
+EXPECT_TRUE(!hasDef || hasDecl);
+
 OS << Input.substr(NextChar, StartOffset - NextChar);
 OS << '$' << T.Kind;
 for (unsigned I = 0;
  I <= static_cast(HighlightingModifier::LastModifier); ++I) {
-  if (T.Modifiers & (1 << I))
-OS << '_' << static_cast(I);
+  if (T.Modifiers & (1 << I)) {
+// _decl_def is common and redundant, just print _def instead.
+if (I != uint32_t(HighlightingModifier::Declaration) || !hasDef)
+  OS << '_' << static_cast(I);
+  }
 }
 OS << "[[" << Input.substr(StartOffset, EndOffset - StartOffset) << "]]";
 NextChar = EndOffset;
@@ -96,52 +105,52 @@
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
   R"cpp(
-  struct $Class_decl[[AS]] {
+  struct $Class_def[[AS]] {
 double $Field_decl[[SomeMember]];
   };
   struct {
-  } $Variable_decl[[S]];
-  void $Function_decl[[foo]](int $Parameter_decl[[A]], $Class[[AS]] $Parameter_decl[[As]]) {
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[VeryLongVariableName]] = 12312;
-$Class[[AS]] $LocalVariable_decl[[AA]];
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
-auto $LocalVariable_decl[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_decl[[A]]) -> void {};
+  } $Variable_def[[S]];
+  void $Function_def[[foo]](int $Parameter_def[[A]], $Class[[AS]] $Parameter_def[[As]]) {
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[VeryLongVariableName]] = 12312;
+$Class[[AS]] $LocalVariable_def[[AA]];
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
+auto $LocalVariable_def[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_def[[A]]) -> void {};
 $LocalVariable[[FN]](12312);
   }
 )cpp",
   R"cpp(
   void $Function_decl[[foo]](int);
   void $Function_decl[[Gah]]();
-  void $Function_decl[[foo]]() {
-auto $LocalVariable_decl[[Bou]] = $Function[[Gah]];
+  void $Function_def[[foo]]() {
+auto $LocalVariable_def[[Bou]] = $Function[[Gah]];
   }
-  struct $Class_decl[[A]] {
+  struct $Class_def[[A]] {
 void $Method_decl[[abc]]();
   };
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
 template
-struct $Class_decl[[A]] {
+

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 466731.
ckandeler added a comment.

Template parameters are definitions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
@@ -18,18 +18,18 @@
 TEST_F(AnnotateHighlightingsTest, Test) {
   EXPECT_AVAILABLE("^vo^id^ ^f(^) {^}^"); // available everywhere.
   EXPECT_AVAILABLE("[[int a; int b;]]");
-  EXPECT_EQ("void /* Function [decl] [globalScope] */f() {}",
+  EXPECT_EQ("void /* Function [decl] [def] [globalScope] */f() {}",
 apply("void ^f() {}"));
 
   EXPECT_EQ(apply("[[int f1(); const int x = f1();]]"),
 "int /* Function [decl] [globalScope] */f1(); "
-"const int /* Variable [decl] [readonly] [fileScope] */x = "
+"const int /* Variable [decl] [def] [readonly] [fileScope] */x = "
 "/* Function [globalScope] */f1();");
 
   // Only the targeted range is annotated.
   EXPECT_EQ(apply("void f1(); void f2() {^}"),
 "void f1(); "
-"void /* Function [decl] [globalScope] */f2() {}");
+"void /* Function [decl] [def] [globalScope] */f2() {}");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -48,12 +48,21 @@
 assert(StartOffset <= EndOffset);
 assert(NextChar <= StartOffset);
 
+bool hasDef =
+T.Modifiers & (1 << uint32_t(HighlightingModifier::Definition));
+bool hasDecl =
+T.Modifiers & (1 << uint32_t(HighlightingModifier::Declaration));
+EXPECT_TRUE(!hasDef || hasDecl);
+
 OS << Input.substr(NextChar, StartOffset - NextChar);
 OS << '$' << T.Kind;
 for (unsigned I = 0;
  I <= static_cast(HighlightingModifier::LastModifier); ++I) {
-  if (T.Modifiers & (1 << I))
-OS << '_' << static_cast(I);
+  if (T.Modifiers & (1 << I)) {
+// _decl_def is common and redundant, just print _def instead.
+if (I != uint32_t(HighlightingModifier::Declaration) || !hasDef)
+  OS << '_' << static_cast(I);
+  }
 }
 OS << "[[" << Input.substr(StartOffset, EndOffset - StartOffset) << "]]";
 NextChar = EndOffset;
@@ -96,52 +105,52 @@
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
   R"cpp(
-  struct $Class_decl[[AS]] {
+  struct $Class_def[[AS]] {
 double $Field_decl[[SomeMember]];
   };
   struct {
-  } $Variable_decl[[S]];
-  void $Function_decl[[foo]](int $Parameter_decl[[A]], $Class[[AS]] $Parameter_decl[[As]]) {
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[VeryLongVariableName]] = 12312;
-$Class[[AS]] $LocalVariable_decl[[AA]];
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
-auto $LocalVariable_decl[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_decl[[A]]) -> void {};
+  } $Variable_def[[S]];
+  void $Function_def[[foo]](int $Parameter_def[[A]], $Class[[AS]] $Parameter_def[[As]]) {
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[VeryLongVariableName]] = 12312;
+$Class[[AS]] $LocalVariable_def[[AA]];
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
+auto $LocalVariable_def[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_def[[A]]) -> void {};
 $LocalVariable[[FN]](12312);
   }
 )cpp",
   R"cpp(
   void $Function_decl[[foo]](int);
   void $Function_decl[[Gah]]();
-  void $Function_decl[[foo]]() {
-auto $LocalVariable_decl[[Bou]] = $Function[[Gah]];
+  void $Function_def[[foo]]() {
+auto $LocalVariable_def[[Bou]] = $Function[[Gah]];
   }
-  struct $Class_decl[[A]] {
+  struct $Class_def[[A]] {
 void $Method_decl[[abc]]();
   };
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
-template
-struct $Class_decl[[A]] {
+template
+struct $Class_def[[A

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Everything good now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

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

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Could you please land it for me? I don't have the privileges. (How does one 
attain those, by the way?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

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

[PATCH] D139926: [clangd] Add semantic tokens for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D139926#4030782 , @nridge wrote:

> It's true that there is an ambiguity between `<` and `>` as operators, vs. 
> template arg/param list delimiters, but, at least in terms of user 
> understanding of code, my sense is that the highlighting of the **preceding** 
> token should be sufficient to disambiguate -- i.e. it would be some sort of 
> type name in the template case, vs. a variable / literal / punctuation ending 
> an expression in the operator case.

We used to do this sort of heuristic in our old libclang-based implementation, 
and it turned out to be rather messy, with a surprising amount of exceptions 
having to be added.

>> This is needed for clients that would like to visualize matching opening and 
>> closing angle brackets, which can be valuable in non-trivial template 
>> declarations or instantiations.
>
> For this use case, could an editor make use of the recently added operator 
> tokens (https://reviews.llvm.org/D136594) instead, inferring that a `<` token 
> which does not have an `operator` semantic token is a template delimiter?

I have a suspicion that this will lead to false positives for invalid code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

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

[PATCH] D139926: [clangd] Add semantic tokens for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D139926#4033152 , @nridge wrote:

> In D139926#4032473 , @ckandeler 
> wrote:
>
>> In D139926#4030782 , @nridge wrote:
>>
>>> It's true that there is an ambiguity between `<` and `>` as operators, vs. 
>>> template arg/param list delimiters, but, at least in terms of user 
>>> understanding of code, my sense is that the highlighting of the 
>>> **preceding** token should be sufficient to disambiguate -- i.e. it would 
>>> be some sort of type name in the template case, vs. a variable / literal / 
>>> punctuation ending an expression in the operator case.
>>
>> We used to do this sort of heuristic in our old libclang-based 
>> implementation, and it turned out to be rather messy, with a surprising 
>> amount of exceptions having to be added.
>
> To clarify, I'm not suggesting that any client-side logic use this heuristic, 
> only that it's probably good enough for a human reader to disambiguate 
> without needing to assign angle brackets and comparison operators different 
> colors.

Note that we use this information to *animate* the matching tokens, i.e. when 
the cursor is on one of them, both it and its counterpart get a special 
highlighting. That's why it's so important that the language server guarantees 
they always come in pairs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

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

[PATCH] D130015: [clangd] Add "usedAsMutablePointer" highlighting modifier

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:592
+  Arg = IC->getSubExprAsWritten();
+if (auto *UO = dyn_cast(Arg)) {
+  if (UO->getOpcode() == UO_AddrOf)

nridge wrote:
> Could you add a test case that exercises the `UnaryOperator` case?
Doesn't the very first hunk in SemanticHighlightingTests.cpp do that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130015

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

[PATCH] D130015: [clangd] Add "usedAsMutablePointer" highlighting modifier

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 473603.
ckandeler marked an inline comment as done.
ckandeler added a comment.

Rebased and updated comment as indicated in review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130015

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.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
@@ -382,7 +382,7 @@
   void $Function_def[[foo]]() {
 $Class[[F]] $LocalVariable_def[[FF]];
 $Class[[G]]<$Class[[F]], &$Class[[F]]::$Method[[f]]> $LocalVariable_def[[GG]];
-$LocalVariable[[GG]].$Method[[foo]](&$LocalVariable[[FF]]);
+$LocalVariable[[GG]].$Method[[foo]](&$LocalVariable_usedAsMutablePointer[[FF]]);
 $Class[[A]]<$Function[[foo]]> $LocalVariable_def[[AA]];
   }
 )cpp",
@@ -781,14 +781,14 @@
   const int* $LocalVariable_def_readonly[[constPtr]];
   int** $LocalVariable_def[[array]];
   $Function[[fun]]($LocalVariable[[val]], $LocalVariable[[val]],
-   $LocalVariable[[ptr]], $LocalVariable_readonly[[constPtr]],
+   $LocalVariable_usedAsMutablePointer[[ptr]], $LocalVariable_readonly[[constPtr]],
$LocalVariable_usedAsMutableReference[[val]], $LocalVariable[[val]],
 
$LocalVariable_usedAsMutableReference[[ptr]],
$LocalVariable_readonly_usedAsMutableReference[[constPtr]],
$LocalVariable_readonly[[constPtr]],
 
-   $LocalVariable[[array]], $LocalVariable_usedAsMutableReference[[array]],
+   $LocalVariable_usedAsMutablePointer[[array]], $LocalVariable_usedAsMutableReference[[array]],
$LocalVariable[[array]]
);
   [](int){}($LocalVariable[[val]]);
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- clang-tools-extra/clangd/test/semantic-tokens.test
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  32771
+# CHECK-NEXT:  65539
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "1"
 # CHECK-NEXT:  }
@@ -49,7 +49,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  32771
+# CHECK-NEXT:  65539
 # CHECK-NEXT:],
 #Inserted at position 1
 # CHECK-NEXT:"deleteCount": 0,
@@ -72,12 +72,12 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  32771,
+# CHECK-NEXT:  65539,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  32771
+# CHECK-NEXT:  65539
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "3"
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -68,6 +68,7 @@
 # CHECK-NEXT:"dependentName",
 # CHECK-NEXT:"defaultLibrary",
 # CHECK-NEXT:"usedAsMutableReference",
+# CHECK-NEXT:"usedAsMutablePointer",
 # CHECK-NEXT:"constructorOrDestructor",
 # CHECK-NEXT:"functionScope",
 # CHECK-NEXT:"classScope",
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -71,6 +71,7 @@
   DependentName,
   DefaultLibrary,
   UsedAsMutableReference,
+  UsedAsMutablePointer,
   ConstructorOrDestructor,
 
   FunctionScope,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -597,19 +597,27 @@
 if (!Arg)
   return;
 
-// Is this parameter passed by non-const reference?
+// Is this parameter passed by non-const pointer or reference?
 // FIXME The condition T->idDependentType() could be relaxed a bit,
 // e.g. std::vector& is dependent but we would want to highlight it
- 

[PATCH] D130015: [clangd] Add "usedAsMutablePointer" highlighting modifier

[Christian Kandeler via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2bf960aef08e: [clangd] Add "usedAsMutablePointer" 
highlighting modifier (authored by ckandeler).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130015

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.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
@@ -382,7 +382,7 @@
   void $Function_def[[foo]]() {
 $Class[[F]] $LocalVariable_def[[FF]];
 $Class[[G]]<$Class[[F]], &$Class[[F]]::$Method[[f]]> $LocalVariable_def[[GG]];
-$LocalVariable[[GG]].$Method[[foo]](&$LocalVariable[[FF]]);
+$LocalVariable[[GG]].$Method[[foo]](&$LocalVariable_usedAsMutablePointer[[FF]]);
 $Class[[A]]<$Function[[foo]]> $LocalVariable_def[[AA]];
   }
 )cpp",
@@ -781,14 +781,14 @@
   const int* $LocalVariable_def_readonly[[constPtr]];
   int** $LocalVariable_def[[array]];
   $Function[[fun]]($LocalVariable[[val]], $LocalVariable[[val]],
-   $LocalVariable[[ptr]], $LocalVariable_readonly[[constPtr]],
+   $LocalVariable_usedAsMutablePointer[[ptr]], $LocalVariable_readonly[[constPtr]],
$LocalVariable_usedAsMutableReference[[val]], $LocalVariable[[val]],
 
$LocalVariable_usedAsMutableReference[[ptr]],
$LocalVariable_readonly_usedAsMutableReference[[constPtr]],
$LocalVariable_readonly[[constPtr]],
 
-   $LocalVariable[[array]], $LocalVariable_usedAsMutableReference[[array]],
+   $LocalVariable_usedAsMutablePointer[[array]], $LocalVariable_usedAsMutableReference[[array]],
$LocalVariable[[array]]
);
   [](int){}($LocalVariable[[val]]);
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- clang-tools-extra/clangd/test/semantic-tokens.test
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  32771
+# CHECK-NEXT:  65539
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "1"
 # CHECK-NEXT:  }
@@ -49,7 +49,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  32771
+# CHECK-NEXT:  65539
 # CHECK-NEXT:],
 #Inserted at position 1
 # CHECK-NEXT:"deleteCount": 0,
@@ -72,12 +72,12 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  32771,
+# CHECK-NEXT:  65539,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  32771
+# CHECK-NEXT:  65539
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "3"
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -68,6 +68,7 @@
 # CHECK-NEXT:"dependentName",
 # CHECK-NEXT:"defaultLibrary",
 # CHECK-NEXT:"usedAsMutableReference",
+# CHECK-NEXT:"usedAsMutablePointer",
 # CHECK-NEXT:"constructorOrDestructor",
 # CHECK-NEXT:"functionScope",
 # CHECK-NEXT:"classScope",
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -71,6 +71,7 @@
   DependentName,
   DefaultLibrary,
   UsedAsMutableReference,
+  UsedAsMutablePointer,
   ConstructorOrDestructor,
 
   FunctionScope,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -597,19 +597,27 @@
 if (!Arg)
   return;
 
-// Is this parameter passed by non-const reference?
+// Is this parameter passed by non-const pointer or reference?
 // FIXME The condition T->idDependentType() could be relaxed a bit,
 // e.g. std::vector& is dependent but we would want 

[PATCH] D137943: [clangd] Mark "override" and "final" as keywords

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

... in semantic highlighting.
These specifiers are keywords for all intents and purposes, except that
they are not, for backward compatibility. As a result, they cannot be
identified by simple lexing (since e.g. variables with these names can
legally be declared), which means they should be semantic tokens.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137943

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  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
@@ -864,7 +864,13 @@
 const char *$LocalVariable_def_readonly[[s]] = 
$LocalVariable_readonly_static[[__func__]];
 }
   )cpp",
+  // override and final
+  R"cpp(
+class $Class_def_abstract[[Base]] { virtual void 
$Method_decl_abstract_virtual[[m]]() = 0; };
+class $Class_def[[Derived]] : public $Class_abstract[[Base]] { void 
$Method_decl_virtual[[m]]() $Keyword[[override]]; };
+class $Class_def[[MoreDerived]] : public $Class[[Derived]] { void 
$Method_decl_virtual[[m]]() $Keyword[[final]]; };
   // Issue 1222: readonly modifier for generic parameter
+  )cpp",
   R"cpp(
 template 
 auto $Function_def[[foo]](const $TemplateParameter[[T]] 
$Parameter_def_readonly[[template_type]], 
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -49,6 +49,7 @@
   Concept,
   Primitive,
   Macro,
+  Keyword,
 
   // This one is different from the other kinds as it's a line style
   // rather than a token style.
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -809,6 +809,18 @@
 return true;
   }
 
+  bool VisitAttr(Attr *A) {
+switch (A->getKind()) {
+case attr::Override:
+case attr::Final:
+  H.addToken(A->getLocation(), HighlightingKind::Keyword);
+  break;
+default:
+  break;
+}
+return true;
+  }
+
   bool VisitDependentNameTypeLoc(DependentNameTypeLoc L) {
 H.addToken(L.getNameLoc(), HighlightingKind::Type)
 .addModifier(HighlightingModifier::DependentName)
@@ -985,6 +997,8 @@
 return OS << "Primitive";
   case HighlightingKind::Macro:
 return OS << "Macro";
+  case HighlightingKind::Keyword:
+return OS << "Keyword";
   case HighlightingKind::InactiveCode:
 return OS << "InactiveCode";
   }
@@ -1119,6 +1133,8 @@
 return "type";
   case HighlightingKind::Macro:
 return "macro";
+  case HighlightingKind::Keyword:
+return "keyword";
   case HighlightingKind::InactiveCode:
 return "comment";
   }


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -864,7 +864,13 @@
 const char *$LocalVariable_def_readonly[[s]] = $LocalVariable_readonly_static[[__func__]];
 }
   )cpp",
+  // override and final
+  R"cpp(
+class $Class_def_abstract[[Base]] { virtual void $Method_decl_abstract_virtual[[m]]() = 0; };
+class $Class_def[[Derived]] : public $Class_abstract[[Base]] { void $Method_decl_virtual[[m]]() $Keyword[[override]]; };
+class $Class_def[[MoreDerived]] : public $Class[[Derived]] { void $Method_decl_virtual[[m]]() $Keyword[[final]]; };
   // Issue 1222: readonly modifier for generic parameter
+  )cpp",
   R"cpp(
 template 
 auto $Function_def[[foo]](const $TemplateParameter[[T]] $Parameter_def_readonly[[template_type]], 
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -49,6 +49,7 @@
   Concept,
   Primitive,
   Macro,
+  Keyword,
 
   // This one is different from the other kinds as it's a line style
   // rather than a token style.
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===

[PATCH] D139926: [clangd] Add semantic token for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 491718.
ckandeler marked an inline comment as done.
ckandeler added a comment.

Added test cases from review comment and made them pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  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
@@ -130,17 +130,17 @@
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
-template
+template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
 struct $Class_def[[A]] {
   $TemplateParameter[[T]] $Field_decl[[t]];
 };
   }
-  template
-  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
+  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]$TemplateParameter[[T]]$Bracket[[>]] {
 typename $TemplateParameter[[T]]::$Type_dependentName[[A]]* $Field_decl[[D]];
   };
-  $Namespace[[abc]]::$Class[[A]] $Variable_def[[AA]];
-  typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]];
+  $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]int$Bracket[[>]] $Variable_def[[AA]];
+  typedef $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]int$Bracket[[>]] $Class_decl[[AAA]];
   struct $Class_def[[B]] {
 $Class_decl_constrDestr[[B]]();
 ~$Class_decl_constrDestr[[B]]();
@@ -243,36 +243,36 @@
   typedef float $Primitive_decl[[F]];
 )cpp",
   R"cpp(
-  template
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]], typename = void$Bracket[[>]]
   class $Class_def[[A]] {
 $TemplateParameter[[T]] $Field_decl[[AA]];
 $TemplateParameter[[T]] $Method_decl[[foo]]();
   };
-  template
+  template$Bracket[[<]]class $TemplateParameter_def[[TT]]$Bracket[[>]]
   class $Class_def[[B]] {
-$Class[[A]]<$TemplateParameter[[TT]]> $Field_decl[[AA]];
+$Class[[A]]$Bracket[[<]]$TemplateParameter[[TT]]$Bracket[[>]] $Field_decl[[AA]];
   };
-  template
+  template$Bracket[[<]]class $TemplateParameter_def[[TT]], class $TemplateParameter_def[[GG]]$Bracket[[>]]
   class $Class_def[[BB]] {};
-  template
-  class $Class_def[[BB]]<$TemplateParameter[[T]], int> {};
-  template
-  class $Class_def[[BB]]<$TemplateParameter[[T]], $TemplateParameter[[T]]*> {};
+  template$Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
+  class $Class_def[[BB]]$Bracket[[<]]$TemplateParameter[[T]], int$Bracket[[>]] {};
+  template$Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
+  class $Class_def[[BB]]$Bracket[[<]]$TemplateParameter[[T]], $TemplateParameter[[T]]*$Bracket[[>]] {};
 
-  template class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]>
-  $TemplateParameter[[T]]<$TemplateParameter[[C]]> $Function_decl[[f]]();
+  template$Bracket[[<]]template$Bracket[[<]]class$Bracket[[>]] class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]$Bracket[[>]]
+  $TemplateParameter[[T]]$Bracket[[<]]$TemplateParameter[[C]]$Bracket[[>]] $Function_decl[[f]]();
 
-  template
+  template$Bracket[[<]]typename$Bracket[[>]]
   class $Class_def[[Foo]] {};
 
-  template
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
   void $Function_decl[[foo]]($TemplateParameter[[T]] ...);
 )cpp",
   R"cpp(
-  template 
+  template $Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
   struct $Class_def[[Tmpl]] {$TemplateParameter[[T]] $Field_decl[[x]] = 0;};
-  extern template struct $Class_def[[Tmpl]];
-  template struct $Class_def[[Tmpl]];
+  extern template struct $Class_def[[Tmpl]]$Bracket[[<]]float$Bracket[[>]];
+  template struct $Class_def[[Tmpl]]$Bracket[[<]]double$Bracket[[>]];
 )cpp",
   // This test is to guard against highlightings disappearing when using
   // conversion operators as their behaviour in the clang AST differ from
@@ -335,17 +335,17 @@
 )cpp",
   R"cpp(
   class $Class_def[[G]] {};
-  template<$Class[[G]] *$TemplateParameter_def_readonly[[U]]>
+  template$Bracket[[<]]$Class[[G]] *$TemplateParameter_def_readonly[[U]]$Bracket[[>]]
   class $Class_def[[GP]] {};
-  template<$Class[[G]] &$TemplateParameter_def_readonly[[U]]>
+  template$Bracket[[<]]$Class[[G]] &$TemplateParameter_def_readonly[[U]]$Bracket[[>]]
   class $Class_def[[GR]] {};
-  templat

[PATCH] D139926: [clangd] Add semantic token for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Thanks for the test cases!
All fixed, except:

> // variable template specialization
> // parameter and argument lists are missing semantic tokens
> template <>
> constexpr int V = 5;

Argument list fixed. I didn't manage to make the parameter list work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

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

[PATCH] D139926: [clangd] Add semantic token for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D139926#4076057 , @nridge wrote:

> One other thought that has occurred to me is that as we add more semantic 
> tolens for punctuation, the test cases in `GetsCorrectTokens` become harder 
> to read.
>
> What would you think about omitting punctuation tokens in the 
> `GetsCorrectTokens` test cases (both `Operator` and `Bracket`, and any future 
> ones), similarly to the way we already omit scope modifiers like 
> `_classScope`, and have another test case like `GetsCorrectPunctuationTokens` 
> where we put specifically test cases that test for punctuation tokens?

I suppose that's an option, but I don't have a strong opinion on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

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

[PATCH] D139926: [clangd] Add semantic token for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D139926#4084801 , @nridge wrote:

> Function calls are still missing some cases:
>
>   

I would have expected these to be handled by VisitDeclRefExpr(), but they 
aren't. Any idea?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

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

[PATCH] D139926: [clangd] Add semantic token for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 492670.
ckandeler marked 3 inline comments as done.
ckandeler added a comment.

Handled more cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  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
@@ -130,17 +130,17 @@
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
-template
+template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
 struct $Class_def[[A]] {
   $TemplateParameter[[T]] $Field_decl[[t]];
 };
   }
-  template
-  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
+  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]$TemplateParameter[[T]]$Bracket[[>]] {
 typename $TemplateParameter[[T]]::$Type_dependentName[[A]]* $Field_decl[[D]];
   };
-  $Namespace[[abc]]::$Class[[A]] $Variable_def[[AA]];
-  typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]];
+  $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]int$Bracket[[>]] $Variable_def[[AA]];
+  typedef $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]int$Bracket[[>]] $Class_decl[[AAA]];
   struct $Class_def[[B]] {
 $Class_decl_constrDestr[[B]]();
 ~$Class_decl_constrDestr[[B]]();
@@ -243,36 +243,36 @@
   typedef float $Primitive_decl[[F]];
 )cpp",
   R"cpp(
-  template
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]], typename = void$Bracket[[>]]
   class $Class_def[[A]] {
 $TemplateParameter[[T]] $Field_decl[[AA]];
 $TemplateParameter[[T]] $Method_decl[[foo]]();
   };
-  template
+  template$Bracket[[<]]class $TemplateParameter_def[[TT]]$Bracket[[>]]
   class $Class_def[[B]] {
-$Class[[A]]<$TemplateParameter[[TT]]> $Field_decl[[AA]];
+$Class[[A]]$Bracket[[<]]$TemplateParameter[[TT]]$Bracket[[>]] $Field_decl[[AA]];
   };
-  template
+  template$Bracket[[<]]class $TemplateParameter_def[[TT]], class $TemplateParameter_def[[GG]]$Bracket[[>]]
   class $Class_def[[BB]] {};
-  template
-  class $Class_def[[BB]]<$TemplateParameter[[T]], int> {};
-  template
-  class $Class_def[[BB]]<$TemplateParameter[[T]], $TemplateParameter[[T]]*> {};
+  template$Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
+  class $Class_def[[BB]]$Bracket[[<]]$TemplateParameter[[T]], int$Bracket[[>]] {};
+  template$Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
+  class $Class_def[[BB]]$Bracket[[<]]$TemplateParameter[[T]], $TemplateParameter[[T]]*$Bracket[[>]] {};
 
-  template class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]>
-  $TemplateParameter[[T]]<$TemplateParameter[[C]]> $Function_decl[[f]]();
+  template$Bracket[[<]]template$Bracket[[<]]class$Bracket[[>]] class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]$Bracket[[>]]
+  $TemplateParameter[[T]]$Bracket[[<]]$TemplateParameter[[C]]$Bracket[[>]] $Function_decl[[f]]();
 
-  template
+  template$Bracket[[<]]typename$Bracket[[>]]
   class $Class_def[[Foo]] {};
 
-  template
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
   void $Function_decl[[foo]]($TemplateParameter[[T]] ...);
 )cpp",
   R"cpp(
-  template 
+  template $Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
   struct $Class_def[[Tmpl]] {$TemplateParameter[[T]] $Field_decl[[x]] = 0;};
-  extern template struct $Class_def[[Tmpl]];
-  template struct $Class_def[[Tmpl]];
+  extern template struct $Class_def[[Tmpl]]$Bracket[[<]]float$Bracket[[>]];
+  template struct $Class_def[[Tmpl]]$Bracket[[<]]double$Bracket[[>]];
 )cpp",
   // This test is to guard against highlightings disappearing when using
   // conversion operators as their behaviour in the clang AST differ from
@@ -335,17 +335,17 @@
 )cpp",
   R"cpp(
   class $Class_def[[G]] {};
-  template<$Class[[G]] *$TemplateParameter_def_readonly[[U]]>
+  template$Bracket[[<]]$Class[[G]] *$TemplateParameter_def_readonly[[U]]$Bracket[[>]]
   class $Class_def[[GP]] {};
-  template<$Class[[G]] &$TemplateParameter_def_readonly[[U]]>
+  template$Bracket[[<]]$Class[[G]] &$TemplateParameter_def_readonly[[U]]$Bracket[[>]]
   class $Class_def[[GR]] {};
-  template
+  template$Bracket[[<]]int *$T

[PATCH] D139926: [clangd] Add semantic token for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D139926#4084868 , @nridge wrote:

> I figured I might as well look through the AST API for classes with 
> getLAngleLoc/getRAngleLoc methods.
>
> It looks like we've got almost all of them (including the ones mentioned in 
> recent comments) except:
>
> - OverloadExpr
> - DependentTemplateSpecializationTypeLoc
> - AutoTypeLoc

Adding AutoTypeLoc broke tons of tests, so I left it out for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

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

[PATCH] D139926: [clangd] Add semantic token for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: 
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:1026
+concept $Concept_decl[[C2]] = true;
+template $Bracket[[<]]C2$Bracket[[<]]int$Bracket[[>]] 
$TemplateParameter_def[[A]]$Bracket[[>]]
+class $Class_def[[B]] {};

Note that C2 does not get a token here. Maybe we can fix that along the way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

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

[PATCH] D139926: [clangd] Add semantic token for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 493247.
ckandeler added a comment.

Support more cases as per review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  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
@@ -130,17 +130,17 @@
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
-template
+template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
 struct $Class_def[[A]] {
   $TemplateParameter[[T]] $Field_decl[[t]];
 };
   }
-  template
-  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
+  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]$TemplateParameter[[T]]$Bracket[[>]] {
 typename $TemplateParameter[[T]]::$Type_dependentName[[A]]* $Field_decl[[D]];
   };
-  $Namespace[[abc]]::$Class[[A]] $Variable_def[[AA]];
-  typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]];
+  $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]int$Bracket[[>]] $Variable_def[[AA]];
+  typedef $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]int$Bracket[[>]] $Class_decl[[AAA]];
   struct $Class_def[[B]] {
 $Class_decl_constrDestr[[B]]();
 ~$Class_decl_constrDestr[[B]]();
@@ -243,36 +243,36 @@
   typedef float $Primitive_decl[[F]];
 )cpp",
   R"cpp(
-  template
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]], typename = void$Bracket[[>]]
   class $Class_def[[A]] {
 $TemplateParameter[[T]] $Field_decl[[AA]];
 $TemplateParameter[[T]] $Method_decl[[foo]]();
   };
-  template
+  template$Bracket[[<]]class $TemplateParameter_def[[TT]]$Bracket[[>]]
   class $Class_def[[B]] {
-$Class[[A]]<$TemplateParameter[[TT]]> $Field_decl[[AA]];
+$Class[[A]]$Bracket[[<]]$TemplateParameter[[TT]]$Bracket[[>]] $Field_decl[[AA]];
   };
-  template
+  template$Bracket[[<]]class $TemplateParameter_def[[TT]], class $TemplateParameter_def[[GG]]$Bracket[[>]]
   class $Class_def[[BB]] {};
-  template
-  class $Class_def[[BB]]<$TemplateParameter[[T]], int> {};
-  template
-  class $Class_def[[BB]]<$TemplateParameter[[T]], $TemplateParameter[[T]]*> {};
+  template$Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
+  class $Class_def[[BB]]$Bracket[[<]]$TemplateParameter[[T]], int$Bracket[[>]] {};
+  template$Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
+  class $Class_def[[BB]]$Bracket[[<]]$TemplateParameter[[T]], $TemplateParameter[[T]]*$Bracket[[>]] {};
 
-  template class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]>
-  $TemplateParameter[[T]]<$TemplateParameter[[C]]> $Function_decl[[f]]();
+  template$Bracket[[<]]template$Bracket[[<]]class$Bracket[[>]] class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]$Bracket[[>]]
+  $TemplateParameter[[T]]$Bracket[[<]]$TemplateParameter[[C]]$Bracket[[>]] $Function_decl[[f]]();
 
-  template
+  template$Bracket[[<]]typename$Bracket[[>]]
   class $Class_def[[Foo]] {};
 
-  template
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
   void $Function_decl[[foo]]($TemplateParameter[[T]] ...);
 )cpp",
   R"cpp(
-  template 
+  template $Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
   struct $Class_def[[Tmpl]] {$TemplateParameter[[T]] $Field_decl[[x]] = 0;};
-  extern template struct $Class_def[[Tmpl]];
-  template struct $Class_def[[Tmpl]];
+  extern template struct $Class_def[[Tmpl]]$Bracket[[<]]float$Bracket[[>]];
+  template struct $Class_def[[Tmpl]]$Bracket[[<]]double$Bracket[[>]];
 )cpp",
   // This test is to guard against highlightings disappearing when using
   // conversion operators as their behaviour in the clang AST differ from
@@ -335,17 +335,17 @@
 )cpp",
   R"cpp(
   class $Class_def[[G]] {};
-  template<$Class[[G]] *$TemplateParameter_def_readonly[[U]]>
+  template$Bracket[[<]]$Class[[G]] *$TemplateParameter_def_readonly[[U]]$Bracket[[>]]
   class $Class_def[[GP]] {};
-  template<$Class[[G]] &$TemplateParameter_def_readonly[[U]]>
+  template$Bracket[[<]]$Class[[G]] &$TemplateParameter_def_readonly[[U]]$Bracket[[>]]
   class $Class_def[[GR]] {};
-  template
+  template$Bracket[[<]]int *$TemplateParameter_def_

[PATCH] D139926: [clangd] Add semantic token for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 493594.
ckandeler marked an inline comment as done.
ckandeler added a comment.

Improved implementation as per review comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  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
@@ -130,17 +130,17 @@
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
-template
+template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
 struct $Class_def[[A]] {
   $TemplateParameter[[T]] $Field_decl[[t]];
 };
   }
-  template
-  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
+  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]$TemplateParameter[[T]]$Bracket[[>]] {
 typename $TemplateParameter[[T]]::$Type_dependentName[[A]]* $Field_decl[[D]];
   };
-  $Namespace[[abc]]::$Class[[A]] $Variable_def[[AA]];
-  typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]];
+  $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]int$Bracket[[>]] $Variable_def[[AA]];
+  typedef $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]int$Bracket[[>]] $Class_decl[[AAA]];
   struct $Class_def[[B]] {
 $Class_decl_constrDestr[[B]]();
 ~$Class_decl_constrDestr[[B]]();
@@ -243,36 +243,36 @@
   typedef float $Primitive_decl[[F]];
 )cpp",
   R"cpp(
-  template
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]], typename = void$Bracket[[>]]
   class $Class_def[[A]] {
 $TemplateParameter[[T]] $Field_decl[[AA]];
 $TemplateParameter[[T]] $Method_decl[[foo]]();
   };
-  template
+  template$Bracket[[<]]class $TemplateParameter_def[[TT]]$Bracket[[>]]
   class $Class_def[[B]] {
-$Class[[A]]<$TemplateParameter[[TT]]> $Field_decl[[AA]];
+$Class[[A]]$Bracket[[<]]$TemplateParameter[[TT]]$Bracket[[>]] $Field_decl[[AA]];
   };
-  template
+  template$Bracket[[<]]class $TemplateParameter_def[[TT]], class $TemplateParameter_def[[GG]]$Bracket[[>]]
   class $Class_def[[BB]] {};
-  template
-  class $Class_def[[BB]]<$TemplateParameter[[T]], int> {};
-  template
-  class $Class_def[[BB]]<$TemplateParameter[[T]], $TemplateParameter[[T]]*> {};
+  template$Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
+  class $Class_def[[BB]]$Bracket[[<]]$TemplateParameter[[T]], int$Bracket[[>]] {};
+  template$Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
+  class $Class_def[[BB]]$Bracket[[<]]$TemplateParameter[[T]], $TemplateParameter[[T]]*$Bracket[[>]] {};
 
-  template class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]>
-  $TemplateParameter[[T]]<$TemplateParameter[[C]]> $Function_decl[[f]]();
+  template$Bracket[[<]]template$Bracket[[<]]class$Bracket[[>]] class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]$Bracket[[>]]
+  $TemplateParameter[[T]]$Bracket[[<]]$TemplateParameter[[C]]$Bracket[[>]] $Function_decl[[f]]();
 
-  template
+  template$Bracket[[<]]typename$Bracket[[>]]
   class $Class_def[[Foo]] {};
 
-  template
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
   void $Function_decl[[foo]]($TemplateParameter[[T]] ...);
 )cpp",
   R"cpp(
-  template 
+  template $Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
   struct $Class_def[[Tmpl]] {$TemplateParameter[[T]] $Field_decl[[x]] = 0;};
-  extern template struct $Class_def[[Tmpl]];
-  template struct $Class_def[[Tmpl]];
+  extern template struct $Class_def[[Tmpl]]$Bracket[[<]]float$Bracket[[>]];
+  template struct $Class_def[[Tmpl]]$Bracket[[<]]double$Bracket[[>]];
 )cpp",
   // This test is to guard against highlightings disappearing when using
   // conversion operators as their behaviour in the clang AST differ from
@@ -335,17 +335,17 @@
 )cpp",
   R"cpp(
   class $Class_def[[G]] {};
-  template<$Class[[G]] *$TemplateParameter_def_readonly[[U]]>
+  template$Bracket[[<]]$Class[[G]] *$TemplateParameter_def_readonly[[U]]$Bracket[[>]]
   class $Class_def[[GP]] {};
-  template<$Class[[G]] &$TemplateParameter_def_readonly[[U]]>
+  template$Bracket[[<]]$Class[[G]] &$TemplateParameter_def_readonly[[U]]$Bracket[[>]]
   class $Class_def[[GR]] {};
-  template
+  t

[PATCH] D139926: [clangd] Add semantic token for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:382
+
+// For the inner element of a nested template instantiation with no space
+// between the '>' characters, TemplateSpecializationLocInfo::RAngleLoc has

kadircet wrote:
> i am actually having a hard time following the logic here. it feels like what 
> you really want is map `>` back to its file location, possibly to a `>>` 
> token, and deal with second case specially.
> so maybe something like:
> ```
> // RLoc might be pointing at a virtual buffer when it's part of a `>>` token.
> RLoc = SM.getFileLoc(RLoc);
> // Make sure token is part of the main file.
> RLoc = getHighlightableSpellingToken(RLoc);
> if(!RLoc.isValid())
>   return;
> 
> const auto *RTok = TB.spelledTokenAt(RLoc);
> // Handle `>>`. RLoc is always pointing at the right location, just change
> // the end to be offset by 1.
> // We'll either point at the beginning of `>>`, hence get a proper spelled
> // or point in the middle of `>>` hence get no spelled tok.
> if (!RTok || RTok->kind() == tok::greatergreater) {
>   Position Begin = sourceLocToPosition(SourceMgr, RLoc);
>   Position End = sourceLocToPosition(SourceMgr, RLoc.getLocWithOffset(1));
>   addToken(*LRange, HighlightingKind::Bracket);
>   addToken({Begin, End}, HighlightingKind::Bracket);
>   return;
> }
> 
> // Easy case, we have the `>` token directly available.
> if (RTok->kind() == tok::greater) {
>   if (auto RRange = getRangeForSourceLocation(RLoc)) {
> addToken(*LRange, HighlightingKind::Bracket);
> addToken(*RRange, HighlightingKind::Bracket);
>   }
>   return;
> }
> ```
> 
> This should also make sure you're handling line continuations properly.
Indeed, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

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

[PATCH] D139926: [clangd] Add semantic token for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 493610.
ckandeler added a comment.

Incorporated review comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  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
@@ -130,17 +130,17 @@
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
-template
+template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
 struct $Class_def[[A]] {
   $TemplateParameter[[T]] $Field_decl[[t]];
 };
   }
-  template
-  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
+  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]$TemplateParameter[[T]]$Bracket[[>]] {
 typename $TemplateParameter[[T]]::$Type_dependentName[[A]]* $Field_decl[[D]];
   };
-  $Namespace[[abc]]::$Class[[A]] $Variable_def[[AA]];
-  typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]];
+  $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]int$Bracket[[>]] $Variable_def[[AA]];
+  typedef $Namespace[[abc]]::$Class[[A]]$Bracket[[<]]int$Bracket[[>]] $Class_decl[[AAA]];
   struct $Class_def[[B]] {
 $Class_decl_constrDestr[[B]]();
 ~$Class_decl_constrDestr[[B]]();
@@ -243,36 +243,36 @@
   typedef float $Primitive_decl[[F]];
 )cpp",
   R"cpp(
-  template
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]], typename = void$Bracket[[>]]
   class $Class_def[[A]] {
 $TemplateParameter[[T]] $Field_decl[[AA]];
 $TemplateParameter[[T]] $Method_decl[[foo]]();
   };
-  template
+  template$Bracket[[<]]class $TemplateParameter_def[[TT]]$Bracket[[>]]
   class $Class_def[[B]] {
-$Class[[A]]<$TemplateParameter[[TT]]> $Field_decl[[AA]];
+$Class[[A]]$Bracket[[<]]$TemplateParameter[[TT]]$Bracket[[>]] $Field_decl[[AA]];
   };
-  template
+  template$Bracket[[<]]class $TemplateParameter_def[[TT]], class $TemplateParameter_def[[GG]]$Bracket[[>]]
   class $Class_def[[BB]] {};
-  template
-  class $Class_def[[BB]]<$TemplateParameter[[T]], int> {};
-  template
-  class $Class_def[[BB]]<$TemplateParameter[[T]], $TemplateParameter[[T]]*> {};
+  template$Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
+  class $Class_def[[BB]]$Bracket[[<]]$TemplateParameter[[T]], int$Bracket[[>]] {};
+  template$Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
+  class $Class_def[[BB]]$Bracket[[<]]$TemplateParameter[[T]], $TemplateParameter[[T]]*$Bracket[[>]] {};
 
-  template class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]>
-  $TemplateParameter[[T]]<$TemplateParameter[[C]]> $Function_decl[[f]]();
+  template$Bracket[[<]]template$Bracket[[<]]class$Bracket[[>]] class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]$Bracket[[>]]
+  $TemplateParameter[[T]]$Bracket[[<]]$TemplateParameter[[C]]$Bracket[[>]] $Function_decl[[f]]();
 
-  template
+  template$Bracket[[<]]typename$Bracket[[>]]
   class $Class_def[[Foo]] {};
 
-  template
+  template$Bracket[[<]]typename $TemplateParameter_def[[T]]$Bracket[[>]]
   void $Function_decl[[foo]]($TemplateParameter[[T]] ...);
 )cpp",
   R"cpp(
-  template 
+  template $Bracket[[<]]class $TemplateParameter_def[[T]]$Bracket[[>]]
   struct $Class_def[[Tmpl]] {$TemplateParameter[[T]] $Field_decl[[x]] = 0;};
-  extern template struct $Class_def[[Tmpl]];
-  template struct $Class_def[[Tmpl]];
+  extern template struct $Class_def[[Tmpl]]$Bracket[[<]]float$Bracket[[>]];
+  template struct $Class_def[[Tmpl]]$Bracket[[<]]double$Bracket[[>]];
 )cpp",
   // This test is to guard against highlightings disappearing when using
   // conversion operators as their behaviour in the clang AST differ from
@@ -335,17 +335,17 @@
 )cpp",
   R"cpp(
   class $Class_def[[G]] {};
-  template<$Class[[G]] *$TemplateParameter_def_readonly[[U]]>
+  template$Bracket[[<]]$Class[[G]] *$TemplateParameter_def_readonly[[U]]$Bracket[[>]]
   class $Class_def[[GP]] {};
-  template<$Class[[G]] &$TemplateParameter_def_readonly[[U]]>
+  template$Bracket[[<]]$Class[[G]] &$TemplateParameter_def_readonly[[U]]$Bracket[[>]]
   class $Class_def[[GR]] {};
-  template
+  template$Bracket[[<]]int *$TemplateParameter_def_readonly[[U]]$