On Tue, Oct 8, 2013 at 5:49 AM, Daniel Jasper <[email protected]> wrote:
> > > > On Tue, Oct 8, 2013 at 2:20 PM, Manuel Klimek <[email protected]> wrote: > >> On Mon, Oct 7, 2013 at 10:11 PM, Daniel Jasper <[email protected]>wrote: >> >>> Author: djasper >>> Date: Tue Oct 8 00:11:18 2013 >>> New Revision: 192168 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=192168&view=rev >>> Log: >>> clang-format: Improve constructor initializer linewrapping. >>> >>> Specifically make ConstructorInitializerAllOnOneLineOrOnePerLine work >>> nicely with BreakConstructorInitializersBeforeComma. >>> >>> This fixes llvm.org/PR17395. >>> >>> Modified: >>> cfe/trunk/lib/Format/ContinuationIndenter.cpp >>> cfe/trunk/lib/Format/TokenAnnotator.cpp >>> cfe/trunk/unittests/Format/FormatTest.cpp >>> >>> Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=192168&r1=192167&r2=192168&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original) >>> +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Tue Oct 8 00:11:18 >>> 2013 >>> @@ -44,6 +44,18 @@ static bool startsSegmentOfBuilderTypeCa >>> return Tok.isMemberAccess() && Tok.Previous && >>> Tok.Previous->closesScope(); >>> } >>> >>> +// Returns \c true if \c Current starts a new parameter. >>> >> I'd add: // If true, 'Current' is the first token of either the next constructor initializer, or the next // parameter of any inner comma separated list. // Note that if Style.BreakConstructorInitializersBeforeComma, inner parameter lists // will still start after the comma. > +static bool startsNextParameter(const FormatToken &Current, >>> + const FormatStyle &Style) { >>> + const FormatToken &Previous = *Current.Previous; >>> + if (Current.Type == TT_CtorInitializerComma && >>> + Style.BreakConstructorInitializersBeforeComma) >>> + return true; >>> + return Previous.is(tok::comma) && !Current.isTrailingComment() && >>> + (Previous.Type != TT_CtorInitializerComma || >>> + !Style.BreakConstructorInitializersBeforeComma); >>> >> >> Isn't that last part redundant, given the first 'if'? >> > > No. For one thing, Previous != Current. > Ah. I missed that. I wonder whether there's a better way to write that down, perhaps in a way that the expression doesn't iterate between previous->current->previous. Here's a proposal that makes it easier to understand for me, to take or leave ;) if (Style.BreakConstructorInitializersBeforeComma) { return Current.Type == TT_CtorInitializerComma || (Previous.Type != TT_CtorInitializerComma && Previous.is(tok::comma) && !Current.isTrailingComment()); } else { return Previous.is(tok::comma) && !Current.isTrailingComment(); } > > +} >>> + >>> ContinuationIndenter::ContinuationIndenter(const FormatStyle &Style, >>> SourceManager &SourceMgr, >>> WhitespaceManager >>> &Whitespaces, >>> @@ -113,15 +125,9 @@ bool ContinuationIndenter::mustBreak(con >>> return true; >>> if (Previous.is(tok::semi) && >>> State.LineContainsContinuedForLoopSection) >>> return true; >>> - if (Style.BreakConstructorInitializersBeforeComma) { >>> - if (Previous.Type == TT_CtorInitializerComma) >>> - return false; >>> - if (Current.Type == TT_CtorInitializerComma) >>> - return true; >>> - } >>> - if ((Previous.isOneOf(tok::comma, tok::semi) || >>> Current.is(tok::question) || >>> - (Current.Type == TT_ConditionalExpr && >>> - !(Current.is(tok::colon) && Previous.is(tok::question)))) && >>> + if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) || >>> + Current.is(tok::question) || >>> + (Current.Type == TT_ConditionalExpr && >>> Previous.isNot(tok::question))) && >>> State.Stack.back().BreakBeforeParameter && >>> !Current.isTrailingComment() && >>> !Current.isOneOf(tok::r_paren, tok::r_brace)) >>> return true; >>> @@ -250,8 +256,7 @@ void ContinuationIndenter::addTokenOnCur >>> if (Previous.opensScope() && Previous.Type != TT_ObjCMethodExpr && >>> Current.Type != TT_LineComment) >>> State.Stack.back().Indent = State.Column + Spaces; >>> - if (Previous.is(tok::comma) && !Current.isTrailingComment() && >>> - State.Stack.back().AvoidBinPacking) >>> + if (State.Stack.back().AvoidBinPacking && >>> startsNextParameter(Current, Style)) >>> State.Stack.back().NoLineBreak = true; >>> if (startsSegmentOfBuilderTypeCall(Current)) >>> State.Stack.back().ContainsUnwrappedBuilder = true; >>> >>> Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=192168&r1=192167&r2=192168&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original) >>> +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Oct 8 00:11:18 2013 >>> @@ -1366,7 +1366,8 @@ bool TokenAnnotator::mustBreakBefore(con >>> // FIXME: Fix horrible hack of using BindingStrength to find >>> top-level <>. >>> return true; >>> } else if (Right.Type == TT_CtorInitializerComma && >>> - Style.BreakConstructorInitializersBeforeComma) { >>> + Style.BreakConstructorInitializersBeforeComma && >>> + !Style.ConstructorInitializerAllOnOneLineOrOnePerLine) { >>> return true; >>> } else if (Right.Previous->BlockKind == BK_Block && >>> Right.Previous->isNot(tok::r_brace) && >>> @@ -1450,6 +1451,9 @@ bool TokenAnnotator::canBreakBefore(cons >>> if (Left.Type == TT_CtorInitializerComma && >>> Style.BreakConstructorInitializersBeforeComma) >>> return false; >>> + if (Right.Type == TT_CtorInitializerComma && >>> + Style.BreakConstructorInitializersBeforeComma) >>> + return true; >>> if (Right.isBinaryOperator() && Style.BreakBeforeBinaryOperators) >>> return true; >>> if (Left.is(tok::greater) && Right.is(tok::greater) && >>> >>> Modified: cfe/trunk/unittests/Format/FormatTest.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=192168&r1=192167&r2=192168&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/unittests/Format/FormatTest.cpp (original) >>> +++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Oct 8 00:11:18 2013 >>> @@ -6587,6 +6587,20 @@ TEST_F(FormatTest, ConstructorInitialize >>> ", b(b)\n" >>> ", c(c) {}", >>> Style); >>> + >>> + Style.ConstructorInitializerAllOnOneLineOrOnePerLine = true; >>> + Style.ConstructorInitializerIndentWidth = 4; >>> + verifyFormat( >>> + "SomeClass::Constructor()\n" >>> + " : aaaaaaaa(aaaaaaaa), aaaaaaaa(aaaaaaaa), aaaaaaaa(aaaaaaaa) >>> {}", >>> + Style); >>> + Style.ConstructorInitializerIndentWidth = 4; >>> + Style.ColumnLimit = 60; >>> + verifyFormat("SomeClass::Constructor()\n" >>> + " : aaaaaaaa(aaaaaaaa)\n" >>> + " , aaaaaaaa(aaaaaaaa)\n" >>> + " , aaaaaaaa(aaaaaaaa) {}", >>> + Style); >>> } >>> >>> TEST_F(FormatTest, FormatsWithWebKitStyle) { >>> >>> >>> _______________________________________________ >>> 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
