Author: sstwcw Date: 2025-10-27T18:41:09Z New Revision: f8a0599d761e4283b3877f0bf4043e01722dc448
URL: https://github.com/llvm/llvm-project/commit/f8a0599d761e4283b3877f0bf4043e01722dc448 DIFF: https://github.com/llvm/llvm-project/commit/f8a0599d761e4283b3877f0bf4043e01722dc448.diff LOG: [clang-format] Align comments following continued aligned lines (#164687) 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. Added: Modified: clang/lib/Format/WhitespaceManager.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ 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<unsigned> Matches, SmallVector<WhitespaceManager::Change, 16> &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<int>(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);", _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
