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:
> kadircet wrote:
> > 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).
> That's what I meant by `isImplicit(ME.getBase())`
> 
> The following passes tests (added to isImplicit)...
> ```
> if (auto *ME = llvm::dyn_cast<MemberExpr>(S)) 
>   if (auto *FD = llvm::dyn_cast<FieldDecl>(ME->getMemberDecl()))
>     if (FD->isAnonymousStructOrUnion())
>       return isImplicit(ME->getBase());
> ```
> 
> > and the member is in an anonymous struct
> 
> Oops, I meant "and the member *is* an anonymous struct".
> It seems more natural to recognize and special-case the field-access to the 
> anon struct itself (which is intuitively implicit!), than the one to a normal 
> field within it.
right, it makes sense. i was missing the isImplicit(base) bit + got stunned by 
the beautiful shape of the AST. thanks for bearing with me :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110825

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

Reply via email to