r192168 - clang-format: Improve constructor initializer linewrapping.

[Daniel Jasper]

On Tue, Oct 8, 2013 at 2:20 PM, Manuel Klimek <klimek at google.com> wrote:

> On Mon, Oct 7, 2013 at 10:11 PM, Daniel Jasper <djasper at google.com> 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.
>> +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.

+}
>> +
>>  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
>> 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/20131008/83ff7f35/attachment.html>