From 1db4303b26210bfb7c8b3f60740a1036ede70425 Mon Sep 17 00:00:00 2001 From: sstwcw Date: Tue, 21 Oct 2025 16:35:27 +0000 Subject: [PATCH 1/3] [clang-format] Align trailing comments for function parameters before ```C++ void foo(int name, // name float name, // name int name) // name {} ``` after ```C++ void foo(int name, // name float name, // name int name) // name {} ``` Fixes #85123. As the bug report explained, the procedure for aligning the function parameters previously failed to update `StartOfTokenColumn`. --- clang/lib/Format/WhitespaceManager.cpp | 9 +++++++++ clang/unittests/Format/FormatTest.cpp | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index 65fc65e79fdc3..cf1f8da7f704f 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -399,6 +399,15 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End, } } } + + // The scope to be aligned may not occupy entire lines. The rest of the line + // needs some book-keeping. + for (unsigned i = End; i < Changes.size() && Changes[i].NewlinesBefore == 0; + ++i) { + Changes[i].StartOfTokenColumn += Shift; + if (i + 1 != Changes.size()) + Changes[i + 1].PreviousEndOfTokenColumn += Shift; + } } // Walk through a subset of the changes, starting at StartAt, and find diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index ce68f91bef02a..9133eeb137cb9 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -19828,6 +19828,14 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) { " Test &operator=(const Test &) = default;\n" "};", Alignment); + + // The comment to the right should still align right. + verifyFormat("void foo(int name, // name\n" + " float name, // name\n" + " int name) // name\n" + "{}", + Alignment); + unsigned OldColumnLimit = Alignment.ColumnLimit; // We need to set ColumnLimit to zero, in order to stress nested alignments, // otherwise the function parameters will be re-flowed onto a single line. From 7f148b2cd539c5deec2108dfa91ecba39a323cdd Mon Sep 17 00:00:00 2001 From: sstwcw Date: Wed, 22 Oct 2025 17:46:17 +0000 Subject: [PATCH 2/3] Address comments --- clang/lib/Format/WhitespaceManager.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index cf1f8da7f704f..e960a8f10c60d 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -399,15 +399,6 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End, } } } - - // The scope to be aligned may not occupy entire lines. The rest of the line - // needs some book-keeping. - for (unsigned i = End; i < Changes.size() && Changes[i].NewlinesBefore == 0; - ++i) { - Changes[i].StartOfTokenColumn += Shift; - if (i + 1 != Changes.size()) - Changes[i + 1].PreviousEndOfTokenColumn += Shift; - } } // Walk through a subset of the changes, starting at StartAt, and find @@ -659,8 +650,16 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches, MatchedIndices.push_back(I); } + // Pass to the function entire lines, so it can update the state of all tokens + // that move. EndOfSequence = I; + while (EndOfSequence < Changes.size() && + Changes[EndOfSequence].NewlinesBefore == 0) { + ++EndOfSequence; + } AlignCurrentSequence(); + // The return value should still be where the level ends. The rest of the line + // may contain stuff to be aligned within the parent level. return I; } From 191a34c1851c715494ea3be89086cd5c98a51474 Mon Sep 17 00:00:00 2001 From: sstwcw Date: Mon, 27 Oct 2025 18:59:52 +0000 Subject: [PATCH 3/3] Address review --- clang/lib/Format/WhitespaceManager.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index e960a8f10c60d..f58d071b9257b 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -650,16 +650,15 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches, MatchedIndices.push_back(I); } - // Pass to the function entire lines, so it can update the state of all tokens - // that move. - EndOfSequence = I; - while (EndOfSequence < Changes.size() && - Changes[EndOfSequence].NewlinesBefore == 0) { - ++EndOfSequence; + // Pass entire lines to the function so that it can update the state of all + // tokens that move. + for (EndOfSequence = I; EndOfSequence < Changes.size() && + Changes[EndOfSequence].NewlinesBefore == 0; + ++EndOfSequence) { } AlignCurrentSequence(); // The return value should still be where the level ends. The rest of the line - // may contain stuff to be aligned within the parent level. + // may contain stuff to be aligned within an outer level. return I; }