[cfe-commits] r172196 - in /cfe/trunk: include/clang/Format/Format.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp
Daniel Jasper
djasper at google.com
Fri Jan 11 03:37:55 PST 2013
Author: djasper
Date: Fri Jan 11 05:37:55 2013
New Revision: 172196
URL: http://llvm.org/viewvc/llvm-project?rev=172196&view=rev
Log:
Improved formatting of constructor initializers
Added option to put each constructor initializer on its own line
if not all initializers fit on a single line. Enabling this for
Google style now as the style guide (arguable) suggests it. Not
sure whether we also want it for LLVM.
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=172196&r1=172195&r2=172196&view=diff
==============================================================================
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Fri Jan 11 05:37:55 2013
@@ -56,6 +56,10 @@
/// \brief The number of spaces to before trailing line comments.
unsigned SpacesBeforeTrailingComments;
+ /// \brief If the constructor initializers don't fit on a line, put each
+ /// initializer on its own line.
+ bool ConstructorInitializerAllOnOneLineOrOnePerLine;
+
/// \brief Add a space in front of an Objective-C protocol list, i.e. use
/// Foo <Protocol> instead of Foo<Protocol>.
bool ObjCSpaceBeforeProtocolList;
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=172196&r1=172195&r2=172196&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Fri Jan 11 05:37:55 2013
@@ -105,6 +105,7 @@
LLVMStyle.SplitTemplateClosingGreater = true;
LLVMStyle.IndentCaseLabels = false;
LLVMStyle.SpacesBeforeTrailingComments = 1;
+ LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
LLVMStyle.ObjCSpaceBeforeProtocolList = true;
LLVMStyle.ObjCSpaceBeforeReturnType = true;
return LLVMStyle;
@@ -119,6 +120,7 @@
GoogleStyle.SplitTemplateClosingGreater = false;
GoogleStyle.IndentCaseLabels = true;
GoogleStyle.SpacesBeforeTrailingComments = 2;
+ GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
GoogleStyle.ObjCSpaceBeforeProtocolList = false;
GoogleStyle.ObjCSpaceBeforeReturnType = false;
return GoogleStyle;
@@ -165,6 +167,26 @@
NewLineText + std::string(Spaces, ' ')));
}
+/// \brief Checks whether the (remaining) \c UnwrappedLine starting with
+/// \p RootToken fits into \p Limit columns.
+bool fitsIntoLimit(const AnnotatedToken &RootToken, unsigned Limit) {
+ unsigned Columns = RootToken.FormatTok.TokenLength;
+ bool FitsOnALine = true;
+ const AnnotatedToken *Tok = &RootToken;
+ while (!Tok->Children.empty()) {
+ Tok = &Tok->Children[0];
+ Columns += (Tok->SpaceRequiredBefore ? 1 : 0) + Tok->FormatTok.TokenLength;
+ // A special case for the colon of a constructor initializer as this only
+ // needs to be put on a new line if the line needs to be split.
+ if (Columns > Limit ||
+ (Tok->MustBreakBefore && Tok->Type != TT_CtorInitializerColon)) {
+ FitsOnALine = false;
+ break;
+ }
+ };
+ return FitsOnALine;
+}
+
class UnwrappedLineFormatter {
public:
UnwrappedLineFormatter(const FormatStyle &Style, SourceManager &SourceMgr,
@@ -190,7 +212,7 @@
LineState State;
State.Column = FirstIndent;
State.NextToken = &RootToken;
- State.Stack.push_back(ParenState(FirstIndent + 4, FirstIndent, 0, false));
+ State.Stack.push_back(ParenState(FirstIndent + 4, FirstIndent));
State.ForLoopVariablePos = 0;
State.LineContainsContinuedForLoopSection = false;
State.StartOfLineLevel = 1;
@@ -206,6 +228,13 @@
unsigned NoBreak = calcPenalty(State, false, UINT_MAX);
unsigned Break = calcPenalty(State, true, NoBreak);
addTokenToState(Break < NoBreak, false, State);
+ if (State.NextToken != NULL &&
+ State.NextToken->Parent->Type == TT_CtorInitializerColon) {
+ if (Style.ConstructorInitializerAllOnOneLineOrOnePerLine &&
+ !fitsIntoLimit(*State.NextToken,
+ getColumnLimit() - State.Column - 1))
+ State.Stack.back().BreakAfterComma = true;
+ }
}
}
return State.Column;
@@ -213,10 +242,9 @@
private:
struct ParenState {
- ParenState(unsigned Indent, unsigned LastSpace, unsigned FirstLessLess,
- bool BreakBeforeClosingBrace)
- : Indent(Indent), LastSpace(LastSpace), FirstLessLess(FirstLessLess),
- BreakBeforeClosingBrace(BreakBeforeClosingBrace) {}
+ ParenState(unsigned Indent, unsigned LastSpace)
+ : Indent(Indent), LastSpace(LastSpace), FirstLessLess(0),
+ BreakBeforeClosingBrace(false), BreakAfterComma(false) {}
/// \brief The position to which a specific parenthesis level needs to be
/// indented.
@@ -242,6 +270,8 @@
/// was a newline after the beginning left brace.
bool BreakBeforeClosingBrace;
+ bool BreakAfterComma;
+
bool operator<(const ParenState &Other) const {
if (Indent != Other.Indent)
return Indent < Other.Indent;
@@ -249,7 +279,9 @@
return LastSpace < Other.LastSpace;
if (FirstLessLess != Other.FirstLessLess)
return FirstLessLess < Other.FirstLessLess;
- return BreakBeforeClosingBrace;
+ if (BreakBeforeClosingBrace != Other.BreakBeforeClosingBrace)
+ return BreakBeforeClosingBrace;
+ return BreakAfterComma;
}
};
@@ -416,7 +448,7 @@
NewIndent = 4 + State.Stack.back().LastSpace;
}
State.Stack.push_back(
- ParenState(NewIndent, State.Stack.back().LastSpace, 0, false));
+ ParenState(NewIndent, State.Stack.back().LastSpace));
}
// If we encounter a closing ), ], } or >, we can remove a level from our
@@ -498,6 +530,10 @@
if (!NewLine && State.NextToken->Parent->is(tok::semi) &&
State.LineContainsContinuedForLoopSection)
return UINT_MAX;
+ if (!NewLine && State.NextToken->Parent->is(tok::comma) &&
+ State.NextToken->Type != TT_LineComment &&
+ State.Stack.back().BreakAfterComma)
+ return UINT_MAX;
unsigned CurrentPenalty = 0;
if (NewLine) {
@@ -1250,8 +1286,12 @@
unsigned Indent = formatFirstToken(Annotator.getRootToken(),
TheLine.Level, TheLine.InPPDirective,
PreviousEndOfLineColumn);
- bool FitsOnALine = fitsOnALine(Annotator.getRootToken(), Indent,
- TheLine.InPPDirective);
+ unsigned Limit = Style.ColumnLimit - (TheLine.InPPDirective ? 1 : 0) -
+ Indent;
+ // Check whether the UnwrappedLine can be put onto a single line. If
+ // so, this is bound to be the optimal solution (by definition) and we
+ // don't need to analyze the entire solution space.
+ bool FitsOnALine = fitsIntoLimit(Annotator.getRootToken(), Limit);
UnwrappedLineFormatter Formatter(Style, SourceMgr, TheLine, Indent,
FitsOnALine, Annotator.getLineType(),
Annotator.getRootToken(), Replaces,
@@ -1300,29 +1340,6 @@
UnwrappedLines.push_back(TheLine);
}
- bool fitsOnALine(const AnnotatedToken &RootToken, unsigned Indent,
- bool InPPDirective) {
- // Check whether the UnwrappedLine can be put onto a single line. If so,
- // this is bound to be the optimal solution (by definition) and we don't
- // need to analyze the entire solution space.
- unsigned Columns = Indent + RootToken.FormatTok.TokenLength;
- bool FitsOnALine = true;
- const AnnotatedToken *Tok = &RootToken;
- while (Tok != NULL) {
- Columns += (Tok->SpaceRequiredBefore ? 1 : 0) +
- Tok->FormatTok.TokenLength;
- // A special case for the colon of a constructor initializer as this only
- // needs to be put on a new line if the line needs to be split.
- if (Columns > Style.ColumnLimit - (InPPDirective ? 1 : 0) ||
- (Tok->MustBreakBefore && Tok->Type != TT_CtorInitializerColon)) {
- FitsOnALine = false;
- break;
- }
- Tok = Tok->Children.empty() ? NULL : &Tok->Children[0];
- }
- return FitsOnALine;
- }
-
/// \brief Add a new line and the required indent before the first Token
/// of the \c UnwrappedLine if there was no structural parsing error.
/// Returns the indent level of the \c UnwrappedLine.
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=172196&r1=172195&r2=172196&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Jan 11 05:37:55 2013
@@ -649,6 +649,11 @@
TEST_F(FormatTest, ConstructorInitializers) {
verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}");
+ verifyFormat("Constructor() : Inttializer(FitsOnTheLine) {}",
+ getLLVMStyleWithColumns(45));
+ verifyFormat("Constructor()\n"
+ " : Inttializer(FitsOnTheLine) {}",
+ getLLVMStyleWithColumns(44));
verifyFormat(
"SomeClass::Constructor()\n"
@@ -656,6 +661,16 @@
verifyFormat(
"SomeClass::Constructor()\n"
+ " : aaaaaaaaaaaaa(aaaaaaaaaaaaaa), aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
+ " aaaaaaaaaaaaa(aaaaaaaaaaaaaa) {}");
+ verifyGoogleFormat(
+ "SomeClass::Constructor()\n"
+ " : aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
+ " aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
+ " aaaaaaaaaaaaa(aaaaaaaaaaaaaa) {}");
+
+ verifyFormat(
+ "SomeClass::Constructor()\n"
" : aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa),\n"
" aaaaaaaaaaaaaaa(aaaaaaaaaaaa) {}");
More information about the cfe-commits
mailing list