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<ExtractionContext>(N, SM, Ctx); - return Target->isExtractable(); + computeExtractionContext(N, SM, Ctx); + return Target && Target->InsertionPoint; ---------------- SureYeaah wrote: > kadircet wrote: > > maybe instead of checking internals like `Target` just make > > `computeExtractionContext` return an `llvm::Error` and check for success? > Should it instead return a bool since there's actually no error? Yes that's also plausible ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:292 + Target = llvm::make_unique<ExtractionContext>(TargetNode, SM, Ctx); + return true; +} ---------------- `return Target->InsertionPoint`? ================ Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:304 // return statement - return ^1; + return [[[[t.bar]](t.z)]]; } ---------------- another case on bar ? ``` [[[[t.b[[a]]r]](t.z)]] ``` ================ Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:349 + } t; + return t.bar([[t.z]]); } ---------------- also on t: ``` return [[t]].bar([[[[t]].z]]); ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://reviews.llvm.org/D64717 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits