[PATCH] D24400: Also cleanup comments around redundant colons/commas in format::cleanup.

[Eric Liu via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: djasper.
ioeric added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D24400

Files:
  lib/Format/Format.cpp
  unittests/Format/CleanupTest.cpp

Index: unittests/Format/CleanupTest.cpp
===================================================================
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -188,39 +188,34 @@
   EXPECT_EQ(Expected, Result);
 }
 
-// FIXME: delete comments too.
-TEST_F(CleanupTest, CtorInitializationCommentAroundCommas) {
-  // Remove redundant commas around comment.
-  std::string Code = "class A {\nA() : x({1}), /* comment */, {} };";
-  std::string Expected = "class A {\nA() : x({1}) /* comment */ {} };";
+TEST_F(CleanupTest, RemoveCommentsAroundDeleteCode) {
+  std::string Code =
+      "class A {\nA() : x({1}), /* comment */, /* comment */ {} };";
+  std::string Expected = "class A {\nA() : x({1}) {} };";
   std::vector<tooling::Range> Ranges;
   Ranges.push_back(tooling::Range(25, 0));
   Ranges.push_back(tooling::Range(40, 0));
   std::string Result = cleanup(Code, Ranges);
   EXPECT_EQ(Expected, Result);
 
-  // Remove trailing comma and ignore comment.
-  Code = "class A {\nA() : x({1}), // comment\n{} };";
-  Expected = "class A {\nA() : x({1}) // comment\n{} };";
+  Code = "class A {\nA() : x({1}), // comment\n {} };";
+  Expected = "class A {\nA() : x({1})\n {} };";
   Ranges = std::vector<tooling::Range>(1, tooling::Range(25, 0));
   Result = cleanup(Code, Ranges);
   EXPECT_EQ(Expected, Result);
 
-  // Remove trailing comma and ignore comment.
   Code = "class A {\nA() : x({1}), // comment\n , y(1),{} };";
-  Expected = "class A {\nA() : x({1}), // comment\n  y(1){} };";
+  Expected = "class A {\nA() : x({1}),  y(1){} };";
   Ranges = std::vector<tooling::Range>(1, tooling::Range(38, 0));
   Result = cleanup(Code, Ranges);
   EXPECT_EQ(Expected, Result);
 
-  // Remove trailing comma and ignore comment.
   Code = "class A {\nA() : x({1}), \n/* comment */, y(1),{} };";
-  Expected = "class A {\nA() : x({1}), \n/* comment */ y(1){} };";
+  Expected = "class A {\nA() : x({1}), \n y(1){} };";
   Ranges = std::vector<tooling::Range>(1, tooling::Range(40, 0));
   Result = cleanup(Code, Ranges);
   EXPECT_EQ(Expected, Result);
 
-  // Remove trailing comma and ignore comment.
   Code = "class A {\nA() : , // comment\n y(1),{} };";
   Expected = "class A {\nA() :  // comment\n y(1){} };";
   Ranges = std::vector<tooling::Range>(1, tooling::Range(17, 0));
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1119,19 +1119,28 @@
   template <typename LeftKind, typename RightKind>
   void cleanupPair(FormatToken *Start, LeftKind LK, RightKind RK,
                    bool DeleteLeft) {
-    auto NextNotDeleted = [this](const FormatToken &Tok) -> FormatToken * {
-      for (auto *Res = Tok.Next; Res; Res = Res->Next)
-        if (!Res->is(tok::comment) &&
-            DeletedTokens.find(Res) == DeletedTokens.end())
+    auto NextNotDeleted = [this](
+        const FormatToken &Tok,
+        llvm::SmallVectorImpl<FormatToken *> &Comments) -> FormatToken * {
+      for (auto *Res = Tok.Next; Res; Res = Res->Next) {
+        if (Res->is(tok::comment))
+          Comments.push_back(Res);
+        else if (DeletedTokens.find(Res) == DeletedTokens.end())
           return Res;
+      }
       return nullptr;
     };
+    llvm::SmallVector<FormatToken *, 1> Comments;
     for (auto *Left = Start; Left;) {
-      auto *Right = NextNotDeleted(*Left);
+      Comments.clear();
+      auto *Right = NextNotDeleted(*Left, Comments);
       if (!Right)
         break;
       if (Left->is(LK) && Right->is(RK)) {
         deleteToken(DeleteLeft ? Left : Right);
+        // Delete all comments between `Left` and `Right`.
+        for (auto *Comment : Comments)
+          deleteToken(Comment);
         // If the right token is deleted, we should keep the left token
         // unchanged and pair it with the new right token.
         if (!DeleteLeft)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D24400.70842.patch
Type: text/x-patch
Size: 3913 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160909/2a58c30f/attachment.bin>