[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
Sat Jan 12 22:55:19 PST 2013
Well, logically it is correct. True really means we do "A or B". False
means, we don't. I am not happy with the name, but I couldn't come up with
a better one.
On Fri, Jan 11, 2013 at 7:09 PM, Matt Beaumont-Gay <matthewbg at google.com>wrote:
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130113/6ef71f94/attachment.html>
More information about the cfe-commits
mailing list