Skip to content

Commit 5a61139

Browse files
committed
clang-format: Add special case for leading comments in braced lists.
A comment following the "{" of a braced list seems to almost always refer to the first element of the list and thus should be aligned to it. Before (with Cpp11 braced list style): SomeFunction({ // Comment 1 "first entry", // Comment 2 "second entry"}); After: SomeFunction({// Comment 1 "first entry", // Comment 2 "second entry"}); llvm-svn: 197725
1 parent 5c6c62f commit 5a61139

File tree

3 files changed

+53
-15
lines changed

3 files changed

+53
-15
lines changed

clang/lib/Format/ContinuationIndenter.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
268268
}
269269

270270
if (Previous.opensScope() && Previous.Type != TT_ObjCMethodExpr &&
271-
Current.Type != TT_LineComment)
271+
(Current.Type != TT_LineComment || Previous.BlockKind == BK_BracedInit))
272272
State.Stack.back().Indent = State.Column + Spaces;
273273
if (State.Stack.back().AvoidBinPacking && startsNextParameter(Current, Style))
274274
State.Stack.back().NoLineBreak = true;
@@ -596,9 +596,11 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
596596
NewIndent = State.Stack.back().LastSpace;
597597
if (Current.opensBlockTypeList(Style)) {
598598
NewIndent += Style.IndentWidth;
599+
NewIndent = std::min(State.Column + 2, NewIndent);
599600
++NewIndentLevel;
600601
} else {
601602
NewIndent += Style.ContinuationIndentWidth;
603+
NewIndent = std::min(State.Column + 1, NewIndent);
602604
}
603605
}
604606
const FormatToken *NextNoComment = Current.getNextNonComment();

clang/lib/Format/TokenAnnotator.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,11 +1072,15 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
10721072
FormatToken *Current = Line.First->Next;
10731073
bool InFunctionDecl = Line.MightBeFunctionDecl;
10741074
while (Current != NULL) {
1075-
if (Current->Type == TT_LineComment)
1076-
Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
1077-
else if (Current->SpacesRequiredBefore == 0 &&
1078-
spaceRequiredBefore(Line, *Current))
1075+
if (Current->Type == TT_LineComment) {
1076+
if (Current->Previous->BlockKind == BK_BracedInit)
1077+
Current->SpacesRequiredBefore = Style.Cpp11BracedListStyle ? 0 : 1;
1078+
else
1079+
Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
1080+
} else if (Current->SpacesRequiredBefore == 0 &&
1081+
spaceRequiredBefore(Line, *Current)) {
10791082
Current->SpacesRequiredBefore = 1;
1083+
}
10801084

10811085
Current->MustBreakBefore =
10821086
Current->MustBreakBefore || mustBreakBefore(Line, *Current);
@@ -1398,7 +1402,8 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
13981402
bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
13991403
const FormatToken &Right) {
14001404
if (Right.is(tok::comment)) {
1401-
return Right.NewlinesBefore > 0;
1405+
return Right.Previous->BlockKind != BK_BracedInit &&
1406+
Right.NewlinesBefore > 0;
14021407
} else if (Right.Previous->isTrailingComment() ||
14031408
(Right.is(tok::string_literal) &&
14041409
Right.Previous->is(tok::string_literal))) {
@@ -1445,7 +1450,10 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
14451450
if (Right.isTrailingComment())
14461451
// We rely on MustBreakBefore being set correctly here as we should not
14471452
// change the "binding" behavior of a comment.
1448-
return false;
1453+
// The first comment in a braced lists is always interpreted as belonging to
1454+
// the first list element. Otherwise, it should be placed outside of the
1455+
// list.
1456+
return Left.BlockKind == BK_BracedInit;
14491457
if (Left.is(tok::question) && Right.is(tok::colon))
14501458
return false;
14511459
if (Right.Type == TT_ConditionalExpr || Right.is(tok::question))

clang/unittests/Format/FormatTest.cpp

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4771,12 +4771,12 @@ TEST_F(FormatTest, LayoutCxx11ConstructorBraceInitializers) {
47714771
verifyFormat("DoSomethingWithVector({} /* No data */);");
47724772
verifyFormat("DoSomethingWithVector({ {} /* No data */ }, { { 1, 2 } });");
47734773
verifyFormat(
4774-
"someFunction(OtherParam, BracedList{\n"
4775-
" // comment 1 (Forcing interesting break)\n"
4776-
" param1, param2,\n"
4777-
" // comment 2\n"
4778-
" param3, param4\n"
4779-
" });");
4774+
"someFunction(OtherParam,\n"
4775+
" BracedList{ // comment 1 (Forcing interesting break)\n"
4776+
" param1, param2,\n"
4777+
" // comment 2\n"
4778+
" param3, param4 });",
4779+
getLLVMStyleWithColumns(75));
47804780
verifyFormat(
47814781
"std::this_thread::sleep_for(\n"
47824782
" std::chrono::nanoseconds{ std::chrono::seconds{ 1 } } / 5);");
@@ -4808,11 +4808,39 @@ TEST_F(FormatTest, LayoutCxx11ConstructorBraceInitializers) {
48084808
" T member = {arg1, arg2};\n"
48094809
"};",
48104810
NoSpaces);
4811+
4812+
// FIXME: The alignment of these trailing comments might be bad. Then again,
4813+
// this might be utterly useless in real code.
48114814
verifyFormat("Constructor::Constructor()\n"
4812-
" : some_value{ //\n"
4813-
" aaaaaaa //\n"
4815+
" : some_value{ //\n"
4816+
" aaaaaaa //\n"
48144817
" } {}",
48154818
NoSpaces);
4819+
4820+
// In braced lists, the first comment is always assumed to belong to the
4821+
// first element. Thus, it can be moved to the next or previous line as
4822+
// appropriate.
4823+
EXPECT_EQ("function({// First element:\n"
4824+
" 1,\n"
4825+
" // Second element:\n"
4826+
" 2});",
4827+
format("function({\n"
4828+
" // First element:\n"
4829+
" 1,\n"
4830+
" // Second element:\n"
4831+
" 2});",
4832+
NoSpaces));
4833+
NoSpaces.ColumnLimit = 30;
4834+
EXPECT_EQ(
4835+
"std::vector<int> MyNumbers{\n"
4836+
" // First element:\n"
4837+
" 1,\n"
4838+
" // Second element:\n"
4839+
" 2};",
4840+
format("std::vector<int> MyNumbers{// First element:\n"
4841+
" 1,\n"
4842+
" // Second element:\n"
4843+
" 2};", NoSpaces));
48164844
}
48174845

48184846
TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {

0 commit comments

Comments
 (0)