Unverified Commit e6b81818 authored by Kadir Cetinkaya's avatar Kadir Cetinkaya
Browse files

[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
parent 497c110e

clang-tools-extra/clangd/Selection.cpp

+15 −4
Original line number Diff line number Diff line
@@ -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"
@@ -634,12 +635,22 @@ private: 
  // 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();

      // 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();

clang-tools-extra/clangd/unittests/HoverTests.cpp

+8 −1
Original line number Diff line number Diff line
@@ -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(

clang-tools-extra/clangd/unittests/SelectionTests.cpp

+5 −0
Original line number Diff line number Diff line
@@ -375,6 +375,11 @@ TEST(SelectionTest, CommonAncestor) { 
            int test(I *f) { return 42 + [[f.^foo]]; }

          )cpp",

          "ObjCPropertyRefExpr"},

      {"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);