Enable bin-packing in Google style.
After some discussions, it seems that this is the better path in
the long run. Does not change Chromium style, as there, bin packing
is forbidden by the style guide.
Also fix two minor bugs wrt. formatting:
1. If a call parameter is a function call itself and is split before
the "." or "->", split before the next parameter.
2. If a call parameter is string literal that has to be split onto
two lines, split before the next parameter.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176177 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index 8bc414c..ea96b8f 100644
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -61,7 +61,7 @@
GoogleStyle.Standard = FormatStyle::LS_Auto;
GoogleStyle.IndentCaseLabels = true;
GoogleStyle.SpacesBeforeTrailingComments = 2;
- GoogleStyle.BinPackParameters = false;
+ GoogleStyle.BinPackParameters = true;
GoogleStyle.AllowAllParametersOfDeclarationOnNextLine = true;
GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
GoogleStyle.AllowShortIfStatementsOnASingleLine = false;
@@ -74,6 +74,7 @@
FormatStyle getChromiumStyle() {
FormatStyle ChromiumStyle = getGoogleStyle();
ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false;
+ ChromiumStyle.BinPackParameters = false;
ChromiumStyle.Standard = FormatStyle::LS_Cpp03;
ChromiumStyle.DerivePointerBinding = false;
return ChromiumStyle;
@@ -541,6 +542,9 @@
for (unsigned i = 0, e = State.Stack.size() - 1; i != e; ++i) {
State.Stack[i].BreakBeforeParameter = true;
}
+ if (Current.is(tok::period) || Current.is(tok::arrow))
+ State.Stack.back().BreakBeforeParameter = true;
+
// If we break after {, we should also break before the corresponding }.
if (Previous.is(tok::l_brace))
State.Stack.back().BreakBeforeClosingBrace = true;
@@ -730,6 +734,8 @@
TailLength -= SplitPoint + 1;
OffsetFromStart = 1;
Penalty += Style.PenaltyExcessCharacter;
+ for (unsigned i = 0, e = State.Stack.size(); i != e; ++i)
+ State.Stack[i].BreakBeforeParameter = true;
}
State.Column = StartColumn + TailLength;
return Penalty;
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index 4fe4595..87e89db 100644
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -293,24 +293,26 @@
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);\n"
" ++aaaaaaaaaaa) {\n}");
+ verifyFormat("for (int aaaaaaaaaaa = 1; aaaaaaaaaaa <= bbbbbbbbbbbbbbb;\n"
+ " aaaaaaaaaaa++, bbbbbbbbbbbbbbbbb++) {\n"
+ "}");
- verifyGoogleFormat(
- "for (int aaaaaaaaaaa = 1; aaaaaaaaaaa <= bbbbbbbbbbbbbbb;\n"
- " aaaaaaaaaaa++, bbbbbbbbbbbbbbbbb++) {\n"
- "}");
- verifyGoogleFormat(
- "for (int aaaaaaaaaaa = 1;\n"
- " aaaaaaaaaaa <= aaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa,\n"
- " aaaaaaaaaaaaaaaa,\n"
- " aaaaaaaaaaaaaaaa,\n"
- " aaaaaaaaaaaaaaaa);\n"
- " aaaaaaaaaaa++, bbbbbbbbbbbbbbbbb++) {\n"
- "}");
- verifyGoogleFormat(
+ FormatStyle NoBinPacking = getLLVMStyle();
+ NoBinPacking.BinPackParameters = false;
+ verifyFormat("for (int aaaaaaaaaaa = 1;\n"
+ " aaaaaaaaaaa <= aaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaa);\n"
+ " aaaaaaaaaaa++, bbbbbbbbbbbbbbbbb++) {\n"
+ "}",
+ NoBinPacking);
+ verifyFormat(
"for (std::vector<UnwrappedLine>::iterator I = UnwrappedLines.begin(),\n"
" E = UnwrappedLines.end();\n"
" I != E;\n"
- " ++I) {\n}");
+ " ++I) {\n}",
+ NoBinPacking);
}
TEST_F(FormatTest, RangeBasedForLoops) {
@@ -548,10 +550,13 @@
format("f(aaaaaaaaaaaaaaaaaaaaaaaaa , \n"
"/* Leading comment for bb... */ bbbbbbbbbbbbbbbbbbbbbbbbb);"));
- verifyGoogleFormat("aaaaaaaa(/* parameter 1 */ aaaaaa,\n"
- " /* parameter 2 */ aaaaaa,\n"
- " /* parameter 3 */ aaaaaa,\n"
- " /* parameter 4 */ aaaaaa);");
+ FormatStyle NoBinPacking = getLLVMStyle();
+ NoBinPacking.BinPackParameters = false;
+ verifyFormat("aaaaaaaa(/* parameter 1 */ aaaaaa,\n"
+ " /* parameter 2 */ aaaaaa,\n"
+ " /* parameter 3 */ aaaaaa,\n"
+ " /* parameter 4 */ aaaaaa);",
+ NoBinPacking);
}
TEST_F(FormatTest, CommentsInStaticInitializers) {
@@ -1146,11 +1151,10 @@
// Here a line could be saved by splitting the second initializer onto two
// lines, but that is not desireable.
- verifyFormat(
- "Constructor()\n"
- " : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaa),\n"
- " aaaaaaaaaaa(aaaaaaaaaaa),\n"
- " aaaaaaaaaaaaaaaaaaaaat(aaaaaaaaaaaaaaaaaaaaaaaaaaaa) {}");
+ verifyFormat("Constructor()\n"
+ " : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaa),\n"
+ " aaaaaaaaaaa(aaaaaaaaaaa),\n"
+ " aaaaaaaaaaaaaaaaaaaaat(aaaaaaaaaaaaaaaaaaaaaaaaaaaa) {}");
FormatStyle OnePerLine = getLLVMStyle();
OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
@@ -1171,13 +1175,14 @@
OnePerLine);
// This test takes VERY long when memoization is broken.
+ OnePerLine.BinPackParameters = false;
std::string input = "Constructor()\n"
" : aaaa(a,\n";
for (unsigned i = 0, e = 80; i != e; ++i) {
input += " a,\n";
}
input += " a) {}";
- verifyGoogleFormat(input);
+ verifyFormat(input, OnePerLine);
}
TEST_F(FormatTest, BreaksAsHighAsPossible) {
@@ -1240,54 +1245,60 @@
}
TEST_F(FormatTest, FormatsOneParameterPerLineIfNecessary) {
- verifyGoogleFormat("f(aaaaaaaaaaaaaaaaaaaa,\n"
- " aaaaaaaaaaaaaaaaaaaa,\n"
- " aaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaa);");
- verifyGoogleFormat(
- "aaaaaaa(aaaaaaaaaaaaa,\n"
- " aaaaaaaaaaaaa,\n"
- " aaaaaaaaaaaaa(aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaa));");
- verifyGoogleFormat(
+ FormatStyle NoBinPacking = getLLVMStyle();
+ NoBinPacking.BinPackParameters = false;
+ verifyFormat("f(aaaaaaaaaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaa);",
+ NoBinPacking);
+ verifyFormat("aaaaaaa(aaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaa(aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaa));",
+ NoBinPacking);
+ verifyFormat(
"aaaaaaaa(aaaaaaaaaaaaa,\n"
" aaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)),\n"
" aaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
- " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)));");
- verifyGoogleFormat(
- "aaaaaaaaaaaaaaa(aaaaaaaaa, aaaaaaaaa, aaaaaaaaaaaaaaaaaaaaa)\n"
- " .aaaaaaaaaaaaaaaaaa();");
- verifyGoogleFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
- " aaaaaaaaaa, aaaaaaaaaa, aaaaaaaaaa, aaaaaaaaaaa);");
+ " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)));",
+ NoBinPacking);
+ verifyFormat("aaaaaaaaaaaaaaa(aaaaaaaaa, aaaaaaaaa, aaaaaaaaaaaaaaaaaaaaa)\n"
+ " .aaaaaaaaaaaaaaaaaa();",
+ NoBinPacking);
+ verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
+ " aaaaaaaaaa, aaaaaaaaaa, aaaaaaaaaa, aaaaaaaaaaa);",
+ NoBinPacking);
- verifyGoogleFormat(
+ verifyFormat(
"aaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaa,\n"
" aaaaaaaaaaaa,\n"
- " aaaaaaaaaaaa);");
- verifyGoogleFormat(
+ " aaaaaaaaaaaa);",
+ NoBinPacking);
+ verifyFormat(
"somefunction(someotherFunction(ddddddddddddddddddddddddddddddddddd,\n"
" ddddddddddddddddddddddddddddd),\n"
- " test);");
+ " test);",
+ NoBinPacking);
- verifyGoogleFormat(
- "std::vector<aaaaaaaaaaaaaaaaaaaaaaa,\n"
- " aaaaaaaaaaaaaaaaaaaaaaa,\n"
- " aaaaaaaaaaaaaaaaaaaaaaa> aaaaaaaaaaaaaaaaaa;");
- verifyGoogleFormat("a(\"a\"\n"
- " \"a\",\n"
- " a);");
+ verifyFormat("std::vector<aaaaaaaaaaaaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaaaaaaaaa> aaaaaaaaaaaaaaaaaa;",
+ NoBinPacking);
+ verifyFormat("a(\"a\"\n"
+ " \"a\",\n"
+ " a);");
- FormatStyle Style = getGoogleStyle();
- Style.AllowAllParametersOfDeclarationOnNextLine = false;
+ NoBinPacking.AllowAllParametersOfDeclarationOnNextLine = false;
verifyFormat("void aaaaaaaaaa(aaaaaaaaa,\n"
" aaaaaaaaa,\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);",
- Style);
+ NoBinPacking);
verifyFormat(
"void f() {\n"
" aaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaa, aaaaaaaaa, aaaaaaaaaaaaaaaaaaaaa)\n"
" .aaaaaaa();\n"
"}",
- Style);
+ NoBinPacking);
}
TEST_F(FormatTest, FormatsBuilderPattern) {
@@ -1432,14 +1443,17 @@
" TheLine.InPPDirective, PreviousEndOfLineColumn);",
getLLVMStyleWithColumns(70));
- verifyGoogleFormat(
+ FormatStyle NoBinPacking = getLLVMStyle();
+ NoBinPacking.BinPackParameters = false;
+ verifyFormat(
"void f() {\n"
" g(aaa,\n"
" aaaaaaaaaa == aaaaaaaaaa ? aaaa : aaaaa,\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa == aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
" ? aaaaaaaaaaaaaaa\n"
" : aaaaaaaaaaaaaaa);\n"
- "}");
+ "}",
+ NoBinPacking);
}
TEST_F(FormatTest, DeclarationsOfMultipleVariables) {
@@ -1580,13 +1594,6 @@
verifyFormat("SomeMap[std::pair(aaaaaaaaaaaa, bbbbbbbbbbbbbbb)]\n"
" .insert(ccccccccccccccccccccccc);");
- verifyGoogleFormat(
- "aaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa)\n"
- " .aaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa)\n"
- " .aaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaa,\n"
- " aaaaaaaaaaaaaaaaaaa,\n"
- " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);");
-
// Here, it is not necessary to wrap at "." or "->".
verifyFormat("if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaa) ||\n"
" aaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {\n}");
@@ -1598,6 +1605,15 @@
verifyFormat(
"aaaaaaaaaaaaaaaaaaaaaaaaa(\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa().aaaaaaaaaaaaaaaaa());");
+
+ FormatStyle NoBinPacking = getLLVMStyle();
+ NoBinPacking.BinPackParameters = false;
+ verifyFormat("aaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa)\n"
+ " .aaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa)\n"
+ " .aaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);",
+ NoBinPacking);
}
TEST_F(FormatTest, WrapsTemplateDeclarations) {
@@ -1929,7 +1945,7 @@
verifyFormat("void f(int i = (kA * kB) & kMask) {}");
verifyFormat("int a = sizeof(int) * b;");
verifyFormat("int a = alignof(int) * b;");
-
+
// These are not casts, but at some point were confused with casts.
verifyFormat("virtual void foo(int *) override;");
verifyFormat("virtual void foo(char &) const;");
@@ -1964,8 +1980,8 @@
"aaaaaaaaaaaaaaaaaaaaaaa;");
verifyGoogleFormat(
- "TypeSpecDecl* TypeSpecDecl::Create(\n"
- " ASTContext& C, DeclContext* DC, SourceLocation L) {}");
+ "TypeSpecDecl* TypeSpecDecl::Create(ASTContext& C, DeclContext* DC,\n"
+ " SourceLocation L) {}");
verifyGoogleFormat(
"some_namespace::LongReturnType\n"
"long_namespace::SomeVeryLongClass::SomeVeryLongFunction(\n"
@@ -2004,13 +2020,16 @@
//===----------------------------------------------------------------------===//
TEST_F(FormatTest, IncompleteParameterLists) {
- verifyGoogleFormat("void aaaaaaaaaaaaaaaaaa(int level,\n"
- " double *min_x,\n"
- " double *max_x,\n"
- " double *min_y,\n"
- " double *max_y,\n"
- " double *min_z,\n"
- " double *max_z, ) {}");
+ FormatStyle NoBinPacking = getLLVMStyle();
+ NoBinPacking.BinPackParameters = false;
+ verifyFormat("void aaaaaaaaaaaaaaaaaa(int level,\n"
+ " double *min_x,\n"
+ " double *max_x,\n"
+ " double *min_y,\n"
+ " double *max_y,\n"
+ " double *min_z,\n"
+ " double *max_z, ) {}",
+ NoBinPacking);
}
TEST_F(FormatTest, IncorrectCodeTrailingStuff) {
@@ -2284,6 +2303,8 @@
"/* */someCall(parameter);",
getLLVMStyleWithColumns(15)));
+ FormatStyle NoBinPacking = getLLVMStyle();
+ NoBinPacking.BinPackParameters = false;
EXPECT_EQ("someFunction(1, /* comment 1 */\n"
" 2, /* comment 2 */\n"
" 3, /* comment 3 */\n"
@@ -2293,7 +2314,7 @@
" 2, /* comment 2 */ \n"
" 3, /* comment 3 */\n"
"aaaa, bbbb );",
- getGoogleStyle()));
+ NoBinPacking));
verifyFormat(
"bool aaaaaaaaaaaaa = /* comment: */ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ||\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaa;");
@@ -2974,7 +2995,8 @@
EXPECT_EQ("variable = f(\n"
" \"long string \"\n"
- " \"literal\", short,\n"
+ " \"literal\",\n"
+ " short,\n"
" loooooooooooooooooooong);",
format("variable = f(\"long string literal\", short, "
"loooooooooooooooooooong);",
