Author: Björn Schäpers Date: 2025-10-20T22:11:11+02:00 New Revision: c318f82a4ad4c91401daa177b8c8ab546901bb4d
URL: https://github.com/llvm/llvm-project/commit/c318f82a4ad4c91401daa177b8c8ab546901bb4d DIFF: https://github.com/llvm/llvm-project/commit/c318f82a4ad4c91401daa177b8c8ab546901bb4d.diff LOG: [clang-format] Respect ColumnLimit while aligning multiline expressions (#163863) Before the patch the added test case would indent the function and moving its second line beyond the column limit. Fixes #68122. Added: Modified: clang/lib/Format/WhitespaceManager.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index b004d738588ab..65fc65e79fdc3 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -506,15 +506,15 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches, MatchedIndices.clear(); }; - unsigned i = StartAt; - for (unsigned e = Changes.size(); i != e; ++i) { - auto &CurrentChange = Changes[i]; + unsigned I = StartAt; + for (unsigned E = Changes.size(); I != E; ++I) { + auto &CurrentChange = Changes[I]; if (CurrentChange.indentAndNestingLevel() < IndentAndNestingLevel) break; if (CurrentChange.NewlinesBefore != 0) { CommasBeforeMatch = 0; - EndOfSequence = i; + EndOfSequence = I; // Whether to break the alignment sequence because of an empty line. bool EmptyLineBreak = @@ -530,8 +530,8 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches, // A new line starts, re-initialize line status tracking bools. // Keep the match state if a string literal is continued on this line. - if (i == 0 || CurrentChange.Tok->isNot(tok::string_literal) || - Changes[i - 1].Tok->isNot(tok::string_literal)) { + if (I == 0 || CurrentChange.Tok->isNot(tok::string_literal) || + Changes[I - 1].Tok->isNot(tok::string_literal)) { FoundMatchOnLine = false; } LineIsComment = true; @@ -547,8 +547,8 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches, IndentAndNestingLevel) { // Call AlignTokens recursively, skipping over this scope block. const auto StoppedAt = - AlignTokens(Style, Matches, Changes, i, ACS, RightJustify); - i = StoppedAt - 1; + AlignTokens(Style, Matches, Changes, I, ACS, RightJustify); + I = StoppedAt - 1; continue; } } @@ -559,7 +559,7 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches, // If there is more than one matching token per line, or if the number of // preceding commas, do not match anymore, end the sequence. if (FoundMatchOnLine || CommasBeforeMatch != CommasBeforeLastMatch) { - MatchedIndices.push_back(i); + MatchedIndices.push_back(I); AlignCurrentSequence(); } @@ -567,29 +567,69 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches, FoundMatchOnLine = true; if (StartOfSequence == 0) - StartOfSequence = i; + StartOfSequence = I; unsigned ChangeWidthLeft = CurrentChange.StartOfTokenColumn; unsigned ChangeWidthAnchor = 0; unsigned ChangeWidthRight = 0; + unsigned CurrentChangeWidthRight = 0; if (RightJustify) if (ACS.PadOperators) ChangeWidthAnchor = CurrentChange.TokenLength; else ChangeWidthLeft += CurrentChange.TokenLength; else - ChangeWidthRight = CurrentChange.TokenLength; - for (unsigned j = i + 1; j != e && Changes[j].NewlinesBefore == 0; ++j) { - ChangeWidthRight += Changes[j].Spaces; + CurrentChangeWidthRight = CurrentChange.TokenLength; + const FormatToken *MatchingParenToEncounter = nullptr; + for (unsigned J = I + 1; + J != E && (Changes[J].NewlinesBefore == 0 || MatchingParenToEncounter); + ++J) { + const auto &Change = Changes[J]; + const auto *Tok = Change.Tok; + + if (Tok->MatchingParen) { + if (Tok->isOneOf(tok::l_paren, tok::l_brace, tok::l_square, + TT_TemplateOpener) && + !MatchingParenToEncounter) { + // If the next token is on the next line, we probably don't need to + // check the following lengths, because it most likely isn't aligned + // with the rest. + if (J + 1 != E && Changes[J + 1].NewlinesBefore == 0) + MatchingParenToEncounter = Tok->MatchingParen; + } else if (MatchingParenToEncounter == Tok->MatchingParen) { + MatchingParenToEncounter = nullptr; + } + } + + if (Change.NewlinesBefore != 0) { + ChangeWidthRight = std::max(ChangeWidthRight, CurrentChangeWidthRight); + const auto ChangeWidthStart = ChangeWidthLeft + ChangeWidthAnchor; + // If the position of the current token is columnwise before the begin + // of the alignment, we drop out here, because the next line does not + // have to be moved with the previous one(s) for the alignment. E.g.: + // int i1 = 1; | <- ColumnLimit | int i1 = 1; + // int j = 0; | Without the break -> | int j = 0; + // int k = bar( | We still want to align the = | int k = bar( + // argument1, | here, even if we can't move | argument1, + // argument2); | the following lines. | argument2); + if (static_cast<unsigned>(Change.Spaces) < ChangeWidthStart) + break; + CurrentChangeWidthRight = Change.Spaces - ChangeWidthStart; + } else { + CurrentChangeWidthRight += Change.Spaces; + } + // Changes are generally 1:1 with the tokens, but a change could also be // inside of a token, in which case it's counted more than once: once for // the whitespace surrounding the token (!IsInsideToken) and once for // each whitespace change within it (IsInsideToken). // Therefore, changes inside of a token should only count the space. - if (!Changes[j].IsInsideToken) - ChangeWidthRight += Changes[j].TokenLength; + if (!Change.IsInsideToken) + CurrentChangeWidthRight += Change.TokenLength; } + ChangeWidthRight = std::max(ChangeWidthRight, CurrentChangeWidthRight); + // If we are restricted by the maximum column width, end the sequence. unsigned NewLeft = std::max(ChangeWidthLeft, WidthLeft); unsigned NewAnchor = std::max(ChangeWidthAnchor, WidthAnchor); @@ -598,7 +638,7 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches, if (Style.ColumnLimit != 0 && Style.ColumnLimit < NewLeft + NewAnchor + NewRight) { AlignCurrentSequence(); - StartOfSequence = i; + StartOfSequence = I; WidthLeft = ChangeWidthLeft; WidthAnchor = ChangeWidthAnchor; WidthRight = ChangeWidthRight; @@ -607,12 +647,12 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches, WidthAnchor = NewAnchor; WidthRight = NewRight; } - MatchedIndices.push_back(i); + MatchedIndices.push_back(I); } - EndOfSequence = i; + EndOfSequence = I; AlignCurrentSequence(); - return i; + return I; } // Aligns a sequence of matching tokens, on the MinColumn column. diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index a3ad978a9357d..ce68f91bef02a 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -20777,6 +20777,30 @@ TEST_F(FormatTest, AlignWithLineBreaks) { "}", Style); // clang-format on + + Style = getLLVMStyleWithColumns(70); + Style.AlignConsecutiveDeclarations.Enabled = true; + verifyFormat( + "ReturnType\n" + "MyFancyIntefaceFunction(Context *context,\n" + " ALongTypeName *response) noexcept override;\n" + "ReturnType func();", + Style); + + verifyFormat( + "ReturnType\n" + "MyFancyIntefaceFunction(B<int> *context,\n" + " decltype(AFunc) *response) noexcept override;\n" + "ReturnType func();", + Style); + + Style.AlignConsecutiveAssignments.Enabled = true; + Style.ColumnLimit = 15; + verifyFormat("int i1 = 1;\n" + "k = bar(\n" + " argument1,\n" + " argument2);", + Style); } TEST_F(FormatTest, AlignWithInitializerPeriods) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
