<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt"><div dir="ltr"><div class="gmail_default" style><br></div><div class="gmail_extra"><div class="gmail_quote">On Mon, Dec 24, 2012 at 9:02 PM, Matt Beaumont-Gay <span dir="ltr"><<a href="mailto:matthewbg@google.com" target="_blank">matthewbg@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Comments from the peanut gallery...<br>
<div><div class="h5"><br>
On Mon, Dec 24, 2012 at 8:51 AM, Daniel Jasper <<a href="mailto:djasper@google.com">djasper@google.com</a>> wrote:<br>
> Author: djasper<br>
> Date: Mon Dec 24 10:51:15 2012<br>
> New Revision: 171039<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=171039&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=171039&view=rev</a><br>
> Log:<br>
> Let clang-format format itself.<br>
><br>
> Apply all formatting changes that clang-format would apply to its own source<br>
> code. All choices seem to improve readability (or at least not make it worse).<br>
> No functional changes.<br>
><br>
> Modified:<br>
> cfe/trunk/include/clang/Format/Format.h<br>
> cfe/trunk/lib/Format/Format.cpp<br>
> cfe/trunk/lib/Format/UnwrappedLineParser.cpp<br>
> cfe/trunk/lib/Format/UnwrappedLineParser.h<br>
><br>
> Modified: cfe/trunk/include/clang/Format/Format.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=171039&r1=171038&r2=171039&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=171039&r1=171038&r2=171039&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Format/Format.h (original)<br>
> +++ cfe/trunk/include/clang/Format/Format.h Mon Dec 24 10:51:15 2012<br>
> @@ -78,4 +78,4 @@<br>
> } // end namespace format<br>
> } // end namespace clang<br>
><br>
> -#endif // LLVM_CLANG_FORMAT_FORMAT_H<br>
> +#endif // LLVM_CLANG_FORMAT_FORMAT_H<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=171039&r1=171038&r2=171039&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=171039&r1=171038&r2=171039&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Format/Format.cpp (original)<br>
> +++ cfe/trunk/lib/Format/Format.cpp Mon Dec 24 10:51:15 2012<br>
> @@ -124,7 +124,7 @@<br>
> bool FitsOnALine = true;<br>
> for (unsigned i = 1, n = Line.Tokens.size(); i != n; ++i) {<br>
> Columns += (Annotations[i].SpaceRequiredBefore ? 1 : 0) +<br>
> - Line.Tokens[i].Tok.getLength();<br>
> + Line.Tokens[i].Tok.getLength();<br>
> // A special case for the colon of a constructor initializer as this only<br>
> // needs to be put on a new line if the line needs to be split.<br>
> if (Columns > Style.ColumnLimit ||<br>
> @@ -248,8 +248,8 @@<br>
> // Indent and extra 4 spaces after '=' as it continues an expression.<br>
> // Don't do that on the top level, as we already indent 4 there.<br>
> State.Column = State.Indent[ParenLevel] + 4;<br>
> - } else if (Line.Tokens[0].Tok.is(tok::kw_for) &&<br>
> - <a href="http://Previous.Tok.is" target="_blank">Previous.Tok.is</a>(tok::comma)) {<br>
> + } else if (<br>
> + Line.Tokens[0].Tok.is(tok::kw_for) && <a href="http://Previous.Tok.is" target="_blank">Previous.Tok.is</a>(tok::comma)) {<br>
<br>
</div></div>This is a little dubious.<br></blockquote><div><br></div><div style>I agree. This has been introduced with the operator precedence patch. The reasons I have not addressed it yet:<br></div><div style>a) It is a reasonably rare case.</div>
<div style>b) It is not terribly bad (I am not aware that it contradicts a style guide).</div><div style>c (And most importantly)) I am not sure what the "correct" formatting is for other binary operators. E.g., which of the following three is best (assuming everything does not fit on the first line)?</div>
<div style><br></div><div style><span style="color:rgb(80,0,80)">}</span> else if (aaaaaaaa ==</div><div style> bbbbbbbb) ...</div><div style><br></div><div style><span style="color:rgb(80,0,80)">}</span> else if (aaaaaaaa ==</div>
<div style> bbbbbbbb) ...</div><div style><br></div><div style><span style="color:rgb(80,0,80)">}</span> else if (</div><div style> aaaaaaaa == bbbbbbbb) ...</div><div style><br></div><div style>(This might need a parallel Font).</div>
<div style><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">
> State.Column = State.ForLoopVariablePos;<br>
> } else {<br>
> State.Column = State.Indent[ParenLevel];<br>
> @@ -384,7 +384,7 @@<br>
> unsigned CurrentPenalty = 0;<br>
> if (NewLine) {<br>
> CurrentPenalty += Parameters.PenaltyIndentLevel * State.Indent.size() +<br>
> - splitPenalty(State.ConsumedTokens - 1);<br>
> + splitPenalty(State.ConsumedTokens - 1);<br>
> } else {<br>
> if (State.Indent.size() < State.StartOfLineLevel)<br>
> CurrentPenalty += Parameters.PenaltyLevelDecrease *<br>
> @@ -668,8 +668,9 @@<br>
> Annotation.SpaceRequiredBefore = true;<br>
> } else if (Annotation.Type == TokenAnnotation::TT_OverloadedOperator) {<br>
> Annotation.SpaceRequiredBefore =<br>
> - Line.Tokens[i].Tok.is(tok::identifier) || Line.Tokens[i].Tok.is(<br>
> - tok::kw_new) || Line.Tokens[i].Tok.is(tok::kw_delete);<br>
> + Line.Tokens[i].Tok.is(tok::identifier) ||<br>
> + Line.Tokens[i].Tok.is(tok::kw_new) ||<br>
> + Line.Tokens[i].Tok.is(tok::kw_delete);<br>
<br>
</div>This is nice!<br>
<div class=""><div class="h5"><br>
> } else if (<br>
> Annotations[i - 1].Type == TokenAnnotation::TT_OverloadedOperator) {<br>
> Annotation.SpaceRequiredBefore = false;<br>
> @@ -691,8 +692,8 @@<br>
> Annotation.MustBreakBefore = true;<br>
> } else if (Line.Tokens[i].Tok.is(tok::colon)) {<br>
> Annotation.SpaceRequiredBefore =<br>
> - Line.Tokens[0].Tok.isNot(tok::kw_case) && !IsObjCMethodDecl &&<br>
> - (i != e - 1);<br>
> + Line.Tokens[0].Tok.isNot(tok::kw_case) && !IsObjCMethodDecl &&<br>
> + (i != e - 1);<br>
> // Don't break at ':' if identifier before it can beak.<br>
> if (IsObjCMethodDecl && Line.Tokens[i - 1].Tok.is(tok::identifier) &&<br>
> Annotations[i - 1].CanBreakBefore)<br>
> @@ -799,8 +800,7 @@<br>
> return getPrecedence(Tok) > prec::Comma;<br>
> }<br>
><br>
> - TokenAnnotation::TokenType determineStarAmpUsage(unsigned Index,<br>
> - bool IsRHS) {<br>
> + TokenAnnotation::TokenType determineStarAmpUsage(unsigned Index, bool IsRHS) {<br>
> if (Index == Annotations.size())<br>
> return TokenAnnotation::TT_Unknown;<br>
><br>
> @@ -866,8 +866,8 @@<br>
> return false;<br>
> if (Right.is(tok::amp) || Right.is(tok::star))<br>
> return Left.isLiteral() ||<br>
> - (Left.isNot(tok::star) && Left.isNot(tok::amp) &&<br>
> - !Style.PointerAndReferenceBindToType);<br>
> + (Left.isNot(tok::star) && Left.isNot(tok::amp) &&<br>
> + !Style.PointerAndReferenceBindToType);<br>
> if (Left.is(tok::amp) || Left.is(tok::star))<br>
> return Right.isLiteral() || Style.PointerAndReferenceBindToType;<br>
> if (Right.is(tok::star) && Left.is(tok::l_paren))<br>
> @@ -889,9 +889,9 @@<br>
> return false;<br>
> if (Right.is(tok::l_paren)) {<br>
> return Left.is(tok::kw_if) || Left.is(tok::kw_for) ||<br>
> - Left.is(tok::kw_while) || Left.is(tok::kw_switch) ||<br>
> - (Left.isNot(tok::identifier) && Left.isNot(tok::kw_sizeof) &&<br>
> - Left.isNot(tok::kw_typeof));<br>
> + Left.is(tok::kw_while) || Left.is(tok::kw_switch) ||<br>
> + (Left.isNot(tok::identifier) && Left.isNot(tok::kw_sizeof) &&<br>
> + Left.isNot(tok::kw_typeof));<br>
> }<br>
> return true;<br>
> }<br>
> @@ -901,11 +901,11 @@<br>
> <a href="http://Right.Tok.is" target="_blank">Right.Tok.is</a>(tok::comment) || <a href="http://Right.Tok.is" target="_blank">Right.Tok.is</a>(tok::greater))<br>
> return false;<br>
> return (isBinaryOperator(Left) && Left.Tok.isNot(tok::lessless)) ||<br>
> - <a href="http://Left.Tok.is" target="_blank">Left.Tok.is</a>(tok::comma) || <a href="http://Right.Tok.is" target="_blank">Right.Tok.is</a>(tok::lessless) ||<br>
> - <a href="http://Right.Tok.is" target="_blank">Right.Tok.is</a>(tok::arrow) || <a href="http://Right.Tok.is" target="_blank">Right.Tok.is</a>(tok::period) ||<br>
> - <a href="http://Right.Tok.is" target="_blank">Right.Tok.is</a>(tok::colon) || <a href="http://Left.Tok.is" target="_blank">Left.Tok.is</a>(tok::semi) ||<br>
> - <a href="http://Left.Tok.is" target="_blank">Left.Tok.is</a>(tok::l_brace) ||<br>
> - (<a href="http://Left.Tok.is" target="_blank">Left.Tok.is</a>(tok::l_paren) && !<a href="http://Right.Tok.is" target="_blank">Right.Tok.is</a>(tok::r_paren));<br>
> + <a href="http://Left.Tok.is" target="_blank">Left.Tok.is</a>(tok::comma) || <a href="http://Right.Tok.is" target="_blank">Right.Tok.is</a>(tok::lessless) ||<br>
> + <a href="http://Right.Tok.is" target="_blank">Right.Tok.is</a>(tok::arrow) || <a href="http://Right.Tok.is" target="_blank">Right.Tok.is</a>(tok::period) ||<br>
> + <a href="http://Right.Tok.is" target="_blank">Right.Tok.is</a>(tok::colon) || <a href="http://Left.Tok.is" target="_blank">Left.Tok.is</a>(tok::semi) ||<br>
> + <a href="http://Left.Tok.is" target="_blank">Left.Tok.is</a>(tok::l_brace) ||<br>
> + (<a href="http://Left.Tok.is" target="_blank">Left.Tok.is</a>(tok::l_paren) && !<a href="http://Right.Tok.is" target="_blank">Right.Tok.is</a>(tok::r_paren));<br>
> }<br>
><br>
> const UnwrappedLine &Line;<br>
><br>
> Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=171039&r1=171038&r2=171039&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=171039&r1=171038&r2=171039&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)<br>
> +++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Mon Dec 24 10:51:15 2012<br>
> @@ -25,9 +25,7 @@<br>
> UnwrappedLineParser::UnwrappedLineParser(const FormatStyle &Style,<br>
> FormatTokenSource &Tokens,<br>
> UnwrappedLineConsumer &Callback)<br>
> - : Style(Style),<br>
> - Tokens(Tokens),<br>
> - Callback(Callback) {<br>
> + : Style(Style), Tokens(Tokens), Callback(Callback) {<br>
> }<br>
><br>
> bool UnwrappedLineParser::parse() {<br>
><br>
> Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=171039&r1=171038&r2=171039&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=171039&r1=171038&r2=171039&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)<br>
> +++ cfe/trunk/lib/Format/UnwrappedLineParser.h Mon Dec 24 10:51:15 2012<br>
> @@ -123,4 +123,4 @@<br>
> } // end namespace format<br>
> } // end namespace clang<br>
><br>
> -#endif // LLVM_CLANG_FORMAT_UNWRAPPED_LINE_PARSER_H<br>
> +#endif // LLVM_CLANG_FORMAT_UNWRAPPED_LINE_PARSER_H<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>
</div></div></blockquote></div><br></div></div></div>