Skip to content

Commit 8f6e5bc

Browse files
[clang-format] Remove special handling of comments after brace/paren
Fixes http://llvm.org/PR55487 (#55487) The code did not match the documentation about Cpp11BracedListStyle. Changed handling of comments after opening braces, which are supposedly function call like to behave exactly like their parenthesis counter part.
1 parent 738e927 commit 8f6e5bc

File tree

5 files changed

+76
-30
lines changed

5 files changed

+76
-30
lines changed

clang/lib/Format/ContinuationIndenter.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -920,12 +920,15 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
920920
// align the commas with the opening paren.
921921
if (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign &&
922922
!CurrentState.IsCSharpGenericTypeConstraint && Previous.opensScope() &&
923-
Previous.isNot(TT_ObjCMethodExpr) && Previous.isNot(TT_RequiresClause) &&
924-
Previous.isNot(TT_TableGenDAGArgOpener) &&
925-
Previous.isNot(TT_TableGenDAGArgOpenerToBreak) &&
923+
Previous.isNotOneOf(TT_ObjCMethodExpr, TT_RequiresClause,
924+
TT_TableGenDAGArgOpener,
925+
TT_TableGenDAGArgOpenerToBreak) &&
926926
!(Current.MacroParent && Previous.MacroParent) &&
927927
(Current.isNot(TT_LineComment) ||
928-
Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen)) &&
928+
(Previous.is(BK_BracedInit) &&
929+
(!Style.Cpp11BracedListStyle || !Previous.Previous ||
930+
Previous.Previous->isNot(TT_StartOfName))) ||
931+
Previous.is(TT_VerilogMultiLineListLParen)) &&
929932
!IsInTemplateString(Current)) {
930933
CurrentState.Indent = State.Column + Spaces;
931934
CurrentState.IsAligned = true;

clang/lib/Format/FormatToken.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,9 @@ struct FormatToken {
645645
return is(K1) || isOneOf(K2, Ks...);
646646
}
647647
template <typename T> bool isNot(T Kind) const { return !is(Kind); }
648+
template <typename... Ts> bool isNotOneOf(Ts... Ks) const {
649+
return !isOneOf(Ks...);
650+
}
648651

649652
bool isIf(bool AllowConstexprMacro = true) const {
650653
return is(tok::kw_if) || endsSequence(tok::kw_constexpr, tok::kw_if) ||

clang/lib/Format/TokenAnnotator.cpp

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4099,16 +4099,9 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const {
40994099
for (auto *Current = First->Next; Current; Current = Current->Next) {
41004100
const FormatToken *Prev = Current->Previous;
41014101
if (Current->is(TT_LineComment)) {
4102-
if (Prev->is(BK_BracedInit) && Prev->opensScope()) {
4103-
Current->SpacesRequiredBefore =
4104-
(Style.Cpp11BracedListStyle && !Style.SpacesInParensOptions.Other)
4105-
? 0
4106-
: 1;
4107-
} else if (Prev->is(TT_VerilogMultiLineListLParen)) {
4108-
Current->SpacesRequiredBefore = 0;
4109-
} else {
4110-
Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
4111-
}
4102+
Current->SpacesRequiredBefore = Prev->is(TT_VerilogMultiLineListLParen)
4103+
? 0
4104+
: Style.SpacesBeforeTrailingComments;
41124105

41134106
// If we find a trailing comment, iterate backwards to determine whether
41144107
// it seems to relate to a specific parameter. If so, break before that

clang/unittests/Format/FormatTest.cpp

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14291,20 +14291,18 @@ TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
1429114291
" CDDDP83848_RBR_REGISTER};",
1429214292
NoBinPacking);
1429314293

14294-
// FIXME: The alignment of these trailing comments might be bad. Then again,
14295-
// this might be utterly useless in real code.
1429614294
verifyFormat("Constructor::Constructor()\n"
14297-
" : some_value{ //\n"
14298-
" aaaaaaa, //\n"
14299-
" bbbbbbb} {}");
14295+
" : some_value{ //\n"
14296+
" aaaaaaa, //\n"
14297+
" bbbbbbb} {}");
1430014298

1430114299
// In braced lists, the first comment is always assumed to belong to the
1430214300
// first element. Thus, it can be moved to the next or previous line as
1430314301
// appropriate.
14304-
verifyFormat("function({// First element:\n"
14305-
" 1,\n"
14306-
" // Second element:\n"
14307-
" 2});",
14302+
verifyFormat("function({ // First element:\n"
14303+
" 1,\n"
14304+
" // Second element:\n"
14305+
" 2});",
1430814306
"function({\n"
1430914307
" // First element:\n"
1431014308
" 1,\n"
@@ -14426,7 +14424,7 @@ TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
1442614424
verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
1442714425
verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
1442814426
verifyFormat("vector< int > x{ // comment 1\n"
14429-
" 1, 2, 3, 4 };",
14427+
" 1, 2, 3, 4 };",
1443014428
SpaceBetweenBraces);
1443114429
SpaceBetweenBraces.ColumnLimit = 20;
1443214430
verifyFormat("vector< int > x{\n"

clang/unittests/Format/FormatTestComments.cpp

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,12 +1474,12 @@ TEST_F(FormatTestComments, CommentsInStaticInitializers) {
14741474
verifyFormat("S s = {{a, b, c}, // Group #1\n"
14751475
" {d, e, f}, // Group #2\n"
14761476
" {g, h, i}}; // Group #3");
1477-
verifyFormat("S s = {{// Group #1\n"
1478-
" a, b, c},\n"
1479-
" {// Group #2\n"
1480-
" d, e, f},\n"
1481-
" {// Group #3\n"
1482-
" g, h, i}};");
1477+
verifyFormat("S s = {{ // Group #1\n"
1478+
" a, b, c},\n"
1479+
" { // Group #2\n"
1480+
" d, e, f},\n"
1481+
" { // Group #3\n"
1482+
" g, h, i}};");
14831483

14841484
EXPECT_EQ("S s = {\n"
14851485
" // Some comment\n"
@@ -4699,6 +4699,55 @@ TEST_F(FormatTestComments, SplitCommentIntroducers) {
46994699
getLLVMStyleWithColumns(10)));
47004700
}
47014701

4702+
TEST_F(FormatTestComments, LineCommentsOnStartOfFunctionCall) {
4703+
auto Style = getLLVMStyle();
4704+
4705+
EXPECT_TRUE(Style.Cpp11BracedListStyle);
4706+
4707+
verifyFormat("T foo( // Comment\n"
4708+
" arg);",
4709+
Style);
4710+
4711+
verifyFormat("T bar{ // Comment\n"
4712+
" arg};",
4713+
Style);
4714+
4715+
verifyFormat("T baz({ // Comment\n"
4716+
" arg});",
4717+
Style);
4718+
4719+
verifyFormat("func( // Comment\n"
4720+
" arg);",
4721+
Style);
4722+
4723+
verifyFormat("func({ // Comment\n"
4724+
" arg});",
4725+
Style);
4726+
4727+
Style.Cpp11BracedListStyle = false;
4728+
4729+
verifyFormat("T foo( // Comment\n"
4730+
" arg);",
4731+
Style);
4732+
4733+
verifyFormat("T bar{ // Comment\n"
4734+
" arg\n"
4735+
"};",
4736+
Style);
4737+
4738+
verifyFormat("T baz({ // Comment\n"
4739+
" arg });",
4740+
Style);
4741+
4742+
verifyFormat("func( // Comment\n"
4743+
" arg);",
4744+
Style);
4745+
4746+
verifyFormat("func({ // Comment\n"
4747+
" arg });",
4748+
Style);
4749+
}
4750+
47024751
} // end namespace
47034752
} // namespace test
47044753
} // end namespace format

0 commit comments

Comments
 (0)