On Fri, May 8, 2015 at 5:16 PM, Tobias Grosser <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
