From ad9fd320091d44d4b8782c28b72a7be21a2bd68d Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Fri, 15 Nov 2019 16:07:34 +0100 Subject: [PATCH] [clangd] Fix SelectionTree behavior on constructor init-lists. Summary: For the constructor Foo() : classmember(arg) {} The AST looks like: - CXXCtorInitializer classmember(arg) - CXXConstructExpr classmember(arg) - DeclRefExpr: arg We want the 'classmember' to be associated with the CXXCtorInitializer, not the CXXConstructExpr. (CXXConstructExpr is known to have bad ranges). So just early-claim it. Thanks @hokein for tracking down/reducing the bug. Reviewers: hokein Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits, hokein Tags: #clang Differential Revision: https://reviews.llvm.org/D70312 --- clang-tools-extra/clangd/Selection.cpp | 4 ++++ clang-tools-extra/clangd/unittests/SelectionTests.cpp | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp index 1424825..04076df 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/DeclCXX.h" #include "clang/AST/Expr.h" #include "clang/AST/PrettyPrinter.h" #include "clang/AST/RecursiveASTVisitor.h" @@ -397,6 +398,9 @@ private: // int (*[[s]])(); else if (auto *VD = llvm::dyn_cast(D)) return VD->getLocation(); + } else if (const auto* CCI = N.get()) { + // : [[b_]](42) + return CCI->getMemberLocation(); } return SourceRange(); } diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp index 8895dd8..7b8b33d 100644 --- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp +++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp @@ -246,6 +246,17 @@ TEST(SelectionTest, CommonAncestor) { // Tricky case: two VarDecls share a specifier. {"[[int ^a]], b;", "VarDecl"}, {"[[int a, ^b]];", "VarDecl"}, + // Tricky case: CXXConstructExpr wants to claim the whole init range. + { + R"cpp( + class X { X(int); }; + class Y { + X x; + Y() : [[^x(4)]] {} + }; + )cpp", + "CXXCtorInitializer", // Not the CXXConstructExpr! + }, // Tricky case: anonymous struct is a sibling of the VarDecl. {"[[st^ruct {int x;}]] y;", "CXXRecordDecl"}, {"[[struct {int x;} ^y]];", "VarDecl"}, -- 2.7.4