Index: cfe/trunk/lib/Format/Format.cpp =================================================================== --- cfe/trunk/lib/Format/Format.cpp +++ cfe/trunk/lib/Format/Format.cpp @@ -1765,6 +1765,15 @@ checkEmptyNamespace(AnnotatedLines); + for (auto &Line : AnnotatedLines) { + if (Line->Affected) { + cleanupRight(Line->First, tok::comma, tok::comma); + cleanupRight(Line->First, TT_CtorInitializerColon, tok::comma); + cleanupLeft(Line->First, TT_CtorInitializerComma, tok::l_brace); + cleanupLeft(Line->First, TT_CtorInitializerColon, tok::l_brace); + } + } + return generateFixes(); } @@ -1853,6 +1862,45 @@ return true; } + // Checks pairs {start, start->next},..., {end->previous, end} and deletes one + // of the token in the pair if the left token has \p LK token kind and the + // right token has \p RK token kind. If \p DeleteLeft is true, the left token + // is deleted on match; otherwise, the right token is deleted. + template + 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()) + return Res; + 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); + // 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; + } + } + + template + void cleanupLeft(FormatToken *Start, LeftKind LK, RightKind RK) { + cleanupPair(Start, LK, RK, /*DeleteLeft=*/true); + } + + template + void cleanupRight(FormatToken *Start, LeftKind LK, RightKind RK) { + cleanupPair(Start, LK, RK, /*DeleteLeft=*/false); + } + // Delete the given token. inline void deleteToken(FormatToken *Tok) { if (Tok) Index: cfe/trunk/lib/Format/TokenAnnotator.cpp =================================================================== --- cfe/trunk/lib/Format/TokenAnnotator.cpp +++ cfe/trunk/lib/Format/TokenAnnotator.cpp @@ -920,6 +920,10 @@ Contexts.back().IsExpression = false; } else if (Current.is(TT_LambdaArrow) || Current.is(Keywords.kw_assert)) { Contexts.back().IsExpression = Style.Language == FormatStyle::LK_Java; + } else if (Current.Previous && + Current.Previous->is(TT_CtorInitializerColon)) { + Contexts.back().IsExpression = true; + Contexts.back().InCtorInitializer = true; } else if (Current.isOneOf(tok::r_paren, tok::greater, tok::comma)) { for (FormatToken *Previous = Current.Previous; Previous && Previous->isOneOf(tok::star, tok::amp); @@ -927,10 +931,6 @@ Previous->Type = TT_PointerOrReference; if (Line.MustBeDeclaration && !Contexts.front().InCtorInitializer) Contexts.back().IsExpression = false; - } else if (Current.Previous && - Current.Previous->is(TT_CtorInitializerColon)) { - Contexts.back().IsExpression = true; - Contexts.back().InCtorInitializer = true; } else if (Current.is(tok::kw_new)) { Contexts.back().CanBeExpression = false; } else if (Current.isOneOf(tok::semi, tok::exclaim)) { Index: cfe/trunk/unittests/Format/CleanupTest.cpp =================================================================== --- cfe/trunk/unittests/Format/CleanupTest.cpp +++ cfe/trunk/unittests/Format/CleanupTest.cpp @@ -113,6 +113,133 @@ EXPECT_EQ(Expected, Result); } +TEST_F(CleanupTest, CtorInitializationSimpleRedundantComma) { + std::string Code = "class A {\nA() : , {} };"; + std::string Expected = "class A {\nA() {} };"; + std::vector Ranges; + Ranges.push_back(tooling::Range(17, 0)); + Ranges.push_back(tooling::Range(19, 0)); + std::string Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); + + Code = "class A {\nA() : x(1), {} };"; + Expected = "class A {\nA() : x(1) {} };"; + Ranges.clear(); + Ranges.push_back(tooling::Range(23, 0)); + Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); + + Code = "class A {\nA() :,,,,{} };"; + Expected = "class A {\nA() {} };"; + Ranges.clear(); + Ranges.push_back(tooling::Range(15, 0)); + Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); +} + +TEST_F(CleanupTest, ListSimpleRedundantComma) { + std::string Code = "void f() { std::vector v = {1,2,,,3,{4,5}}; }"; + std::string Expected = "void f() { std::vector v = {1,2,3,{4,5}}; }"; + std::vector Ranges; + Ranges.push_back(tooling::Range(40, 0)); + std::string Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); + + Code = "int main() { f(1,,2,3,,4);}"; + Expected = "int main() { f(1,2,3,4);}"; + Ranges.clear(); + Ranges.push_back(tooling::Range(17, 0)); + Ranges.push_back(tooling::Range(22, 0)); + Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); +} + +TEST_F(CleanupTest, CtorInitializationBracesInParens) { + std::string Code = "class A {\nA() : x({1}),, {} };"; + std::string Expected = "class A {\nA() : x({1}) {} };"; + std::vector Ranges; + Ranges.push_back(tooling::Range(24, 0)); + Ranges.push_back(tooling::Range(26, 0)); + std::string Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); +} + +TEST_F(CleanupTest, RedundantCommaNotInAffectedRanges) { + std::string Code = + "class A {\nA() : x({1}), /* comment */, { int x = 0; } };"; + std::string Expected = + "class A {\nA() : x({1}), /* comment */, { int x = 0; } };"; + // Set the affected range to be "int x = 0", which does not intercept the + // constructor initialization list. + std::vector Ranges(1, tooling::Range(42, 9)); + std::string Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); + + 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 */ {} };"; + std::vector 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{} };"; + Ranges = std::vector(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){} };"; + Ranges = std::vector(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){} };"; + Ranges = std::vector(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(1, tooling::Range(17, 0)); + Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); +} + +TEST_F(CleanupTest, CtorInitializerInNamespace) { + std::string Code = "namespace A {\n" + "namespace B {\n" // missing r_brace + "} // namespace A\n\n" + "namespace C {\n" + "class A { A() : x(0),, {} };\n" + "inline namespace E { namespace { } }\n" + "}"; + std::string Expected = "namespace A {\n" + "\n\n\nnamespace C {\n" + "class A { A() : x(0) {} };\n \n" + "}"; + std::vector Ranges(1, tooling::Range(0, Code.size())); + std::string Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); +} + } // end namespace } // end namespace format } // end namespace clang