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

[Haojian Wu via Phabricator via cfe-commits]

hokein marked an inline comment as done.
hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/Selection.cpp:758
+      // ranges of children are not overlapped with their parent's.
+      // But there are some AST-weird cases, e.g.
+      //    auto fun = [bar = foo]() { ... }
----------------
sammccall wrote:
> I don't think this is an e.g., but rather is suitable for specifically this case.
> 
> If there are other cases, we likely should be testing DeclaratorDecl here, and excluding Constructor/Destructor as before.
refined the comment.

Testing DeclaratorDecl is safer, but we miss opportunities to know other cases (hopefully, there should be no other cases). For now, I'm leaning towards merely handling this specific case. If it turns out more cases, we can definitely use the DeclaratorDecl.


================
Comment at: clang-tools-extra/clangd/Selection.cpp:773
   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 {}.
----------------
sammccall wrote:
> I wonder whether we want to keep the CXXConstructExpr hack now, up to you.
if we remove it, than we need to add the `CXXCtorInitializer` case back to the earlySourceRange :(


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117472/new/

https://reviews.llvm.org/D117472