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