From db15e31212436ae51c04e8b5fcc2f140db4d6626 Mon Sep 17 00:00:00 2001 From: owenca Date: Wed, 1 Jun 2022 01:06:02 -0700 Subject: [PATCH] [clang-format] Handle do-while loops for RemoveBracesLLVM Also updates the unit tests to match the updated LLVM Coding Standards. Differential Revision: https://reviews.llvm.org/D126758 --- clang/lib/Format/UnwrappedLineParser.cpp | 9 +++- clang/lib/Format/UnwrappedLineParser.h | 1 + clang/unittests/Format/FormatTest.cpp | 71 +++++++++++++++++++++----------- 3 files changed, 56 insertions(+), 25 deletions(-) diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 9a5d85c..8de7fae9 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -480,6 +480,7 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace, : TT_Unknown; const bool IsPrecededByCommentOrPPDirective = !Style.RemoveBracesLLVM || precededByCommentOrPPDirective(); + bool HasDoWhile = false; bool HasLabel = false; unsigned StatementCount = 0; bool SwitchLabelEncountered = false; @@ -495,8 +496,9 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace, kind = tok::r_brace; auto ParseDefault = [this, OpeningBrace, IfKind, NextLevelLBracesType, - &HasLabel, &StatementCount] { + &HasDoWhile, &HasLabel, &StatementCount] { parseStructuralElement(IfKind, !OpeningBrace, NextLevelLBracesType, + HasDoWhile ? nullptr : &HasDoWhile, HasLabel ? nullptr : &HasLabel); ++StatementCount; assert(StatementCount > 0 && "StatementCount overflow!"); @@ -536,7 +538,7 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace, return false; } if (FormatTok->isNot(tok::r_brace) || StatementCount != 1 || HasLabel || - IsPrecededByCommentOrPPDirective || + HasDoWhile || IsPrecededByCommentOrPPDirective || precededByCommentOrPPDirective()) { return false; } @@ -1415,6 +1417,7 @@ void UnwrappedLineParser::readTokenWithJavaScriptASI() { void UnwrappedLineParser::parseStructuralElement(IfStmtKind *IfKind, bool IsTopLevel, TokenType NextLBracesType, + bool *HasDoWhile, bool *HasLabel) { if (Style.Language == FormatStyle::LK_TableGen && FormatTok->is(tok::pp_include)) { @@ -1476,6 +1479,8 @@ void UnwrappedLineParser::parseStructuralElement(IfStmtKind *IfKind, break; } parseDoWhile(); + if (HasDoWhile) + *HasDoWhile = true; return; case tok::kw_switch: if (Style.isJavaScript() && Line->MustBeDeclaration) { diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h index ffba36c..38f83a8 100644 --- a/clang/lib/Format/UnwrappedLineParser.h +++ b/clang/lib/Format/UnwrappedLineParser.h @@ -115,6 +115,7 @@ private: void parseStructuralElement(IfStmtKind *IfKind = nullptr, bool IsTopLevel = false, TokenType NextLBracesType = TT_Unknown, + bool *HasDoWhile = nullptr, bool *HasLabel = nullptr); bool tryToParseBracedList(); bool parseBracedList(bool ContinueOnSemicolons = false, bool IsEnum = false, diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index c125705..f8f2715 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -25100,12 +25100,12 @@ TEST_F(FormatTest, RemoveBraces) { FormatStyle Style = getLLVMStyle(); Style.RemoveBracesLLVM = true; - // The following eight test cases are fully-braced versions of the examples at + // The following test cases are fully-braced versions of the examples at // "llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single- // statement-bodies-of-if-else-loop-statements". - // 1. Omit the braces, since the body is simple and clearly associated with - // the if. + // Omit the braces since the body is simple and clearly associated with the + // `if`. verifyFormat("if (isa(D))\n" " handleFunctionDecl(D);\n" "else if (isa(D))\n" @@ -25117,7 +25117,7 @@ TEST_F(FormatTest, RemoveBraces) { "}", Style); - // 2. Here we document the condition itself and not the body. + // Here we document the condition itself and not the body. verifyFormat("if (isa(D)) {\n" " // It is necessary that we explain the situation with this\n" " // surprisingly long comment, so it would be unclear\n" @@ -25125,32 +25125,29 @@ TEST_F(FormatTest, RemoveBraces) { " // the scope of the `if`.\n" " // Because the condition is documented, we can't really\n" " // hoist this comment that applies to the body above the\n" - " // if.\n" + " // `if`.\n" " handleOtherDecl(D);\n" "}", Style); - // 3. Use braces on the outer `if` to avoid a potential dangling else + // Use braces on the outer `if` to avoid a potential dangling `else` // situation. verifyFormat("if (isa(D)) {\n" - " for (auto *A : D.attrs())\n" - " if (shouldProcessAttr(A))\n" - " handleAttr(A);\n" + " if (shouldProcessAttr(A))\n" + " handleAttr(A);\n" "}", "if (isa(D)) {\n" - " for (auto *A : D.attrs()) {\n" - " if (shouldProcessAttr(A)) {\n" - " handleAttr(A);\n" - " }\n" + " if (shouldProcessAttr(A)) {\n" + " handleAttr(A);\n" " }\n" "}", Style); - // 4. Use braces for the `if` block to keep it uniform with the else block. + // Use braces for the `if` block to keep it uniform with the `else` block. verifyFormat("if (isa(D)) {\n" " handleFunctionDecl(D);\n" "} else {\n" - " // In this else case, it is necessary that we explain the\n" + " // In this `else` case, it is necessary that we explain the\n" " // situation with this surprisingly long comment, so it\n" " // would be unclear without the braces whether the\n" " // following statement is in the scope of the `if`.\n" @@ -25158,9 +25155,9 @@ TEST_F(FormatTest, RemoveBraces) { "}", Style); - // 5. This should also omit braces. The `for` loop contains only a single + // This should also omit braces. The `for` loop contains only a single // statement, so it shouldn't have braces. The `if` also only contains a - // single simple statement (the for loop), so it also should omit braces. + // single simple statement (the `for` loop), so it also should omit braces. verifyFormat("if (isa(D))\n" " for (auto *A : D.attrs())\n" " handleAttr(A);", @@ -25171,18 +25168,26 @@ TEST_F(FormatTest, RemoveBraces) { "}", Style); - // 6. Use braces for the outer `if` since the nested `for` is braced. + // Use braces for a `do-while` loop and its enclosing statement. + verifyFormat("if (Tok->is(tok::l_brace)) {\n" + " do {\n" + " Tok = Tok->Next;\n" + " } while (Tok);\n" + "}", + Style); + + // Use braces for the outer `if` since the nested `for` is braced. verifyFormat("if (isa(D)) {\n" " for (auto *A : D.attrs()) {\n" - " // In this for loop body, it is necessary that we explain\n" - " // the situation with this surprisingly long comment,\n" - " // forcing braces on the `for` block.\n" + " // In this `for` loop body, it is necessary that we\n" + " // explain the situation with this surprisingly long\n" + " // comment, forcing braces on the `for` block.\n" " handleAttr(A);\n" " }\n" "}", Style); - // 7. Use braces on the outer block because there are more than two levels of + // Use braces on the outer block because there are more than two levels of // nesting. verifyFormat("if (isa(D)) {\n" " for (auto *A : D.attrs())\n" @@ -25198,7 +25203,7 @@ TEST_F(FormatTest, RemoveBraces) { "}", Style); - // 8. Use braces on the outer block because of a nested `if`, otherwise the + // Use braces on the outer block because of a nested `if`; otherwise the // compiler would warn: `add explicit braces to avoid dangling else` verifyFormat("if (auto *D = dyn_cast(D)) {\n" " if (shouldProcess(D))\n" @@ -25385,6 +25390,26 @@ TEST_F(FormatTest, RemoveBraces) { "}", Style); + verifyFormat("if (isa(D)) {\n" + " for (auto *A : D.attrs())\n" + " if (shouldProcessAttr(A))\n" + " handleAttr(A);\n" + "}", + "if (isa(D)) {\n" + " for (auto *A : D.attrs()) {\n" + " if (shouldProcessAttr(A)) {\n" + " handleAttr(A);\n" + " }\n" + " }\n" + "}", + Style); + + verifyFormat("do {\n" + " ++I;\n" + "} while (hasMore() && Filter(*I));", + "do { ++I; } while (hasMore() && Filter(*I));", + Style); + verifyFormat("if (a)\n" " if (b)\n" " c;\n" -- 2.7.4