diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp --- a/clang-tools-extra/clangd/Selection.cpp +++ b/clang-tools-extra/clangd/Selection.cpp @@ -639,11 +639,10 @@ // Nodes *usually* nest nicely: a child's getSourceRange() lies within the // parent's, and a node (transitively) owns all tokens in its range. // - // Exception 1: child range claims tokens that should be owned by the parent. - // e.g. in `void foo(int);`, the FunctionTypeLoc should own - // `void (int)` but the parent FunctionDecl should own `foo`. - // To handle this case, certain nodes claim small token ranges *before* - // their children are traversed. (see earlySourceRange). + // Exception 1: when declarators nest, *inner* declarator is the *outer* type. + // e.g. void foo[5](int) is an array of functions. + // To handle this case, declarators are careful to only claim the tokens they + // own, rather than claim a range and rely on claim ordering. // // Exception 2: siblings both claim the same node. // e.g. `int x, y;` produces two sibling VarDecls. @@ -690,16 +689,13 @@ } // Pushes a node onto the ancestor stack. Pairs with pop(). - // Performs early hit detection for some nodes (on the earlySourceRange). void push(DynTypedNode Node) { - SourceRange Early = earlySourceRange(Node); dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy), indent()); Nodes.emplace_back(); Nodes.back().ASTNode = std::move(Node); Nodes.back().Parent = Stack.top(); Nodes.back().Selected = NoTokens; Stack.push(&Nodes.back()); - claimRange(Early, Nodes.back().Selected); } // Pops a node off the ancestor stack, and finalizes it. Pairs with push(). @@ -721,52 +717,46 @@ Stack.pop(); } - // Returns the range of tokens that this node will claim directly, and - // is not available to the node's children. - // Usually empty, but sometimes children cover tokens but shouldn't own them. - SourceRange earlySourceRange(const DynTypedNode &N) { - if (const Decl *D = N.get()) { - // 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(D) || isa(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]](); - // int (*[[s]])(); - // struct X { int [[hash]] [32]; [[operator]] int();} - if (const auto *DD = llvm::dyn_cast(D)) - return DD->getLocation(); - } else if (const auto *CCI = N.get()) { - // : [[b_]](42) - return CCI->getMemberLocation(); - } - return SourceRange(); - } - // Claim tokens for N, after processing its children. // By default this claims all unclaimed tokens in getSourceRange(). // We override this if we want to claim fewer tokens (e.g. there are gaps). void claimTokensFor(const DynTypedNode &N, SelectionTree::Selection &Result) { + // CXXConstructExpr often shows implicit construction, like `string s;`. + // Don't associate any tokens with it unless there's some syntax like {}. + // This prevents it from claiming 's', its primary location. + if (const auto *CCE = N.get()) { + claimRange(CCE->getParenOrBraceRange(), Result); + return; + } + // ExprWithCleanups is always implicit. It often wraps CXXConstructExpr. + // Prevent it claiming 's' in the case above. + if (N.get()) + return; + + // Declarators nest "inside out", with parent types inside child ones. + // Instead of claiming the whole range (clobbering parent tokens), carefully + // claim the tokens owned by this node and non-declarator children. + // (We could manipulate traversal order instead, but this is easier). if (const auto *TL = N.get()) { - // e.g. EltType Foo[OuterSize][InnerSize]; - // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ArrayTypeLoc (Outer) - // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |-ArrayTypeLoc (Inner) - // ~~~~~~~ | |-RecordType - // ~~~~~~~~~ | `-Expr (InnerSize) - // ~~~~~~~~~ `-Expr (OuterSize) - // Inner ATL must not claim its whole SourceRange, or it clobbers Outer. - if (TL->getAs()) { - claimRange(TL->getLocalSourceRange(), Result); + if (auto PTL = TL->getAs()) { + claimRange(PTL.getLParenLoc(), Result); + claimRange(PTL.getRParenLoc(), Result); + return; + } + if (auto ATL = TL->getAs()) { + claimRange(ATL.getBracketsRange(), Result); + return; + } + if (auto PTL = TL->getAs()) { + claimRange(PTL.getStarLoc(), Result); + return; + } + if (auto FTL = TL->getAs()) { + claimRange( + SourceRange(FTL.getParensRange().getBegin(), FTL.getEndLoc()), + Result); return; } - // FIXME: maybe LocalSourceRange is a better default for TypeLocs. - // It doesn't seem to be usable for FunctionTypeLocs. } claimRange(getSourceRange(N), Result); } @@ -774,7 +764,7 @@ // Perform hit-testing of a complete Node against the selection. // This runs for every node in the AST, and must be fast in common cases. // This is usually called from pop(), so we can take children into account. - // The existing state of Result is relevant (early/late claims can interact). + // The existing state of Result is relevant. void claimRange(SourceRange S, SelectionTree::Selection &Result) { for (const auto &ClaimedRange : UnclaimedExpandedTokens.erase(TokenBuf.expandedTokens(S))) diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp --- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp +++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp @@ -204,9 +204,9 @@ { R"cpp( struct S { S(const char*); }; - S [[s ^= "foo"]]; + [[S s ^= "foo"]]; )cpp", - "CXXConstructExpr", + "VarDecl", }, { R"cpp( @@ -231,7 +231,7 @@ R"cpp( [[void (^*S)(int)]] = nullptr; )cpp", - "FunctionProtoTypeLoc", + "PointerTypeLoc", }, { R"cpp( @@ -243,7 +243,7 @@ R"cpp( [[void ^(*S)(int)]] = nullptr; )cpp", - "FunctionProtoTypeLoc", + "ParenTypeLoc", }, { R"cpp( @@ -326,6 +326,8 @@ {"const int x = 1, y = 2; [[i^nt]] array[x][10][y];", "BuiltinTypeLoc"}, {"void func(int x) { int v_array[^[[x]]][10]; }", "DeclRefExpr"}, + {"int (*getFunc([[do^uble]]))(int);", "BuiltinTypeLoc"}, + // FIXME: the AST has no location info for qualifiers. {"const [[a^uto]] x = 42;", "AutoTypeLoc"}, {"[[co^nst auto x = 42]];", "VarDecl"},