r192168 - clang-format: Improve constructor initializer linewrapping.

[Manuel Klimek]

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'?


> +}
> +
>  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/f6f46fc8/attachment.html>