[PATCH] D72594: [clangd] Include expression in DecltypeTypeLoc sourcerange while building SelectionTree

2020-01-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5465e74ef4c: [clangd] Include expression in DecltypeTypeLoc 
sourcerange while building… (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D72594?vs=237610=237743#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72594

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -323,6 +323,12 @@
 Foo x = [[^12_ud]];
   )cpp",
   "UserDefinedLiteral"},
+  {
+  R"cpp(
+int a;
+decltype([[^a]] + a) b;
+)cpp",
+  "DeclRefExpr"},
   };
   for (const Case  : Cases) {
 Annotations Test(C.Code);
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -527,6 +527,19 @@
   // don't intersect the selection may be recursively skipped.
   bool canSafelySkipNode(const DynTypedNode ) {
 SourceRange S = N.getSourceRange();
+if (auto *TL = N.get()) {
+  // DeclTypeTypeLoc::getSourceRange() is incomplete, which would lead to
+  // failing
+  // to descend into the child expression.
+  // decltype(2+2);
+  // ~ <-- correct range
+  //   <-- range reported by getSourceRange()
+  //   <-- range with this hack(i.e, missing closing paren)
+  // FIXME: Alter DecltypeTypeLoc to contain parentheses locations and get
+  // rid of this patch.
+  if (auto DT = TL->getAs())
+S.setEnd(DT.getUnderlyingExpr()->getEndLoc());
+}
 if (!SelChecker.mayHit(S)) {
   dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
   dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -323,6 +323,12 @@
 Foo x = [[^12_ud]];
   )cpp",
   "UserDefinedLiteral"},
+  {
+  R"cpp(
+int a;
+decltype([[^a]] + a) b;
+)cpp",
+  "DeclRefExpr"},
   };
   for (const Case  : Cases) {
 Annotations Test(C.Code);
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -527,6 +527,19 @@
   // don't intersect the selection may be recursively skipped.
   bool canSafelySkipNode(const DynTypedNode ) {
 SourceRange S = N.getSourceRange();
+if (auto *TL = N.get()) {
+  // DeclTypeTypeLoc::getSourceRange() is incomplete, which would lead to
+  // failing
+  // to descend into the child expression.
+  // decltype(2+2);
+  // ~ <-- correct range
+  //   <-- range reported by getSourceRange()
+  //   <-- range with this hack(i.e, missing closing paren)
+  // FIXME: Alter DecltypeTypeLoc to contain parentheses locations and get
+  // rid of this patch.
+  if (auto DT = TL->getAs())
+S.setEnd(DT.getUnderlyingExpr()->getEndLoc());
+}
 if (!SelChecker.mayHit(S)) {
   dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
   dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72594: [clangd] Include expression in DecltypeTypeLoc sourcerange while building SelectionTree

2020-01-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet marked 3 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Selection.cpp:530
 SourceRange S = N.getSourceRange();
+if (auto *TL = N.get()) {
+  // DecltypeTypeLoc only contains the SourceRange for `decltype` keyword.

sammccall wrote:
> Alternately, we could just always return false if it's a DecltypeTypeLoc.
i would rather keep it that way to not regress performance, even if it should 
be negligible most of the time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72594



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

[PATCH] D72594: [clangd] Include expression in DecltypeTypeLoc sourcerange while building SelectionTree

2020-01-13 Thread Sam McCall via Phabricator via cfe-commits 
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks!




Comment at: clang-tools-extra/clangd/Selection.cpp:530
 SourceRange S = N.getSourceRange();
+if (auto *TL = N.get()) {
+  // DecltypeTypeLoc only contains the SourceRange for `decltype` keyword.

Alternately, we could just always return false if it's a DecltypeTypeLoc.



Comment at: clang-tools-extra/clangd/Selection.cpp:531
+if (auto *TL = N.get()) {
+  // DecltypeTypeLoc only contains the SourceRange for `decltype` keyword.
+  // We are extending the SourceRange up until the end of the expression

I think this comment could be a little clearer on the three ranges, and that 
the AST one is just incorrect.

e.g.
```
// DeclTypeTypeLoc::getSourceRange() is incomplete, which would lead to failing
// to descend into the child expression.
// decltype(2+2);
// X <-- correct range
//   <-- range reported by getSourceRange()
//   <-- range with this hack
```



Comment at: clang-tools-extra/clangd/Selection.cpp:534
+  // inside decltype. Note that this will not include the closing
+  // parenthese.
+  // FIXME: Alter DecltypeTypeLoc to contain parentheses locations and get

nit: parenthesis


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72594



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

[PATCH] D72594: [clangd] Include expression in DecltypeTypeLoc sourcerange while building SelectionTree

2020-01-13 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61742 tests passed, 0 failed 
and 780 were skipped.

{icon question-circle color=gray} clang-tidy: unknown.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72594



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

[PATCH] D72594: [clangd] Include expression in DecltypeTypeLoc sourcerange while building SelectionTree

2020-01-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Currently AST only contains the location for `decltype` keyword,
therefore we were skipping expressions inside decltype while building selection
tree.

This patch extends source range in such cases to contain the expression as well.
A proper fix would require changes to Sema and DecltypeTypeLoc to contain these
location information.

Fixes https://github.com/clangd/clangd/issues/250.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72594

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -323,6 +323,12 @@
 Foo x = [[^12_ud]];
   )cpp",
   "UserDefinedLiteral"},
+  {
+  R"cpp(
+int a;
+decltype([[^a]] + a) b;
+)cpp",
+  "DeclRefExpr"},
   };
   for (const Case  : Cases) {
 Annotations Test(C.Code);
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -527,6 +527,16 @@
   // don't intersect the selection may be recursively skipped.
   bool canSafelySkipNode(const DynTypedNode ) {
 SourceRange S = N.getSourceRange();
+if (auto *TL = N.get()) {
+  // DecltypeTypeLoc only contains the SourceRange for `decltype` keyword.
+  // We are extending the SourceRange up until the end of the expression
+  // inside decltype. Note that this will not include the closing
+  // parenthese.
+  // FIXME: Alter DecltypeTypeLoc to contain parentheses locations and get
+  // rid of this patch.
+  if (auto DT = TL->getAs())
+S.setEnd(DT.getUnderlyingExpr()->getEndLoc());
+}
 if (!SelChecker.mayHit(S)) {
   dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
   dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -323,6 +323,12 @@
 Foo x = [[^12_ud]];
   )cpp",
   "UserDefinedLiteral"},
+  {
+  R"cpp(
+int a;
+decltype([[^a]] + a) b;
+)cpp",
+  "DeclRefExpr"},
   };
   for (const Case  : Cases) {
 Annotations Test(C.Code);
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -527,6 +527,16 @@
   // don't intersect the selection may be recursively skipped.
   bool canSafelySkipNode(const DynTypedNode ) {
 SourceRange S = N.getSourceRange();
+if (auto *TL = N.get()) {
+  // DecltypeTypeLoc only contains the SourceRange for `decltype` keyword.
+  // We are extending the SourceRange up until the end of the expression
+  // inside decltype. Note that this will not include the closing
+  // parenthese.
+  // FIXME: Alter DecltypeTypeLoc to contain parentheses locations and get
+  // rid of this patch.
+  if (auto DT = TL->getAs())
+S.setEnd(DT.getUnderlyingExpr()->getEndLoc());
+}
 if (!SelChecker.mayHit(S)) {
   dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
   dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits