<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Dec 25, 2012 at 11:16 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank" class="cremed">richard@metafoo.co.uk</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">On Mon, Dec 24, 2012 at 8:51 AM, Daniel Jasper <<a href="mailto:djasper@google.com" class="cremed">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" class="cremed">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>
</div>Awesome! :)<br>
<div><div class="h5"><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" class="cremed">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" class="cremed">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" class="cremed">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" class="cremed">Previous.Tok.is</a>(tok::comma)) {<br>
<br>
</div></div>This looks ugly to me; the older code was clearer and not any longer.<br>
We have only 6 matches for /if ($/ in all of Clang and LLVM, and all<br>
are in lib/Format/Format.cpp. Could we teach the formatter to only<br>
split the if-expression here as a last resort (and likewise for 'for'<br>
and 'while')?<br></blockquote><div><br></div><div style>We certainly could. Or we could say that we never should have " (" (space and open parenthesis) at the end of the line.. I am just not yet convinced that we need a special solution for that. Why would it be different from:</div>
<div style><br></div><div style>someFunctionWithBoolParameter(</div><div style> param1 && param2);</div><div style><br></div><div style>Just because it is very short? What if s/someFunctionWithBoolParameter/func/?</div>
<div style><br></div><div style>I think this is in fact fairly easy to solve, I just didn't assign a high priority to it and I want to get it right (see previous email).</div><div style><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
> 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>
> } 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" class="cremed">Right.Tok.is</a>(tok::comment) || <a href="http://Right.Tok.is" target="_blank" class="cremed">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" class="cremed">Left.Tok.is</a>(tok::comma) || <a href="http://Right.Tok.is" target="_blank" class="cremed">Right.Tok.is</a>(tok::lessless) ||<br>
> - <a href="http://Right.Tok.is" target="_blank" class="cremed">Right.Tok.is</a>(tok::arrow) || <a href="http://Right.Tok.is" target="_blank" class="cremed">Right.Tok.is</a>(tok::period) ||<br>
> - <a href="http://Right.Tok.is" target="_blank" class="cremed">Right.Tok.is</a>(tok::colon) || <a href="http://Left.Tok.is" target="_blank" class="cremed">Left.Tok.is</a>(tok::semi) ||<br>
> - <a href="http://Left.Tok.is" target="_blank" class="cremed">Left.Tok.is</a>(tok::l_brace) ||<br>
> - (<a href="http://Left.Tok.is" target="_blank" class="cremed">Left.Tok.is</a>(tok::l_paren) && !<a href="http://Right.Tok.is" target="_blank" class="cremed">Right.Tok.is</a>(tok::r_paren));<br>
> + <a href="http://Left.Tok.is" target="_blank" class="cremed">Left.Tok.is</a>(tok::comma) || <a href="http://Right.Tok.is" target="_blank" class="cremed">Right.Tok.is</a>(tok::lessless) ||<br>
> + <a href="http://Right.Tok.is" target="_blank" class="cremed">Right.Tok.is</a>(tok::arrow) || <a href="http://Right.Tok.is" target="_blank" class="cremed">Right.Tok.is</a>(tok::period) ||<br>
> + <a href="http://Right.Tok.is" target="_blank" class="cremed">Right.Tok.is</a>(tok::colon) || <a href="http://Left.Tok.is" target="_blank" class="cremed">Left.Tok.is</a>(tok::semi) ||<br>
> + <a href="http://Left.Tok.is" target="_blank" class="cremed">Left.Tok.is</a>(tok::l_brace) ||<br>
> + (<a href="http://Left.Tok.is" target="_blank" class="cremed">Left.Tok.is</a>(tok::l_paren) && !<a href="http://Right.Tok.is" target="_blank" class="cremed">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" class="cremed">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" class="cremed">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" class="cremed">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div></div>