[clang-format] Don't unwrap lines preceded by line comments

[owenpan]

Summary

Fixes https://github.com/llvm/llvm-project/issues/53495.

[curdeius]

LGTM.

[owenpan]

Reset the Spaces of the closing brace only if it can be unwrapped.

[MyDeveloperDay]

I'm wondering if the presence of the comment would impact the CellCount, I might go back and add some more unit tests for the "non rectangular" change I made.

So this fixes the "}" going up onto the comment line, but not the indentation right?

[owenpan]

In D121576#3378934, @MyDeveloperDay wrote:

I'm wondering if the presence of the comment would impact the CellCount, I might go back and add some more unit tests for the "non rectangular" change I made.

So this fixes the "}" going up onto the comment line, but not the indentation right?

Right. I think we should have a specification (including when not to align the cells) and re-design the entire thing. It's not maintainable as is. I have some ideas but need to remove/change some of the existing test cases, e.g., those involving concatenated string literals. If you (and @curdeius and @HazardyKnusperkeks) agree, I can give it a shot.

[MyDeveloperDay]

Please do, to be honest I spent enough time on this and I got to the same conclusion, it needs a rethink. Please go ahead, I don't use this feature myself so I'm not likely to see all the issues.

[curdeius]

In D121576#3381295, @owenpan wrote:

Right. I think we should have a specification (including when not to align the cells) and re-design the entire thing. It's not maintainable as is. I have some ideas but need to remove/change some of the existing test cases, e.g., those involving concatenated string literals. If you (and @curdeius and @HazardyKnusperkeks) agree, I can give it a shot.

It will be appreciated!

clang/lib/Format/WhitespaceManager.cpp

for (auto i = 0U; i < CellDescs.CellCounts[0]; ++i, ++CellIter) {
     if (isSplitCell(*CellIter))
       NetWidth = getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
     auto CellWidth = getMaximumCellWidth(CellIter, NetWidth);
 
     if (Changes[CellIter->Index].Tok->is(tok::r_brace)) {
       // So in here we want to see if there is a brace that falls
       // on a line that was split. If so on that line we make sure that
       // the spaces in front of the brace are enough.
-      Changes[CellIter->Index].NewlinesBefore = 0;
-      Changes[CellIter->Index].Spaces = 0;
-      for (const auto *Next = CellIter->NextColumnElement; Next != nullptr;
-           Next = Next->NextColumnElement) {
+      const auto *Next = CellIter;
+      do {
+        const FormatToken *Previous = Changes[Next->Index].Tok->Previous;
+        if (Previous && Previous->isNot(TT_LineComment)) {
           Changes[Next->Index].Spaces = 0;
           Changes[Next->Index].NewlinesBefore = 0;
         }
+        Next = Next->NextColumnElement;
+      } while (Next);
       // Unless the array is empty, we need the position of all the
       // immediately adjacent cells
       if (CellIter != Cells.begin()) {
         auto ThisNetWidth =
             getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
         auto MaxNetWidth = getMaximumNetWidth(
             Cells.begin(), CellIter, CellDescs.InitialSpaces,
             CellDescs.CellCounts[0], CellDescs.CellCounts.size());

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
   // This caused a core dump by enabling Alignment in the LLVMStyle globally
   Style = getLLVMStyleWithColumns(50);
   Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
   verifyFormat("static A x = {\n"
                "    {{init1, init2, init3, init4},\n"
                "     {init1, init2, init3, init4}}\n"
                "};",
                Style);
+  // TODO: Fix the indentations below when this option is fully functional.
+  verifyFormat("int a[][] = {\n"
+               "    {\n"
+               "     {0, 2}, //\n"
+               " {1, 2}  //\n"
+               "    }\n"
+               "};",
+               Style);
   Style.ColumnLimit = 100;
   EXPECT_EQ(
       "test demo[] = {\n"
       "    {56,    23,\n"
       "     \"hello world i am a very long line that really, in any just world"
       ", ought to be split over \"\n"
       "     \"multiple lines\"  },\n"
       "    {-1, 93463, \"world\"},\n"