Also cleanup comments around redundant colons/commas in format::cleanup.

[djasper]

lib/Format/Format.cpp

1141 ↗

(On Diff #70842)

Couldn't you just do:

for (auto *Tok = Left->Next; Tok && Tok != Right; Tok = Tok->Next)
  if (Tok->is(tok::comment))
    deleteToken(Comment);

That way you wouldn't have to store the comments in a vector.

[ioeric]

• Addressed review comments.

[djasper]

One remark, otherwise looks good.

lib/Format/Format.cpp

1136 ↗

(On Diff #70863)

Hm. I think this "if" actually doesn't change behavior. If we remove it, we'd just add already deleted tokens to the set again, which shouldn't hurt (and we do it for comments at the moment). I'd just remove this line.

[ioeric]

• Addressed review comments.

lib/Format/Format.cpp

1136 ↗

(On Diff #70863)

Wow, from 20 LOC to 2 LOC! Thanks!

cfe/trunk/lib/Format/Format.cpp

auto NextNotDeleted = [this](const FormatToken &Tok) -> FormatToken * {
       return nullptr;
     };
     for (auto *Left = Start; Left;) {
       auto *Right = NextNotDeleted(*Left);
       if (!Right)
         break;
       if (Left->is(LK) && Right->is(RK)) {
         deleteToken(DeleteLeft ? Left : Right);
+        for (auto *Tok = Left->Next; Tok && Tok != Right; Tok = Tok->Next)
+          deleteToken(Tok);
         // If the right token is deleted, we should keep the left token
         // unchanged and pair it with the new right token.
         if (!DeleteLeft)
           continue;
       }
       Left = Right;
     }
   }

cfe/trunk/unittests/Format/CleanupTest.cpp

TEST_F(CleanupTest, RedundantCommaNotInAffectedRanges) {
   Code = "class A {\nA() : x(1), {} };";
   Expected = "class A {\nA() : x(1), {} };";
   // No range. Fixer should do nothing.
   Ranges.clear();
   Result = cleanup(Code, Ranges);
   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));
   Result = cleanup(Code, Ranges);
   EXPECT_EQ(Expected, Result);
 }
 
 TEST_F(CleanupTest, CtorInitializerInNamespace) {