[clang] [clang-format] Align trailing comments for function parameters (PR #164458)

[via cfe-commits]

https://github.com/sstwcw updated https://github.com/llvm/llvm-project/pull/164458

>From 1db4303b26210bfb7c8b3f60740a1036ede70425 Mon Sep 17 00:00:00 2001
From: sstwcw <su3e8a96kzlver at posteo.net>
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 <su3e8a96kzlver at posteo.net>
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 <su3e8a96kzlver at posteo.net>
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;
 }