llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-format Author: Rudolf Lovrenčić (rudolflovrencic) <details> <summary>Changes</summary> 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). --- Full diff: https://github.com/llvm/llvm-project/pull/196021.diff 2 Files Affected: - (modified) clang/lib/Format/UnwrappedLineFormatter.cpp (+16-16) - (modified) clang/unittests/Format/FormatTest.cpp (+26-28) ``````````diff 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" `````````` </details> https://github.com/llvm/llvm-project/pull/196021 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
