https://github.com/rudolflovrencic created https://github.com/llvm/llvm-project/pull/196021
If the opening brace of control statement blocks, record block, and function blocks is not on the same line, the block can never be short (the closing brace is always placed into a separate line). The format options AllowShortIfStatementsOnASingleLine and AllowShortLoopsOnASingleLine now correctly control only the braceless variants of these statements. Fixes #183705 and #187993 With these fixes, I was able to perform clang `v21->v23` migration on a 40k line codebase with a custom `.clang-format` file without major formatting changes (only a few functions declarations and braced initialization constructor calls were formatted differently due other changes unrelated to this). From 43921127d7333681a39a5a49b793166e88035640 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rudolf=20Lovren=C4=8Di=C4=87?= <[email protected]> Date: Wed, 6 May 2026 10:00:04 +0200 Subject: [PATCH] [clang-format] Disable short blocks if brace is on the new line If the opening brace of control statement blocks, record block, and function blocks is not on the same line, the block can never be short (the closing brace is always placed into a separate line). The format options AllowShortIfStatementsOnASingleLine and AllowShortLoopsOnASingleLine now correctly control only the braceless variants of these statements. Fixes #183705 and #187993 --- clang/lib/Format/UnwrappedLineFormatter.cpp | 32 ++++++------ clang/unittests/Format/FormatTest.cpp | 54 ++++++++++----------- 2 files changed, 42 insertions(+), 44 deletions(-) diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 42eabc065b1a8..f02ae5b5fecb8 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -264,13 +264,10 @@ class LineJoiner { : Limit - TheLine->Last->TotalLength; if (TheLine->Last->is(TT_FunctionLBrace) && - TheLine->First == TheLine->Last) { - const bool EmptyFunctionBody = NextLine.First->is(tok::r_brace); - if ((EmptyFunctionBody && !Style.BraceWrapping.SplitEmptyFunction) || - (!EmptyFunctionBody && - Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Always)) { - return tryMergeSimpleBlock(I, E, Limit); - } + TheLine->First == TheLine->Last && + !Style.BraceWrapping.SplitEmptyFunction && + NextLine.First->is(tok::r_brace)) { + return tryMergeSimpleBlock(I, E, Limit); } // Try merging record blocks that have had their left brace wrapped into @@ -647,11 +644,8 @@ class LineJoiner { const bool IsEmptyBlock = Line->Last->is(tok::l_brace) && NextLine->First->is(tok::r_brace); - if ((IsEmptyBlock && !Style.BraceWrapping.SplitEmptyRecord) || - (!IsEmptyBlock && - Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Always)) { + if (IsEmptyBlock && !Style.BraceWrapping.SplitEmptyRecord) return tryMergeSimpleBlock(I, E, Limit); - } } return 0; @@ -895,9 +889,12 @@ class LineJoiner { Line.startsWithExportBlock()) { if (IsSplitBlock) return 0; + const bool BracedBlocksAlwaysOnSingleLine = + Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Always; // Don't merge when we can't except the case when // the control statement block is empty - if (!Style.AllowShortIfStatementsOnASingleLine && + if (!BracedBlocksAlwaysOnSingleLine && + !Style.AllowShortIfStatementsOnASingleLine && Line.First->isOneOf(tok::kw_if, tok::kw_else) && !Style.BraceWrapping.AfterControlStatement && I[1]->First->isNot(tok::r_brace)) { @@ -910,7 +907,8 @@ class LineJoiner { I + 2 != E && I[2]->First->isNot(tok::r_brace)) { return 0; } - if (!Style.AllowShortLoopsOnASingleLine && + if (!BracedBlocksAlwaysOnSingleLine && + !Style.AllowShortLoopsOnASingleLine && Line.First->isOneOf(tok::kw_while, tok::kw_do, tok::kw_for, TT_ForEachMacro) && !Style.BraceWrapping.AfterControlStatement && @@ -988,9 +986,11 @@ class LineJoiner { if (I[1]->Last->is(TT_LineComment)) return 0; do { - if (Tok->isOneOf(tok::l_brace, tok::r_brace) && - Tok->isNot(BK_BracedInit)) { - return 0; + if (Tok->isOneOf(tok::l_brace, tok::r_brace)) { + const FormatToken *Open = + Tok->is(tok::l_brace) ? Tok : Tok->MatchingParen; + if (!Open || Open->isNot(BK_BracedInit)) + return 0; } Tok = Tok->Next; } while (Tok); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index f5e496652e15e..c5c6b048ab0bb 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -1607,10 +1607,7 @@ TEST_F(FormatTest, FormatShortBracedStatements) { AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never; verifyFormat("if (true) {}", AllowSimpleBracedStatements); - verifyFormat("if (true) {\n" - " f();\n" - "}", - AllowSimpleBracedStatements); + verifyFormat("if (true) { f(); }", AllowSimpleBracedStatements); verifyFormat("if (true) {\n" " f();\n" "} else {\n" @@ -1618,10 +1615,7 @@ TEST_F(FormatTest, FormatShortBracedStatements) { "}", AllowSimpleBracedStatements); verifyFormat("MYIF (true) {}", AllowSimpleBracedStatements); - verifyFormat("MYIF (true) {\n" - " f();\n" - "}", - AllowSimpleBracedStatements); + verifyFormat("MYIF (true) { f(); }", AllowSimpleBracedStatements); verifyFormat("MYIF (true) {\n" " f();\n" "} else {\n" @@ -1631,19 +1625,11 @@ TEST_F(FormatTest, FormatShortBracedStatements) { AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = false; verifyFormat("while (true) {}", AllowSimpleBracedStatements); - verifyFormat("while (true) {\n" - " f();\n" - "}", - AllowSimpleBracedStatements); + verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements); verifyFormat("for (;;) {}", AllowSimpleBracedStatements); - verifyFormat("for (;;) {\n" - " f();\n" - "}", - AllowSimpleBracedStatements); + verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements); verifyFormat("BOOST_FOREACH (int v, vec) {}", AllowSimpleBracedStatements); - verifyFormat("BOOST_FOREACH (int v, vec) {\n" - " f();\n" - "}", + verifyFormat("BOOST_FOREACH (int v, vec) { f(); }", AllowSimpleBracedStatements); AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = @@ -2432,12 +2418,8 @@ TEST_F(FormatTest, ForEachLoops) { " int j = 1;\n" " Q_FOREACH (int &v, vec)\n" " v *= 2;\n" - " for (;;) {\n" - " int j = 1;\n" - " }\n" - " Q_FOREACH (int &v, vec) {\n" - " int j = 1;\n" - " }\n" + " for (;;) { int j = 1; }\n" + " Q_FOREACH (int &v, vec) { int j = 1; }\n" "}", ShortBlocks); @@ -15342,7 +15324,19 @@ TEST_F(FormatTest, MergeShortFunctionBody) { Style.BraceWrapping.AfterFunction = true; verifyFormat("int foo()\n" - "{ return 1; }", + "{\n" + " return 1;\n" + "}\n", + Style); + verifyFormat("int foo()\n" + "{\n" + " if (true) { return 42; };\n" + "}\n", + Style); + verifyFormat("int foo()\n" + "{\n" + " static constexpr auto lambda = []() -> int { return 42; };\n" + "}\n", Style); } @@ -15691,11 +15685,15 @@ TEST_F(FormatTest, AllowShortRecordOnASingleLine) { Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Never; verifyFormat("class foo\n" - "{ int i; };", + "{\n" + " int i;\n" + "};", Style); Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Empty; verifyFormat("class foo\n" - "{ int i; };", + "{\n" + " int i;\n" + "};", Style); Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Always; verifyFormat("class foo\n" _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
