Skip to content

Commit f8a0599

Browse files
authored
[clang-format] Align comments following continued aligned lines (llvm#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.
1 parent 88558d5 commit f8a0599

File tree

2 files changed

+36
-8
lines changed

2 files changed

+36
-8
lines changed

clang/lib/Format/WhitespaceManager.cpp

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,9 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
288288
ArrayRef<unsigned> Matches,
289289
SmallVector<WhitespaceManager::Change, 16> &Changes) {
290290
int Shift = 0;
291+
// Set when the shift is applied anywhere in the line. Cleared when the line
292+
// ends.
293+
bool LineShifted = false;
291294

292295
// ScopeStack keeps track of the current scope depth. It contains the levels
293296
// 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,
339342
Changes[i - 1].Tok->is(tok::string_literal);
340343
bool SkipMatchCheck = InsideNestedScope || ContinuedStringLiteral;
341344

342-
if (CurrentChange.NewlinesBefore > 0 && !SkipMatchCheck)
343-
Shift = 0;
345+
if (CurrentChange.NewlinesBefore > 0) {
346+
LineShifted = false;
347+
if (!SkipMatchCheck)
348+
Shift = 0;
349+
}
344350

345351
// If this is the first matching token to be aligned, remember by how many
346352
// 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,
349355
Shift = Column - (RightJustify ? CurrentChange.TokenLength : 0) -
350356
CurrentChange.StartOfTokenColumn;
351357
ScopeStack = {CurrentChange.indentAndNestingLevel()};
352-
CurrentChange.Spaces += Shift;
353358
}
354359

355360
if (Shift == 0)
@@ -358,8 +363,10 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
358363
// This is for lines that are split across multiple lines, as mentioned in
359364
// the ScopeStack comment. The stack size being 1 means that the token is
360365
// not in a scope that should not move.
361-
if (ScopeStack.size() == 1u && CurrentChange.NewlinesBefore > 0 &&
362-
(ContinuedStringLiteral || InsideNestedScope)) {
366+
if ((!Matches.empty() && Matches[0] == i) ||
367+
(ScopeStack.size() == 1u && CurrentChange.NewlinesBefore > 0 &&
368+
(ContinuedStringLiteral || InsideNestedScope))) {
369+
LineShifted = true;
363370
CurrentChange.Spaces += Shift;
364371
}
365372

@@ -369,9 +376,11 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
369376
static_cast<int>(Changes[i].Tok->SpacesRequiredBefore) ||
370377
CurrentChange.Tok->is(tok::eof));
371378

372-
CurrentChange.StartOfTokenColumn += Shift;
373-
if (i + 1 != Changes.size())
374-
Changes[i + 1].PreviousEndOfTokenColumn += Shift;
379+
if (LineShifted) {
380+
CurrentChange.StartOfTokenColumn += Shift;
381+
if (i + 1 != Changes.size())
382+
Changes[i + 1].PreviousEndOfTokenColumn += Shift;
383+
}
375384

376385
// If PointerAlignment is PAS_Right, keep *s or &s next to the token,
377386
// except if the token is equal, then a space is needed.

clang/unittests/Format/FormatTest.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19615,6 +19615,25 @@ TEST_F(FormatTest, AlignConsecutiveAssignments) {
1961519615
"};",
1961619616
Alignment);
1961719617

19618+
// Aligning lines should not mess up the comments. However, feel free to
19619+
// change the test if it turns out that comments inside the closure should not
19620+
// be aligned with those outside it.
19621+
verifyFormat("auto aaaaaaaaaaaaaaaaaaaaa = {}; //\n"
19622+
"auto b = [] { //\n"
19623+
" return; //\n"
19624+
"};",
19625+
Alignment);
19626+
verifyFormat("auto aaaaaaaaaaaaaaaaaaaaa = {}; //\n"
19627+
"auto b = [] { //\n"
19628+
" return aaaaaaaaaaaaaaaaaaaaa; //\n"
19629+
"};",
19630+
Alignment);
19631+
verifyFormat("auto aaaaaaaaaaaaaaa = {}; //\n"
19632+
"auto b = [] { //\n"
19633+
" return aaaaaaaaaaaaaaaaaaaaa; //\n"
19634+
"};",
19635+
Alignment);
19636+
1961819637
verifyFormat("auto b = f(aaaaaaaaaaaaaaaaaaaaaaaaa,\n"
1961919638
" ccc ? aaaaa : bbbbb,\n"
1962019639
" dddddddddddddddddddddddddd);",

0 commit comments

Comments
 (0)