[clang] [clang-format] Continue aligned lines better (PR #161903)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 3 13:38:52 PDT 2025
https://github.com/sstwcw created https://github.com/llvm/llvm-project/pull/161903
after with config `{AlignConsecutiveAssignments: true}`
```C++
auto aaaaaaaaaaaaaaaaaaaaa = {};
auto b = [] {
x = {.one_foooooooooooooooo = 2, //
.two_fooooooooooooo = 3, //
.three_fooooooooooooo = 4};
};
A B = {"Hello "
"World"};
BYTE payload = 2;
float i2 = 0;
auto v = type{i2 = 1, //
i = 3};
```
before
```C++
auto aaaaaaaaaaaaaaaaaaaaa = {};
auto b = [] {
x = {.one_foooooooooooooooo = 2, //
.two_fooooooooooooo = 3, //
.three_fooooooooooooo = 4};
};
A B = {"Hello "
"World"};
BYTE payload = 2;
float i2 = 0;
auto v = type{i2 = 1, //
i = 3};
```
When a line gets aligned, the following lines may need to move with it. This patch replaces the old algorithm with a simpler one. It uses the `IsAligned` attribute. It makes most of the scope stack irrelevant. Now the stack only needs to keep track of 2 levels.
The old algorithm had problems like these.
- Whether lines inside a braced list moved depended on whether there was a type at the start. It should depend on whether the inside was aligned to the brace. The first case that came up with a type name at the start happened to have a comma at the end of the list so the inside was not aligned to the brace.
- Excluding lines inside closures did not always work.
- A continued string could move twice as much as it should.
The following problems are not fixed yet.
A token that opens a scope is needed. Operator precedence is not enough.
```C++
auto aaaaaaaaaaaaaaaaaaaaa = {};
auto b = 0 + //
0;
```
The algorithm has trouble when things that should not move and things that should move are nested. This is related to the `IsAligned` attribute being a boolean. It also affects how spaces and tabs are selected.
```C++
auto aaaaaaaaaaaaaaaaaaaaa = {};
auto b = {.a = {
.a = 0,
}};
```
>From e220825c7db1b2acdcf048811de17b07ba6e3a0e Mon Sep 17 00:00:00 2001
From: sstwcw <su3e8a96kzlver at posteo.net>
Date: Fri, 3 Oct 2025 20:22:20 +0000
Subject: [PATCH] [clang-format] Continue aligned lines better
after with config `{AlignConsecutiveAssignments: true}`
```C++
auto aaaaaaaaaaaaaaaaaaaaa = {};
auto b = [] {
x = {.one_foooooooooooooooo = 2, //
.two_fooooooooooooo = 3, //
.three_fooooooooooooo = 4};
};
A B = {"Hello "
"World"};
BYTE payload = 2;
float i2 = 0;
auto v = type{i2 = 1, //
i = 3};
```
before
```C++
auto aaaaaaaaaaaaaaaaaaaaa = {};
auto b = [] {
x = {.one_foooooooooooooooo = 2, //
.two_fooooooooooooo = 3, //
.three_fooooooooooooo = 4};
};
A B = {"Hello "
"World"};
BYTE payload = 2;
float i2 = 0;
auto v = type{i2 = 1, //
i = 3};
```
When a line gets aligned, the following lines may need to move with it.
This patch replaces the old algorithm with a simpler one. It uses the
`IsAligned` attribute. It makes most of the scope stack irrelevant.
Now the stack only needs to keep track of 2 levels.
The old algorithm had problems like these.
- Whether lines inside a braced list moved depended on whether there was
a type at the start. It should depend on whether the inside was
aligned to the brace. The first case that came up with a type name at
the start happened to have a comma at the end of the list so the
inside was not aligned to the brace.
- Excluding lines inside closures did not always work.
- A continued string could move twice as much as it should.
The following problems are not fixed yet.
A token that opens a scope is needed. Operator precedence is not
enough.
```C++
auto aaaaaaaaaaaaaaaaaaaaa = {};
auto b = 0 + //
0;
```
The algorithm has trouble when things that should not move and things
that should move are nested. This is related to the `IsAligned`
attribute being a boolean. It also affects how spaces and tabs are
selected.
```C++
auto aaaaaaaaaaaaaaaaaaaaa = {};
auto b = {.a = {
.a = 0,
}};
```
---
clang/lib/Format/WhitespaceManager.cpp | 128 +++++--------------------
clang/unittests/Format/FormatTest.cpp | 48 ++++++++++
2 files changed, 74 insertions(+), 102 deletions(-)
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index 54f366fc02502..8e2f8cc8cdd4a 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -289,17 +289,20 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
SmallVector<WhitespaceManager::Change, 16> &Changes) {
int Shift = 0;
- // ScopeStack keeps track of the current scope depth. It contains indices of
- // the first token on each scope.
+ // 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
+ // in. The second one is the one that should not be moved by this procedure.
// The "Matches" indices should only have tokens from the outer-most scope.
// However, we do need to pay special attention to one class of tokens
- // that are not in the outer-most scope, and that is function parameters
- // which are split across multiple lines, as illustrated by this example:
+ // that are not in the outer-most scope, and that is the continuations of an
+ // unwrapped line whose positions are derived from a token to the right of the
+ // aligned token, as illustrated by this example:
// double a(int x);
// int b(int y,
// double z);
// In the above example, we need to take special care to ensure that
- // 'double z' is indented along with it's owning function 'b'.
+ // 'double z' is indented along with its owning function 'b', because its
+ // position is derived from the '(' token to the right of the 'b' token.
// The same holds for calling a function:
// double a = foo(x);
// int b = bar(foo(y),
@@ -309,32 +312,28 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
// auto s = "Hello"
// "World";
// Special handling is required for 'nested' ternary operators.
- SmallVector<unsigned, 16> ScopeStack;
+ SmallVector<std::tuple<unsigned, unsigned, unsigned>, 2> ScopeStack;
for (unsigned i = Start; i != End; ++i) {
auto &CurrentChange = Changes[i];
if (!Matches.empty() && Matches[0] < i)
Matches.consume_front();
assert(Matches.empty() || Matches[0] >= i);
- if (!ScopeStack.empty() &&
- CurrentChange.indentAndNestingLevel() <
- Changes[ScopeStack.back()].indentAndNestingLevel()) {
+ while (!ScopeStack.empty() &&
+ CurrentChange.indentAndNestingLevel() < ScopeStack.back()) {
ScopeStack.pop_back();
}
- // Compare current token to previous non-comment token to ensure whether
- // it is in a deeper scope or not.
- unsigned PreviousNonComment = i - 1;
- while (PreviousNonComment > Start &&
- Changes[PreviousNonComment].Tok->is(tok::comment)) {
- --PreviousNonComment;
- }
- if (i != Start && CurrentChange.indentAndNestingLevel() >
- Changes[PreviousNonComment].indentAndNestingLevel()) {
- ScopeStack.push_back(i);
+ // Keep track of the level that should not move with the aligned token.
+ if (ScopeStack.size() == 1u && CurrentChange.NewlinesBefore != 0u &&
+ CurrentChange.indentAndNestingLevel() > ScopeStack[0] &&
+ !CurrentChange.IsAligned) {
+ ScopeStack.push_back(CurrentChange.indentAndNestingLevel());
}
- bool InsideNestedScope = !ScopeStack.empty();
+ bool InsideNestedScope =
+ !ScopeStack.empty() &&
+ CurrentChange.indentAndNestingLevel() > ScopeStack[0];
bool ContinuedStringLiteral = i > Start &&
CurrentChange.Tok->is(tok::string_literal) &&
Changes[i - 1].Tok->is(tok::string_literal);
@@ -349,94 +348,22 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
if (!Matches.empty() && Matches[0] == i) {
Shift = Column - (RightJustify ? CurrentChange.TokenLength : 0) -
CurrentChange.StartOfTokenColumn;
+ ScopeStack = {CurrentChange.indentAndNestingLevel()};
CurrentChange.Spaces += Shift;
}
if (Shift == 0)
continue;
- // This is for function parameters that are split across multiple lines,
- // as mentioned in the ScopeStack comment.
- if (InsideNestedScope && CurrentChange.NewlinesBefore > 0) {
- unsigned ScopeStart = ScopeStack.back();
+ // This is for lines that are split across multiple lines, as mentioned in
+ // the ScopeStack comment. The stack size being 1 means that the token is
+ // not in a scope that should not move.
+ if (ScopeStack.size() == 1u && CurrentChange.NewlinesBefore > 0) {
auto ShouldShiftBeAdded = [&] {
- // Function declaration
- if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName))
- return true;
-
- // Lambda.
- if (Changes[ScopeStart - 1].Tok->is(TT_LambdaLBrace))
- return false;
-
- // Continued function declaration
- if (ScopeStart > Start + 1 &&
- Changes[ScopeStart - 2].Tok->is(TT_FunctionDeclarationName)) {
- return true;
- }
-
- // Continued (template) function call.
- if (ScopeStart > Start + 1 &&
- Changes[ScopeStart - 2].Tok->isOneOf(tok::identifier,
- TT_TemplateCloser) &&
- Changes[ScopeStart - 1].Tok->is(tok::l_paren) &&
- Changes[ScopeStart].Tok->isNot(TT_LambdaLSquare)) {
- if (CurrentChange.Tok->MatchingParen &&
- CurrentChange.Tok->MatchingParen->is(TT_LambdaLBrace)) {
- return false;
- }
- if (Changes[ScopeStart].NewlinesBefore > 0)
- return false;
- if (CurrentChange.Tok->is(tok::l_brace) &&
- CurrentChange.Tok->is(BK_BracedInit)) {
- return true;
- }
- return Style.BinPackArguments;
- }
-
- // Ternary operator
- if (CurrentChange.Tok->is(TT_ConditionalExpr))
- return true;
-
- // Period Initializer .XXX = 1.
- if (CurrentChange.Tok->is(TT_DesignatedInitializerPeriod))
- return true;
-
- // Continued ternary operator
- if (CurrentChange.Tok->Previous &&
- CurrentChange.Tok->Previous->is(TT_ConditionalExpr)) {
- return true;
- }
-
- // Continued direct-list-initialization using braced list.
- if (ScopeStart > Start + 1 &&
- Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
- Changes[ScopeStart - 1].Tok->is(tok::l_brace) &&
- CurrentChange.Tok->is(tok::l_brace) &&
- CurrentChange.Tok->is(BK_BracedInit)) {
- return true;
- }
-
- // Continued braced list.
- if (ScopeStart > Start + 1 &&
- Changes[ScopeStart - 2].Tok->isNot(tok::identifier) &&
- Changes[ScopeStart - 1].Tok->is(tok::l_brace) &&
- CurrentChange.Tok->isNot(tok::r_brace)) {
- for (unsigned OuterScopeStart : llvm::reverse(ScopeStack)) {
- // Lambda.
- if (OuterScopeStart > Start &&
- Changes[OuterScopeStart - 1].Tok->is(TT_LambdaLBrace)) {
- return false;
- }
- }
- if (Changes[ScopeStart].NewlinesBefore > 0)
- return false;
+ if (ContinuedStringLiteral)
return true;
- }
-
- // Continued template parameter.
- if (Changes[ScopeStart - 1].Tok->is(TT_TemplateOpener))
+ if (InsideNestedScope && CurrentChange.IsAligned)
return true;
-
return false;
};
@@ -444,9 +371,6 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
CurrentChange.Spaces += Shift;
}
- if (ContinuedStringLiteral)
- CurrentChange.Spaces += Shift;
-
// We should not remove required spaces unless we break the line before.
assert(Shift > 0 || Changes[i].NewlinesBefore > 0 ||
CurrentChange.Spaces >=
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index fef70365b5e18..b3aebd50cf400 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -19575,6 +19575,12 @@ TEST_F(FormatTest, AlignConsecutiveAssignments) {
" return;\n"
"};",
Alignment);
+ verifyFormat("auto aaaaaaaaaaaaaaaaaaaaa = {};\n"
+ "auto b = g([] {\n"
+ " return \"Hello \"\n"
+ " \"World\";\n"
+ "});",
+ Alignment);
verifyFormat("auto aaaaaaaaaaaaaaaaaaaaa = {};\n"
"auto b = g([] {\n"
" f();\n"
@@ -20192,6 +20198,11 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
" i = 3 //\n"
"};",
Alignment);
+ // When assignments are nested, each level should be aligned.
+ verifyFormat("float i2 = 0;\n"
+ "auto v = type{i2 = 1, //\n"
+ " i = 3};",
+ Alignment);
Alignment.AlignConsecutiveAssignments.Enabled = false;
verifyFormat(
@@ -20681,6 +20692,21 @@ TEST_F(FormatTest, AlignWithLineBreaks) {
"}",
Style);
+ verifyFormat("void foo() {\n"
+ " int myVar = 5;\n"
+ " double x = 3.14;\n"
+ " auto str = (\"Hello \"\n"
+ " \"World\");\n"
+ " auto s = (\"Hello \"\n"
+ " \"Again\");\n"
+ "}",
+ Style);
+
+ verifyFormat("A B = {\"Hello \"\n"
+ " \"World\"};\n"
+ "BYTE payload = 2;",
+ Style);
+
// clang-format off
verifyFormat("void foo() {\n"
" const int capacityBefore = Entries.capacity();\n"
@@ -20763,6 +20789,28 @@ TEST_F(FormatTest, AlignWithInitializerPeriods) {
"}",
Style);
+ // The lines inside the braces are supposed to be indented by
+ // BracedInitializerIndentWidth from the start of the line. They should not
+ // move with the opening brace.
+ verifyFormat("void foo2(void) {\n"
+ " BYTE p[1] = 1;\n"
+ " A B = {\n"
+ " .one_foooooooooooooooo = 2,\n"
+ " .two_fooooooooooooo = 3,\n"
+ " .three_fooooooooooooo = 4,\n"
+ " };\n"
+ " BYTE payload = 2;\n"
+ "}",
+ Style);
+
+ verifyFormat("auto aaaaaaaaaaaaaaaaaaaaa = {};\n"
+ "auto b = g([] {\n"
+ " x = {.one_foooooooooooooooo = 2, //\n"
+ " .two_fooooooooooooo = 3, //\n"
+ " .three_fooooooooooooo = 4};\n"
+ "});",
+ Style);
+
Style.AlignConsecutiveAssignments.Enabled = false;
Style.AlignConsecutiveDeclarations.Enabled = true;
verifyFormat("void foo3(void) {\n"
More information about the cfe-commits
mailing list