clang-format: Fixed line merging of more than two lines

[mxbOctasic]

Summary

getNextMergedLine merged pairs of adjacent lines instead of merging them all with the first one.

Consider AnnotatedLine A, B and C.

The function merges A with B then B with C and returns a pointer to A.
A.Last is not updated when B and C are merged together. (A.Last == B.Last)

Subsequent lines need to be merged with the first.
By merging A with B then with C, A.Last points to the proper token. (A.Last == C.Last)

[djasper]

Nice catch.

unittests/Format/FormatTest.cpp

285 ↗

(On Diff #53572)

How does this break? I generally add an Before and After of one of the test cases into my patch descriptions.

[mxbOctasic]

unittests/Format/FormatTest.cpp

285 ↗

(On Diff #53572)

This is a fun one.

The newline between Foo() {} and void Funk() {} is removed.
This is a combination of the line merge problem and KeepEmptyLinesAtTheStartOfBlocks.

Last on the merged line for Foo() {} points to { instead of }, so the newline before void is seen as a newline at the start of a block. (Checks PreviousLine->Last->is(tok::l_brace))

[djasper]

Sorry for the delay. Looks good.

[cameron314]

I'll commit this in a few days, when I have some time to reintegrate it to the trunk. Thanks!

cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp

const AnnotatedLine *getNextMergedLine(bool DryRun,
     if (MergedLines > 0 && Style.ColumnLimit == 0)
       // Disallow line merging if there is a break at the start of one of the
       // input lines.
       for (unsigned i = 0; i < MergedLines; ++i)
         if (Next[i + 1]->First->NewlinesBefore > 0)
           MergedLines = 0;
     if (!DryRun)
       for (unsigned i = 0; i < MergedLines; ++i)
-        join(*Next[i], *Next[i + 1]);
+        join(*Next[0], *Next[i + 1]);
     Next = Next + MergedLines + 1;
     return Current;
   }
 
 private:
   /// \brief Calculates how many lines can be merged into 1 starting at \p I.
   unsigned
   tryFitMultipleLinesInOne(unsigned Indent,

cfe/trunk/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, RemovesEmptyLines) {
   EXPECT_EQ("namespace {\n"
             "int i;\n"
             "\n"
             "} // namespace",
             format("namespace {\n"
                    "int i;\n"
                    "\n"
                    "}  // namespace"));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  Style.MaxEmptyLinesToKeep = 2;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = true;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.KeepEmptyLinesAtTheStartOfBlocks = false;
+
+  EXPECT_EQ("class Foo\n"
+            "{\n"
+            "  Foo() {}\n"
+            "\n"
+            "  void funk() {}\n"
+            "};",
+            format("class Foo\n"
+                   "{\n"
+                   "  Foo()\n"
+                   "  {\n"
+                   "  }\n"
+                   "\n"
+                   "  void funk() {}\n"
+                   "};",
+                   Style));
 }
 
 TEST_F(FormatTest, RecognizesBinaryOperatorKeywords) {
   verifyFormat("x = (a) and (b);");
   verifyFormat("x = (a) or (b);");
   verifyFormat("x = (a) bitand (b);");
   verifyFormat("x = (a) bitor (b);");
   verifyFormat("x = (a) not_eq (b);");