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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to