[cfe-commits] r172617 - in /cfe/trunk: include/clang/Format/Format.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp
Daniel Jasper
djasper at google.com
Wed Jan 16 06:59:02 PST 2013
Author: djasper
Date: Wed Jan 16 08:59:02 2013
New Revision: 172617
URL: http://llvm.org/viewvc/llvm-project?rev=172617&view=rev
Log:
Add option to avoid "bin-packing" of parameters.
"Bin-packing" here means allowing multiple parameters on one line, if a
function call/declaration is spread over multiple lines.
This is required by the Chromium style guide and probably desired for
the Google style guide. Not making changes to LLVM style as I don't have
enough data.
With this enabled, we format stuff like:
aaaaaaaaaaaaaaa(aaaaaaaaaa,
aaaaaaaaaa,
aaaaaaaaaaaaaaaaaaaaaaa).aaaaaaaaaaaaaaaaaa();
Modified:
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/include/clang/Format/Format.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=172617&r1=172616&r2=172617&view=diff
==============================================================================
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Wed Jan 16 08:59:02 2013
@@ -57,6 +57,10 @@
/// \brief The number of spaces to before trailing line comments.
unsigned SpacesBeforeTrailingComments;
+ /// \brief If false, a function call's or function definition's parameters
+ /// will either all be on the same line or will have one line each.
+ bool BinPackParameters;
+
/// \brief If the constructor initializers don't fit on a line, put each
/// initializer on its own line.
bool ConstructorInitializerAllOnOneLineOrOnePerLine;
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=172617&r1=172616&r2=172617&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Jan 16 08:59:02 2013
@@ -73,7 +73,7 @@
AnnotatedToken(const FormatToken &FormatTok)
: FormatTok(FormatTok), Type(TT_Unknown), SpaceRequiredBefore(false),
CanBreakBefore(false), MustBreakBefore(false),
- ClosesTemplateDeclaration(false), Parent(NULL) {}
+ ClosesTemplateDeclaration(false), MatchingParen(NULL), Parent(NULL) {}
bool is(tok::TokenKind Kind) const { return FormatTok.Tok.is(Kind); }
bool isNot(tok::TokenKind Kind) const { return FormatTok.Tok.isNot(Kind); }
@@ -92,6 +92,8 @@
bool ClosesTemplateDeclaration;
+ AnnotatedToken *MatchingParen;
+
/// \brief The total length of the line up to and including this token.
unsigned TotalLength;
@@ -146,6 +148,7 @@
LLVMStyle.SplitTemplateClosingGreater = true;
LLVMStyle.IndentCaseLabels = false;
LLVMStyle.SpacesBeforeTrailingComments = 1;
+ LLVMStyle.BinPackParameters = true;
LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
LLVMStyle.ObjCSpaceBeforeProtocolList = true;
@@ -162,6 +165,7 @@
GoogleStyle.SplitTemplateClosingGreater = false;
GoogleStyle.IndentCaseLabels = true;
GoogleStyle.SpacesBeforeTrailingComments = 2;
+ GoogleStyle.BinPackParameters = false;
GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
GoogleStyle.AllowShortIfStatementsOnASingleLine = true;
GoogleStyle.ObjCSpaceBeforeProtocolList = false;
@@ -318,7 +322,8 @@
struct ParenState {
ParenState(unsigned Indent, unsigned LastSpace)
: Indent(Indent), LastSpace(LastSpace), FirstLessLess(0),
- BreakBeforeClosingBrace(false), BreakAfterComma(false) {}
+ BreakBeforeClosingBrace(false), BreakAfterComma(false),
+ HasMultiParameterLine(false) {}
/// \brief The position to which a specific parenthesis level needs to be
/// indented.
@@ -345,6 +350,7 @@
bool BreakBeforeClosingBrace;
bool BreakAfterComma;
+ bool HasMultiParameterLine;
bool operator<(const ParenState &Other) const {
if (Indent != Other.Indent)
@@ -357,6 +363,8 @@
return BreakBeforeClosingBrace;
if (BreakAfterComma != Other.BreakAfterComma)
return BreakAfterComma;
+ if (HasMultiParameterLine != Other.HasMultiParameterLine)
+ return HasMultiParameterLine;
return false;
}
};
@@ -484,6 +492,9 @@
if (Previous.is(tok::l_paren) || Previous.is(tok::l_brace) ||
State.NextToken->Parent->Type == TT_TemplateOpener)
State.Stack[ParenLevel].Indent = State.Column + Spaces;
+ if (Previous.is(tok::comma))
+ State.Stack[ParenLevel].HasMultiParameterLine = true;
+
// Top-level spaces that are not part of assignments are exempt as that
// mostly leads to better results.
@@ -492,9 +503,18 @@
(ParenLevel != 0 || getPrecedence(Previous) == prec::Assignment))
State.Stack[ParenLevel].LastSpace = State.Column;
}
- if (Newline && Previous.is(tok::l_brace)) {
+
+ // If we break after an {, we should also break before the corresponding }.
+ if (Newline && Previous.is(tok::l_brace))
State.Stack.back().BreakBeforeClosingBrace = true;
- }
+
+ // If we are breaking after '(', '{', '<' or ',', we need to break after
+ // future commas as well to avoid bin packing.
+ if (!Style.BinPackParameters && Newline &&
+ (Previous.is(tok::comma) || Previous.is(tok::l_paren) ||
+ Previous.is(tok::l_brace) || Previous.Type == TT_TemplateOpener))
+ State.Stack.back().BreakAfterComma = true;
+
moveStateToNextToken(State);
}
@@ -523,6 +543,17 @@
}
State.Stack.push_back(
ParenState(NewIndent, State.Stack.back().LastSpace));
+
+ // If the entire set of parameters will not fit on the current line, we
+ // will need to break after commas on this level to avoid bin-packing.
+ if (!Style.BinPackParameters && Current.MatchingParen != NULL &&
+ !Current.Children.empty()) {
+ if (getColumnLimit() < State.Column + Current.FormatTok.TokenLength +
+ Current.MatchingParen->TotalLength -
+ Current.Children[0].TotalLength) {
+ State.Stack.back().BreakAfterComma = true;
+ }
+ }
}
// If we encounter a closing ), ], } or >, we can remove a level from our
@@ -623,6 +654,11 @@
State.NextToken->Type != TT_LineComment &&
State.Stack.back().BreakAfterComma)
return UINT_MAX;
+ // Trying to insert a parameter on a new line if there are already more than
+ // one parameter on the current line is bin packing.
+ if (NewLine && State.NextToken->Parent->is(tok::comma) &&
+ State.Stack.back().HasMultiParameterLine && !Style.BinPackParameters)
+ return UINT_MAX;
if (!NewLine && State.NextToken->Type == TT_CtorInitializerColon)
return UINT_MAX;
@@ -631,7 +667,8 @@
CurrentPenalty += Parameters.PenaltyIndentLevel * State.Stack.size() +
splitPenalty(*State.NextToken->Parent);
} else {
- if (State.Stack.size() < State.StartOfLineLevel)
+ if (State.Stack.size() < State.StartOfLineLevel &&
+ State.NextToken->is(tok::identifier))
CurrentPenalty += Parameters.PenaltyLevelDecrease *
(State.StartOfLineLevel - State.Stack.size());
}
@@ -706,8 +743,13 @@
ColonIsObjCMethodExpr(false) {}
bool parseAngle() {
+ if (CurrentToken == NULL)
+ return false;
+ AnnotatedToken *Left = CurrentToken->Parent;
while (CurrentToken != NULL) {
if (CurrentToken->is(tok::greater)) {
+ Left->MatchingParen = CurrentToken;
+ CurrentToken->MatchingParen = Left;
CurrentToken->Type = TT_TemplateCloser;
next();
return true;
@@ -725,10 +767,15 @@
}
bool parseParens() {
- if (CurrentToken != NULL && CurrentToken->is(tok::caret))
- CurrentToken->Parent->Type = TT_ObjCBlockLParen;
+ if (CurrentToken == NULL)
+ return false;
+ AnnotatedToken *Left = CurrentToken->Parent;
+ if (CurrentToken->is(tok::caret))
+ Left->Type = TT_ObjCBlockLParen;
while (CurrentToken != NULL) {
if (CurrentToken->is(tok::r_paren)) {
+ Left->MatchingParen = CurrentToken;
+ CurrentToken->MatchingParen = Left;
next();
return true;
}
@@ -781,8 +828,14 @@
}
bool parseBrace() {
+ // Lines are fine to end with '{'.
+ if (CurrentToken == NULL)
+ return true;
+ AnnotatedToken *Left = CurrentToken->Parent;
while (CurrentToken != NULL) {
if (CurrentToken->is(tok::r_brace)) {
+ Left->MatchingParen = CurrentToken;
+ CurrentToken->MatchingParen = Left;
next();
return true;
}
@@ -791,7 +844,6 @@
if (!consumeToken())
return false;
}
- // Lines can currently end with '{'.
return true;
}
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=172617&r1=172616&r2=172617&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Jan 16 08:59:02 2013
@@ -484,10 +484,11 @@
TEST_F(FormatTest, NestedStaticInitializers) {
verifyFormat("static A x = { { {} } };\n");
verifyFormat(
- "static A x = {\n"
- " { { init1, init2, init3, init4 }, { init1, init2, init3, init4 } }\n"
- "};\n");
- verifyFormat(
+ "static A x = { { { init1, init2, init3, init4 },\n"
+ " { init1, init2, init3, init4 } } };");
+
+ // FIXME: Fix this in general an verify that it works in LLVM style again.
+ verifyGoogleFormat(
"somes Status::global_reps[3] = {\n"
" { kGlobalRef, OK_CODE, NULL, NULL, NULL },\n"
" { kGlobalRef, CANCELLED_CODE, NULL, NULL, NULL },\n"
@@ -670,8 +671,7 @@
"#define A B\n"
" withMoreParamters,\n"
" whichStronglyInfluenceTheLayout),\n"
- " andMoreParameters),\n"
- " trailing);", getLLVMStyleWithColumns(69));
+ " andMoreParameters), trailing);", getLLVMStyleWithColumns(69));
}
TEST_F(FormatTest, LayoutBlockInsideParens) {
@@ -763,22 +763,13 @@
"}");
// This test takes VERY long when memoization is broken.
- verifyGoogleFormat(
- "Constructor()\n"
- " : aaaa(a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a,"
- " a, a, a,\n"
- " a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a,"
- " a, a, a,\n"
- " a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a,"
- " a, a, a,\n"
- " a, a, a, a, a, a, a, a, a, a, a)\n"
- " aaaa(a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a,"
- " a, a, a,\n"
- " a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a,"
- " a, a, a,\n"
- " a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a,"
- " a, a, a,\n"
- " a, a, a, a, a, a, a, a, a, a, a) {}\n");
+ std::string input = "Constructor()\n"
+ " : aaaa(a,\n";
+ for (unsigned i = 0, e = 80; i != e; ++i) {
+ input += " a,\n";
+ }
+ input += " a) {}";
+ verifyGoogleFormat(input);
}
TEST_F(FormatTest, BreaksAsHighAsPossible) {
@@ -829,6 +820,31 @@
" }\n }\n}");
}
+TEST_F(FormatTest, FormatsOneParameterPerLineIfNecessary) {
+ verifyGoogleFormat(
+ "aaaaaaaa(aaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
+ " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)),\n"
+ " aaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
+ " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)));");
+ verifyGoogleFormat(
+ "aaaaaaaaaaaaaaa(aaaaaaaaa,\n"
+ " aaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaaaaaaaaa).aaaaaaaaaaaaaaaaaa();");
+ verifyGoogleFormat(
+ "somefunction(someotherFunction(ddddddddddddddddddddddddddddddddddd,\n"
+ " ddddddddddddddddddddddddddddd),\n"
+ " test);");
+
+ verifyGoogleFormat(
+ "std::vector<aaaaaaaaaaaaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaaaaaaaaa,\n"
+ " aaaaaaaaaaaaaaaaaaaaaaa> aaaaaaaaaaaaaaaaaa;");
+ verifyGoogleFormat("a(\"a\"\n"
+ " \"a\",\n"
+ " a);");
+}
+
TEST_F(FormatTest, DoesNotBreakTrailingAnnotation) {
verifyFormat("void aaaaaaaaaaaa(int aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
" GUARDED_BY(aaaaaaaaaaaaa);");
More information about the cfe-commits
mailing list