Explorar o código

[clang-format] Fix cleanup of `AnnotatedLine` to include children nodes.

Summary:
AnnotatedLine has a tree structure, and things like the body of a lambda will be
a child of the lambda expression. For example,

    [&]() { foo(a); };

will have an AnnotatedLine with a child:

    [&]() {};
     '- foo(a);

Currently, when the `Cleaner` class analyzes the affected lines, it does not
cleanup the lines' children nodes, which results in missed cleanup
opportunities, like the lambda body in the example above.

This revision extends the algorithm to visit children, thereby fixing the above problem.

Patch by Eric Li.

Reviewers: krasimir

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D67659

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@372129 91177308-0d34-0410-b5e6-96231b3b80d8
Yitzhak Mandelbaum %!s(int64=6) %!d(string=hai) anos
pai
achega
e71234de99
Modificáronse 2 ficheiros con 25 adicións e 13 borrados
  1. 18 11
      lib/Format/Format.cpp
  2. 7 2
      unittests/Format/CleanupTest.cpp

+ 18 - 11
lib/Format/Format.cpp

@@ -1417,22 +1417,29 @@ public:
 
 
     checkEmptyNamespace(AnnotatedLines);
     checkEmptyNamespace(AnnotatedLines);
 
 
-    for (auto &Line : AnnotatedLines) {
-      if (Line->Affected) {
-        cleanupRight(Line->First, tok::comma, tok::comma);
-        cleanupRight(Line->First, TT_CtorInitializerColon, tok::comma);
-        cleanupRight(Line->First, tok::l_paren, tok::comma);
-        cleanupLeft(Line->First, tok::comma, tok::r_paren);
-        cleanupLeft(Line->First, TT_CtorInitializerComma, tok::l_brace);
-        cleanupLeft(Line->First, TT_CtorInitializerColon, tok::l_brace);
-        cleanupLeft(Line->First, TT_CtorInitializerColon, tok::equal);
-      }
-    }
+    for (auto *Line : AnnotatedLines)
+      cleanupLine(Line);
 
 
     return {generateFixes(), 0};
     return {generateFixes(), 0};
   }
   }
 
 
 private:
 private:
+  void cleanupLine(AnnotatedLine *Line) {
+    for (auto *Child : Line->Children) {
+      cleanupLine(Child);
+    }
+
+    if (Line->Affected) {
+      cleanupRight(Line->First, tok::comma, tok::comma);
+      cleanupRight(Line->First, TT_CtorInitializerColon, tok::comma);
+      cleanupRight(Line->First, tok::l_paren, tok::comma);
+      cleanupLeft(Line->First, tok::comma, tok::r_paren);
+      cleanupLeft(Line->First, TT_CtorInitializerComma, tok::l_brace);
+      cleanupLeft(Line->First, TT_CtorInitializerColon, tok::l_brace);
+      cleanupLeft(Line->First, TT_CtorInitializerColon, tok::equal);
+    }
+  }
+
   bool containsOnlyComments(const AnnotatedLine &Line) {
   bool containsOnlyComments(const AnnotatedLine &Line) {
     for (FormatToken *Tok = Line.First; Tok != nullptr; Tok = Tok->Next) {
     for (FormatToken *Tok = Line.First; Tok != nullptr; Tok = Tok->Next) {
       if (Tok->isNot(tok::comment))
       if (Tok->isNot(tok::comment))

+ 7 - 2
unittests/Format/CleanupTest.cpp

@@ -183,10 +183,15 @@ TEST_F(CleanupTest, TrailingCommaInParens) {
   std::string Code = "int main() { f(,1,,2,3,f(1,2,),4,,);}";
   std::string Code = "int main() { f(,1,,2,3,f(1,2,),4,,);}";
   std::string Expected = "int main() { f(1,2,3,f(1,2),4);}";
   std::string Expected = "int main() { f(1,2,3,f(1,2),4);}";
   EXPECT_EQ(Expected, cleanupAroundOffsets({15, 18, 29, 33}, Code));
   EXPECT_EQ(Expected, cleanupAroundOffsets({15, 18, 29, 33}, Code));
+
+  // Lambda contents are also checked for trailing commas.
+  Code = "int main() { [](){f(,1,,2,3,f(1,2,),4,,);}();}";
+  Expected = "int main() { [](){f(1,2,3,f(1,2),4);}();}";
+  EXPECT_EQ(Expected, cleanupAroundOffsets({20, 23, 34, 38}, Code));
 }
 }
 
 
 TEST_F(CleanupTest, TrailingCommaInBraces) {
 TEST_F(CleanupTest, TrailingCommaInBraces) {
-  // Trainling comma is allowed in brace list.
+  // Trailing comma is allowed in brace list.
   // If there was trailing comma in the original code, then trailing comma is
   // If there was trailing comma in the original code, then trailing comma is
   // preserved. In this example, element between the last two commas is deleted
   // preserved. In this example, element between the last two commas is deleted
   // causing the second-last comma to be redundant.
   // causing the second-last comma to be redundant.
@@ -194,7 +199,7 @@ TEST_F(CleanupTest, TrailingCommaInBraces) {
   std::string Expected = "void f() { std::vector<int> v = {1,2,3,}; }";
   std::string Expected = "void f() { std::vector<int> v = {1,2,3,}; }";
   EXPECT_EQ(Expected, cleanupAroundOffsets({39}, Code));
   EXPECT_EQ(Expected, cleanupAroundOffsets({39}, Code));
 
 
-  // If there was no trailing comma in the original code, then trainling comma
+  // If there was no trailing comma in the original code, then trailing comma
   // introduced by replacements should be cleaned up. In this example, the
   // introduced by replacements should be cleaned up. In this example, the
   // element after the last comma is deleted causing the last comma to be
   // element after the last comma is deleted causing the last comma to be
   // redundant.
   // redundant.