dgoldman added inline comments.
================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:418 + if (const auto *ME = dyn_cast<MemberExpr>(E)) + if (const auto *TE = dyn_cast<CXXThisExpr>(ME->getBase())) + if (TE->isImplicit()) ---------------- sammccall wrote: > dgoldman wrote: > > sammccall wrote: > > > dgoldman wrote: > > > > sammccall wrote: > > > > > oops, I forgot one detail: we want ME->getBase()->IgnoreImpCasts() > > > > > > > > > > (in `void nonConstMethod() { constMethod(); }` there's an > > > > > ImplicitCastExpr in there... > > > > Hmm, is this right, I tested out that example and it seems like that's > > > > actually a `CXXMemberCallExpr` which this doesn't handle? > > > The callee of the CXXMemberCallExpr is the MemberExpr i'm talking about > > > here. > > Right but this code here doesn't handle CXXMemberCallExpr, only MemberExpr > > - so we don't even see the callee here. If I add the following above: > > > > ``` > > if (const auto *MCE = dyn_cast<CXXMemberCallExpr>(E)) > > E = MCE->getCallee(); > > ``` > > > > and do `ME->getBase()->IgnoreImpCasts()` then: > > > > > > ``` > > class Test { > > int constMethod() const { return 1; } > > int nonConstMethod() { return [[constMethod()]]; } > > }; > > ``` > > > > will be unavailable. > > Right but this code here doesn't handle CXXMemberCallExpr, only MemberExpr > > - so we don't even see the callee here. > > I don't understand what you mean. > > If you select ` int nonConstMethod() { return [[constMethod]](); }`, then > isn't N such a MemberExpr? Nope, still seems to be `CXXMemberCallExpr`. I'll go ahead and land this with `ME->getBase()->IgnoreImpCasts()` check although the example isn't working as expected. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124486/new/ https://reviews.llvm.org/D124486 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits