Skip to content

Commit

Permalink
[clangd] Fix early selection for non-vardecl declarators
Browse files Browse the repository at this point in the history
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 clangd/clangd#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 e6b8181)

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

Fixes clangd/clangd#421
  • Loading branch information
kadircet authored and sam-mccall committed Jun 10, 2020
1 parent 52f2d6d commit 357e79c
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
21 changes: 16 additions & 5 deletions clang-tools-extra/clangd/Selection.cpp
Expand Up @@ -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"
Expand Down Expand Up @@ -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();
}
Expand Down
9 changes: 8 additions & 1 deletion clang-tools-extra/clangd/unittests/HoverTests.cpp
Expand Up @@ -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";
Expand All @@ -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(
Expand Down
6 changes: 6 additions & 0 deletions clang-tools-extra/clangd/unittests/SelectionTests.cpp
Expand Up @@ -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);
Expand Down

0 comments on commit 357e79c

Please sign in to comment.