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