Skip to content

Commit 87a08c8

Browse files
HazardyKnusperkeksaokblast
authored andcommitted
[clang-format] Respect ColumnLimit while aligning multiline expressions (llvm#163863)
Before the patch the added test case would indent the function and moving its second line beyond the column limit. Fixes llvm#68122.
1 parent 3159802 commit 87a08c8

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
@@ -506,15 +506,15 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
506506
MatchedIndices.clear();
507507
};
508508

509-
unsigned i = StartAt;
510-
for (unsigned e = Changes.size(); i != e; ++i) {
511-
auto &CurrentChange = Changes[i];
509+
unsigned I = StartAt;
510+
for (unsigned E = Changes.size(); I != E; ++I) {
511+
auto &CurrentChange = Changes[I];
512512
if (CurrentChange.indentAndNestingLevel() < IndentAndNestingLevel)
513513
break;
514514

515515
if (CurrentChange.NewlinesBefore != 0) {
516516
CommasBeforeMatch = 0;
517-
EndOfSequence = i;
517+
EndOfSequence = I;
518518

519519
// Whether to break the alignment sequence because of an empty line.
520520
bool EmptyLineBreak =
@@ -530,8 +530,8 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
530530

531531
// A new line starts, re-initialize line status tracking bools.
532532
// Keep the match state if a string literal is continued on this line.
533-
if (i == 0 || CurrentChange.Tok->isNot(tok::string_literal) ||
534-
Changes[i - 1].Tok->isNot(tok::string_literal)) {
533+
if (I == 0 || CurrentChange.Tok->isNot(tok::string_literal) ||
534+
Changes[I - 1].Tok->isNot(tok::string_literal)) {
535535
FoundMatchOnLine = false;
536536
}
537537
LineIsComment = true;
@@ -547,8 +547,8 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
547547
IndentAndNestingLevel) {
548548
// Call AlignTokens recursively, skipping over this scope block.
549549
const auto StoppedAt =
550-
AlignTokens(Style, Matches, Changes, i, ACS, RightJustify);
551-
i = StoppedAt - 1;
550+
AlignTokens(Style, Matches, Changes, I, ACS, RightJustify);
551+
I = StoppedAt - 1;
552552
continue;
553553
}
554554
}
@@ -559,37 +559,77 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
559559
// If there is more than one matching token per line, or if the number of
560560
// preceding commas, do not match anymore, end the sequence.
561561
if (FoundMatchOnLine || CommasBeforeMatch != CommasBeforeLastMatch) {
562-
MatchedIndices.push_back(i);
562+
MatchedIndices.push_back(I);
563563
AlignCurrentSequence();
564564
}
565565

566566
CommasBeforeLastMatch = CommasBeforeMatch;
567567
FoundMatchOnLine = true;
568568

569569
if (StartOfSequence == 0)
570-
StartOfSequence = i;
570+
StartOfSequence = I;
571571

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

631+
ChangeWidthRight = std::max(ChangeWidthRight, CurrentChangeWidthRight);
632+
593633
// If we are restricted by the maximum column width, end the sequence.
594634
unsigned NewLeft = std::max(ChangeWidthLeft, WidthLeft);
595635
unsigned NewAnchor = std::max(ChangeWidthAnchor, WidthAnchor);
@@ -598,7 +638,7 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
598638
if (Style.ColumnLimit != 0 &&
599639
Style.ColumnLimit < NewLeft + NewAnchor + NewRight) {
600640
AlignCurrentSequence();
601-
StartOfSequence = i;
641+
StartOfSequence = I;
602642
WidthLeft = ChangeWidthLeft;
603643
WidthAnchor = ChangeWidthAnchor;
604644
WidthRight = ChangeWidthRight;
@@ -607,12 +647,12 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
607647
WidthAnchor = NewAnchor;
608648
WidthRight = NewRight;
609649
}
610-
MatchedIndices.push_back(i);
650+
MatchedIndices.push_back(I);
611651
}
612652

613-
EndOfSequence = i;
653+
EndOfSequence = I;
614654
AlignCurrentSequence();
615-
return i;
655+
return I;
616656
}
617657

618658
// 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
@@ -20777,6 +20777,30 @@ TEST_F(FormatTest, AlignWithLineBreaks) {
2077720777
"}",
2077820778
Style);
2077920779
// clang-format on
20780+
20781+
Style = getLLVMStyleWithColumns(70);
20782+
Style.AlignConsecutiveDeclarations.Enabled = true;
20783+
verifyFormat(
20784+
"ReturnType\n"
20785+
"MyFancyIntefaceFunction(Context *context,\n"
20786+
" ALongTypeName *response) noexcept override;\n"
20787+
"ReturnType func();",
20788+
Style);
20789+
20790+
verifyFormat(
20791+
"ReturnType\n"
20792+
"MyFancyIntefaceFunction(B<int> *context,\n"
20793+
" decltype(AFunc) *response) noexcept override;\n"
20794+
"ReturnType func();",
20795+
Style);
20796+
20797+
Style.AlignConsecutiveAssignments.Enabled = true;
20798+
Style.ColumnLimit = 15;
20799+
verifyFormat("int i1 = 1;\n"
20800+
"k = bar(\n"
20801+
" argument1,\n"
20802+
" argument2);",
20803+
Style);
2078020804
}
2078120805

2078220806
TEST_F(FormatTest, AlignWithInitializerPeriods) {

0 commit comments

Comments
 (0)