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

[Kadir Cetinkaya via Phabricator via cfe-commits]

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 nesting by early returns and add braces to outer if statements if there are any nested statements left.


================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:299
       // return statement
       return ^1;
     }
----------------
left out this one ?


================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:302
     void f() {
-      int a = 5 + [[4 ^* ^xyz^()]];
+      int a = 5 + [[4 * [[xyz()]]]];
       // multivariable initialization
----------------
how come these changes part of that patch?

is it possible that this tweak was changed to not trigger on empty selections, but tests were not updated? If that's the case could you please send these on a different patch and rebase this patch on that one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64717