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

via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 22 10:46:47 PDT 2025


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/2] [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 77b5fb551bdc2df8a8f826b4573465d4748a671f 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/2] 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..657436e1b9aaf 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 scope ends. The rest of the line
+  // may contain stuff to be aligned within a larger scope.
   return I;
 }
 



More information about the cfe-commits mailing list