galenelias created this revision.
galenelias added a project: clang-format.
Herald added projects: All, clang.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
galenelias requested review of this revision.

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

Currently AlignArrayOfStructures=Left is hard coding setting `Spaces` to
`0` for the token following the initial opening brace, but not touching
`Spaces` for the subsequent lines, which leads to the array being
misaligned.  Additionally, it's not adding a space before the trailing
`}` which is generally done when Cpp11BracedListStyle=false.

I'm not exactly sure why this function needs to override the `Spaces` as
it seems to generally already be set to either `0` or `1` according to
the other formatting settings, but I'm going with an explicit fix where
I just force the padding to `1` when Cpp11BracedListStyle=false.

AlignArrayOfStructures=Right doesn't have any alignment problems, but
isn't adding the expected padding around the braces either, so I'm
giving that the same treatment.


https://reviews.llvm.org/D158795

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20880,6 +20880,16 @@
                "});\n",
                Style);
 
+  Style.Cpp11BracedListStyle = false;
+  verifyFormat("struct test demo[] = {\n"
+               "  { 56,    23, \"hello\" },\n"
+               "  { -1, 93463, \"world\" },\n"
+               "  {  7,     5,    \"!!\" }\n"
+               "};\n",
+               Style);
+
+  Style.Cpp11BracedListStyle = true;
+
   Style.ColumnLimit = 0;
   EXPECT_EQ(
       "test demo[] = {\n"
@@ -21112,6 +21122,15 @@
                "});\n",
                Style);
 
+  Style.Cpp11BracedListStyle = false;
+  verifyFormat("struct test demo[] = {\n"
+               "  { 56, 23,    \"hello\" },\n"
+               "  { -1, 93463, \"world\" },\n"
+               "  { 7,  5,     \"!!\"    }\n"
+               "};\n",
+               Style);
+  Style.Cpp11BracedListStyle = true;
+
   Style.ColumnLimit = 0;
   EXPECT_EQ(
       "test demo[] = {\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===================================================================
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1226,6 +1226,7 @@
   if (!CellDescs.isRectangular())
     return;
 
+  const auto BracePadding = Style.Cpp11BracedListStyle ? 0 : 1;
   auto &Cells = CellDescs.Cells;
   // Now go through and fixup the spaces.
   auto *CellIter = Cells.begin();
@@ -1243,7 +1244,7 @@
       do {
         const FormatToken *Previous = Changes[Next->Index].Tok->Previous;
         if (Previous && Previous->isNot(TT_LineComment)) {
-          Changes[Next->Index].Spaces = 0;
+          Changes[Next->Index].Spaces = BracePadding;
           Changes[Next->Index].NewlinesBefore = 0;
         }
         Next = Next->NextColumnElement;
@@ -1276,7 +1277,7 @@
           NetWidth;
       if (Changes[CellIter->Index].NewlinesBefore == 0) {
         Changes[CellIter->Index].Spaces = (CellWidth - (ThisWidth + NetWidth));
-        Changes[CellIter->Index].Spaces += (i > 0) ? 1 : 0;
+        Changes[CellIter->Index].Spaces += (i > 0) ? 1 : BracePadding;
       }
       alignToStartOfCell(CellIter->Index, CellIter->EndIndex);
       for (const auto *Next = CellIter->NextColumnElement; Next;
@@ -1285,7 +1286,7 @@
             calculateCellWidth(Next->Index, Next->EndIndex, true) + NetWidth;
         if (Changes[Next->Index].NewlinesBefore == 0) {
           Changes[Next->Index].Spaces = (CellWidth - ThisWidth);
-          Changes[Next->Index].Spaces += (i > 0) ? 1 : 0;
+          Changes[Next->Index].Spaces += (i > 0) ? 1 : BracePadding;
         }
         alignToStartOfCell(Next->Index, Next->EndIndex);
       }
@@ -1299,12 +1300,13 @@
   if (!CellDescs.isRectangular())
     return;
 
+  const auto BracePadding = Style.Cpp11BracedListStyle ? 0 : 1;
   auto &Cells = CellDescs.Cells;
   // Now go through and fixup the spaces.
   auto *CellIter = Cells.begin();
   // The first cell needs to be against the left brace.
   if (Changes[CellIter->Index].NewlinesBefore == 0)
-    Changes[CellIter->Index].Spaces = 0;
+    Changes[CellIter->Index].Spaces = BracePadding;
   else
     Changes[CellIter->Index].Spaces = CellDescs.InitialSpaces;
   ++CellIter;
@@ -1317,7 +1319,7 @@
     if (Changes[CellIter->Index].NewlinesBefore == 0) {
       Changes[CellIter->Index].Spaces =
           MaxNetWidth - ThisNetWidth +
-          (Changes[CellIter->Index].Tok->isNot(tok::r_brace) ? 1 : 0);
+          (Changes[CellIter->Index].Tok->isNot(tok::r_brace) ? 1 : BracePadding);
     }
     auto RowCount = 1U;
     auto Offset = std::distance(Cells.begin(), CellIter);
@@ -1331,7 +1333,7 @@
       if (Changes[Next->Index].NewlinesBefore == 0) {
         Changes[Next->Index].Spaces =
             MaxNetWidth - ThisNetWidth +
-            (Changes[Next->Index].Tok->isNot(tok::r_brace) ? 1 : 0);
+            (Changes[Next->Index].Tok->isNot(tok::r_brace) ? 1 : BracePadding);
       }
       ++RowCount;
     }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to