Skip to content

Commit c5243c6

Browse files
[clang-format] Fix aligning with linebreaks
Breaking a string literal or a function calls arguments with AlignConsecutiveDeclarations or AlignConsecutiveAssignments did misalign the continued line. E.g.: void foo() { int myVar = 5; double x = 3.14; auto str = "Hello" "World"; } or void foo() { int myVar = 5; double x = 3.14; auto str = "Hello" "World"; } Differential Revision: https://reviews.llvm.org/D98214
1 parent 8c6c357 commit c5243c6

File tree

2 files changed

+146
-9
lines changed

2 files changed

+146
-9
lines changed

clang/lib/Format/WhitespaceManager.cpp

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,14 @@ AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
278278
// double z);
279279
// In the above example, we need to take special care to ensure that
280280
// 'double z' is indented along with it's owning function 'b'.
281+
// The same holds for calling a function:
282+
// double a = foo(x);
283+
// int b = bar(foo(y),
284+
// foor(z));
285+
// Similar for broken string literals:
286+
// double x = 3.14;
287+
// auto s = "Hello"
288+
// "World";
281289
// Special handling is required for 'nested' ternary operators.
282290
SmallVector<unsigned, 16> ScopeStack;
283291

@@ -298,16 +306,20 @@ AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
298306
ScopeStack.push_back(i);
299307

300308
bool InsideNestedScope = ScopeStack.size() != 0;
309+
bool ContinuedStringLiteral = i > Start &&
310+
Changes[i].Tok->is(tok::string_literal) &&
311+
Changes[i - 1].Tok->is(tok::string_literal);
312+
bool SkipMatchCheck = InsideNestedScope || ContinuedStringLiteral;
301313

302-
if (Changes[i].NewlinesBefore > 0 && !InsideNestedScope) {
314+
if (Changes[i].NewlinesBefore > 0 && !SkipMatchCheck) {
303315
Shift = 0;
304316
FoundMatchOnLine = false;
305317
}
306318

307319
// If this is the first matching token to be aligned, remember by how many
308320
// spaces it has to be shifted, so the rest of the changes on the line are
309321
// shifted by the same amount
310-
if (!FoundMatchOnLine && !InsideNestedScope && Matches(Changes[i])) {
322+
if (!FoundMatchOnLine && !SkipMatchCheck && Matches(Changes[i])) {
311323
FoundMatchOnLine = true;
312324
Shift = Column - Changes[i].StartOfTokenColumn;
313325
Changes[i].Spaces += Shift;
@@ -317,15 +329,41 @@ AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
317329
// as mentioned in the ScopeStack comment.
318330
if (InsideNestedScope && Changes[i].NewlinesBefore > 0) {
319331
unsigned ScopeStart = ScopeStack.back();
320-
if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName) ||
321-
(ScopeStart > Start + 1 &&
322-
Changes[ScopeStart - 2].Tok->is(TT_FunctionDeclarationName)) ||
323-
Changes[i].Tok->is(TT_ConditionalExpr) ||
324-
(Changes[i].Tok->Previous &&
325-
Changes[i].Tok->Previous->is(TT_ConditionalExpr)))
332+
auto ShouldShiftBeAdded = [&] {
333+
// Function declaration
334+
if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName))
335+
return true;
336+
337+
// Continued function declaration
338+
if (ScopeStart > Start + 1 &&
339+
Changes[ScopeStart - 2].Tok->is(TT_FunctionDeclarationName))
340+
return true;
341+
342+
// Continued function call
343+
if (ScopeStart > Start + 1 &&
344+
Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
345+
Changes[ScopeStart - 1].Tok->is(tok::l_paren))
346+
return true;
347+
348+
// Ternary operator
349+
if (Changes[i].Tok->is(TT_ConditionalExpr))
350+
return true;
351+
352+
// Continued ternary operator
353+
if (Changes[i].Tok->Previous &&
354+
Changes[i].Tok->Previous->is(TT_ConditionalExpr))
355+
return true;
356+
357+
return false;
358+
};
359+
360+
if (ShouldShiftBeAdded())
326361
Changes[i].Spaces += Shift;
327362
}
328363

364+
if (ContinuedStringLiteral)
365+
Changes[i].Spaces += Shift;
366+
329367
assert(Shift >= 0);
330368
Changes[i].StartOfTokenColumn += Shift;
331369
if (i + 1 != Changes.size())
@@ -434,7 +472,10 @@ static unsigned AlignTokens(
434472
AlignCurrentSequence();
435473

436474
// A new line starts, re-initialize line status tracking bools.
437-
FoundMatchOnLine = false;
475+
// Keep the match state if a string literal is continued on this line.
476+
if (i == 0 || !Changes[i].Tok->is(tok::string_literal) ||
477+
!Changes[i - 1].Tok->is(tok::string_literal))
478+
FoundMatchOnLine = false;
438479
LineIsComment = true;
439480
}
440481

clang/unittests/Format/FormatTest.cpp

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14297,6 +14297,102 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
1429714297
Alignment);
1429814298
}
1429914299

14300+
TEST_F(FormatTest, AlignWithLineBreaks) {
14301+
auto Style = getLLVMStyleWithColumns(120);
14302+
14303+
EXPECT_EQ(Style.AlignConsecutiveAssignments, FormatStyle::ACS_None);
14304+
EXPECT_EQ(Style.AlignConsecutiveDeclarations, FormatStyle::ACS_None);
14305+
verifyFormat("void foo() {\n"
14306+
" int myVar = 5;\n"
14307+
" double x = 3.14;\n"
14308+
" auto str = \"Hello \"\n"
14309+
" \"World\";\n"
14310+
" auto s = \"Hello \"\n"
14311+
" \"Again\";\n"
14312+
"}",
14313+
Style);
14314+
14315+
// clang-format off
14316+
verifyFormat("void foo() {\n"
14317+
" const int capacityBefore = Entries.capacity();\n"
14318+
" const auto newEntry = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
14319+
" std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
14320+
" const X newEntry2 = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
14321+
" std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
14322+
"}",
14323+
Style);
14324+
// clang-format on
14325+
14326+
Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
14327+
verifyFormat("void foo() {\n"
14328+
" int myVar = 5;\n"
14329+
" double x = 3.14;\n"
14330+
" auto str = \"Hello \"\n"
14331+
" \"World\";\n"
14332+
" auto s = \"Hello \"\n"
14333+
" \"Again\";\n"
14334+
"}",
14335+
Style);
14336+
14337+
// clang-format off
14338+
verifyFormat("void foo() {\n"
14339+
" const int capacityBefore = Entries.capacity();\n"
14340+
" const auto newEntry = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
14341+
" std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
14342+
" const X newEntry2 = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
14343+
" std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
14344+
"}",
14345+
Style);
14346+
// clang-format on
14347+
14348+
Style.AlignConsecutiveAssignments = FormatStyle::ACS_None;
14349+
Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
14350+
verifyFormat("void foo() {\n"
14351+
" int myVar = 5;\n"
14352+
" double x = 3.14;\n"
14353+
" auto str = \"Hello \"\n"
14354+
" \"World\";\n"
14355+
" auto s = \"Hello \"\n"
14356+
" \"Again\";\n"
14357+
"}",
14358+
Style);
14359+
14360+
// clang-format off
14361+
verifyFormat("void foo() {\n"
14362+
" const int capacityBefore = Entries.capacity();\n"
14363+
" const auto newEntry = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
14364+
" std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
14365+
" const X newEntry2 = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
14366+
" std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
14367+
"}",
14368+
Style);
14369+
// clang-format on
14370+
14371+
Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
14372+
Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
14373+
14374+
verifyFormat("void foo() {\n"
14375+
" int myVar = 5;\n"
14376+
" double x = 3.14;\n"
14377+
" auto str = \"Hello \"\n"
14378+
" \"World\";\n"
14379+
" auto s = \"Hello \"\n"
14380+
" \"Again\";\n"
14381+
"}",
14382+
Style);
14383+
14384+
// clang-format off
14385+
verifyFormat("void foo() {\n"
14386+
" const int capacityBefore = Entries.capacity();\n"
14387+
" const auto newEntry = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
14388+
" std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
14389+
" const X newEntry2 = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
14390+
" std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
14391+
"}",
14392+
Style);
14393+
// clang-format on
14394+
}
14395+
1430014396
TEST_F(FormatTest, LinuxBraceBreaking) {
1430114397
FormatStyle LinuxBraceStyle = getLLVMStyle();
1430214398
LinuxBraceStyle.BreakBeforeBraces = FormatStyle::BS_Linux;

0 commit comments

Comments
 (0)