https://github.com/sstwcw updated https://github.com/llvm/llvm-project/pull/164458
>From 1db4303b26210bfb7c8b3f60740a1036ede70425 Mon Sep 17 00:00:00 2001 From: sstwcw <[email protected]> Date: Tue, 21 Oct 2025 16:35:27 +0000 Subject: [PATCH 1/5] [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 <[email protected]> Date: Wed, 22 Oct 2025 17:46:17 +0000 Subject: [PATCH 2/5] 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 <[email protected]> Date: Mon, 27 Oct 2025 18:59:52 +0000 Subject: [PATCH 3/5] 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; } >From 8b6d6bbd51876a95a2478d78eb876287083e6977 Mon Sep 17 00:00:00 2001 From: sstwcw <[email protected]> Date: Wed, 29 Oct 2025 16:15:21 +0000 Subject: [PATCH 4/5] Move the variable out of the loop --- clang/lib/Format/WhitespaceManager.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index f58d071b9257b..4c700f2e15928 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -507,7 +507,8 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches, }; unsigned I = StartAt; - for (unsigned E = Changes.size(); I != E; ++I) { + unsigned E = Changes.size(); + for (; I != E; ++I) { auto &CurrentChange = Changes[I]; if (CurrentChange.indentAndNestingLevel() < IndentAndNestingLevel) break; @@ -652,8 +653,8 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches, // 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; + for (EndOfSequence = I; + EndOfSequence < E && Changes[EndOfSequence].NewlinesBefore == 0; ++EndOfSequence) { } AlignCurrentSequence(); >From 3168df91c25a5f0d3cec35371beb63003a897fc9 Mon Sep 17 00:00:00 2001 From: sstwcw <[email protected]> Date: Mon, 3 Nov 2025 03:54:58 +0000 Subject: [PATCH 5/5] Use auto type --- clang/lib/Format/WhitespaceManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index 4c700f2e15928..7c2c786055a13 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -507,7 +507,7 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches, }; unsigned I = StartAt; - unsigned E = Changes.size(); + const auto E = Changes.size(); for (; I != E; ++I) { auto &CurrentChange = Changes[I]; if (CurrentChange.indentAndNestingLevel() < IndentAndNestingLevel) _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
