Sure. I'll add this as an option and add a default style for Chromium (we have other stuff we want to configure according to the Chromium style guide).
Douglas: Should I set this to break or no break for the default LLVM Style? On Mon, Jan 14, 2013 at 3:52 PM, Douglas Gregor <[email protected]> wrote: > > On Jan 14, 2013, at 6:14 AM, Daniel Jasper <[email protected]> wrote: > > > Author: djasper > > Date: Mon Jan 14 08:14:23 2013 > > New Revision: 172413 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=172413&view=rev > > Log: > > Put short if statements on a single line. > > > > Before: if (a) > > return; > > After: if (a) return; > > > > Not yet sure, whether this is always desired, but we can add options and > > make this a style parameter as we go along. > > I'd prefer this to be a style parameter… > > - Doug > > > Modified: > > cfe/trunk/lib/Format/Format.cpp > > cfe/trunk/unittests/Format/FormatTest.cpp > > > > Modified: cfe/trunk/lib/Format/Format.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=172413&r1=172412&r2=172413&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/Format/Format.cpp (original) > > +++ cfe/trunk/lib/Format/Format.cpp Mon Jan 14 08:14:23 2013 > > @@ -69,12 +69,9 @@ > > CanBreakBefore(false), MustBreakBefore(false), > > ClosesTemplateDeclaration(false), Parent(NULL) {} > > > > - bool is(tok::TokenKind Kind) const { > > - return FormatTok.Tok.is(Kind); > > - } > > - bool isNot(tok::TokenKind Kind) const { > > - return FormatTok.Tok.isNot(Kind); > > - } > > + bool is(tok::TokenKind Kind) const { return FormatTok.Tok.is(Kind); } > > + bool isNot(tok::TokenKind Kind) const { return > FormatTok.Tok.isNot(Kind); } > > + > > bool isObjCAtKeyword(tok::ObjCKeywordKind Kind) const { > > return FormatTok.Tok.isObjCAtKeyword(Kind); > > } > > @@ -185,8 +182,8 @@ > > /// \p RootToken fits into \p Limit columns on a single line. > > /// > > /// If true, sets \p Length to the required length. > > -static bool fitsIntoLimit(const AnnotatedToken &RootToken, > > - unsigned Limit, unsigned* Length = 0) { > > +static bool fitsIntoLimit(const AnnotatedToken &RootToken, unsigned > Limit, > > + unsigned *Length = 0) { > > unsigned Columns = RootToken.FormatTok.TokenLength; > > const AnnotatedToken *Tok = &RootToken; > > while (!Tok->Children.empty()) { > > @@ -781,8 +778,8 @@ > > Tok->Type = TT_ObjCMethodExpr; > > break; > > case tok::l_paren: { > > - bool ParensWereObjCReturnType = > > - Tok->Parent && Tok->Parent->Type == TT_ObjCMethodSpecifier; > > + bool ParensWereObjCReturnType = Tok->Parent && > Tok->Parent->Type == > > + TT_ObjCMethodSpecifier; > > if (!parseParens()) > > return false; > > if (CurrentToken != NULL && CurrentToken->is(tok::colon)) { > > @@ -1347,8 +1344,8 @@ > > > > class Formatter : public UnwrappedLineConsumer { > > public: > > - Formatter(DiagnosticsEngine &Diag, const FormatStyle &Style, > > - Lexer &Lex, SourceManager &SourceMgr, > > + Formatter(DiagnosticsEngine &Diag, const FormatStyle &Style, Lexer > &Lex, > > + SourceManager &SourceMgr, > > const std::vector<CharSourceRange> &Ranges) > > : Diag(Diag), Style(Style), Lex(Lex), SourceMgr(SourceMgr), > > Ranges(Ranges) {} > > @@ -1407,25 +1404,46 @@ > > // Check whether the UnwrappedLine can be put onto a single line. If > > // so, this is bound to be the optimal solution (by definition) and > we > > // don't need to analyze the entire solution space. > > - unsigned LengthLine1 = 0; > > - if (!fitsIntoLimit(I->First, Limit, &LengthLine1)) > > + unsigned Length = 0; > > + if (!fitsIntoLimit(I->First, Limit, &Length)) > > return false; > > + Limit -= Length + 1; // One space. > > + if (I + 1 == E) > > + return true; > > + > > + if (I->Last->is(tok::l_brace)) { > > + tryMergeSimpleBlock(I, E, Limit); > > + } else if (I->First.is(tok::kw_if)) { > > + tryMergeSimpleIf(I, E, Limit); > > + } > > + return true; > > + } > > > > + void tryMergeSimpleIf(std::vector<AnnotatedLine>::iterator &I, > > + std::vector<AnnotatedLine>::iterator E, > > + unsigned Limit) { > > + AnnotatedLine &Line = *I; > > + if (!fitsIntoLimit((I + 1)->First, Limit)) > > + return; > > + if ((I + 1)->First.is(tok::kw_if) || (I + 1)->First.Type == > TT_LineComment) > > + return; > > + // Only inline simple if's (no nested if or else). > > + if (I + 2 != E && (I + 2)->First.is(tok::kw_else)) > > + return; > > + join(Line, *(++I)); > > + } > > + > > + void tryMergeSimpleBlock(std::vector<AnnotatedLine>::iterator &I, > > + std::vector<AnnotatedLine>::iterator E, > > + unsigned Limit){ > > // Check that we still have three lines and they fit into the limit. > > - if (I + 1 == E || I + 2 == E) > > - return true; > > - unsigned LengthLine2 = 0; > > - unsigned LengthLine3 = 0; > > - if (!fitsIntoLimit((I + 1)->First, Limit, &LengthLine2) || > > - !fitsIntoLimit((I + 2)->First, Limit, &LengthLine3)) > > - return true; > > - if (LengthLine1 + LengthLine2 + LengthLine3 + 2 > Limit) // Two > spaces. > > - return true; > > + if (I + 2 == E || !nextTwoLinesFitInto(I, Limit)) > > + return; > > > > // First, check that the current line allows merging. This is the > case if > > // we're not in a control flow statement and the last token is an > opening > > // brace. > > - AnnotatedLine& Line = *I; > > + AnnotatedLine &Line = *I; > > bool AllowedTokens = > > Line.First.isNot(tok::kw_if) && Line.First.isNot(tok::kw_while) > && > > Line.First.isNot(tok::kw_do) && Line.First.isNot(tok::r_brace) && > > @@ -1434,17 +1452,17 @@ > > // This gets rid of all ObjC @ keywords and methods. > > Line.First.isNot(tok::at) && Line.First.isNot(tok::minus) && > > Line.First.isNot(tok::plus); > > - if (Line.Last->isNot(tok::l_brace) || !AllowedTokens) > > - return true; > > + if (!AllowedTokens) > > + return; > > > > // Second, check that the next line does not contain any braces - if > it > > // does, readability declines when putting it into a single line. > > const AnnotatedToken *Tok = &(I + 1)->First; > > if ((I + 1)->Last->Type == TT_LineComment || Tok->MustBreakBefore) > > - return true; > > + return; > > do { > > if (Tok->is(tok::l_brace) || Tok->is(tok::r_brace)) > > - return true; > > + return; > > Tok = Tok->Children.empty() ? NULL : &Tok->Children.back(); > > } while (Tok != NULL); > > > > @@ -1452,7 +1470,7 @@ > > Tok = &(I + 2)->First; > > if (!Tok->Children.empty() || Tok->isNot(tok::r_brace) || > > Tok->MustBreakBefore) > > - return true; > > + return; > > > > // If the merged line fits, we use that instead and skip the next > two lines. > > Line.Last->Children.push_back((I + 1)->First); > > @@ -1464,7 +1482,16 @@ > > join(Line, *(I + 1)); > > join(Line, *(I + 2)); > > I += 2; > > - return true; > > + } > > + > > + bool nextTwoLinesFitInto(std::vector<AnnotatedLine>::iterator I, > > + unsigned Limit) { > > + unsigned LengthLine1 = 0; > > + unsigned LengthLine2 = 0; > > + if (!fitsIntoLimit((I + 1)->First, Limit, &LengthLine1) || > > + !fitsIntoLimit((I + 2)->First, Limit, &LengthLine2)) > > + return false; > > + return LengthLine1 + LengthLine2 + 1 <= Limit; // One space. > > } > > > > void join(AnnotatedLine &A, const AnnotatedLine &B) { > > > > Modified: cfe/trunk/unittests/Format/FormatTest.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=172413&r1=172412&r2=172413&view=diff > > > ============================================================================== > > --- cfe/trunk/unittests/Format/FormatTest.cpp (original) > > +++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jan 14 08:14:23 2013 > > @@ -130,12 +130,14 @@ > > > //===----------------------------------------------------------------------===// > > > > TEST_F(FormatTest, FormatIfWithoutCompountStatement) { > > - verifyFormat("if (true)\n f();\ng();"); > > - verifyFormat("if (a)\n if (b)\n if (c)\n g();\nh();"); > > + verifyFormat("if (true) f();\ng();"); > > + verifyFormat("if (a)\n if (b)\n if (c) g();\nh();"); > > verifyFormat("if (a)\n if (b) {\n f();\n }\ng();"); > > verifyFormat("if (a)\n" > > " // comment\n" > > " f();"); > > + verifyFormat("if (a) return;", getLLVMStyleWithColumns(14)); > > + verifyFormat("if (a)\n return;", getLLVMStyleWithColumns(13)); > > } > > > > TEST_F(FormatTest, ParseIfElse) { > > @@ -152,8 +154,7 @@ > > verifyFormat("if (true)\n" > > " if (true)\n" > > " if (true) {\n" > > - " if (true)\n" > > - " f();\n" > > + " if (true) f();\n" > > " } else {\n" > > " g();\n" > > " }\n" > > @@ -1454,8 +1455,7 @@ > > > > verifyFormat("@implementation Foo\n" > > "+ (id)init {\n" > > - " if (true)\n" > > - " return nil;\n" > > + " if (true) return nil;\n" > > "}\n" > > "// Look, a comment!\n" > > "- (int)answerWith:(int)i {\n" > > > > > > _______________________________________________ > > cfe-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
