Index: lib/Format/FormatToken.h =================================================================== --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -37,6 +37,7 @@ TYPE(ConflictAlternative) \ TYPE(ConflictEnd) \ TYPE(ConflictStart) \ + TYPE(CompoundStatementLBrace) \ TYPE(CtorInitializerColon) \ TYPE(CtorInitializerComma) \ TYPE(DesignatedInitializerPeriod) \ @@ -75,6 +76,7 @@ TYPE(PointerOrReference) \ TYPE(PureVirtualSpecifier) \ TYPE(RangeBasedForLoopColon) \ + TYPE(RecordLBrace) \ TYPE(RegexLiteral) \ TYPE(SelectorName) \ TYPE(StartOfName) \ Index: lib/Format/TokenAnnotator.cpp =================================================================== --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -828,7 +828,8 @@ if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro, TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow, - TT_RegexLiteral)) + TT_RegexLiteral, TT_CompoundStatementLBrace, + TT_RecordLBrace)) CurrentToken->Type = TT_Unknown; CurrentToken->Role.reset(); CurrentToken->MatchingParen = nullptr; Index: lib/Format/UnwrappedLineFormatter.cpp =================================================================== --- lib/Format/UnwrappedLineFormatter.cpp +++ lib/Format/UnwrappedLineFormatter.cpp @@ -185,50 +185,34 @@ ? 0 : Limit - TheLine->Last->TotalLength; - // FIXME: TheLine->Level != 0 might or might not be the right check to do. - // If necessary, change to something smarter. - bool MergeShortFunctions = - Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All || - (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty && - I[1]->First->is(tok::r_brace)) || - (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline && - TheLine->Level != 0); - if (TheLine->Last->is(TT_FunctionLBrace) && - TheLine->First != TheLine->Last) { + TheLine->First != TheLine->Last || + I[1]->First->is(TT_FunctionLBrace) && I[1]->First == I[1]->Last) { + // FIXME: TheLine->Level != 0 might or might not be the right check to do. + // If necessary, change to something smarter. + bool MergeShortFunctions = + Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All || + (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty && + (TheLine->Last->is(tok::l_brace) && I[1]->First->is(tok::r_brace) || + !TheLine->Last->is(tok::l_brace) && I[1]->First->is(tok::l_brace) && + I + 2 != E && I[2]->First->is(tok::r_brace))) || + (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline && + TheLine->Level != 0); return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0; } - if (TheLine->Last->is(tok::l_brace)) { - return !Style.BraceWrapping.AfterFunction - ? tryMergeSimpleBlock(I, E, Limit) - : 0; + if (TheLine->Last->is(TT_RecordLBrace) && TheLine->First != TheLine->Last || + I[1]->First->is(TT_RecordLBrace)) { + return tryMergeSimpleBlock(I, E, Limit); } - if (I[1]->First->is(TT_FunctionLBrace) && - Style.BraceWrapping.AfterFunction) { - if (I[1]->Last->is(TT_LineComment)) - return 0; - - // Check for Limit <= 2 to account for the " {". - if (Limit <= 2 || (Style.ColumnLimit == 0 && containsMustBreak(TheLine))) - return 0; - Limit -= 2; - - unsigned MergedLines = 0; - if (MergeShortFunctions) { - MergedLines = tryMergeSimpleBlock(I + 1, E, Limit); - // If we managed to merge the block, count the function header, which is - // on a separate line. - if (MergedLines > 0) - ++MergedLines; - } - return MergedLines; + if (TheLine->Last->is(tok::l_brace) && TheLine->Last->is(TT_Unknown)) { + return tryMergeSimpleBlock(I, E, Limit); } - if (TheLine->First->is(tok::kw_if)) { + if (TheLine->First->isOneOf(tok::kw_if, tok::kw_else)) { return Style.AllowShortIfStatementsOnASingleLine ? tryMergeSimpleControlStatement(I, E, Limit) : 0; } - if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) { + if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) { return Style.AllowShortLoopsOnASingleLine ? tryMergeSimpleControlStatement(I, E, Limit) : 0; @@ -263,14 +247,15 @@ SmallVectorImpl::const_iterator E, unsigned Limit) { if (Limit == 0) return 0; - if (Style.BraceWrapping.AfterControlStatement && - (I[1]->First->is(tok::l_brace) && !Style.AllowShortBlocksOnASingleLine)) - return 0; if (I[1]->InPPDirective != (*I)->InPPDirective || (I[1]->InPPDirective && I[1]->First->HasUnescapedNewline)) return 0; Limit = limitConsideringMacros(I + 1, E, Limit); AnnotatedLine &Line = **I; + if (I[0]->Last->is(TT_CompoundStatementLBrace) || + I[1]->First->is(TT_CompoundStatementLBrace) && + I[1]->First == I[1]->Last) + return tryMergeSimpleBlock(I, E, Limit); if (Line.Last->isNot(tok::r_paren)) return 0; if (1 + I[1]->Last->TotalLength > Limit) @@ -292,6 +277,9 @@ if (Limit == 0 || I + 1 == E || I[1]->First->isOneOf(tok::kw_case, tok::kw_default)) return 0; + if (I[0]->Last->is(tok::l_brace) || + I[1]->First->is(tok::l_brace) && I[1]->First == I[1]->Last) + return 0; unsigned NumStmts = 0; unsigned Length = 0; bool InPPDirective = I[0]->InPPDirective; @@ -320,6 +308,15 @@ unsigned Limit) { AnnotatedLine &Line = **I; + // FIXME: Consider an option to allow short exception handling clauses on + // a single line. + // FIXME: This isn't covered by tests. + // FIXME: For catch, __except, __finally the first token on the line + // is '}', so this isn't correct here. + if (Line.First->isOneOf(tok::kw_try, tok::kw___try, tok::kw_catch, + Keywords.kw___except, tok::kw___finally)) + return 0; + // Don't merge ObjC @ keywords and methods. // FIXME: If an option to allow short exception handling clauses on a single // line is added, change this to not return for @try and friends. @@ -327,40 +324,33 @@ Line.First->isOneOf(tok::at, tok::minus, tok::plus)) return 0; - // Check that the current line allows merging. This depends on whether we - // are in a control flow statements as well as several style flags. - if (Line.First->isOneOf(tok::kw_else, tok::kw_case) || - (Line.First->Next && Line.First->Next->is(tok::kw_else))) - return 0; - if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do, tok::kw_try, - tok::kw___try, tok::kw_catch, tok::kw___finally, - tok::kw_for, tok::r_brace, Keywords.kw___except)) { - if (!Style.AllowShortBlocksOnASingleLine) - return 0; - if (!Style.AllowShortIfStatementsOnASingleLine && - Line.startsWith(tok::kw_if)) - return 0; - if (!Style.AllowShortLoopsOnASingleLine && - Line.First->isOneOf(tok::kw_while, tok::kw_do, tok::kw_for)) - return 0; - // FIXME: Consider an option to allow short exception handling clauses on - // a single line. - // FIXME: This isn't covered by tests. - // FIXME: For catch, __except, __finally the first token on the line - // is '}', so this isn't correct here. - if (Line.First->isOneOf(tok::kw_try, tok::kw___try, tok::kw_catch, - Keywords.kw___except, tok::kw___finally)) + // There is no brace on this line, check if the opening brace is on the + // next and belong to a construct (compound statement, function or record). + // Move the iterator to merge the block with the first line. + // Ex: if (1) { exit 0; } + if (Line.Last->isNot(tok::l_brace) && I[1]->First == I[1]->Last && + I[1]->Last->is(tok::l_brace) && I[1]->Last->isNot(TT_Unknown)) { + if (I + 1 == E || Limit <= 2 || + (Style.ColumnLimit == 0 && containsMustBreak(&Line))) return 0; + Limit -= 2; + ++I; } - + assert(I[0]->Last->is(tok::l_brace)); + FormatToken *BlockLBrace = I[0]->Last; FormatToken *Tok = I[1]->First; + + if (BlockLBrace->is(TT_CompoundStatementLBrace) && + !Style.AllowShortBlocksOnASingleLine) + return 0; + if (Tok->is(tok::r_brace) && !Tok->MustBreakBefore && (Tok->getNextNonComment() == nullptr || Tok->getNextNonComment()->is(tok::semi))) { // We merge empty blocks even if the line exceeds the column limit. Tok->SpacesRequiredBefore = 0; Tok->CanBreakBefore = true; - return 1; + return BlockLBrace == Line.Last ? 1 : 2; } else if (Limit != 0 && !Line.startsWith(tok::kw_namespace) && !startsExternCBlock(Line)) { // We don't merge short records. @@ -368,6 +358,11 @@ Keywords.kw_interface)) return 0; + if (BlockLBrace->isNot(TT_FunctionLBrace) && + Line.First->isNot(tok::kw_export) && + !Style.AllowShortBlocksOnASingleLine) + return 0; + // Check that we still have three lines and they fit into the limit. if (I + 2 == E || I[2]->Type == LT_Invalid) return 0; @@ -395,7 +390,7 @@ if (Tok->Next && Tok->Next->is(tok::kw_else)) return 0; - return 2; + return BlockLBrace == Line.Last ? 2 : 3; } return 0; } Index: lib/Format/UnwrappedLineParser.cpp =================================================================== --- lib/Format/UnwrappedLineParser.cpp +++ lib/Format/UnwrappedLineParser.cpp @@ -154,14 +154,29 @@ CompoundStatementIndenter(UnwrappedLineParser *Parser, const FormatStyle &Style, unsigned &LineLevel) : LineLevel(LineLevel), OldLineLevel(LineLevel) { - if (Style.BraceWrapping.AfterControlStatement) - Parser->addUnwrappedLine(); - if (Style.BraceWrapping.IndentBraces) - ++LineLevel; + init(Parser, Style, Style.BraceWrapping.AfterControlStatement); + } + CompoundStatementIndenter(UnwrappedLineParser *Parser, + const FormatStyle &Style, unsigned &LineLevel, + bool wrapBrace) + : LineLevel(LineLevel), OldLineLevel(LineLevel) { + init(Parser, Style, wrapBrace); } ~CompoundStatementIndenter() { LineLevel = OldLineLevel; } private: + void init(UnwrappedLineParser *Parser, const FormatStyle &Style, + bool wrapBrace) { + assert(Parser->FormatTok->is(tok::l_brace)); + Parser->FormatTok->Type = TT_CompoundStatementLBrace; + if (wrapBrace) { + Parser->addUnwrappedLine(); + if (Style.BraceWrapping.IndentBraces) + ++LineLevel; + } + } + +private: unsigned &LineLevel; unsigned OldLineLevel; }; @@ -771,8 +786,8 @@ case tok::objc_autoreleasepool: nextToken(); if (FormatTok->Tok.is(tok::l_brace)) { - if (Style.BraceWrapping.AfterObjCDeclaration) - addUnwrappedLine(); + CompoundStatementIndenter Indenter( + this, Style, Line->Level, Style.BraceWrapping.AfterObjCDeclaration); parseBlock(/*MustBeDeclaration=*/false); } addUnwrappedLine(); @@ -1370,7 +1385,9 @@ if (FormatTok->Tok.is(tok::l_brace)) { CompoundStatementIndenter Indenter(this, Style, Line->Level); parseBlock(/*MustBeDeclaration=*/false); - if (Style.BraceWrapping.BeforeElse) + if (Style.BraceWrapping.BeforeElse || + Style.BraceWrapping.AfterControlStatement && + Style.BraceWrapping.IndentBraces) addUnwrappedLine(); else NeedsUnwrappedLine = true; @@ -1520,11 +1537,16 @@ void UnwrappedLineParser::parseForOrWhileLoop() { assert(FormatTok->isOneOf(tok::kw_for, tok::kw_while, TT_ForEachMacro) && "'for', 'while' or foreach macro expected"); + bool isMacro = FormatTok->is(TT_ForEachMacro); nextToken(); if (FormatTok->Tok.is(tok::l_paren)) parseParens(); if (FormatTok->Tok.is(tok::l_brace)) { CompoundStatementIndenter Indenter(this, Style, Line->Level); + // FIXME: TT_ForEachMacro not treated like tok::kw_for by the rest of + // system. + if (isMacro) + FormatTok->Type = TT_Unknown; parseBlock(/*MustBeDeclaration=*/false); addUnwrappedLine(); } else { @@ -1791,6 +1813,9 @@ } } if (FormatTok->Tok.is(tok::l_brace)) { + if (InitialToken.isOneOf(tok::kw_class, tok::kw_union, tok::kw_struct, + Keywords.kw_interface)) + FormatTok->Type = TT_RecordLBrace; if (ShouldBreakBeforeBrace(Style, InitialToken)) addUnwrappedLine(); Index: unittests/Format/FormatTest.cpp =================================================================== --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -423,6 +423,119 @@ " f();\n" "}", AllowSimpleBracedStatements); + + AllowSimpleBracedStatements.BreakBeforeBraces = FormatStyle::BS_Custom; + AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true; + AllowSimpleBracedStatements.BraceWrapping.BeforeElse = true; + + AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = false; + AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false; + + verifyFormat("if (true)\n" + "{\n" + " f();\n" + "}\n" + "else\n" + "{\n" + " f();\n" + "}\n", + AllowSimpleBracedStatements); + + AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = false; + AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true; + + verifyFormat("if (true)\n" + "{\n" + " f();\n" + "}\n" + "else\n" + "{\n" + " f();\n" + "}\n", + AllowSimpleBracedStatements); + + AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true; + AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false; + + verifyFormat("if (true)\n" + "{\n" + " f();\n" + "}\n" + "else\n" + "{\n" + " f();\n" + "}\n", + AllowSimpleBracedStatements); + + AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true; + AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true; + + EXPECT_EQ("if (true) { f(); }\nelse { f(); }", + format("if (true)\n" + "{\n" + " f();\n" + "}\n" + "else\n" + "{\n" + " f();\n" + "}", + AllowSimpleBracedStatements)); + + AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = false; + AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = false; + + verifyFormat("while (true)\n" + "{\n" + " f();\n" + "}", + AllowSimpleBracedStatements); + verifyFormat("for (;;)\n" + "{\n" + " f();\n" + "}", + AllowSimpleBracedStatements); + + AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = false; + AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true; + + verifyFormat("while (true)\n" + "{\n" + " f();\n" + "}", + AllowSimpleBracedStatements); + verifyFormat("for (;;)\n" + "{\n" + " f();\n" + "}", + AllowSimpleBracedStatements); + + AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true; + AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = false; + + verifyFormat("while (true)\n" + "{\n" + " f();\n" + "}", + AllowSimpleBracedStatements); + verifyFormat("for (;;)\n" + "{\n" + " f();\n" + "}", + AllowSimpleBracedStatements); + + AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true; + AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true; + + EXPECT_EQ("while (true) { f(); }", format("while (true)\n" + "{\n" + " f();\n" + "}", + AllowSimpleBracedStatements)); + EXPECT_EQ("for (;;) { f(); }", format("for (;;)\n" + "{\n" + " f();\n" + "}", + AllowSimpleBracedStatements)); } TEST_F(FormatTest, ParseIfElse) { @@ -2338,7 +2451,9 @@ "}"); // Function-level try statements. - verifyFormat("int f() try { return 4; } catch (...) {\n" + verifyFormat("int f() try {\n" + " return 4;\n" + "} catch (...) {\n" " return 5;\n" "}"); verifyFormat("class A {\n" @@ -2995,7 +3110,7 @@ } TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) { - verifyFormat("{\n { a #c; }\n}"); + verifyFormat("{\n {\n a #c;\n }\n}"); } TEST_F(FormatTest, FormatUnbalancedStructuralElements) { @@ -6007,7 +6122,7 @@ // FIXME: single value wrapped with paren will be treated as cast. verifyFormat("void f(int i = (kValue)*kMask) {}"); - verifyFormat("{ (void)F; }"); + verifyFormat("{\n (void)F;\n}"); // Don't break after a cast's verifyFormat("int aaaaaaaaaaaaaaaaaaaaaaaaaaa =\n" @@ -6797,6 +6912,22 @@ " void f() { int i; } \\\n" " int j;", getLLVMStyleWithColumns(23)); + + FormatStyle CustomBraceStyle = getLLVMStyle(); + CustomBraceStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty; + CustomBraceStyle.BreakBeforeBraces = FormatStyle::BS_Custom; + CustomBraceStyle.BraceWrapping.AfterFunction = true; + + verifyFormat("void empty() {}", CustomBraceStyle); + + EXPECT_EQ("void empty() {}", format("void empty()\n" + "{\n" + "}", + CustomBraceStyle)); + + EXPECT_EQ("void empty() {}", format("void empty() {\n" + "}", + CustomBraceStyle)); } TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) { @@ -9743,6 +9874,128 @@ WebKitBraceStyle); } +TEST_F(FormatTest, IndentBracesBreaking) { + FormatStyle Style = getLLVMStyle(); + + Style.BreakBeforeBraces = FormatStyle::BS_Custom; + Style.BraceWrapping.IndentBraces = true; + + Style.BraceWrapping.AfterControlStatement = true; + Style.BraceWrapping.BeforeElse = false; + Style.BraceWrapping.AfterFunction = true; + verifyFormat("void A(bool b)\n" + "{\n" + " if (b)\n" + " {\n" + " Funk(rand() == 4);\n" + " }\n" + "}", + Style); + verifyFormat("void B(bool b)\n" + "{\n" + " if (b)\n" + " {\n" + " Funk(rand() == 4);\n" + " }\n" + " else\n" + " {\n" + " Funk(rand() == 2);\n" + " }\n" + "}", + Style); + verifyFormat("void C(bool b)\n" + "{\n" + " if (b)\n" + " {\n" + " Funk(rand() == 4);\n" + " }\n" + " else\n" + " Funk(rand() == 2);\n" + "}", + Style); + + Style.BraceWrapping.BeforeElse = true; + verifyFormat("void D(bool b)\n" + "{\n" + " if (b)\n" + " {\n" + " Funk(rand() == 4);\n" + " }\n" + "}", + Style); + verifyFormat("void E(bool b)\n" + "{\n" + " if (b)\n" + " {\n" + " Funk(rand() == 4);\n" + " }\n" + " else\n" + " {\n" + " Funk(rand() == 2);\n" + " }\n" + "}", + Style); + verifyFormat("void F(bool b)\n" + "{\n" + " if (b)\n" + " {\n" + " Funk(rand() == 4);\n" + " }\n" + " else\n" + " Funk(rand() == 2);\n" + "}", + Style); + + Style.BraceWrapping.AfterControlStatement = false; + Style.BraceWrapping.BeforeElse = false; + Style.BraceWrapping.AfterFunction = false; + verifyFormat("void G(bool b) {\n" + " if (b) {\n" + " Funk(rand() == 4);\n" + " }\n" + "}", + Style); + verifyFormat("void H(bool b) {\n" + " if (b) {\n" + " Funk(rand() == 4);\n" + " } else {\n" + " Funk(rand() == 2);\n" + " }\n" + "}", + Style); + verifyFormat("void I(bool b) {\n" + " if (b) {\n" + " Funk(rand() == 4);\n" + " } else\n" + " Funk(rand() == 2);\n" + "}", + Style); + Style.BraceWrapping.BeforeElse = true; + verifyFormat("void J(bool b) {\n" + " if (b) {\n" + " Funk(rand() == 4);\n" + " }\n" + "}", + Style); + verifyFormat("void K(bool b) {\n" + " if (b) {\n" + " Funk(rand() == 4);\n" + " }\n" + " else {\n" + " Funk(rand() == 2);\n" + " }\n" + "}", + Style); + verifyFormat("void L(bool b) {\n" + " if (b) {\n" + " Funk(rand() == 4);\n" + " }\n" + " else\n" + " Funk(rand() == 2);\n" + "}", + Style); +} + TEST_F(FormatTest, CatchExceptionReferenceBinding) { verifyFormat("void f() {\n" " try {\n"