[clang] 7ab2ff4 - [clang-format] Fix incorrect trailing comment and escaped newlines when AlignArrayOfStructures is enabled (#180305)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Mar 14 00:05:53 PDT 2026
Author: Ben Dunkin
Date: 2026-03-14T00:05:47-07:00
New Revision: 7ab2ff44d3bfbfed0452c7701d1627e72acf2452
URL: https://github.com/llvm/llvm-project/commit/7ab2ff44d3bfbfed0452c7701d1627e72acf2452
DIFF: https://github.com/llvm/llvm-project/commit/7ab2ff44d3bfbfed0452c7701d1627e72acf2452.diff
LOG: [clang-format] Fix incorrect trailing comment and escaped newlines when AlignArrayOfStructures is enabled (#180305)
This change fixes how the spaces are modified during alignment.
Previously it was inconsistent whether the `StartOfTokenColumn` and
`PreviousEndOfTokenColumn` members of `WhitespaceManager::Change`s were
also updated when their `Spaces` member was changed to align tokens.
A new function has been added that properly maintains the relationship
between these members, and all places that directly modified `Spaces`
have been replaced with calls to this new function.
Fixes https://github.com/llvm/llvm-project/issues/138151. Fixes
https://github.com/llvm/llvm-project/issues/85937. Fixes
https://github.com/llvm/llvm-project/issues/53442. Tests have been added
to ensure they stay fixed.
Attribution Note - I have been authorized to contribute this change on
behalf of my company: ArenaNet LLC
Added:
Modified:
clang/lib/Format/WhitespaceManager.cpp
clang/lib/Format/WhitespaceManager.h
clang/unittests/Format/FormatTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index 947578cbff9ad..0a79e708269e2 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -284,6 +284,43 @@ void WhitespaceManager::calculateLineBreakInformation() {
}
}
+// Sets the spaces in front of a Change, and updates the start/end columns of
+// subsequent tokens so that trailing comments and escaped newlines can be
+// aligned properly.
+static void
+SetChangeSpaces(unsigned Start, unsigned Spaces,
+ MutableArrayRef<WhitespaceManager::Change> Changes) {
+ auto &FirstChange = Changes[Start];
+ const int ColumnChange = Spaces - FirstChange.Spaces;
+
+ if (ColumnChange == 0)
+ return;
+
+ FirstChange.Spaces += ColumnChange;
+ FirstChange.StartOfTokenColumn += ColumnChange;
+
+ for (auto I = Start + 1; I < Changes.size(); I++) {
+ auto &Change = Changes[I];
+
+ Change.PreviousEndOfTokenColumn += ColumnChange;
+
+ if (Change.NewlinesBefore > 0)
+ break;
+
+ Change.StartOfTokenColumn += ColumnChange;
+ }
+}
+
+// Changes the spaces in front of a change by Delta, and updates the start/end
+// columns of subsequent tokens so that trailing comments and escaped newlines
+// can be aligned properly.
+static void
+IncrementChangeSpaces(unsigned Start, int Delta,
+ MutableArrayRef<WhitespaceManager::Change> Changes) {
+ assert(Delta > 0 || (abs(Delta) <= Changes[Start].Spaces));
+ SetChangeSpaces(Start, Changes[Start].Spaces + Delta, Changes);
+}
+
// Align a single sequence of tokens, see AlignTokens below.
// Column - The tokens indexed in Matches are moved to this column.
// RightJustify - Whether it is the token's right end or left end that gets
@@ -295,9 +332,6 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
SmallVector<WhitespaceManager::Change, 16> &Changes) {
unsigned OriginalMatchColumn = 0;
int Shift = 0;
- // Set when the shift is applied anywhere in the line. Cleared when the line
- // ends.
- bool LineShifted = false;
// ScopeStack keeps track of the current scope depth. It contains the levels
// of at most 2 scopes. The first one is the one that the matched token is
@@ -347,11 +381,8 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
(CurrentChange.indentAndNestingLevel() == ScopeStack[0] &&
CurrentChange.IndentedFromColumn >= OriginalMatchColumn));
- if (CurrentChange.NewlinesBefore > 0) {
- LineShifted = false;
- if (!InsideNestedScope)
- Shift = 0;
- }
+ if (CurrentChange.NewlinesBefore > 0 && !InsideNestedScope)
+ Shift = 0;
// If this is the first matching token to be aligned, remember by how many
// spaces it has to be shifted, so the rest of the changes on the line are
@@ -372,9 +403,8 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
if ((!Matches.empty() && Matches[0] == i) ||
(ScopeStack.size() == 1u && CurrentChange.NewlinesBefore > 0 &&
InsideNestedScope)) {
- LineShifted = true;
CurrentChange.IndentedFromColumn += Shift;
- CurrentChange.Spaces += Shift;
+ IncrementChangeSpaces(i, Shift, Changes);
}
// We should not remove required spaces unless we break the line before.
@@ -383,12 +413,6 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
static_cast<int>(Changes[i].Tok->SpacesRequiredBefore) ||
CurrentChange.Tok->is(tok::eof));
- if (LineShifted) {
- CurrentChange.StartOfTokenColumn += Shift;
- if (i + 1 != Changes.size())
- Changes[i + 1].PreviousEndOfTokenColumn += Shift;
- }
-
// If PointerAlignment is PAS_Right, keep *s or &s next to the token,
// except if the token is equal, then a space is needed.
if ((Style.PointerAlignment == FormatStyle::PAS_Right ||
@@ -409,9 +433,9 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
} else if (Style.PointerAlignment != FormatStyle::PAS_Right) {
continue;
}
- Changes[Previous + 1].Spaces -= Shift;
- Changes[Previous].Spaces += Shift;
- Changes[Previous].StartOfTokenColumn += Shift;
+
+ IncrementChangeSpaces(Previous + 1, -Shift, Changes);
+ IncrementChangeSpaces(Previous, Shift, Changes);
}
}
}
@@ -692,27 +716,19 @@ static void AlignMatchingTokenSequence(
SmallVector<WhitespaceManager::Change, 16> &Changes) {
if (StartOfSequence > 0 && StartOfSequence < EndOfSequence) {
bool FoundMatchOnLine = false;
- int Shift = 0;
for (unsigned I = StartOfSequence; I != EndOfSequence; ++I) {
- if (Changes[I].NewlinesBefore > 0) {
- Shift = 0;
+ if (Changes[I].NewlinesBefore > 0)
FoundMatchOnLine = false;
- }
// If this is the first matching token to be aligned, remember by how many
// spaces it has to be shifted, so the rest of the changes on the line are
// shifted by the same amount.
if (!FoundMatchOnLine && Matches(Changes[I])) {
FoundMatchOnLine = true;
- Shift = MinColumn - Changes[I].StartOfTokenColumn;
- Changes[I].Spaces += Shift;
+ int Shift = MinColumn - Changes[I].StartOfTokenColumn;
+ IncrementChangeSpaces(I, Shift, Changes);
}
-
- assert(Shift >= 0);
- Changes[I].StartOfTokenColumn += Shift;
- if (I + 1 != Changes.size())
- Changes[I + 1].PreviousEndOfTokenColumn += Shift;
}
}
@@ -1064,7 +1080,10 @@ void WhitespaceManager::alignTrailingComments() {
// leave the comments.
if (RestoredLineLength >= Style.ColumnLimit && Style.ColumnLimit > 0)
break;
- C.Spaces = C.NewlinesBefore > 0 ? C.Tok->OriginalColumn : OriginalSpaces;
+
+ int Spaces =
+ C.NewlinesBefore > 0 ? C.Tok->OriginalColumn : OriginalSpaces;
+ setChangeSpaces(I, Spaces);
continue;
}
@@ -1185,10 +1204,8 @@ void WhitespaceManager::alignTrailingComments(unsigned Start, unsigned End,
}
if (Shift <= 0)
continue;
- Changes[i].Spaces += Shift;
- if (i + 1 != Changes.size())
- Changes[i + 1].PreviousEndOfTokenColumn += Shift;
- Changes[i].StartOfTokenColumn += Shift;
+
+ setChangeSpaces(i, Changes[i].Spaces + Shift);
}
}
@@ -1294,8 +1311,8 @@ void WhitespaceManager::alignArrayInitializersRightJustified(
do {
const FormatToken *Previous = Changes[Next->Index].Tok->Previous;
if (Previous && Previous->isNot(TT_LineComment)) {
- Changes[Next->Index].Spaces = BracePadding;
Changes[Next->Index].NewlinesBefore = 0;
+ setChangeSpaces(Next->Index, BracePadding);
}
Next = Next->NextColumnElement;
} while (Next);
@@ -1308,7 +1325,7 @@ void WhitespaceManager::alignArrayInitializersRightJustified(
Cells.begin(), CellIter, CellDescs.InitialSpaces,
CellDescs.CellCounts[0], CellDescs.CellCounts.size());
if (ThisNetWidth < MaxNetWidth)
- Changes[CellIter->Index].Spaces = (MaxNetWidth - ThisNetWidth);
+ setChangeSpaces(CellIter->Index, MaxNetWidth - ThisNetWidth);
auto RowCount = 1U;
auto Offset = std::distance(Cells.begin(), CellIter);
for (const auto *Next = CellIter->NextColumnElement; Next;
@@ -1319,7 +1336,7 @@ void WhitespaceManager::alignArrayInitializersRightJustified(
auto *End = Start + Offset;
ThisNetWidth = getNetWidth(Start, End, CellDescs.InitialSpaces);
if (ThisNetWidth < MaxNetWidth)
- Changes[Next->Index].Spaces = (MaxNetWidth - ThisNetWidth);
+ setChangeSpaces(Next->Index, MaxNetWidth - ThisNetWidth);
++RowCount;
}
}
@@ -1328,8 +1345,10 @@ void WhitespaceManager::alignArrayInitializersRightJustified(
calculateCellWidth(CellIter->Index, CellIter->EndIndex, true) +
NetWidth;
if (Changes[CellIter->Index].NewlinesBefore == 0) {
- Changes[CellIter->Index].Spaces = (CellWidth - (ThisWidth + NetWidth));
- Changes[CellIter->Index].Spaces += (i > 0) ? 1 : BracePadding;
+ int Spaces = (CellWidth - (ThisWidth + NetWidth));
+ Spaces += (i > 0) ? 1 : BracePadding;
+
+ setChangeSpaces(CellIter->Index, Spaces);
}
alignToStartOfCell(CellIter->Index, CellIter->EndIndex);
for (const auto *Next = CellIter->NextColumnElement; Next;
@@ -1337,8 +1356,10 @@ void WhitespaceManager::alignArrayInitializersRightJustified(
ThisWidth =
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 : BracePadding;
+ int Spaces = (CellWidth - ThisWidth);
+ Spaces += (i > 0) ? 1 : BracePadding;
+
+ setChangeSpaces(Next->Index, Spaces);
}
alignToStartOfCell(Next->Index, Next->EndIndex);
}
@@ -1360,8 +1381,9 @@ void WhitespaceManager::alignArrayInitializersLeftJustified(
// The first cell of every row needs to be against the left brace.
for (const auto *Next = CellIter; Next; Next = Next->NextColumnElement) {
auto &Change = Changes[Next->Index];
- Change.Spaces =
+ int Spaces =
Change.NewlinesBefore == 0 ? BracePadding : CellDescs.InitialSpaces;
+ setChangeSpaces(Next->Index, Spaces);
}
++CellIter;
for (auto i = 1U; i < CellDescs.CellCounts[0]; i++, ++CellIter) {
@@ -1371,10 +1393,11 @@ void WhitespaceManager::alignArrayInitializersLeftJustified(
auto ThisNetWidth =
getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
if (Changes[CellIter->Index].NewlinesBefore == 0) {
- Changes[CellIter->Index].Spaces =
+ int Spaces =
MaxNetWidth - ThisNetWidth +
(Changes[CellIter->Index].Tok->isNot(tok::r_brace) ? 1
: BracePadding);
+ setChangeSpaces(CellIter->Index, Spaces);
}
auto RowCount = 1U;
auto Offset = std::distance(Cells.begin(), CellIter);
@@ -1386,9 +1409,10 @@ void WhitespaceManager::alignArrayInitializersLeftJustified(
auto *End = Start + Offset;
auto ThisNetWidth = getNetWidth(Start, End, CellDescs.InitialSpaces);
if (Changes[Next->Index].NewlinesBefore == 0) {
- Changes[Next->Index].Spaces =
+ int Spaces =
MaxNetWidth - ThisNetWidth +
(Changes[Next->Index].Tok->isNot(tok::r_brace) ? 1 : BracePadding);
+ setChangeSpaces(Next->Index, Spaces);
}
++RowCount;
}
@@ -1466,11 +1490,11 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start,
Changes[j].Tok->isNot(tok::r_brace)) {
Changes[j].NewlinesBefore = 1;
// Account for the added token lengths
- Changes[j].Spaces = InitialSpaces - InitialTokenLength;
+ setChangeSpaces(j, InitialSpaces - InitialTokenLength);
}
} else if (C.Tok->is(tok::comment) && C.Tok->NewlinesBefore == 0) {
// Trailing comments stay at a space past the last token
- C.Spaces = Changes[i - 1].Tok->is(tok::comma) ? 1 : 2;
+ setChangeSpaces(i, Changes[i - 1].Tok->is(tok::comma) ? 1 : 2);
} else if (C.Tok->is(tok::l_brace)) {
// We need to make sure that the ending braces is aligned to the
// start of our initializer
@@ -1481,7 +1505,7 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start,
}
} else if (Depth == 0 && C.Tok->is(tok::r_brace)) {
C.NewlinesBefore = 1;
- C.Spaces = EndSpaces;
+ setChangeSpaces(i, EndSpaces);
}
if (C.Tok->StartsColumn) {
// This gets us past tokens that have been split over multiple
@@ -1509,12 +1533,12 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start,
auto LineLimit = Changes[j].Spaces + Changes[j].TokenLength;
if (LineLimit < Style.ColumnLimit) {
Changes[i].NewlinesBefore = 0;
- Changes[i].Spaces = 1;
+ setChangeSpaces(i, 1);
}
}
}
while (Changes[i].NewlinesBefore > 0 && Changes[i].Tok == C.Tok) {
- Changes[i].Spaces = InitialSpaces;
+ setChangeSpaces(i, InitialSpaces);
++i;
HasSplit = true;
}
@@ -1546,7 +1570,7 @@ void WhitespaceManager::alignToStartOfCell(unsigned Start, unsigned End) {
// is aligned to the parent
for (auto i = Start + 1; i < End; i++)
if (Changes[i].NewlinesBefore > 0)
- Changes[i].Spaces = Changes[Start].Spaces;
+ setChangeSpaces(i, Changes[Start].Spaces);
}
WhitespaceManager::CellDescriptions
@@ -1565,6 +1589,10 @@ WhitespaceManager::linkCells(CellDescriptions &&CellDesc) {
return std::move(CellDesc);
}
+void WhitespaceManager::setChangeSpaces(unsigned Start, unsigned Spaces) {
+ SetChangeSpaces(Start, Spaces, Changes);
+}
+
void WhitespaceManager::generateChanges() {
for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
const Change &C = Changes[i];
diff --git a/clang/lib/Format/WhitespaceManager.h b/clang/lib/Format/WhitespaceManager.h
index 64a8f9b4fa857..9b6cde54af0af 100644
--- a/clang/lib/Format/WhitespaceManager.h
+++ b/clang/lib/Format/WhitespaceManager.h
@@ -357,6 +357,8 @@ class WhitespaceManager {
/// Link the Cell pointers in the list of Cells.
static CellDescriptions linkCells(CellDescriptions &&CellDesc);
+ void setChangeSpaces(unsigned Start, unsigned Spaces);
+
/// Fill \c Replaces with the replacements for all effective changes.
void generateChanges();
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index d066b3f482b21..a18d21914501f 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -22694,6 +22694,68 @@ TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) {
Style);
}
+TEST_F(FormatTest, AlignArrayOfStructuresGithubIssues) {
+ // https://github.com/llvm/llvm-project/issues/138151
+ // Summary: Aligning arrays of structures with UseTab: AlignWithSpaces does
+ // not use spaces to align columns
+ FormatStyle Style = getGoogleStyle();
+ Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+ Style.UseTab = FormatStyle::UT_AlignWithSpaces;
+ Style.IndentWidth = 4;
+ Style.TabWidth = 4;
+
+ verifyFormat(
+ "std::vector<Foo> foos = {\n"
+ "\t{LONG_NAME, 0, i | j},\n"
+ "\t{LONG_NAME, 0, i | j},\n"
+ "\t{LONGER_NAME, 0, i | j},\n"
+ "\t{LONGER_NAME, 0, i },\n"
+ "\t{THIS_IS_A_VERY_LONG_NAME, 0, j },\n"
+ "\t{LONGER_NAME, THIS_IS_A_VERY_LONG_NAME, i },\n"
+ "\t{LONG_NAME, THIS_IS_A_VERY_LONG_NAME, j }\n"
+ "};\n",
+ Style);
+
+ // https://github.com/llvm/llvm-project/issues/85937
+ // Summary: Macro escaped newlines are not aligned properly when both
+ // AlignEscapedNewLines and AlignArrayOfStructures are used
+ Style = getLLVMStyleWithColumns(80);
+ Style.AlignEscapedNewlines = FormatStyle::ENAS_Left;
+ Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+
+ verifyFormat(R"(
+#define DEFINE_COMMAND_PROCESS_TABLE(Enum) \
+ const STExample TCommand::EXPL_MAIN[] = { \
+ {Enum::GetName(), " shows help " }, \
+ {Enum::GetAttribute(), " do something "}, \
+ {Enum::GetState(), " do whatever " }, \
+ };
+)",
+ Style);
+
+ // https://github.com/llvm/llvm-project/issues/53442
+ // Summary: alignment of columns does not use spaces when UseTab:
+ // AlignWithSpaces
+ Style = getLLVMStyle();
+ Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+ Style.IndentWidth = 4;
+ Style.TabWidth = 4;
+ Style.UseTab = FormatStyle::UT_AlignWithSpaces;
+ Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+
+ verifyFormat(
+ "const map<string, int64_t> CoreReport::GetGameCountersRolloverInfo()\n"
+ "{\n"
+ "\tstatic map<string, int64_t> counterRolloverInfo{\n"
+ "\t\t{\"CashIn\", 4000000000},\n"
+ "\t\t{\"CoinIn\", 4000000000},\n"
+ "\t\t{\"QuantityMultiProgressive\", 65535 },\n"
+ "\t};\n"
+ "\treturn counterRolloverInfo;\n"
+ "}\n",
+ Style);
+}
+
TEST_F(FormatTest, UnderstandsPragmas) {
verifyFormat("#pragma omp reduction(| : var)");
verifyFormat("#pragma omp reduction(+ : var)");
More information about the cfe-commits
mailing list