Author: Marek Kurdej Date: 2022-02-13T21:22:17+01:00 New Revision: 9cb944597907458ce9c2f6bd0ecc9723b674b77f
URL: https://github.com/llvm/llvm-project/commit/9cb944597907458ce9c2f6bd0ecc9723b674b77f DIFF: https://github.com/llvm/llvm-project/commit/9cb944597907458ce9c2f6bd0ecc9723b674b77f.diff LOG: [clang-format] Correctly format loops and `if` statements even if preceded with comments. Fixes https://github.com/llvm/llvm-project/issues/53758. Braces in loops and in `if` statements with leading (block) comments were formatted according to `BraceWrapping.AfterFunction` and not `AllowShortBlocksOnASingleLine`/`AllowShortLoopsOnASingleLine`/`AllowShortIfStatementsOnASingleLine`. Previously, the code: ``` while (true) { f(); } /*comment*/ while (true) { f(); } ``` was incorrectly formatted to: ``` while (true) { f(); } /*comment*/ while (true) { f(); } ``` when using config: ``` BasedOnStyle: LLVM BreakBeforeBraces: Custom BraceWrapping: AfterFunction: false AllowShortBlocksOnASingleLine: false AllowShortLoopsOnASingleLine: false ``` and it was (correctly but by chance) formatted to: ``` while (true) { f(); } /*comment*/ while (true) { f(); } ``` when using enabling brace wrapping after functions: ``` BasedOnStyle: LLVM BreakBeforeBraces: Custom BraceWrapping: AfterFunction: true AllowShortBlocksOnASingleLine: false AllowShortLoopsOnASingleLine: false ``` Reviewed By: MyDeveloperDay, HazardyKnusperkeks, owenpan Differential Revision: https://reviews.llvm.org/D119649 Added: Modified: clang/lib/Format/UnwrappedLineFormatter.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 88efda487eeb6..883030cb3dc16 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -319,6 +319,15 @@ class LineJoiner { bool MergeShortFunctions = ShouldMergeShortFunctions(); + const FormatToken *FirstNonComment = TheLine->First; + if (FirstNonComment->is(tok::comment)) { + FirstNonComment = FirstNonComment->getNextNonComment(); + if (!FirstNonComment) + return 0; + } + // FIXME: There are probably cases where we should use FirstNonComment + // instead of TheLine->First. + if (Style.CompactNamespaces) { if (auto nsToken = TheLine->First->getNamespaceToken()) { int i = 0; @@ -358,9 +367,9 @@ class LineJoiner { if (TheLine->Last->is(TT_FunctionLBrace) && TheLine->First != TheLine->Last) return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0; // Try to merge a control statement block with left brace unwrapped. - if (TheLine->Last->is(tok::l_brace) && TheLine->First != TheLine->Last && - TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for, - TT_ForEachMacro)) { + if (TheLine->Last->is(tok::l_brace) && FirstNonComment != TheLine->Last && + FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for, + TT_ForEachMacro)) { return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never ? tryMergeSimpleBlock(I, E, Limit) : 0; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 0d315734bc951..40de29e58737f 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -1520,6 +1520,36 @@ TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) { TEST_F(FormatTest, FormatShortBracedStatements) { FormatStyle AllowSimpleBracedStatements = getLLVMStyle(); + EXPECT_EQ(AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine, false); + EXPECT_EQ(AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine, + FormatStyle::SIS_Never); + EXPECT_EQ(AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine, false); + EXPECT_EQ(AllowSimpleBracedStatements.BraceWrapping.AfterFunction, false); + verifyFormat("for (;;) {\n" + " f();\n" + "}"); + verifyFormat("/*comment*/ for (;;) {\n" + " f();\n" + "}"); + verifyFormat("BOOST_FOREACH (int v, vec) {\n" + " f();\n" + "}"); + verifyFormat("/*comment*/ BOOST_FOREACH (int v, vec) {\n" + " f();\n" + "}"); + verifyFormat("while (true) {\n" + " f();\n" + "}"); + verifyFormat("/*comment*/ while (true) {\n" + " f();\n" + "}"); + verifyFormat("if (true) {\n" + " f();\n" + "}"); + verifyFormat("/*comment*/ if (true) {\n" + " f();\n" + "}"); + AllowSimpleBracedStatements.IfMacros.push_back("MYIF"); // Where line-lengths matter, a 2-letter synonym that maintains line length. // Not IF to avoid any confusion that IF is somehow special. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits