Skip to content

Commit 0b63b05

Browse files
[clang-format] Respect ColumnLimit while aligning multiline expressions
Before the patch the added test case would indent the function and moving its second line beyond the column limit.
1 parent 2505df0 commit 0b63b05

File tree

2 files changed

+83
-19
lines changed

2 files changed

+83
-19
lines changed

clang/lib/Format/WhitespaceManager.cpp

Lines changed: 59 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -502,15 +502,15 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
502502
MatchedIndices.clear();
503503
};
504504

505-
unsigned i = StartAt;
506-
for (unsigned e = Changes.size(); i != e; ++i) {
507-
auto &CurrentChange = Changes[i];
505+
unsigned I = StartAt;
506+
for (unsigned E = Changes.size(); I != E; ++I) {
507+
auto &CurrentChange = Changes[I];
508508
if (CurrentChange.indentAndNestingLevel() < IndentAndNestingLevel)
509509
break;
510510

511511
if (CurrentChange.NewlinesBefore != 0) {
512512
CommasBeforeMatch = 0;
513-
EndOfSequence = i;
513+
EndOfSequence = I;
514514

515515
// Whether to break the alignment sequence because of an empty line.
516516
bool EmptyLineBreak =
@@ -526,8 +526,8 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
526526

527527
// A new line starts, re-initialize line status tracking bools.
528528
// Keep the match state if a string literal is continued on this line.
529-
if (i == 0 || CurrentChange.Tok->isNot(tok::string_literal) ||
530-
Changes[i - 1].Tok->isNot(tok::string_literal)) {
529+
if (I == 0 || CurrentChange.Tok->isNot(tok::string_literal) ||
530+
Changes[I - 1].Tok->isNot(tok::string_literal)) {
531531
FoundMatchOnLine = false;
532532
}
533533
LineIsComment = true;
@@ -541,8 +541,8 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
541541
} else if (CurrentChange.indentAndNestingLevel() > IndentAndNestingLevel) {
542542
// Call AlignTokens recursively, skipping over this scope block.
543543
unsigned StoppedAt =
544-
AlignTokens(Style, Matches, Changes, i, ACS, RightJustify);
545-
i = StoppedAt - 1;
544+
AlignTokens(Style, Matches, Changes, I, ACS, RightJustify);
545+
I = StoppedAt - 1;
546546
continue;
547547
}
548548

@@ -552,37 +552,77 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
552552
// If there is more than one matching token per line, or if the number of
553553
// preceding commas, do not match anymore, end the sequence.
554554
if (FoundMatchOnLine || CommasBeforeMatch != CommasBeforeLastMatch) {
555-
MatchedIndices.push_back(i);
555+
MatchedIndices.push_back(I);
556556
AlignCurrentSequence();
557557
}
558558

559559
CommasBeforeLastMatch = CommasBeforeMatch;
560560
FoundMatchOnLine = true;
561561

562562
if (StartOfSequence == 0)
563-
StartOfSequence = i;
563+
StartOfSequence = I;
564564

565565
unsigned ChangeWidthLeft = CurrentChange.StartOfTokenColumn;
566566
unsigned ChangeWidthAnchor = 0;
567567
unsigned ChangeWidthRight = 0;
568+
unsigned CurrentChangeWidthRight = 0;
568569
if (RightJustify)
569570
if (ACS.PadOperators)
570571
ChangeWidthAnchor = CurrentChange.TokenLength;
571572
else
572573
ChangeWidthLeft += CurrentChange.TokenLength;
573574
else
574-
ChangeWidthRight = CurrentChange.TokenLength;
575-
for (unsigned j = i + 1; j != e && Changes[j].NewlinesBefore == 0; ++j) {
576-
ChangeWidthRight += Changes[j].Spaces;
575+
CurrentChangeWidthRight = CurrentChange.TokenLength;
576+
const FormatToken *MatchingParenToEncounter = nullptr;
577+
for (unsigned J = I + 1;
578+
J != E && (Changes[J].NewlinesBefore == 0 || MatchingParenToEncounter);
579+
++J) {
580+
const auto &Change = Changes[J];
581+
const auto *Tok = Change.Tok;
582+
583+
if (Tok->MatchingParen) {
584+
if (Tok->isOneOf(tok::l_paren, tok::l_brace, tok::l_square,
585+
TT_TemplateOpener) &&
586+
!MatchingParenToEncounter) {
587+
// If the next token is on the next line, we probably don't need to
588+
// check the following lengths, because it most likely isn't aligned
589+
// with the rest.
590+
if (J + 1 != E && Changes[J + 1].NewlinesBefore == 0)
591+
MatchingParenToEncounter = Tok->MatchingParen;
592+
} else if (MatchingParenToEncounter == Tok->MatchingParen) {
593+
MatchingParenToEncounter = nullptr;
594+
}
595+
}
596+
597+
if (Change.NewlinesBefore != 0) {
598+
ChangeWidthRight = std::max(ChangeWidthRight, CurrentChangeWidthRight);
599+
const auto ChangeWidthStart = ChangeWidthLeft + ChangeWidthAnchor;
600+
// If the position of the current token is columnwise before the begin
601+
// of the alignment, we drop out here, because the next line does not
602+
// have to be moved with the previous one(s) for the alignment. E.g.:
603+
// int i1 = 1; | <- ColumnLimit | int i1 = 1;
604+
// int j = 0; | Without the break -> | int j = 0;
605+
// int k = bar( | We still want to align the = | int k = bar(
606+
// argument1, | here, even if we can't move | argument1,
607+
// argument2); | the following lines. | argument2);
608+
if (static_cast<unsigned>(Change.Spaces) < ChangeWidthStart)
609+
break;
610+
CurrentChangeWidthRight = Change.Spaces - ChangeWidthStart;
611+
} else {
612+
CurrentChangeWidthRight += Change.Spaces;
613+
}
614+
577615
// Changes are generally 1:1 with the tokens, but a change could also be
578616
// inside of a token, in which case it's counted more than once: once for
579617
// the whitespace surrounding the token (!IsInsideToken) and once for
580618
// each whitespace change within it (IsInsideToken).
581619
// Therefore, changes inside of a token should only count the space.
582-
if (!Changes[j].IsInsideToken)
583-
ChangeWidthRight += Changes[j].TokenLength;
620+
if (!Change.IsInsideToken)
621+
CurrentChangeWidthRight += Change.TokenLength;
584622
}
585623

624+
ChangeWidthRight = std::max(ChangeWidthRight, CurrentChangeWidthRight);
625+
586626
// If we are restricted by the maximum column width, end the sequence.
587627
unsigned NewLeft = std::max(ChangeWidthLeft, WidthLeft);
588628
unsigned NewAnchor = std::max(ChangeWidthAnchor, WidthAnchor);
@@ -591,7 +631,7 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
591631
if (Style.ColumnLimit != 0 &&
592632
Style.ColumnLimit < NewLeft + NewAnchor + NewRight) {
593633
AlignCurrentSequence();
594-
StartOfSequence = i;
634+
StartOfSequence = I;
595635
WidthLeft = ChangeWidthLeft;
596636
WidthAnchor = ChangeWidthAnchor;
597637
WidthRight = ChangeWidthRight;
@@ -600,12 +640,12 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
600640
WidthAnchor = NewAnchor;
601641
WidthRight = NewRight;
602642
}
603-
MatchedIndices.push_back(i);
643+
MatchedIndices.push_back(I);
604644
}
605645

606-
EndOfSequence = i;
646+
EndOfSequence = I;
607647
AlignCurrentSequence();
608-
return i;
648+
return I;
609649
}
610650

611651
// Aligns a sequence of matching tokens, on the MinColumn column.

clang/unittests/Format/FormatTest.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20772,6 +20772,30 @@ TEST_F(FormatTest, AlignWithLineBreaks) {
2077220772
"}",
2077320773
Style);
2077420774
// clang-format on
20775+
20776+
Style = getLLVMStyleWithColumns(70);
20777+
Style.AlignConsecutiveDeclarations.Enabled = true;
20778+
verifyFormat(
20779+
"ReturnType\n"
20780+
"MyFancyIntefaceFunction(Context *context,\n"
20781+
" ALongTypeName *response) noexcept override;\n"
20782+
"ReturnType func();",
20783+
Style);
20784+
20785+
verifyFormat(
20786+
"ReturnType\n"
20787+
"MyFancyIntefaceFunction(B<int> *context,\n"
20788+
" decltype(AFunc) *response) noexcept override;\n"
20789+
"ReturnType func();",
20790+
Style);
20791+
20792+
Style.AlignConsecutiveAssignments.Enabled = true;
20793+
Style.ColumnLimit = 15;
20794+
verifyFormat("int i1 = 1;\n"
20795+
"k = bar(\n"
20796+
" argument1,\n"
20797+
" argument2);",
20798+
Style);
2077520799
}
2077620800

2077720801
TEST_F(FormatTest, AlignWithInitializerPeriods) {

0 commit comments

Comments
 (0)