Can you make sure that all changes are covered in the test? For example, I see no 'break' statement in the test.
Thanks! /Manuel On Thu, Aug 1, 2013 at 3:17 PM, Frank Miller <fmil...@sjm.com> wrote: > Allman style is the opposite of attach. Braces are always on their own > line. > > http://llvm-reviews.chandlerc.com/D1257 > > Files: > include/clang/Format/Format.h > lib/Format/Format.cpp > lib/Format/UnwrappedLineParser.cpp > unittests/Format/FormatTest.cpp > > Index: include/clang/Format/Format.h > =================================================================== > --- include/clang/Format/Format.h > +++ include/clang/Format/Format.h > @@ -163,7 +163,9 @@ > /// class definitions. > BS_Linux, > /// Like \c Attach, but break before function definitions. > - BS_Stroustrup > + BS_Stroustrup, > + /// Always break before braces > + BS_Allman > }; > > /// \brief The brace breaking style to use. > Index: lib/Format/Format.cpp > =================================================================== > --- lib/Format/Format.cpp > +++ lib/Format/Format.cpp > @@ -50,6 +50,7 @@ > IO.enumCase(Value, "Attach", clang::format::FormatStyle::BS_Attach); > IO.enumCase(Value, "Linux", clang::format::FormatStyle::BS_Linux); > IO.enumCase(Value, "Stroustrup", > clang::format::FormatStyle::BS_Stroustrup); > + IO.enumCase(Value, "Allman", clang::format::FormatStyle::BS_Allman); > } > }; > > Index: lib/Format/UnwrappedLineParser.cpp > =================================================================== > --- lib/Format/UnwrappedLineParser.cpp > +++ lib/Format/UnwrappedLineParser.cpp > @@ -594,7 +594,8 @@ > // FIXME: Figure out cases where this is not true, and add > projections > // for them (the one we know is missing are lambdas). > if (Style.BreakBeforeBraces == FormatStyle::BS_Linux || > - Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup) > + Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup || > + Style.BreakBeforeBraces == FormatStyle::BS_Allman) > addUnwrappedLine(); > parseBlock(/*MustBeDeclaration=*/false); > addUnwrappedLine(); > @@ -759,8 +760,13 @@ > parseParens(); > bool NeedsUnwrappedLine = false; > if (FormatTok->Tok.is(tok::l_brace)) { > + if (Style.BreakBeforeBraces == FormatStyle::BS_Allman) > + addUnwrappedLine(); > parseBlock(/*MustBeDeclaration=*/false); > - NeedsUnwrappedLine = true; > + if (Style.BreakBeforeBraces == FormatStyle::BS_Allman) > + addUnwrappedLine(); > + else > + NeedsUnwrappedLine = true; > } else { > addUnwrappedLine(); > ++Line->Level; > @@ -770,6 +776,8 @@ > if (FormatTok->Tok.is(tok::kw_else)) { > nextToken(); > if (FormatTok->Tok.is(tok::l_brace)) { > + if (Style.BreakBeforeBraces == FormatStyle::BS_Allman) > + addUnwrappedLine(); > parseBlock(/*MustBeDeclaration=*/false); > addUnwrappedLine(); > } else if (FormatTok->Tok.is(tok::kw_if)) { > @@ -791,7 +799,8 @@ > if (FormatTok->Tok.is(tok::identifier)) > nextToken(); > if (FormatTok->Tok.is(tok::l_brace)) { > - if (Style.BreakBeforeBraces == FormatStyle::BS_Linux) > + if (Style.BreakBeforeBraces == FormatStyle::BS_Linux || > + Style.BreakBeforeBraces == FormatStyle::BS_Allman) > addUnwrappedLine(); > > bool AddLevel = Style.NamespaceIndentation == FormatStyle::NI_All || > @@ -814,6 +823,8 @@ > if (FormatTok->Tok.is(tok::l_paren)) > parseParens(); > if (FormatTok->Tok.is(tok::l_brace)) { > + if (Style.BreakBeforeBraces == FormatStyle::BS_Allman) > + addUnwrappedLine(); > parseBlock(/*MustBeDeclaration=*/false); > addUnwrappedLine(); > } else { > @@ -828,6 +839,8 @@ > assert(FormatTok->Tok.is(tok::kw_do) && "'do' expected"); > nextToken(); > if (FormatTok->Tok.is(tok::l_brace)) { > + if (Style.BreakBeforeBraces == FormatStyle::BS_Allman) > + addUnwrappedLine(); > parseBlock(/*MustBeDeclaration=*/false); > } else { > addUnwrappedLine(); > @@ -854,9 +867,15 @@ > if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0)) > --Line->Level; > if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) > { > + if (Style.BreakBeforeBraces == FormatStyle::BS_Allman) > + addUnwrappedLine(); > parseBlock(/*MustBeDeclaration=*/false); > - if (FormatTok->Tok.is(tok::kw_break)) > - parseStructuralElement(); // "break;" after "}" goes on the same > line. > + if (FormatTok->Tok.is(tok::kw_break)) { > + // "break;" after "}" on its own line only for BS_Allman > + if (Style.BreakBeforeBraces == FormatStyle::BS_Allman) > + addUnwrappedLine(); > + parseStructuralElement(); > + } > } > addUnwrappedLine(); > Line->Level = OldLineLevel; > @@ -877,6 +896,8 @@ > if (FormatTok->Tok.is(tok::l_paren)) > parseParens(); > if (FormatTok->Tok.is(tok::l_brace)) { > + if (Style.BreakBeforeBraces == FormatStyle::BS_Allman) > + addUnwrappedLine(); > parseBlock(/*MustBeDeclaration=*/false); > addUnwrappedLine(); > } else { > @@ -973,7 +994,8 @@ > } > } > if (FormatTok->Tok.is(tok::l_brace)) { > - if (Style.BreakBeforeBraces == FormatStyle::BS_Linux) > + if (Style.BreakBeforeBraces == FormatStyle::BS_Linux || > + Style.BreakBeforeBraces == FormatStyle::BS_Allman) > addUnwrappedLine(); > > parseBlock(/*MustBeDeclaration=*/true); > Index: unittests/Format/FormatTest.cpp > =================================================================== > --- unittests/Format/FormatTest.cpp > +++ unittests/Format/FormatTest.cpp > @@ -5419,6 +5419,30 @@ > BreakBeforeBrace); > } > > +TEST_F(FormatTest, AllmanBraceBreaking) { > + FormatStyle BreakBeforeBrace = getLLVMStyle(); > + BreakBeforeBrace.BreakBeforeBraces = FormatStyle::BS_Allman; > + verifyFormat("namespace a\n" > + "{\n" > + "class A\n" > + "{\n" > + " void f()\n" > + " {\n" > + " if (true)\n" > + " {\n" > + " a();\n" > + " b();\n" > + " }\n" > + " }\n" > + " void g()\n" > + " {\n" > + " return;\n" > + " }\n" > + "}\n" > + "}", > + BreakBeforeBrace); > +} > + > bool allStylesEqual(ArrayRef<FormatStyle> Styles) { > for (size_t i = 1; i < Styles.size(); ++i) > if (!(Styles[0] == Styles[i])) >
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
