[cfe-commits] r172196 - in /cfe/trunk: include/clang/Format/Format.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp
Matt Beaumont-Gay
matthewbg at google.com
Fri Jan 11 10:09:20 PST 2013
On Fri, Jan 11, 2013 at 3:37 AM, Daniel Jasper <djasper at google.com> wrote:
> 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;
I don't think this is a great name for a bool, since it's not clear
which behavior is indicated by 'true'. Maybe
'ConstructorInitializerListOnePerLine' or something like that instead?
> +
> /// \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) {}");
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list