r197725 - clang-format: Add special case for leading comments in braced lists.
Daniel Jasper
djasper at google.com
Thu Dec 19 13:41:37 PST 2013
Author: djasper
Date: Thu Dec 19 15:41:37 2013
New Revision: 197725
URL: http://llvm.org/viewvc/llvm-project?rev=197725&view=rev
Log:
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"});
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=197725&r1=197724&r2=197725&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Dec 19 15:41:37 2013
@@ -268,7 +268,7 @@ void ContinuationIndenter::addTokenOnCur
}
if (Previous.opensScope() && Previous.Type != TT_ObjCMethodExpr &&
- Current.Type != TT_LineComment)
+ (Current.Type != TT_LineComment || Previous.BlockKind == BK_BracedInit))
State.Stack.back().Indent = State.Column + Spaces;
if (State.Stack.back().AvoidBinPacking && startsNextParameter(Current, Style))
State.Stack.back().NoLineBreak = true;
@@ -596,9 +596,11 @@ unsigned ContinuationIndenter::moveState
NewIndent = State.Stack.back().LastSpace;
if (Current.opensBlockTypeList(Style)) {
NewIndent += Style.IndentWidth;
+ NewIndent = std::min(State.Column + 2, NewIndent);
++NewIndentLevel;
} else {
NewIndent += Style.ContinuationIndentWidth;
+ NewIndent = std::min(State.Column + 1, NewIndent);
}
}
const FormatToken *NextNoComment = Current.getNextNonComment();
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=197725&r1=197724&r2=197725&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Dec 19 15:41:37 2013
@@ -1072,11 +1072,15 @@ void TokenAnnotator::calculateFormatting
FormatToken *Current = Line.First->Next;
bool InFunctionDecl = Line.MightBeFunctionDecl;
while (Current != NULL) {
- if (Current->Type == TT_LineComment)
- Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
- else if (Current->SpacesRequiredBefore == 0 &&
- spaceRequiredBefore(Line, *Current))
+ if (Current->Type == TT_LineComment) {
+ if (Current->Previous->BlockKind == BK_BracedInit)
+ Current->SpacesRequiredBefore = Style.Cpp11BracedListStyle ? 0 : 1;
+ else
+ Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
+ } else if (Current->SpacesRequiredBefore == 0 &&
+ spaceRequiredBefore(Line, *Current)) {
Current->SpacesRequiredBefore = 1;
+ }
Current->MustBreakBefore =
Current->MustBreakBefore || mustBreakBefore(Line, *Current);
@@ -1398,7 +1402,8 @@ bool TokenAnnotator::spaceRequiredBefore
bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
const FormatToken &Right) {
if (Right.is(tok::comment)) {
- return Right.NewlinesBefore > 0;
+ return Right.Previous->BlockKind != BK_BracedInit &&
+ Right.NewlinesBefore > 0;
} else if (Right.Previous->isTrailingComment() ||
(Right.is(tok::string_literal) &&
Right.Previous->is(tok::string_literal))) {
@@ -1445,7 +1450,10 @@ bool TokenAnnotator::canBreakBefore(cons
if (Right.isTrailingComment())
// We rely on MustBreakBefore being set correctly here as we should not
// change the "binding" behavior of a comment.
- return false;
+ // The first comment in a braced lists is always interpreted as belonging to
+ // the first list element. Otherwise, it should be placed outside of the
+ // list.
+ return Left.BlockKind == BK_BracedInit;
if (Left.is(tok::question) && Right.is(tok::colon))
return false;
if (Right.Type == TT_ConditionalExpr || Right.is(tok::question))
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=197725&r1=197724&r2=197725&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Dec 19 15:41:37 2013
@@ -4771,12 +4771,12 @@ TEST_F(FormatTest, LayoutCxx11Constructo
verifyFormat("DoSomethingWithVector({} /* No data */);");
verifyFormat("DoSomethingWithVector({ {} /* No data */ }, { { 1, 2 } });");
verifyFormat(
- "someFunction(OtherParam, BracedList{\n"
- " // comment 1 (Forcing interesting break)\n"
- " param1, param2,\n"
- " // comment 2\n"
- " param3, param4\n"
- " });");
+ "someFunction(OtherParam,\n"
+ " BracedList{ // comment 1 (Forcing interesting break)\n"
+ " param1, param2,\n"
+ " // comment 2\n"
+ " param3, param4 });",
+ getLLVMStyleWithColumns(75));
verifyFormat(
"std::this_thread::sleep_for(\n"
" std::chrono::nanoseconds{ std::chrono::seconds{ 1 } } / 5);");
@@ -4808,11 +4808,39 @@ TEST_F(FormatTest, LayoutCxx11Constructo
" T member = {arg1, arg2};\n"
"};",
NoSpaces);
+
+ // FIXME: The alignment of these trailing comments might be bad. Then again,
+ // this might be utterly useless in real code.
verifyFormat("Constructor::Constructor()\n"
- " : some_value{ //\n"
- " aaaaaaa //\n"
+ " : some_value{ //\n"
+ " aaaaaaa //\n"
" } {}",
NoSpaces);
+
+ // In braced lists, the first comment is always assumed to belong to the
+ // first element. Thus, it can be moved to the next or previous line as
+ // appropriate.
+ EXPECT_EQ("function({// First element:\n"
+ " 1,\n"
+ " // Second element:\n"
+ " 2});",
+ format("function({\n"
+ " // First element:\n"
+ " 1,\n"
+ " // Second element:\n"
+ " 2});",
+ NoSpaces));
+ NoSpaces.ColumnLimit = 30;
+ EXPECT_EQ(
+ "std::vector<int> MyNumbers{\n"
+ " // First element:\n"
+ " 1,\n"
+ " // Second element:\n"
+ " 2};",
+ format("std::vector<int> MyNumbers{// First element:\n"
+ " 1,\n"
+ " // Second element:\n"
+ " 2};", NoSpaces));
}
TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
More information about the cfe-commits
mailing list