Skip to content

Commit 3e76e5a

Browse files
HazardyKnusperkeksHazardyKnusperkeks
committed
[clang-format] Don't align comments over scopes
We now stop aligning trailing comments on all closing braces, for classes etc. we even check for the semicolon between the comment and the brace. Fixes #67906.
1 parent febc4ff commit 3e76e5a

File tree

4 files changed

+218
-21
lines changed

4 files changed

+218
-21
lines changed

‎clang/docs/ClangFormatStyleOptions.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,10 @@ the configuration (without a prefix: ``Auto``).
980980
**AlignTrailingComments** (``TrailingCommentsAlignmentStyle``) :versionbadge:`clang-format 3.7` :ref:`<AlignTrailingComments>`
981981
Control of trailing comments.
982982

983+
The alignment stops at closing braces after a line break, and only
984+
followed by other closing braces, a (``do-``) ``while``, a lambda call, or
985+
a semicolon.
986+
983987

984988
.. note::
985989

‎clang/include/clang/Format/Format.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,10 @@ struct FormatStyle {
539539

540540
/// Control of trailing comments.
541541
///
542+
/// The alignment stops at closing braces after a line break, and only
543+
/// followed by other closing braces, a (``do-``) ``while``, a lambda call, or
544+
/// a semicolon.
545+
///
542546
/// \note
543547
/// As of clang-format 16 this option is not a bool but can be set
544548
/// to the options. Conventional bool options still can be parsed as before.

‎clang/lib/Format/WhitespaceManager.cpp

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,6 +1048,9 @@ void WhitespaceManager::alignChainedConditionals() {
10481048
}
10491049

10501050
void WhitespaceManager::alignTrailingComments() {
1051+
if (Style.AlignTrailingComments.Kind == FormatStyle::TCAS_Never)
1052+
return;
1053+
10511054
const int Size = Changes.size();
10521055
int MinColumn = 0;
10531056
int StartOfSequence = 0;
@@ -1118,16 +1121,48 @@ void WhitespaceManager::alignTrailingComments() {
11181121
}
11191122
}
11201123

1121-
// We don't want to align namespace end comments.
1122-
const bool DontAlignThisComment =
1123-
I > 0 && C.NewlinesBefore == 0 &&
1124-
Changes[I - 1].Tok->is(TT_NamespaceRBrace);
1125-
if (Style.AlignTrailingComments.Kind == FormatStyle::TCAS_Never ||
1126-
DontAlignThisComment) {
1124+
// We don't want to align comments which end a scope, which are here
1125+
// identified by most closing braces.
1126+
auto DontAlignThisComment = [](const auto *Tok) {
1127+
if (Tok->is(tok::semi)) {
1128+
Tok = Tok->getPreviousNonComment();
1129+
if (!Tok)
1130+
return false;
1131+
}
1132+
if (Tok->is(tok::r_paren)) {
1133+
// Back up past the parentheses and a `TT_DoWhile` that may precede.
1134+
Tok = Tok->MatchingParen;
1135+
if (!Tok)
1136+
return false;
1137+
Tok = Tok->getPreviousNonComment();
1138+
if (!Tok)
1139+
return false;
1140+
if (Tok->is(TT_DoWhile)) {
1141+
const auto *Prev = Tok->getPreviousNonComment();
1142+
if (!Prev) {
1143+
// A do-while-loop without braces.
1144+
return true;
1145+
}
1146+
Tok = Prev;
1147+
}
1148+
}
1149+
1150+
if (Tok->isNot(tok::r_brace))
1151+
return false;
1152+
1153+
while (Tok->Previous && Tok->Previous->is(tok::r_brace))
1154+
Tok = Tok->Previous;
1155+
return Tok->NewlinesBefore > 0;
1156+
};
1157+
1158+
if (I > 0 && C.NewlinesBefore == 0 &&
1159+
DontAlignThisComment(Changes[I - 1].Tok)) {
11271160
alignTrailingComments(StartOfSequence, I, MinColumn);
1128-
MinColumn = ChangeMinColumn;
1129-
MaxColumn = ChangeMinColumn;
1130-
StartOfSequence = I;
1161+
// Reset to initial values, but skip this change for the next alignment
1162+
// pass.
1163+
MinColumn = 0;
1164+
MaxColumn = INT_MAX;
1165+
StartOfSequence = I + 1;
11311166
} else if (BreakBeforeNext || Newlines > NewLineThreshold ||
11321167
(ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||
11331168
// Break the comment sequence if the previous line did not end

‎clang/unittests/Format/FormatTestComments.cpp

Lines changed: 166 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
182182
"int a; // This is unrelated"));
183183
EXPECT_EQ("class C {\n"
184184
" void f() { // This does something ..\n"
185-
" } // awesome..\n"
185+
" } // awesome..\n"
186186
"\n"
187187
" int a; // This is unrelated\n"
188188
"};",
@@ -3102,7 +3102,8 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
31023102
StringRef Input = "namespace A {\n"
31033103
" TESTSUITE(B) {\n"
31043104
" namespace C {\n"
3105-
" namespace D {} // namespace D\n"
3105+
" namespace D { //\n"
3106+
" } // namespace D\n"
31063107
" std::string Foo = Bar; // Comment\n"
31073108
" std::string BazString = Baz; // C2\n"
31083109
" } // namespace C\n"
@@ -3114,7 +3115,8 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
31143115
verifyFormat("namespace A {\n"
31153116
" TESTSUITE(B) {\n"
31163117
" namespace C {\n"
3117-
" namespace D {} // namespace D\n"
3118+
" namespace D { //\n"
3119+
" } // namespace D\n"
31183120
" std::string Foo = Bar; // Comment\n"
31193121
" std::string BazString = Baz; // C2\n"
31203122
" } // namespace C\n"
@@ -3126,7 +3128,8 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
31263128
verifyFormat("namespace A {\n"
31273129
" TESTSUITE(B) {\n"
31283130
" namespace C {\n"
3129-
" namespace D {} // namespace D\n"
3131+
" namespace D { //\n"
3132+
" } // namespace D\n"
31303133
" std::string Foo = Bar; // Comment\n"
31313134
" std::string BazString = Baz; // C2\n"
31323135
" } // namespace C\n"
@@ -3138,7 +3141,8 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
31383141
verifyFormat("namespace A {\n"
31393142
" TESTSUITE(B) {\n"
31403143
" namespace C {\n"
3141-
" namespace D {} // namespace D\n"
3144+
" namespace D { //\n"
3145+
" } // namespace D\n"
31423146
" std::string Foo = Bar; // Comment\n"
31433147
" std::string BazString = Baz; // C2\n"
31443148
" } // namespace C\n"
@@ -3151,7 +3155,8 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
31513155
verifyFormat("namespace A {\n"
31523156
" TESTSUITE(B) {\n"
31533157
" namespace C {\n"
3154-
" namespace D {} // namespace D\n"
3158+
" namespace D { //\n"
3159+
" } // namespace D\n"
31553160
" std::string Foo = Bar; // Comment\n"
31563161
" std::string BazString = Baz; // C2\n"
31573162
" } // namespace C\n"
@@ -3163,7 +3168,8 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
31633168
verifyFormat("namespace A {\n"
31643169
" TESTSUITE(B) {\n"
31653170
" namespace C {\n"
3166-
" namespace D {} // namespace D\n"
3171+
" namespace D { //\n"
3172+
" } // namespace D\n"
31673173
" std::string Foo = Bar; // Comment\n"
31683174
" std::string BazString = Baz; // C2\n"
31693175
" } // namespace C\n"
@@ -3175,7 +3181,8 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
31753181
verifyFormat("namespace A {\n"
31763182
" TESTSUITE(B) {\n"
31773183
" namespace C {\n"
3178-
" namespace D {} // namespace D\n"
3184+
" namespace D { //\n"
3185+
" } // namespace D\n"
31793186
" std::string Foo = Bar; // Comment\n"
31803187
" std::string BazString = Baz; // C2\n"
31813188
" } // namespace C\n"
@@ -3191,20 +3198,167 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
31913198
"}\n"
31923199
"// Comment";
31933200

3194-
#if 0
3195-
// FIXME: The following comment is aligned with the namespace comment.
31963201
verifyFormat("namespace A {\n"
31973202
" int Foo;\n"
31983203
" int Bar;\n"
31993204
"} // namespace A\n"
3200-
" // Comment",
3205+
"// Comment",
32013206
Input, Style);
3202-
#endif
32033207

32043208
Style.FixNamespaceComments = false;
32053209
verifyFormat(Input, Style);
32063210
}
32073211

3212+
TEST_F(FormatTestComments, DontAlignOverScope) {
3213+
verifyFormat("if (foo) {\n"
3214+
" int aLongVariable; // with comment\n"
3215+
" int f; // aligned\n"
3216+
"} // not aligned\n"
3217+
"int bar; // new align\n"
3218+
"int foobar; // group");
3219+
3220+
verifyFormat("if (foo) {\n"
3221+
" // something\n"
3222+
"} else {\n"
3223+
" int aLongVariable; // with comment\n"
3224+
" int f; // aligned\n"
3225+
"} // not aligned\n"
3226+
"int bar; // new align\n"
3227+
"int foobar; // group");
3228+
3229+
verifyFormat("if (foo) {\n"
3230+
" // something\n"
3231+
"} else if (foo) {\n"
3232+
" int aLongVariable; // with comment\n"
3233+
" int f; // aligned\n"
3234+
"} // not aligned\n"
3235+
"int bar; // new align\n"
3236+
"int foobar; // group");
3237+
3238+
verifyFormat("while (foo) {\n"
3239+
" int aLongVariable; // with comment\n"
3240+
" int f; // aligned\n"
3241+
"} // not aligned\n"
3242+
"int bar; // new align\n"
3243+
"int foobar; // group");
3244+
3245+
verifyFormat("for (;;) {\n"
3246+
" int aLongVariable; // with comment\n"
3247+
" int f; // aligned\n"
3248+
"} // not aligned\n"
3249+
"int bar; // new align\n"
3250+
"int foobar; // group");
3251+
3252+
verifyFormat("do {\n"
3253+
" int aLongVariable; // with comment\n"
3254+
" int f; // aligned\n"
3255+
"} while (foo); // not aligned\n"
3256+
"int bar; // new align\n"
3257+
"int foobar; // group");
3258+
3259+
verifyFormat("do\n"
3260+
" int aLongVariable; // with comment\n"
3261+
"while (foo); // not aigned\n"
3262+
"int bar; // new align\n"
3263+
"int foobar; // group");
3264+
3265+
verifyFormat("do\n"
3266+
" int aLongVariable; // with comment\n"
3267+
"/**/ while (foo); // not aigned\n"
3268+
"int bar; // new align\n"
3269+
"int foobar; // group");
3270+
3271+
verifyFormat("switch (foo) {\n"
3272+
"case 7: {\n"
3273+
" int aLongVariable; // with comment\n"
3274+
" int f; // aligned\n"
3275+
"} // case not aligned\n"
3276+
"} // switch also not aligned\n"
3277+
"int bar; // new align\n"
3278+
"int foobar; // group");
3279+
3280+
verifyFormat("switch (foo) {\n"
3281+
"default: {\n"
3282+
" int aLongVariable; // with comment\n"
3283+
" int f; // aligned\n"
3284+
"} // case not aligned\n"
3285+
"} // switch also not aligned\n"
3286+
"int bar; // new align\n"
3287+
"int foobar; // group");
3288+
3289+
verifyFormat("class C {\n"
3290+
" int aLongVariable; // with comment\n"
3291+
" int f; // aligned\n"
3292+
"}; // not aligned\n"
3293+
"int bar; // new align\n"
3294+
"int foobar; // group");
3295+
3296+
verifyFormat("struct S {\n"
3297+
" int aLongVariable; // with comment\n"
3298+
" int f; // aligned\n"
3299+
"}; // not aligned\n"
3300+
"int bar; // new align\n"
3301+
"int foobar; // group");
3302+
3303+
verifyFormat("union U {\n"
3304+
" int aLongVariable; // with comment\n"
3305+
" int f; // aligned\n"
3306+
"}; // not aligned\n"
3307+
"int bar; // new align\n"
3308+
"int foobar; // group");
3309+
3310+
verifyFormat("enum E {\n"
3311+
" aLongVariable, // with comment\n"
3312+
" f // aligned\n"
3313+
"}; // not aligned\n"
3314+
"int bar; // new align\n"
3315+
"int foobar; // group");
3316+
3317+
verifyFormat("void foo() {\n"
3318+
" {\n"
3319+
" int aLongVariable; // with comment\n"
3320+
" int f; // aligned\n"
3321+
" } // not aligned\n"
3322+
" int bar; // new align\n"
3323+
" int foobar; // group\n"
3324+
"}");
3325+
3326+
verifyFormat("auto longLambda = [] { // comment\n"
3327+
" int aLongVariable; // with comment\n"
3328+
" int f; // aligned\n"
3329+
"}; // not aligned\n"
3330+
"int bar; // new align\n"
3331+
"int foobar; // group\n"
3332+
"auto shortLambda = [] { return 5; }; // aligned");
3333+
3334+
verifyFormat("auto longLambdaResult = [] { // comment\n"
3335+
" int aLongVariable; // with comment\n"
3336+
" int f; // aligned\n"
3337+
"}(); // not aligned\n"
3338+
"int bar; // new align\n"
3339+
"int foobar; // group\n"
3340+
"auto shortLambda = [] { return 5; }(); // aligned");
3341+
3342+
verifyFormat(
3343+
"auto longLambdaResult = [](auto I, auto J) { // comment\n"
3344+
" int aLongVariable; // with comment\n"
3345+
" int f; // aligned\n"
3346+
"}(\"Input\", 5); // not aligned\n"
3347+
"int bar; // new align\n"
3348+
"int foobar; // group\n"
3349+
"auto shortL = [](auto I, auto J) { return 5; }(\"In\", 5); // aligned");
3350+
3351+
verifyFormat("enum E1 { V1, V2 }; // Aligned\n"
3352+
"enum E2 { LongerNames, InThis, Enum }; // Comments");
3353+
3354+
verifyFormat("class C {\n"
3355+
" int aLongVariable; // with comment\n"
3356+
" int f; // aligned\n"
3357+
"} /* middle comment */; // not aligned\n"
3358+
"int bar; // new align\n"
3359+
"int foobar; // group");
3360+
}
3361+
32083362
TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
32093363
EXPECT_EQ("/*\n"
32103364
" */",

0 commit comments

Comments
 (0)