r196378 - Leave constructor initializer lists on one line in styles with no column limit.

Alexander Kornienko alexfh at google.com
Mon Dec 16 07:23:53 PST 2013


I've committed the patch posted on the other thread. Starting from r197386
clang-format forces line break before the colon in constructor initializer
lists when formatting with WebKit style.


On Sat, Dec 14, 2013 at 5:07 AM, Nico Weber <thakis at chromium.org> wrote:

> Huh, why this change? The only style without a column limit is WebKit,
> which explicitly asks for one line per initializer.
>
>
> On Wed, Dec 4, 2013 at 4:21 AM, Alexander Kornienko <alexfh at google.com>wrote:
>
>> Author: alexfh
>> Date: Wed Dec  4 06:21:08 2013
>> New Revision: 196378
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=196378&view=rev
>> Log:
>> Leave constructor initializer lists on one line in styles with no column
>> limit.
>>
>> Summary:
>> Allow tryFitMultipleLinesInOne join unwrapped lines when
>> ContinuationIndenter::mustBreak doesn't agree. But don't merge any lines,
>> that
>> are separate in the input.
>>
>> Reviewers: djasper
>>
>> Reviewed By: djasper
>>
>> CC: cfe-commits, klimek
>>
>> Differential Revision: http://llvm-reviews.chandlerc.com/D2321
>>
>> Modified:
>>     cfe/trunk/lib/Format/Format.cpp
>>     cfe/trunk/unittests/Format/FormatTest.cpp
>>
>> Modified: cfe/trunk/lib/Format/Format.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=196378&r1=196377&r2=196378&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Format/Format.cpp (original)
>> +++ cfe/trunk/lib/Format/Format.cpp Wed Dec  4 06:21:08 2013
>> @@ -439,12 +439,13 @@ public:
>>
>>    /// \brief Formats the line starting at \p State, simply keeping all
>> of the
>>    /// input's line breaking decisions.
>> -  void format(unsigned FirstIndent, const AnnotatedLine *Line) {
>> +  void format(unsigned FirstIndent, const AnnotatedLine *Line,
>> +              bool LineIsMerged) {
>>      LineState State =
>>          Indenter->getInitialState(FirstIndent, Line, /*DryRun=*/false);
>>      while (State.NextToken != NULL) {
>>        bool Newline =
>> -          Indenter->mustBreak(State) ||
>> +          (!LineIsMerged && Indenter->mustBreak(State)) ||
>>            (Indenter->canBreak(State) && State.NextToken->NewlinesBefore
>> > 0);
>>        Indenter->addTokenToState(State, Newline, /*DryRun=*/false);
>>      }
>> @@ -468,9 +469,6 @@ public:
>>      if (TheLine->Last->Type == TT_LineComment)
>>        return 0;
>>
>> -    if (Indent > Style.ColumnLimit)
>> -      return 0;
>> -
>>      unsigned Limit =
>>          Style.ColumnLimit == 0 ? UINT_MAX : Style.ColumnLimit - Indent;
>>      // If we already exceed the column limit, we set 'Limit' to 0. The
>> different
>> @@ -479,6 +477,9 @@ public:
>>                  ? 0
>>                  : Limit - TheLine->Last->TotalLength;
>>
>> +    if (Indent > Limit)
>> +      return 0;
>> +
>>      if (I + 1 == E || I[1]->Type == LT_Invalid)
>>        return 0;
>>
>> @@ -658,6 +659,14 @@ public:
>>        if (static_cast<int>(Indent) + Offset >= 0)
>>          Indent += Offset;
>>        unsigned MergedLines = Joiner.tryFitMultipleLinesInOne(Indent, I,
>> E);
>> +      if (MergedLines > 0 && Style.ColumnLimit == 0) {
>> +        // Disallow line merging if there is a break at the start of one
>> of the
>> +        // input lines.
>> +        for (unsigned i = 0; i < MergedLines; ++i) {
>> +          if (I[i + 1]->First->NewlinesBefore > 0)
>> +            MergedLines = 0;
>> +        }
>> +      }
>>        if (!DryRun) {
>>          for (unsigned i = 0; i < MergedLines; ++i) {
>>            join(*I[i], *I[i + 1]);
>> @@ -702,7 +711,8 @@ public:
>>            // FIXME: Implement nested blocks for ColumnLimit = 0.
>>            NoColumnLimitFormatter Formatter(Indenter);
>>            if (!DryRun)
>> -            Formatter.format(Indent, &TheLine);
>> +            Formatter.format(Indent, &TheLine,
>> +                             /*LineIsMerged=*/MergedLines > 0);
>>          } else {
>>            Penalty += format(TheLine, Indent, DryRun);
>>          }
>>
>> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=196378&r1=196377&r2=196378&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
>> +++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Dec  4 06:21:08 2013
>> @@ -4891,6 +4891,28 @@ TEST_F(FormatTest, PullTrivialFunctionDe
>>
>>    verifyFormat("void f() {}", getLLVMStyleWithColumns(11));
>>    verifyFormat("void f() {\n}", getLLVMStyleWithColumns(10));
>> +
>> +  FormatStyle NoColumnLimit = getLLVMStyle();
>> +  NoColumnLimit.ColumnLimit = 0;
>> +  EXPECT_EQ("A() : b(0) {}", format("A():b(0){}", NoColumnLimit));
>> +  EXPECT_EQ("class C {\n"
>> +            "  A() : b(0) {}\n"
>> +            "};", format("class C{A():b(0){}};", NoColumnLimit));
>> +  EXPECT_EQ("A()\n"
>> +            "    : b(0) {\n"
>> +            "}",
>> +            format("A()\n:b(0)\n{\n}", NoColumnLimit));
>> +
>> +  FormatStyle DoNotMergeNoColumnLimit = NoColumnLimit;
>> +  DoNotMergeNoColumnLimit.AllowShortFunctionsOnASingleLine = false;
>> +  EXPECT_EQ("A()\n"
>> +            "    : b(0) {\n"
>> +            "}",
>> +            format("A():b(0){}", DoNotMergeNoColumnLimit));
>> +  EXPECT_EQ("A()\n"
>> +            "    : b(0) {\n"
>> +            "}",
>> +            format("A()\n:b(0)\n{\n}", DoNotMergeNoColumnLimit));
>>  }
>>
>>  TEST_F(FormatTest, UnderstandContextOfRecordTypeKeywords) {
>> @@ -7340,12 +7362,18 @@ TEST_F(FormatTest, FormatsWithWebKitStyl
>>
>>    // Constructor initializers are formatted one per line with the "," on
>> the
>>    // new line.
>> -  verifyFormat("Constructor()\n"
>> -               "    :
>> aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
>> -               "    , aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaa, //
>> break\n"
>> -               "                               aaaaaaaaaaaaaa)\n"
>> -               "    , aaaaaaaaaaaaaaaaaaaaaaa() {}",
>> -               Style);
>> +  EXPECT_EQ(
>> +      "Constructor()\n"
>> +      "    : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
>> +      "    , aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaa, // break\n"
>> +      "                               aaaaaaaaaaaaaa)\n"
>> +      "    , aaaaaaaaaaaaaaaaaaaaaaa() {}",
>> +      format("Constructor()\n"
>> +             "    :
>> aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
>> +             "    , aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaa, // break\n"
>> +             "                               aaaaaaaaaaaaaa)\n"
>> +             "    , aaaaaaaaaaaaaaaaaaaaaaa() {}",
>> +             Style));
>>
>>    // Access specifiers should be aligned left.
>>    verifyFormat("class C {\n"
>>
>>
>> _______________________________________________
>> 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/20131216/61d29510/attachment.html>


More information about the cfe-commits mailing list