[PATCH] D117472: [clangd] Bring back early-claim approach to fix a selection-tree regression.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

The early-claim hack was removed in 96f5cc1ee417f863f85756d1e56b1bed1bd76a7e <https://reviews.llvm.org/rG96f5cc1ee417f863f85756d1e56b1bed1bd76a7e>,
we see a regression about captured var-decl in lambda, see https://github.com/clangd/clangd/issues/990.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117472

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


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -509,6 +509,13 @@
         enum Bar : [[Fo^o]];
       )cpp",
        "TypedefTypeLoc"},
+
+      // lambda captured var-decl
+      {R"cpp(
+        void test(int bar) {
+          auto l = [^[[foo = bar]]] { };
+        })cpp",
+       "VarDecl"},
   };
 
   for (const Case &C : Cases) {
Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -716,13 +716,16 @@
   }
 
   // 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().
@@ -744,6 +747,25 @@
     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<Decl>()) {
+      // We want the name in the var-decl to be claimed by the decl itself and
+      // not by any children. Ususally, we don't need this, because source
+      // ranges of children are not overlapped with their parent's.
+      // But there are some AST-weird cases, e.g.
+      //    auto fun = [bar = foo]() { ... }
+      //                ~~~~~~~~~   VarDecl
+      //                ~~~         |- AutoTypeLoc
+      if (const auto *DD = llvm::dyn_cast<VarDecl>(D))
+        return DD->getLocation();
+    }
+
+    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).


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117472.400505.patch
Type: text/x-patch
Size: 2489 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220117/c3ce148a/attachment.bin>