owenpan created this revision.
owenpan added reviewers: HazardyKnusperkeks, MyDeveloperDay, curdeius.
owenpan added a project: clang-format.
Herald added a project: All.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When the opening brace of a control statement block is wrapped, we must check
the previous line to determine whether to try to merge the block.
Fixes https://github.com/llvm/llvm-project/issues/38639.
Fixes https://github.com/llvm/llvm-project/issues/48007.
Fixes https://github.com/llvm/llvm-project/issues/57421.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D133093
Files:
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1860,6 +1860,69 @@
" f();\n"
"}",
AllowSimpleBracedStatements);
+
+ FormatStyle Style = getLLVMStyle();
+ Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+ Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
+
+ verifyFormat("while (i > 0)\n"
+ "{\n"
+ " --i;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a)\n"
+ "{\n"
+ " ++b;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a)\n"
+ "{\n"
+ " b = 1;\n"
+ "} else\n"
+ "{\n"
+ " b = 0;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a)\n"
+ "{\n"
+ " b = 1;\n"
+ "} else if (c)\n"
+ "{\n"
+ " b = 2;\n"
+ "} else\n"
+ "{\n"
+ " b = 0;\n"
+ "}",
+ Style);
+
+ Style.BraceWrapping.BeforeElse = true;
+
+ verifyFormat("if (a)\n"
+ "{\n"
+ " b = 1;\n"
+ "}\n"
+ "else\n"
+ "{\n"
+ " b = 0;\n"
+ "}",
+ Style);
+
+ verifyFormat("if (a)\n"
+ "{\n"
+ " b = 1;\n"
+ "}\n"
+ "else if (c)\n"
+ "{\n"
+ " b = 2;\n"
+ "}\n"
+ "else\n"
+ "{\n"
+ " b = 0;\n"
+ "}",
+ Style);
}
TEST_F(FormatTest, UnderstandsMacros) {
@@ -25276,8 +25339,6 @@
Style.BreakBeforeBraces = FormatStyle::BS_Custom;
Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
- // TODO: Delete the following line after #57421 is fixed.
- Style.BraceWrapping.AfterFunction = true;
verifyFormat("if (a) //\n"
"{\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -711,16 +711,23 @@
if (Tok && Tok->is(tok::colon))
return 0;
}
- if (Line.First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while, tok::kw_do,
- tok::kw_try, tok::kw___try, tok::kw_catch,
- tok::kw___finally, tok::kw_for, TT_ForEachMacro,
- tok::r_brace, Keywords.kw___except)) {
- if (Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never)
- return 0;
- if (Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Empty &&
- !I[1]->First->is(tok::r_brace)) {
+
+ auto IsCtrlStmt = [](const auto &Line) {
+ return Line.First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
+ tok::kw_do, tok::kw_for, TT_ForEachMacro);
+ };
+
+ const bool IsSplitBlock =
+ Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never ||
+ (Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Empty &&
+ I[1]->First->isNot(tok::r_brace));
+
+ if (IsCtrlStmt(Line) ||
+ Line.First->isOneOf(tok::kw_try, tok::kw___try, tok::kw_catch,
+ tok::kw___finally, tok::r_brace,
+ Keywords.kw___except)) {
+ if (IsSplitBlock)
return 0;
- }
// Don't merge when we can't except the case when
// the control statement block is empty
if (!Style.AllowShortIfStatementsOnASingleLine &&
@@ -763,6 +770,11 @@
}
if (Line.Last->is(tok::l_brace)) {
+ if (IsSplitBlock && Line.First == Line.Last &&
+ I > AnnotatedLines.begin() &&
+ (I[-1]->endsWith(tok::kw_else) || IsCtrlStmt(*I[-1]))) {
+ return 0;
+ }
FormatToken *Tok = I[1]->First;
auto ShouldMerge = [Tok]() {
if (Tok->isNot(tok::r_brace) || Tok->MustBreakBefore)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits