ioeric updated this revision to Diff 56696.
ioeric added a comment.
- Extended redundant comma cleanup to general lists, and change the way
constructor initializer list is handled. Removed comments cleanup, leave it for
a future patch.
http://reviews.llvm.org/D19804
Files:
lib/Format/Format.cpp
unittests/Format/CleanupTest.cpp
Index: unittests/Format/CleanupTest.cpp
===================================================================
--- unittests/Format/CleanupTest.cpp
+++ 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<tooling::Range> 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<int> v = {1,2,,,3}; }";
+ std::string Expected = "void f() { std::vector<int> v = {1,2,3}; }";
+ std::vector<tooling::Range> 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<tooling::Range> 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<tooling::Range> 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<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{} };";
+ 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){} };";
+ 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){} };";
+ 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) {
+ 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<tooling::Range> 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
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1816,6 +1816,13 @@
checkEmptyNamespace(AnnotatedLines);
+ for (auto &Line : AnnotatedLines) {
+ if (Line->Affected) {
+ cleanupRight(Line->First, Line->Last, tok::comma, tok::comma);
+ checkConstructorInitList(*Line);
+ }
+ }
+
return generateFixes();
}
@@ -1904,6 +1911,99 @@
return true;
}
+ FormatToken *getNextTokenNotDeletedUntilEnd(const FormatToken *Tok,
+ const FormatToken *End,
+ bool SkipComment) {
+ assert(Tok);
+ auto *Res = Tok->Next;
+ while (Res && ((SkipComment && Res->is(tok::comment)) ||
+ DeletedTokens.find(Res) != DeletedTokens.end())) {
+ if (Res == End)
+ return nullptr;
+ Res = Res->Next;
+ }
+ return Res;
+ }
+
+ // 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.
+ void cleanupTokenInMatchedPair(FormatToken *Start, FormatToken *End,
+ tok::TokenKind LK, tok::TokenKind RK,
+ bool DeleteLeft) {
+ for (auto *Left = Start; Left != nullptr && Left != End;) {
+ auto *Right =
+ getNextTokenNotDeletedUntilEnd(Left, End, /*SkipComment=*/true);
+ 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 = getNextTokenNotDeletedUntilEnd(Left, End, /*SkipComment=*/true);
+ }
+ }
+
+ void cleanupLeft(FormatToken *Start, FormatToken *End, tok::TokenKind LK,
+ tok::TokenKind RK) {
+ cleanupTokenInMatchedPair(Start, End, LK, RK, /*DeleteLeft=*/true);
+ }
+
+ void cleanupRight(FormatToken *Start, FormatToken *End, tok::TokenKind LK,
+ tok::TokenKind RK) {
+ cleanupTokenInMatchedPair(Start, End, LK, RK, /*DeleteLeft=*/false);
+ }
+
+ // Looks for and removes redundant colon/commas in the constructor initializer
+ // list.
+ void checkConstructorInitList(AnnotatedLine &Line) {
+ FormatToken *Tok = Line.First;
+ while (Tok && Tok->Type != TT_CtorInitializerColon) {
+ Tok = Tok->Next;
+ }
+ if (!Tok)
+ return;
+
+ assert(Tok->is(tok::colon) && Tok->Type == TT_CtorInitializerColon);
+
+ // Clean up redundant comma after the constructor initializer colon.
+ cleanupRight(Tok, getNextTokenNotDeletedUntilEnd(Tok, Line.Last,
+ /*SkipComment=*/true),
+ tok::colon, tok::comma);
+
+ // Find the left brace at the end of the initializer list.
+ Tok = Line.Last;
+ while (Tok && Tok->is(tok::comment)) {
+ Tok = Tok->Previous;
+ }
+ if (!Tok || Tok->isNot(tok::l_brace))
+ return; // FIXME: error handling. Expecting tok::l_brace at the end.
+
+ // Clean up redundant commas/redundant colon right before the left brace.
+ Tok = Tok->Previous;
+ while (Tok && (Tok->is(tok::comment) ||
+ DeletedTokens.find(Tok) != DeletedTokens.end())) {
+ Tok = Tok->Previous;
+ }
+ // We only need to check one token before the left brace.
+ //
+ // If the token before the left brace is a comma, then the token before the
+ // comma must not be the initializer colon; otherwise, the comma should have
+ // been deleted in the previous steps. It must not be a comma either since a
+ // comma right before another comma should be deleted in
+ // `cleanupLeft(tok::comma, tok::comma)`.
+ //
+ // If the token before the left brace is the intiailizer colon, then we can
+ // simply delete the redundant colon since the initializer list is empty.
+ if (Tok && (Tok->Type == TT_CtorInitializerColon || Tok->is(tok::comma))) {
+ deleteToken(Tok);
+ }
+ }
+
// Delete the given token.
inline void deleteToken(FormatToken *Tok) {
if (Tok)
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
