r196378 - Leave constructor initializer lists on one line in styles with no column limit.
Nico Weber
thakis at chromium.org
Mon Dec 16 12:21:58 PST 2013
Thanks!
Current head now does:
thakis$ echo "A():b(0),c(0) {}" | Release+Asserts/bin/clang-format
-style=WebKit
A()
: b(0)
, c(0) {}
The {} is still wrong according to
http://www.webkit.org/coding/coding-style.html "Function definitions: place
each brace on its own line."
What was the motivation for b/10132568 ? The bug looks wrong to me.
On Mon, Dec 16, 2013 at 7:23 AM, Alexander Kornienko <alexfh at google.com>wrote:
> 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/9ae1feea/attachment.html>
More information about the cfe-commits
mailing list