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 <[email protected]>wrote: > On Fri, Jan 11, 2013 at 3:37 AM, Daniel Jasper <[email protected]> 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 > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
