[clang] [clang-format] Don't align comments over scopes (PR #68743)
Björn Schäpers via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 20 04:20:26 PDT 2023
https://github.com/HazardyKnusperkeks updated https://github.com/llvm/llvm-project/pull/68743
>From be24095877e6ddd25e0884ad8aaf3eca4846f38d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern at hazardy.de>
Date: Fri, 20 Oct 2023 13:00:39 +0200
Subject: [PATCH 1/2] [clang-format] Annotate do while while
So we can differentiate on the while keyword between a do-while-loop and a
normal while-loop.
---
clang/lib/Format/FormatToken.h | 1 +
clang/lib/Format/UnwrappedLineParser.cpp | 2 ++
clang/unittests/Format/TokenAnnotatorTest.cpp | 10 ++++++++++
3 files changed, 13 insertions(+)
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 606e9e790ad833b..acd24f836199da1 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -67,6 +67,7 @@ namespace format {
TYPE(DesignatedInitializerLSquare) \
TYPE(DesignatedInitializerPeriod) \
TYPE(DictLiteral) \
+ TYPE(DoWhile) \
TYPE(ElseLBrace) \
TYPE(ElseRBrace) \
TYPE(EnumLBrace) \
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 7bb487d020ea6f7..488d8dc07b1eae3 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3137,6 +3137,8 @@ void UnwrappedLineParser::parseDoWhile() {
return;
}
+ FormatTok->setFinalizedType(TT_DoWhile);
+
// If in Whitesmiths mode, the line with the while() needs to be indented
// to the same level as the block.
if (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths)
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 4dbe2a532c5fdb2..290d0103bb3cf87 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2334,6 +2334,16 @@ TEST_F(TokenAnnotatorTest, UnderstandsControlStatements) {
EXPECT_TOKEN(Tokens[5], tok::r_brace, TT_ControlStatementRBrace);
}
+TEST_F(TokenAnnotatorTest, UnderstandsDoWhile) {
+ auto Tokens = annotate("do { ++i; } while ( i > 5 );");
+ ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+ EXPECT_TOKEN(Tokens[6], tok::kw_while, TT_DoWhile);
+
+ Tokens = annotate("do ++i; while ( i > 5 );");
+ ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+ EXPECT_TOKEN(Tokens[4], tok::kw_while, TT_DoWhile);
+}
+
} // namespace
} // namespace format
} // namespace clang
>From 141443c2df008fa6fb7f283a025135712cce92a7 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 | 44 ++++-
clang/unittests/Format/FormatTestComments.cpp | 176 ++++++++++++++++--
2 files changed, 199 insertions(+), 21 deletions(-)
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index dc81060671c1712..67e5dd4c04445eb 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,39 @@ 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 || C.NewlinesBefore > 0)
+ return false;
+ const auto *Tok = Changes[I - 1].Tok;
+ if (Tok->is(tok::semi)) {
+ Tok = Tok->getPreviousNonComment();
+ if (!Tok)
+ return false;
+ }
+ if (Tok->isNot(tok::r_brace)) {
+ if (Tok->isNot(tok::r_paren) || !Tok->MatchingParen)
+ return false;
+ auto BeforeParent = Tok->MatchingParen->getPreviousNonComment();
+ return BeforeParent && BeforeParent->is(TT_DoWhile);
+ }
+
+ for (auto Prev = Tok->getPreviousNonComment();
+ Prev && Prev->is(tok::r_brace);
+ Prev = Tok->getPreviousNonComment()) {
+ Tok = Prev;
+ }
+ return Tok->NewlinesBefore > 0;
+ }();
+
+ 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/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp
index 1198329b7b5a8f0..a37505926ec63a8 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"
"};",
@@ -3102,7 +3102,9 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
StringRef Input = "namespace A {\n"
" TESTSUITE(B) {\n"
" namespace C {\n"
- " namespace D {} // namespace D\n"
+ " namespace D {\n"
+ " // ...\n"
+ " } // namespace D\n"
" std::string Foo = Bar; // Comment\n"
" std::string BazString = Baz; // C2\n"
" } // namespace C\n"
@@ -3114,7 +3116,9 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
verifyFormat("namespace A {\n"
" TESTSUITE(B) {\n"
" namespace C {\n"
- " namespace D {} // namespace D\n"
+ " namespace D {\n"
+ " // ...\n"
+ " } // namespace D\n"
" std::string Foo = Bar; // Comment\n"
" std::string BazString = Baz; // C2\n"
" } // namespace C\n"
@@ -3126,7 +3130,9 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
verifyFormat("namespace A {\n"
" TESTSUITE(B) {\n"
" namespace C {\n"
- " namespace D {} // namespace D\n"
+ " namespace D {\n"
+ " // ...\n"
+ " } // namespace D\n"
" std::string Foo = Bar; // Comment\n"
" std::string BazString = Baz; // C2\n"
" } // namespace C\n"
@@ -3138,7 +3144,9 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
verifyFormat("namespace A {\n"
" TESTSUITE(B) {\n"
" namespace C {\n"
- " namespace D {} // namespace D\n"
+ " namespace D {\n"
+ " // ...\n"
+ " } // namespace D\n"
" std::string Foo = Bar; // Comment\n"
" std::string BazString = Baz; // C2\n"
" } // namespace C\n"
@@ -3151,7 +3159,9 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
verifyFormat("namespace A {\n"
" TESTSUITE(B) {\n"
" namespace C {\n"
- " namespace D {} // namespace D\n"
+ " namespace D {\n"
+ " // ...\n"
+ " } // namespace D\n"
" std::string Foo = Bar; // Comment\n"
" std::string BazString = Baz; // C2\n"
" } // namespace C\n"
@@ -3163,7 +3173,9 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
verifyFormat("namespace A {\n"
" TESTSUITE(B) {\n"
" namespace C {\n"
- " namespace D {} // namespace D\n"
+ " namespace D {\n"
+ " // ...\n"
+ " } // namespace D\n"
" std::string Foo = Bar; // Comment\n"
" std::string BazString = Baz; // C2\n"
" } // namespace C\n"
@@ -3175,7 +3187,9 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
verifyFormat("namespace A {\n"
" TESTSUITE(B) {\n"
" namespace C {\n"
- " namespace D {} // namespace D\n"
+ " namespace D {\n"
+ " // ...\n"
+ " } // namespace D\n"
" std::string Foo = Bar; // Comment\n"
" std::string BazString = Baz; // C2\n"
" } // namespace C\n"
@@ -3191,20 +3205,158 @@ 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("do\n"
+ " int aLongVariable; // with comment\n"
+ "while (foo); // not aigned\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"
+ "}");
+
+ verifyFormat("auto longLambda = [] { // comment\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "}; // not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group\n"
+ "auto shortLambda = [] { return 5; }; // aligned");
+
+ verifyFormat("enum E1 { V1, V2 }; // Aligned\n"
+ "enum E2 { LongerNames, InThis, Enum }; // Comments");
+
+ verifyFormat("class C {\n"
+ " int aLongVariable; // with comment\n"
+ " int f; // aligned\n"
+ "} /* middle comment */; // not aligned\n"
+ "int bar; // new align\n"
+ "int foobar; // group");
+
+ auto Style = getLLVMStyle();
+ Style.CompactNamespaces = true;
+ Style.FixNamespaceComments = false;
+
+ verifyFormat("namespace A { namespace B {\n"
+ "int I; // Comment\n"
+ "}} // namespace A::B",
+ Style);
+
+ verifyFormat("namespace A { namespace B {\n"
+ "int I; // Comment\n"
+ "} /* Who comments here? */ } // namespace A::B",
+ Style);
+}
+
TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
EXPECT_EQ("/*\n"
" */",
More information about the cfe-commits
mailing list