[PATCH] D89307: [clangd] Disable extract variable for RHS of assignments

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: adamcz.
Herald added subscribers: cfe-commits, usaxena95, arphaman.
Herald added a project: clang.
kadircet requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89307

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -215,26 +215,26 @@
         int x = [[1]], y = [[a + 1]], a = [[1]], z = a + 1;
       // if without else
       if([[1]])
-        a = [[1]];
+        a = [[1]] + 1;
       // if with else
       if(a < [[3]])
         if(a == [[4]])
-          a = [[5]];
+          a = [[5]] + 1;
         else
-          a = [[5]];
+          a = [[5]] + 1;
       else if (a < [[4]])
-        a = [[4]];
+        a = [[4]] + 1;
       else
-        a = [[5]];
+        a = [[5]] + 1;
       // for loop
-      for(a = [[1]]; a > [[[[3]] + [[4]]]]; a++)
-        a = [[2]];
+      for(a = [[1]] + 1; a > [[[[3]] + [[4]]]]; a++)
+        a = [[2]] + 1;
       // while
       while(a < [[1]])
-        a = [[1]];
+        a = [[1]] + 1;
       // do while
       do
-        a = [[1]];
+        a = [[1]] + 1;
       while(a < [[3]]);
     }
   )cpp";
@@ -291,6 +291,7 @@
       xyz([[a *= 5]]);
       // Variable DeclRefExpr
       a = [[b]];
+      a = [[xyz()]];
       // statement expression
       [[xyz()]];
       while (a)
@@ -373,10 +374,10 @@
                  })cpp"},
           // attribute testing
           {R"cpp(void f(int a) {
-                    [ [gsl::suppress("type")] ] for (;;) a = [[1]];
+                    [ [gsl::suppress("type")] ] for (;;) a = [[1]] + 1;
                  })cpp",
            R"cpp(void f(int a) {
-                    auto dummy = 1; [ [gsl::suppress("type")] ] for (;;) a = dummy;
+                    auto dummy = 1; [ [gsl::suppress("type")] ] for (;;) a = dummy + 1;
                  })cpp"},
           // MemberExpr
           {R"cpp(class T {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -382,17 +382,27 @@
   if (BinOp.parse(*N) && BinaryOperator::isAssignmentOp(BinOp.Kind))
     return false;
 
+  const SelectionTree::Node &OuterImplicit = N->outerImplicit();
+  const auto *Parent = OuterImplicit.Parent;
+  if (!Parent)
+    return false;
   // We don't want to extract expressions used as statements, that would leave
   // a `dummy;` around that has no effect.
   // Unfortunately because the AST doesn't have ExprStmt, we have to check in
   // this roundabout way.
-  const SelectionTree::Node &OuterImplicit = N->outerImplicit();
-  if (!OuterImplicit.Parent ||
-      childExprIsStmt(OuterImplicit.Parent->ASTNode.get<Stmt>(),
+  if (childExprIsStmt(Parent->ASTNode.get<Stmt>(),
                       OuterImplicit.ASTNode.get<Expr>()))
     return false;
 
-  // FIXME: ban extracting the RHS of an assignment: `a = [[foo()]]`
+  // Disable extraction of full RHS on assignment operations, e.g:
+  // auto x = [[RHS_EXPR]];
+  // This would just result in duplicating the code.
+  if (const auto *BO = Parent->ASTNode.get<BinaryOperator>()) {
+    if (BO->isAssignmentOp() &&
+        BO->getRHS() == OuterImplicit.ASTNode.get<Expr>())
+      return false;
+  }
+
   return true;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89307.297818.patch
Type: text/x-patch
Size: 3306 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201013/cdff1553/attachment.bin>