From 2ab5ad28113faf239867f27f5856e21c4652ebeb Mon Sep 17 00:00:00 2001 From: sstwcw Date: Wed, 22 Oct 2025 19:03:46 +0000 Subject: [PATCH] [clang-format] Align comments following continued aligned lines new ```C++ auto aaaaaaaaaaaaaaaaaaaaa = {}; // auto b = [] { // return; // }; auto aaaaaaaaaaaaaaaaaaaaa = {}; // auto b = [] { // return aaaaaaaaaaaaaaaaaaaaa; // }; ``` old ```C++ auto aaaaaaaaaaaaaaaaaaaaa = {}; // auto b = [] { // return; // }; auto aaaaaaaaaaaaaaaaaaaaa = {}; // auto b = [] { // return aaaaaaaaaaaaaaaaaaaaa; // }; ``` Aligning a line to another line involves keeping track of the tokens' positions. Previously the shift was incorrectly added to some tokens that did not move. Then the comments would end up in the wrong places. --- clang/lib/Format/WhitespaceManager.cpp | 25 +++++++++++++++++-------- clang/unittests/Format/FormatTest.cpp | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index 65fc65e79fdc3..f24b8ab14bdce 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -288,6 +288,9 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End, ArrayRef Matches, SmallVector &Changes) { int Shift = 0; + // Set when the shift is applied anywhere in the line. Cleared when the line + // ends. + bool LineShifted = false; // ScopeStack keeps track of the current scope depth. It contains the levels // of at most 2 scopes. The first one is the one that the matched token is @@ -339,8 +342,11 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End, Changes[i - 1].Tok->is(tok::string_literal); bool SkipMatchCheck = InsideNestedScope || ContinuedStringLiteral; - if (CurrentChange.NewlinesBefore > 0 && !SkipMatchCheck) - Shift = 0; + if (CurrentChange.NewlinesBefore > 0) { + LineShifted = false; + if (!SkipMatchCheck) + Shift = 0; + } // If this is the first matching token to be aligned, remember by how many // spaces it has to be shifted, so the rest of the changes on the line are @@ -349,7 +355,6 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End, Shift = Column - (RightJustify ? CurrentChange.TokenLength : 0) - CurrentChange.StartOfTokenColumn; ScopeStack = {CurrentChange.indentAndNestingLevel()}; - CurrentChange.Spaces += Shift; } if (Shift == 0) @@ -358,8 +363,10 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End, // This is for lines that are split across multiple lines, as mentioned in // the ScopeStack comment. The stack size being 1 means that the token is // not in a scope that should not move. - if (ScopeStack.size() == 1u && CurrentChange.NewlinesBefore > 0 && - (ContinuedStringLiteral || InsideNestedScope)) { + if ((!Matches.empty() && Matches[0] == i) || + (ScopeStack.size() == 1u && CurrentChange.NewlinesBefore > 0 && + (ContinuedStringLiteral || InsideNestedScope))) { + LineShifted = true; CurrentChange.Spaces += Shift; } @@ -369,9 +376,11 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End, static_cast(Changes[i].Tok->SpacesRequiredBefore) || CurrentChange.Tok->is(tok::eof)); - CurrentChange.StartOfTokenColumn += Shift; - if (i + 1 != Changes.size()) - Changes[i + 1].PreviousEndOfTokenColumn += Shift; + if (LineShifted) { + CurrentChange.StartOfTokenColumn += Shift; + if (i + 1 != Changes.size()) + Changes[i + 1].PreviousEndOfTokenColumn += Shift; + } // If PointerAlignment is PAS_Right, keep *s or &s next to the token, // except if the token is equal, then a space is needed. diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index ce68f91bef02a..d45babe1b82ad 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -19615,6 +19615,25 @@ TEST_F(FormatTest, AlignConsecutiveAssignments) { "};", Alignment); + // Aligning lines should not mess up the comments. However, feel free to + // change the test if it turns out that comments inside the closure should not + // be aligned with those outside it. + verifyFormat("auto aaaaaaaaaaaaaaaaaaaaa = {}; //\n" + "auto b = [] { //\n" + " return; //\n" + "};", + Alignment); + verifyFormat("auto aaaaaaaaaaaaaaaaaaaaa = {}; //\n" + "auto b = [] { //\n" + " return aaaaaaaaaaaaaaaaaaaaa; //\n" + "};", + Alignment); + verifyFormat("auto aaaaaaaaaaaaaaa = {}; //\n" + "auto b = [] { //\n" + " return aaaaaaaaaaaaaaaaaaaaa; //\n" + "};", + Alignment); + verifyFormat("auto b = f(aaaaaaaaaaaaaaaaaaaaaaaaa,\n" " ccc ? aaaaa : bbbbb,\n" " dddddddddddddddddddddddddd);",