r236851 - clang-format: Several improvements around formatting braced lists.
Daniel Jasper
djasper at google.com
Fri May 8 08:18:05 PDT 2015
On Fri, May 8, 2015 at 5:16 PM, Tobias Grosser <tobias at grosser.es> wrote:
> On 05/08/2015 03:51 PM, Daniel Jasper wrote:
>
>> Author: djasper
>> Date: Fri May 8 08:51:14 2015
>> New Revision: 236851
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=236851&view=rev
>> Log:
>> clang-format: Several improvements around formatting braced lists.
>>
>> In particular:
>> * If the difference between the longest and shortest element, we copped
>> out of column format completely. Now, we instead allow to arrange
>> these in a single column, essentially enforcing a one-per-line format.
>> * Allow column layout even if there are braced lists. Especially, if
>> there are many short lists, this can be beneficial. The bad case,
>> where there is a long nested init list is usually caught as we now
>> limit the length difference of the longest and shortest element.
>>
>
> Hi Daniel,
>
> this change caused the following formatting change in Polly:
>
> - Value *Args[] = {SubFn, SubFnParam, NumberOfThreads, LB, UB, Stride};
> + Value *Args[] = {SubFn,
> + SubFnParam,
> + NumberOfThreads,
> + LB,
> + UB,
> + Stride};
>
> I will change this in Polly. It might be useful to add some test case
> that shows that in certain cases we want to format a list in column layout
> even though it fits a single line.
Do we? Do you actually think the new format is better?
> Best,
> Tobias
>
>
> Modified:
>> cfe/trunk/lib/Format/FormatToken.cpp
>> cfe/trunk/unittests/Format/FormatTest.cpp
>>
>> Modified: cfe/trunk/lib/Format/FormatToken.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.cpp?rev=236851&r1=236850&r2=236851&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Format/FormatToken.cpp (original)
>> +++ cfe/trunk/lib/Format/FormatToken.cpp Fri May 8 08:51:14 2015
>> @@ -199,13 +199,14 @@ void CommaSeparatedList::precomputeForma
>> // create a column layout. If it has a nested list, column layout
>> ensures one
>> // list element per line. If the difference between the shortest and
>> longest
>> // element is too large, column layout would create too much
>> whitespace.
>> - if (HasNestedBracedList || Commas.size() < 5 || Token->NestingLevel !=
>> 0 ||
>> - MaxItemLength - MinItemLength > 10)
>> + if (Commas.size() < 5 || Token->NestingLevel != 0)
>> return;
>>
>> // We can never place more than ColumnLimit / 3 items in a row
>> (because of the
>> // spaces and the comma).
>> - for (unsigned Columns = 1; Columns <= Style.ColumnLimit / 3;
>> ++Columns) {
>> + unsigned MaxColumns =
>> + MaxItemLength - MinItemLength > 10 ? 1 : Style.ColumnLimit / 3;
>> + for (unsigned Columns = 1; Columns <= MaxColumns; ++Columns) {
>> ColumnFormat Format;
>> Format.Columns = Columns;
>> Format.ColumnSizes.resize(Columns);
>>
>> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=236851&r1=236850&r2=236851&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
>> +++ cfe/trunk/unittests/Format/FormatTest.cpp Fri May 8 08:51:14 2015
>> @@ -6223,9 +6223,18 @@ TEST_F(FormatTest, LayoutCxx11BraceIniti
>> ExtraSpaces);
>> verifyFormat(
>> "std::vector<MyValues> aaaaaaaaaaaaaaaaaaa{\n"
>> - " aaaaaaa, aaaaaaaaaa, aaaaa, aaaaaaaaaaaaaaa, aaa, aaaaaaaaaa,
>> a,\n"
>> - " aaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaa,\n"
>> - " aaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaa, aaaaaaa, a};");
>> + " aaaaaaa,\n"
>> + " aaaaaaaaaa,\n"
>> + " aaaaa,\n"
>> + " aaaaaaaaaaaaaaa,\n"
>> + " aaa,\n"
>> + " aaaaaaaaaa,\n"
>> + " a,\n"
>> + " aaaaaaaaaaaaaaaaaaaaa,\n"
>> + " aaaaaaaaaaaa,\n"
>> + " aaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaa,\n"
>> + " aaaaaaa,\n"
>> + " a};");
>> verifyFormat("vector<int> foo = { ::SomeGlobalFunction() };",
>> ExtraSpaces);
>> }
>>
>> @@ -6263,6 +6272,19 @@ TEST_F(FormatTest, FormatsBracedListsInC
>> verifyFormat("vector<int> aaaaaaaaaaaaaaaaaaaaaa = {\n"
>> " 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1};",
>> getLLVMStyleWithColumns(43));
>> + verifyFormat(
>> + "static unsigned SomeValues[10][3] = {\n"
>> + " {1, 4, 0}, {4, 9, 0}, {4, 5, 9}, {8, 5, 4}, {1, 8, 4},\n"
>> + " {10, 1, 6}, {11, 0, 9}, {2, 11, 9}, {5, 2, 9}, {11, 2, 7}};");
>> + verifyFormat("static auto fields = new vector<string>{\n"
>> + " \"aaaaaaaaaaaaa\",\n"
>> + " \"aaaaaaaaaaaaa\",\n"
>> + " \"aaaaaaaaaaaa\",\n"
>> + " \"aaaaaaaaaaaaaa\",\n"
>> + " \"aaaaaaaaaaaaaaaaaaaaaaaaa\",\n"
>> + " \"aaaaaaaaaaaa\",\n"
>> + " \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\n"
>> + "};");
>>
>> // Trailing commas.
>> verifyFormat("vector<int> x = {\n"
>> @@ -6277,15 +6299,15 @@ TEST_F(FormatTest, FormatsBracedListsInC
>> " 1, 1, 1, 1,\n"
>> " /**/ /**/};",
>> getLLVMStyleWithColumns(39));
>> +
>> + // With nested lists, we should either format one item per line or all
>> nested
>> + // lists one on line.
>> + // FIXME: For some nested lists, we can do better.
>> verifyFormat("return {{aaaaaaaaaaaaaaaaaaaaa},\n"
>> " {aaaaaaaaaaaaaaaaaaa},\n"
>> " {aaaaaaaaaaaaaaaaaaaaa},\n"
>> " {aaaaaaaaaaaaaaaaa}};",
>> getLLVMStyleWithColumns(60));
>> -
>> - // With nested lists, we should either format one item per line or all
>> nested
>> - // lists one one line.
>> - // FIXME: For some nested lists, we can do better.
>> verifyFormat(
>> "SomeStruct my_struct_array = {\n"
>> " {aaaaaa, aaaaaaaa, aaaaaaaaaa, aaaaaaaaa, aaaaaaaaa,
>> aaaaaaaaaa,\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/20150508/d2bac17f/attachment.html>
More information about the cfe-commits
mailing list