[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree

Thu Sep 30 07:14:14 PDT 2021

kadircet added inline comments.

================
Comment at: clang-tools-extra/clangd/Selection.cpp:443
   // It would be nice if RAV handled this (!shouldTraverseImplicitCode()).
   if (auto *CTI = llvm::dyn_cast<CXXThisExpr>(S))
     if (CTI->isImplicit())
----------------
sammccall wrote:
> seems like it'd be more consistent to handle it here?
> 
> If it's a MemberExpr and the member is in an anonymous struct, then it's implicit if `isImplicit(ME.getBase())`.
the test case was actually to demonstrate why we can't do it here. we want to still keep traversing the AST after hitting a field inside an anon struct. handling here terminates the traversal for that subtree completely, and the real node we are interested might be down the tree (`y.[[x]]` in the test case).

================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:368

+TEST(LocateSymbol, AnonymousStructFields) {
+  auto Code = Annotations(R"cpp(
----------------
sammccall wrote:
> This example is quite complicated to the point where it's hard to see what it's testing... The example you have in the comment is much clearer.
> 
> If we also want to test named fields with inline struct types, maybe that should be a second test...
it is actually important to demonstrate we are not claiming the whole range at the first encounter (and to make sure traversal is not stopped for any other reason).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110825