[PATCH] D13811: [clang-format] AllowShortFunctionsOnASingleLine: true/Empty didn't work with BreakBeforeBraces: Linux/Allman.

[Daniel Jasper via cfe-commits]

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