Skip to content

Commit

Permalink
[clang-format] Fix AlignArrayOfStructures + Cpp11BracedListStyle=false
Browse files Browse the repository at this point in the history
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.

Fixes #57611.

Differential Revision: https://reviews.llvm.org/D158795
  • Loading branch information
Galen Elias authored and owenca committed Aug 31, 2023
1 parent c9ecaf3 commit 58c67e7
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
15 changes: 9 additions & 6 deletions clang/lib/Format/WhitespaceManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1230,6 +1230,7 @@ void WhitespaceManager::alignArrayInitializersRightJustified(
if (!CellDescs.isRectangular())
return;

const int BracePadding = Style.Cpp11BracedListStyle ? 0 : 1;
auto &Cells = CellDescs.Cells;
// Now go through and fixup the spaces.
auto *CellIter = Cells.begin();
Expand All @@ -1247,7 +1248,7 @@ void WhitespaceManager::alignArrayInitializersRightJustified(
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;
Expand Down Expand Up @@ -1280,7 +1281,7 @@ void WhitespaceManager::alignArrayInitializersRightJustified(
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;
Expand All @@ -1289,7 +1290,7 @@ void WhitespaceManager::alignArrayInitializersRightJustified(
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);
}
Expand All @@ -1303,12 +1304,13 @@ void WhitespaceManager::alignArrayInitializersLeftJustified(
if (!CellDescs.isRectangular())
return;

const int 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;
Expand All @@ -1321,7 +1323,8 @@ void WhitespaceManager::alignArrayInitializersLeftJustified(
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);
Expand All @@ -1335,7 +1338,7 @@ void WhitespaceManager::alignArrayInitializersLeftJustified(
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;
}
Expand Down
18 changes: 18 additions & 0 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20651,6 +20651,15 @@ TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
"});",
Style);

Style.Cpp11BracedListStyle = false;
verifyFormat("struct test demo[] = {\n"
" { 56, 23, \"hello\" },\n"
" { -1, 93463, \"world\" },\n"
" { 7, 5, \"!!\" }\n"
"};",
Style);
Style.Cpp11BracedListStyle = true;

Style.ColumnLimit = 0;
verifyFormat(
"test demo[] = {\n"
Expand Down Expand Up @@ -20882,6 +20891,15 @@ TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) {
"});",
Style);

Style.Cpp11BracedListStyle = false;
verifyFormat("struct test demo[] = {\n"
" { 56, 23, \"hello\" },\n"
" { -1, 93463, \"world\" },\n"
" { 7, 5, \"!!\" }\n"
"};",
Style);
Style.Cpp11BracedListStyle = true;

Style.ColumnLimit = 0;
verifyFormat(
"test demo[] = {\n"
Expand Down

0 comments on commit 58c67e7

Please sign in to comment.