[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

This revision was automatically updated to reflect the committed changes. Closed by commit rGb0ed5bea8812: [Clangd] Fixed ExtractVariable for certain types of Exprs (authored by SureYeaah). Changed prior to commit: https://reviews.llvm.org/D64717?vs=210803&id=211423#toc Repository: rG LLVM G

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:427 R"cpp(void f(int a) { auto dummy = 1; label: [ [gsl::suppress("type")] ] for (;;) a = dummy; })cpp"}, kadircet w

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah updated this revision to Diff 210803. SureYeaah marked an inline comment as done. SureYeaah added a comment. Added test for label Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://reviews.llvm.org/D64717 Files: clang-too

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Thanks, LGTM from my side. Do you have any concerns @sammccall ? Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:427 R"cpp(void f(int a) {

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah updated this revision to Diff 210800. SureYeaah marked 5 inline comments as done. SureYeaah added a comment. Minor changes and disabled extraction from label statement Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://revie

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:208 const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor(); - if (!N) -return false; - Target = llvm::make_unique(N, SM, Ctx); - return Target->isExtrac

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah updated this revision to Diff 210590. SureYeaah added a comment. Merged with trigger on non empty selection only Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://reviews.llvm.org/D64717 Files: clang-tools-extra/clangd/r

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah updated this revision to Diff 210560. SureYeaah marked 4 inline comments as done. SureYeaah added a comment. Fixed tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://reviews.llvm.org/D64717 Files: clang-tools-extra/

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:292 + Target = llvm::make_unique(TargetNode, SM, Ctx); + return true; +} kadircet wrote: > `return Target->InsertionPoint`? Changed to checking if target i

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:208 const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor(); - if (!N) -return false; - Target = llvm::make_unique(N, SM, Ctx); - return Target->isExtrac

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah updated this revision to Diff 210545. SureYeaah added a comment. Refactored code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://reviews.llvm.org/D64717 Files: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.c

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah marked 5 inline comments as done. SureYeaah added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:208 const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor(); - if (!N) -return false; - Target = llvm::make_u

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:195 Intent intent() const override { return Refactor; } + // Compute the extraction context for the Selection + void computeExtractionContext(const SelectionTree::Node *N

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah updated this revision to Diff 210047. SureYeaah added a comment. Fixed crash Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://reviews.llvm.org/D64717 Files: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah updated this revision to Diff 210045. SureYeaah added a comment. Fixed wrong function name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://reviews.llvm.org/D64717 Files: clang-tools-extra/clangd/refactor/tweaks/Extract

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah updated this revision to Diff 209922. SureYeaah added a comment. Added fix for selecting the callExpr of a MemberExpr/Function DeclRefExpr Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://reviews.llvm.org/D64717 Files:

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

sammccall added a comment. In D64717#1585682 , @SureYeaah wrote: > In D64717#1585632 , @sammccall wrote: > > > In D64717#1585542 , @SureYeaah > > wrote: > > > > > In D64717#

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah added a comment. In D64717#1585632 , @sammccall wrote: > In D64717#1585542 , @SureYeaah wrote: > > > In D64717#1585512 , @sammccall > > wrote: > > > > > Are you su

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

sammccall added a comment. In D64717#1585542 , @SureYeaah wrote: > In D64717#1585512 , @sammccall wrote: > > > Are you sure we want to disable extraction here, rather than just do the > > extraction at a higher lev

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah updated this revision to Diff 209853. SureYeaah marked 2 inline comments as done. SureYeaah added a comment. Added comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://reviews.llvm.org/D64717 Files: clang-tools-ext

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:323 while(a < ^1) -^a++; +[[a++]]; // do while SureYeaah wrote: > kadircet wrote: > > I thought extractor didn't handle this case(missing

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah marked an inline comment as done. SureYeaah added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:323 while(a < ^1) -^a++; +[[a++]]; // do while kadircet wrote: > I thought extractor didn't h

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah added a comment. In D64717#1585512 , @sammccall wrote: > Are you sure we want to disable extraction here, rather than just do the > extraction at a higher level? > > E.g. if `bar(1,2,3, f[[o]]o(4,5));` seems like it should extract the call too

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:299 // return statement return ^1; } SureYeaah wrote: > kadircet wrote: > > left out this one ? > Since that's an IntegerLiteral, this patch doesn't a

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

sammccall added a comment. Are you sure we want to disable extraction here, rather than just do the extraction at a higher level? E.g. if `bar(1,2,3, f[[o]]o(4,5));` seems like it should extract the call too `foo(4,5)`, not fail to trigger entirely. Comment at: clang-tools-e

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah updated this revision to Diff 209844. SureYeaah marked 5 inline comments as done. SureYeaah added a comment. Removed unrelated changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://reviews.llvm.org/D64717 Files: clan

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:299 // return statement return ^1; } kadircet wrote: > left out this one ? Since that's an IntegerLiteral, this patch doesn't apply to it?

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:302 void f() { - int a = 5 + [[4 ^* ^xyz^()]]; + int a = 5 + [[4 * [[xyz(); // multivariable initialization kadircet wrote: > how come these ch

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:83 static bool isExtractableExpr(const clang::Expr *Expr) { if (Expr) { // FIXME: check if we need to cover any other types nit: Could you reduce nes

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah updated this revision to Diff 209809. SureYeaah added a comment. Fixed comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://reviews.llvm.org/D64717 Files: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

SureYeaah created this revision. SureYeaah added reviewers: sammccall, kadircet. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. - Modified ExtractVariable to stop extraction for MemberExprs and Assignment Expr - Fixed unittests