<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Apr 21, 2014 at 3:33 PM, Dinesh Dwivedi <span dir="ltr"><<a href="mailto:dinesh.d@samsung.com" target="_blank">dinesh.d@samsung.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi djasper, chapuni, klimek,<br>
<br>
if Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All, clang-format tries to join short functions and prints in single line.<br>
This code was not taking care of cases where there is something in between function header and body e.g. attached test cases where<br>
there are preprocessor directives in between function header and body.<br>
<br>
Attached patch fixes this issue by checking if we have both header and body and nothing between them while trying to merge functions.<br>
<br>
Please let me know if this is ok.<br>
<br>
TEST PLAN<br>
Added testcases for different BreakBeforeBraces styles.<br>
<br>
<a href="http://reviews.llvm.org/D3439" target="_blank">http://reviews.llvm.org/D3439</a><br>
<br>
Files:<br>
lib/Format/Format.cpp<br>
unittests/Format/FormatTest.cpp<br>
<br>
Index: lib/Format/Format.cpp<br>
===================================================================<br>
--- lib/Format/Format.cpp<br>
+++ lib/Format/Format.cpp<br>
@@ -559,7 +559,7 @@<br>
Limit -= 2;<br>
<br>
unsigned MergedLines = 0;<br>
- if (MergeShortFunctions) {<br>
+ if (MergeShortFunctions && TheLine->MustBeDeclaration) {<br></blockquote><div><br></div><div>I don't think this is the right fix; I think we'll need to figure out whether there is a "gap" in the line, and in that case not merge it.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);<br>
// If we managed to merge the block, count the function header, which is<br>
// on a separate line.<br>
Index: unittests/Format/FormatTest.cpp<br>
===================================================================<br>
--- unittests/Format/FormatTest.cpp<br>
+++ unittests/Format/FormatTest.cpp<br>
@@ -7336,6 +7336,26 @@<br>
"}\n"<br>
"}",<br>
BreakBeforeBrace);<br>
+<br>
+ verifyFormat("#ifdef _DEBUG\n"<br>
+ "int foo(int i = 0)\n"<br>
+ "#else\n"<br>
+ "int foo(int i = 5)\n"<br>
+ "#endif\n"<br>
+ "{\n"<br>
+ " return i;\n"<br>
+ "}\n"<br>
+ "void foo1()\n"<br>
+ "#ifdef _DEBUG\n"<br>
+ "{\n"<br>
+ " return 0;\n"<br>
+ "}\n"<br>
+ "#else\n"<br>
+ "{\n"<br>
+ " return 5;\n"<br>
+ "}\n"<br>
+ "#endif",<br>
+ BreakBeforeBrace);<br>
}<br>
<br>
TEST_F(FormatTest, AllmanBraceBreaking) {<br>
@@ -7418,6 +7438,26 @@<br>
"}\n",<br>
BreakBeforeBrace);<br>
<br>
+ verifyFormat("#ifdef _DEBUG\n"<br>
+ "int foo(int i = 0)\n"<br>
+ "#else\n"<br>
+ "int foo(int i = 5)\n"<br>
+ "#endif\n"<br>
+ "{\n"<br>
+ " return i;\n"<br>
+ "}\n"<br>
+ "void foo1()\n"<br>
+ "#ifdef _DEBUG\n"<br>
+ "{\n"<br>
+ " return 0;\n"<br>
+ "}\n"<br>
+ "#else\n"<br>
+ "{\n"<br>
+ " return 5;\n"<br>
+ "}\n"<br>
+ "#endif",<br>
+ BreakBeforeBrace);<br>
+<br>
BreakBeforeBrace.ColumnLimit = 19;<br>
verifyFormat("void f() { int i; }", BreakBeforeBrace);<br>
BreakBeforeBrace.ColumnLimit = 18;<br>
@@ -7538,6 +7578,26 @@<br>
" Y = 0,\n"<br>
"}\n",<br>
GNUBraceStyle);<br>
+<br>
+ verifyFormat("#ifdef _DEBUG\n"<br>
+ "int foo(int i = 0)\n"<br>
+ "#else\n"<br>
+ "int foo(int i = 5)\n"<br>
+ "#endif\n"<br>
+ "{\n"<br>
+ " return i;\n"<br>
+ "}\n"<br>
+ "void foo1()\n"<br>
+ "#ifdef _DEBUG\n"<br>
+ "{\n"<br>
+ " return 0;\n"<br>
+ "}\n"<br>
+ "#else\n"<br>
+ "{\n"<br>
+ " return 5;\n"<br>
+ "}\n"<br>
+ "#endif",<br>
+ GNUBraceStyle);<br>
}<br>
TEST_F(FormatTest, CatchExceptionReferenceBinding) {<br>
verifyFormat("void f() {\n"<br>
</blockquote></div><br></div></div>