[clang] [clang-format] Don't align comments over scopes (PR #68743)
Björn Schäpers via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 10 13:54:06 PDT 2023
https://github.com/HazardyKnusperkeks created https://github.com/llvm/llvm-project/pull/68743
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.
>From 358cbf4fd25d2d323e21774a3d4f5a605c4f1479 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern at hazardy.de>
Date: Mon, 9 Oct 2023 21:28:01 +0200
Subject: [PATCH 1/2] [clang-format][NFC] Annotate control statement r_braces
Annotating switch braces for the first time.
Also in preparation of #67906.
---
clang/lib/Format/FormatToken.h | 2 ++
clang/lib/Format/UnwrappedLineParser.cpp | 20 ++++++++++--
clang/unittests/Format/TokenAnnotatorTest.cpp | 31 +++++++++++++++++++
3 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 527f1d744a58089..606e9e790ad833b 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -52,6 +52,7 @@ namespace format {
TYPE(ConflictStart) \
/* l_brace of if/for/while */ \
TYPE(ControlStatementLBrace) \
+ TYPE(ControlStatementRBrace) \
TYPE(CppCastLParen) \
TYPE(CSharpGenericTypeConstraint) \
TYPE(CSharpGenericTypeConstraintColon) \
@@ -67,6 +68,7 @@ namespace format {
TYPE(DesignatedInitializerPeriod) \
TYPE(DictLiteral) \
TYPE(ElseLBrace) \
+ TYPE(ElseRBrace) \
TYPE(EnumLBrace) \
TYPE(EnumRBrace) \
TYPE(FatArrow) \
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 3275d7b6a71aaa0..9769d536bee32aa 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2756,6 +2756,10 @@ FormatToken *UnwrappedLineParser::parseIfThenElse(IfStmtKind *IfKind,
CompoundStatementIndenter Indenter(this, Style, Line->Level);
parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
/*MunchSemi=*/true, KeepIfBraces, &IfBlockKind);
+ if (auto Prev = FormatTok->getPreviousNonComment();
+ Prev && Prev->is(tok::r_brace)) {
+ Prev->setFinalizedType(TT_ControlStatementRBrace);
+ }
if (Style.BraceWrapping.BeforeElse)
addUnwrappedLine();
else
@@ -2794,6 +2798,10 @@ FormatToken *UnwrappedLineParser::parseIfThenElse(IfStmtKind *IfKind,
FormatToken *IfLBrace =
parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
/*MunchSemi=*/true, KeepElseBraces, &ElseBlockKind);
+ if (auto Prev = FormatTok->getPreviousNonComment();
+ Prev && Prev->is(tok::r_brace)) {
+ Prev->setFinalizedType(TT_ElseRBrace);
+ }
if (FormatTok->is(tok::kw_else)) {
KeepElseBraces = KeepElseBraces ||
ElseBlockKind == IfStmtKind::IfOnly ||
@@ -3057,12 +3065,15 @@ void UnwrappedLineParser::parseLoopBody(bool KeepBraces, bool WrapRightBrace) {
keepAncestorBraces();
if (isBlockBegin(*FormatTok)) {
- if (!KeepBraces)
- FormatTok->setFinalizedType(TT_ControlStatementLBrace);
+ FormatTok->setFinalizedType(TT_ControlStatementLBrace);
FormatToken *LeftBrace = FormatTok;
CompoundStatementIndenter Indenter(this, Style, Line->Level);
parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
/*MunchSemi=*/true, KeepBraces);
+ if (auto Prev = FormatTok->getPreviousNonComment();
+ Prev && Prev->is(tok::r_brace)) {
+ Prev->setFinalizedType(TT_ControlStatementRBrace);
+ }
if (!KeepBraces) {
assert(!NestedTooDeep.empty());
if (!NestedTooDeep.back())
@@ -3196,7 +3207,12 @@ void UnwrappedLineParser::parseSwitch() {
if (FormatTok->is(tok::l_brace)) {
CompoundStatementIndenter Indenter(this, Style, Line->Level);
+ FormatTok->setFinalizedType(TT_ControlStatementLBrace);
parseBlock();
+ if (auto Prev = FormatTok->getPreviousNonComment();
+ Prev && Prev->is(tok::r_brace)) {
+ Prev->setFinalizedType(TT_ControlStatementRBrace);
+ }
addUnwrappedLine();
} else {
addUnwrappedLine();
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 2d590f2af05e63a..b6d4cf166de0281 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2151,6 +2151,37 @@ TEST_F(TokenAnnotatorTest, UnderstandsAttributes) {
EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_AttributeRParen);
}
+TEST_F(TokenAnnotatorTest, UnderstandsControlStatements) {
+ auto Tokens = annotate("while (true) {}");
+ ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+ EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_ControlStatementLBrace);
+ EXPECT_TOKEN(Tokens[5], tok::r_brace, TT_ControlStatementRBrace);
+
+ Tokens = annotate("for (;;) {}");
+ ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+ EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_ControlStatementLBrace);
+ EXPECT_TOKEN(Tokens[6], tok::r_brace, TT_ControlStatementRBrace);
+
+ Tokens = annotate("do {} while (true);");
+ ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::l_brace, TT_ControlStatementLBrace);
+ EXPECT_TOKEN(Tokens[2], tok::r_brace, TT_ControlStatementRBrace);
+
+ Tokens = annotate("if (true) {} else if (false) {} else {}");
+ ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+ EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_ControlStatementLBrace);
+ EXPECT_TOKEN(Tokens[5], tok::r_brace, TT_ControlStatementRBrace);
+ EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_ControlStatementLBrace);
+ EXPECT_TOKEN(Tokens[12], tok::r_brace, TT_ControlStatementRBrace);
+ EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_ElseLBrace);
+ EXPECT_TOKEN(Tokens[15], tok::r_brace, TT_ElseRBrace);
+
+ Tokens = annotate("switch (foo) {}");
+ ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+ EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_ControlStatementLBrace);
+ EXPECT_TOKEN(Tokens[5], tok::r_brace, TT_ControlStatementRBrace);
+}
+
} // namespace
} // namespace format
} // namespace clang
>From a43c1a2abb7c7e851ebc0b34c3ffdf2a13f10b67 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern at hazardy.de>
Date: Tue, 10 Oct 2023 22:50:43 +0200
Subject: [PATCH 2/2] [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.
---
clang/lib/Format/WhitespaceManager.cpp | 45 +++++--
clang/unittests/Format/FormatTest.cpp | 2 +-
clang/unittests/Format/FormatTestComments.cpp | 110 +++++++++++++++++-
3 files changed, 142 insertions(+), 15 deletions(-)
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index dc81060671c1712..74f62ddc4cc3bb2 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -1048,6 +1048,9 @@ void WhitespaceManager::alignChainedConditionals() {
}
void WhitespaceManager::alignTrailingComments() {
+ if (Style.AlignTrailingComments.Kind == FormatStyle::TCAS_Never)
+ return;
+
const int Size = Changes.size();
int MinColumn = 0;
int StartOfSequence = 0;
@@ -1118,16 +1121,40 @@ void WhitespaceManager::alignTrailingComments() {
}
}
- // We don't want to align namespace end comments.
- const bool DontAlignThisComment =
- I > 0 && C.NewlinesBefore == 0 &&
- Changes[I - 1].Tok->is(TT_NamespaceRBrace);
- if (Style.AlignTrailingComments.Kind == FormatStyle::TCAS_Never ||
- DontAlignThisComment) {
+ // We don't want to align comments which end a scope, which are here
+ // identified by most closing braces.
+ const bool DontAlignThisComment = [&] {
+ if (I == 0)
+ return false;
+ if (C.NewlinesBefore != 0)
+ return false;
+ const auto &PrevChange = Changes[I - 1];
+ if (PrevChange.Tok->is(tok::r_brace))
+ return true;
+ if (PrevChange.Tok->is(tok::semi)) {
+ if (auto PrevNonComment = PrevChange.Tok->getPreviousNonComment()) {
+ if (PrevNonComment->is(tok::r_paren) &&
+ PrevNonComment->MatchingParen &&
+ PrevNonComment->MatchingParen->endsSequence(
+ tok::l_paren, tok::kw_while, TT_ControlStatementRBrace)) {
+ return true;
+ }
+ return PrevNonComment->isOneOf(
+ TT_ClassRBrace, TT_ControlStatementRBrace, TT_ElseRBrace,
+ TT_EnumRBrace, TT_NamespaceRBrace, TT_RecordRBrace,
+ TT_StructRBrace, TT_UnionRBrace);
+ }
+ }
+ return false;
+ }();
+
+ if (DontAlignThisComment) {
alignTrailingComments(StartOfSequence, I, MinColumn);
- MinColumn = ChangeMinColumn;
- MaxColumn = ChangeMinColumn;
- StartOfSequence = I;
+ // Reset to initial values, but skip this change for the next alignment
+ // pass.
+ MinColumn = 0;
+ MaxColumn = INT_MAX;
+ StartOfSequence = I + 1;
} else if (BreakBeforeNext || Newlines > NewLineThreshold ||
(ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||
// Break the comment sequence if the previous line did not end
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 2ef3c9b299bcad4..c6eec7602b6b813 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -20794,7 +20794,7 @@ TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
verifyFormat("int a[][] = {\n"
" {\n"
" {0, 2}, //\n"
- " {1, 2} //\n"
+ " {1, 2} //\n"
" }\n"
"};",
Style);
diff --git a/clang/unittests/Format/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp
index 1198329b7b5a8f0..28c216b531516f2 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -182,7 +182,7 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
"int a; // This is unrelated"));
EXPECT_EQ("class C {\n"
" void f() { // This does something ..\n"
- " } // awesome..\n"
+ " } // awesome..\n"
"\n"
" int a; // This is unrelated\n"
"};",
@@ -3191,20 +3191,120 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
"}\n"
"// Comment";
-#if 0
- // FIXME: The following comment is aligned with the namespace comment.
verifyFormat("namespace A {\n"
" int Foo;\n"
" int Bar;\n"
"} // namespace A\n"
- " // Comment",
+ "// Comment",
Input, Style);
-#endif
Style.FixNamespaceComments = false;
verifyFormat(Input, Style);
}
+TEST_F(FormatTestComments, DontAlignOverScope) {
+ verifyFormat("if (foo) {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "} // not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ verifyFormat("if (foo) {\n"
+ " // something\n"
+ "} else {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "} // not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ verifyFormat("if (foo) {\n"
+ " // something\n"
+ "} else if (foo) {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "} // not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ verifyFormat("while (foo) {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "} // not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ verifyFormat("for (;;) {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "} // not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ verifyFormat("do {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "} while (foo); // not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ verifyFormat("switch (foo) {\n"
+ "case 7: {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "} // case not aligned\n"
+ "} // switch also not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ verifyFormat("switch (foo) {\n"
+ "default: {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "} // case not aligned\n"
+ "} // switch also not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ verifyFormat("class C {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "}; // not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ verifyFormat("struct S {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "}; // not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ verifyFormat("union U {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "}; // not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ verifyFormat("enum E {\n"
+ " aLongVariable, // with comment\n"
+ " f // aligned\n"
+ "}; // not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ verifyFormat("void foo() {\n"
+ " {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ " } // not aligned\n"
+ " int bar; // new align\n"
+ " int foobar; // group\n"
+ "}");
+}
+
TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
EXPECT_EQ("/*\n"
" */",
More information about the cfe-commits
mailing list