djasper added inline comments.
================
Comment at: lib/Format/UnwrappedLineFormatter.cpp:193
@@ -192,3 +192,3 @@
(Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
- I[1]->First->is(tok::r_brace)) ||
+ I[1]->First->isOneOf(tok::l_brace, tok::r_brace)) ||
(Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline &&
----------------
From a first look, this doesn't make sense. We specifically check that the next
line is a closing brace to ensure that this is an empty function. Now, this
seems to work with the existing tests, but because of very intricate reasons:
* If we are not wrapping with Allman/Linux style, the first token of I[1] is
unlikely to be an open brace. It can actually be one: void f() { {
DoSomething(); } }, but in this case there are too many unwrapped lines to
merge anything.
* There aren't any other tests with Allman/Linux style *and*
AllowShortFunctionsOnASingleLine set to "Empty". If there were, they'd show,
that this code now also merges non-empty functinos.
Instead, pull out an additional variable:
bool EmptyFunction = (Style.BraceWrapping.AfterFunction ? I[2] :
I[1])->First->is(tok::r_brace);
and then use that.
Please also add a test showing that non-empty functions do not get merged.
================
Comment at: unittests/Format/FormatTest.cpp:6523
@@ -6522,1 +6522,3 @@
+TEST_F(FormatTest, PullEmptyFunctionDefinitionsIntoSingleLineLinux) {
+ FormatStyle MergeEmptyLinux = getLLVMStyle();
----------------
Just extend PullInlineFunctionDefinitionsIntoSingleLine instead.
Repository:
rL LLVM
http://reviews.llvm.org/D13811
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
