[llvm-branch-commits] [clang-tools-extra] 357e79c - [clangd] Fix early selection for non-vardecl declarators

[Sam McCall via llvm-branch-commits]

Author: Kadir Cetinkaya
Date: 2020-06-10T13:44:26+02:00
New Revision: 357e79c2895736c9d202c79380e3e1f507080df3

URL: https://github.com/llvm/llvm-project/commit/357e79c2895736c9d202c79380e3e1f507080df3
DIFF: https://github.com/llvm/llvm-project/commit/357e79c2895736c9d202c79380e3e1f507080df3.diff

LOG: [clangd] Fix early selection for non-vardecl declarators

Summary:
Selection tree was performing an early claim only for VarDecls, but
there are other cases where we can have declarators, e.g. FieldDecls. This patch
extends the early claim logic to all types of declarators.

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

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D75106

(cherry picked from commit e6b8181895b96740dbe54aca036aa237e0a8363d)

Modified the cherry-picked test as diagnostics differ on the branch.

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp
index 14c05d1c0b49..d7630f4481f2 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -10,6 +10,7 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
@@ -594,13 +595,23 @@ class SelectionVisitor : public RecursiveASTVisitor<SelectionVisitor> {
   // Usually empty, but sometimes children cover tokens but shouldn't own them.
   SourceRange earlySourceRange(const DynTypedNode &N) {
     if (const Decl *D = N.get<Decl>()) {
+      // We want constructor name to be claimed by TypeLoc not the constructor
+      // itself. Similar for deduction guides, we rather want to select the
+      // underlying TypeLoc.
+      // FIXME: Unfortunately this doesn't work, even though RecursiveASTVisitor
+      // traverses the underlying TypeLoc inside DeclarationName, it is null for
+      // constructors.
+      if (isa<CXXConstructorDecl>(D) || isa<CXXDeductionGuideDecl>(D))
+        return SourceRange();
+      // This will capture Field, Function, MSProperty, NonTypeTemplateParm and
+      // VarDecls. We want the name in the declarator to be claimed by the decl
+      // and not by any children. For example:
       // void [[foo]]();
-      if (auto *FD = llvm::dyn_cast<FunctionDecl>(D))
-        return FD->getNameInfo().getSourceRange();
       // int (*[[s]])();
-      else if (auto *VD = llvm::dyn_cast<VarDecl>(D))
-        return VD->getLocation();
-    } else if (const auto* CCI = N.get<CXXCtorInitializer>()) {
+      // struct X { int [[hash]] [32]; [[operator]] int();}
+      if (const auto *DD = llvm::dyn_cast<DeclaratorDecl>(D))
+        return DD->getLocation();
+    } else if (const auto *CCI = N.get<CXXCtorInitializer>()) {
       // : [[b_]](42)
       return CCI->getMemberLocation();
     }

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 89b529d9b419..eb47b5a541a6 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -339,7 +339,7 @@ class Foo {})cpp";
          HI.Definition = "~X()";
          HI.Parameters.emplace();
        }},
-      {"class X { operator [[in^t]](); };",
+      {"class X { [[op^erator]] int(); };",
        [](HoverInfo &HI) {
          HI.NamespaceScope = "";
          HI.Name = "operator int";
@@ -348,6 +348,13 @@ class Foo {})cpp";
          HI.Definition = "operator int()";
          HI.Parameters.emplace();
        }},
+      {"class X { operator [[^X]]*(); };",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "";
+         HI.Name = "X";
+         HI.Kind = index::SymbolKind::Class;
+         HI.Definition = "class X {}";
+       }},
 
       // auto on lambda
       {R"cpp(

diff  --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 581309b1dccc..0cd9ca09401e 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -329,6 +329,12 @@ TEST(SelectionTest, CommonAncestor) {
         decltype([[^a]] + a) b;
         )cpp",
           "DeclRefExpr"},
+
+      {"struct foo { [[int has^h<:32:>]]; };", "FieldDecl"},
+      {"struct foo { [[op^erator int()]]; };", "CXXConversionDecl"},
+      {"struct foo { [[^~foo()]]; };", "CXXDestructorDecl"},
+      // FIXME: The following to should be class itself instead.
+      {"struct foo { [[fo^o(){}]] };", "CXXConstructorDecl"},
   };
   for (const Case &C : Cases) {
     Annotations Test(C.Code);