[PATCH] Fix bug in clang-format while merging short function (PR19461)

Daniel Jasper djasper at google.com
Mon Apr 28 07:38:17 PDT 2014


Ah right, this is for styles that break before the braces (I got confused).

Two small comments, otherwise this seems like a strict improvement now.

================
Comment at: lib/Format/Format.cpp:562
@@ -561,3 +561,3 @@
       unsigned MergedLines = 0;
-      if (MergeShortFunctions) {
+      if (MergeShortFunctions && !I[1]->First->MustBreakBefore) {
         MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);
----------------
How about moving the MustBreakBefore check to line 535, i.e.:

    if (I + 1 == E || I[1]->Type == LT_Invalid || I[1]->MustBreakBefore)
      return 0;

I think it is trivially correct and much easier to see that I[1] is guaranteed to exist.

================
Comment at: unittests/Format/FormatTest.cpp:7365
@@ +7364,3 @@
+
+  verifyFormat("#ifdef _DEBUG\n"
+               "int foo(int i = 0)\n"
----------------
Can you split these into smaller verifyFormat() calls that test each case (or smaller group of cases)? It has proven to be much easier to figure out what is going wrong if the tests are as small as possible. Otherwise you get a large diff in the test output and can't easily see what is going on. Same below.

http://reviews.llvm.org/D3439






More information about the cfe-commits mailing list