[cfe-commits] r171039 - in /cfe/trunk: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h
Richard Smith
richard at metafoo.co.uk
Wed Dec 26 02:24:18 PST 2012
On Wed, Dec 26, 2012 at 1:57 AM, Daniel Jasper <djasper at google.com> wrote:
>
>
>
> On Tue, Dec 25, 2012 at 11:16 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>>
>> On Mon, Dec 24, 2012 at 8:51 AM, Daniel Jasper <djasper at google.com> wrote:
>> > Author: djasper
>> > Date: Mon Dec 24 10:51:15 2012
>> > New Revision: 171039
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=171039&view=rev
>> > Log:
>> > Let clang-format format itself.
>> >
>> > Apply all formatting changes that clang-format would apply to its own
>> > source
>> > code. All choices seem to improve readability (or at least not make it
>> > worse).
>> > No functional changes.
>>
>> Awesome! :)
>>
>> > Modified:
>> > cfe/trunk/include/clang/Format/Format.h
>> > cfe/trunk/lib/Format/Format.cpp
>> > cfe/trunk/lib/Format/UnwrappedLineParser.cpp
>> > cfe/trunk/lib/Format/UnwrappedLineParser.h
>> >
>> > Modified: cfe/trunk/include/clang/Format/Format.h
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=171039&r1=171038&r2=171039&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/include/clang/Format/Format.h (original)
>> > +++ cfe/trunk/include/clang/Format/Format.h Mon Dec 24 10:51:15 2012
>> > @@ -78,4 +78,4 @@
>> > } // end namespace format
>> > } // end namespace clang
>> >
>> > -#endif // LLVM_CLANG_FORMAT_FORMAT_H
>> > +#endif // LLVM_CLANG_FORMAT_FORMAT_H
>> >
>> > Modified: cfe/trunk/lib/Format/Format.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=171039&r1=171038&r2=171039&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/Format/Format.cpp (original)
>> > +++ cfe/trunk/lib/Format/Format.cpp Mon Dec 24 10:51:15 2012
>> > @@ -124,7 +124,7 @@
>> > bool FitsOnALine = true;
>> > for (unsigned i = 1, n = Line.Tokens.size(); i != n; ++i) {
>> > Columns += (Annotations[i].SpaceRequiredBefore ? 1 : 0) +
>> > - Line.Tokens[i].Tok.getLength();
>> > + Line.Tokens[i].Tok.getLength();
>> > // A special case for the colon of a constructor initializer as
>> > this only
>> > // needs to be put on a new line if the line needs to be split.
>> > if (Columns > Style.ColumnLimit ||
>> > @@ -248,8 +248,8 @@
>> > // Indent and extra 4 spaces after '=' as it continues an
>> > expression.
>> > // Don't do that on the top level, as we already indent 4
>> > there.
>> > State.Column = State.Indent[ParenLevel] + 4;
>> > - } else if (Line.Tokens[0].Tok.is(tok::kw_for) &&
>> > - Previous.Tok.is(tok::comma)) {
>> > + } else if (
>> > + Line.Tokens[0].Tok.is(tok::kw_for) &&
>> > Previous.Tok.is(tok::comma)) {
>>
>> This looks ugly to me; the older code was clearer and not any longer.
>> We have only 6 matches for /if ($/ in all of Clang and LLVM, and all
>> are in lib/Format/Format.cpp. Could we teach the formatter to only
>> split the if-expression here as a last resort (and likewise for 'for'
>> and 'while')?
>
>
> 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:
>
> someFunctionWithBoolParameter(
> param1 && param2);
>
> Just because it is very short? What if
> s/someFunctionWithBoolParameter/func/?
I think the difference here is that 1) we should prefer not to break a
line after "if (", because 'if' is always followed by an indented
statement, so decreasing the indent increases the visual ambiguity,
and conversely 2) we should prefer not to break a line in the middle
of a function argument because the line break might be confused for
the end of an argument.
> 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).
>
>>
>> > State.Column = State.ForLoopVariablePos;
>> > } else {
>> > State.Column = State.Indent[ParenLevel];
>> > @@ -384,7 +384,7 @@
>> > unsigned CurrentPenalty = 0;
>> > if (NewLine) {
>> > CurrentPenalty += Parameters.PenaltyIndentLevel *
>> > State.Indent.size() +
>> > - splitPenalty(State.ConsumedTokens - 1);
>> > + splitPenalty(State.ConsumedTokens - 1);
>> > } else {
>> > if (State.Indent.size() < State.StartOfLineLevel)
>> > CurrentPenalty += Parameters.PenaltyLevelDecrease *
>> > @@ -668,8 +668,9 @@
>> > Annotation.SpaceRequiredBefore = true;
>> > } else if (Annotation.Type ==
>> > TokenAnnotation::TT_OverloadedOperator) {
>> > Annotation.SpaceRequiredBefore =
>> > - Line.Tokens[i].Tok.is(tok::identifier) ||
>> > Line.Tokens[i].Tok.is(
>> > - tok::kw_new) || Line.Tokens[i].Tok.is(tok::kw_delete);
>> > + Line.Tokens[i].Tok.is(tok::identifier) ||
>> > + Line.Tokens[i].Tok.is(tok::kw_new) ||
>> > + Line.Tokens[i].Tok.is(tok::kw_delete);
>> > } else if (
>> > Annotations[i - 1].Type ==
>> > TokenAnnotation::TT_OverloadedOperator) {
>> > Annotation.SpaceRequiredBefore = false;
>> > @@ -691,8 +692,8 @@
>> > Annotation.MustBreakBefore = true;
>> > } else if (Line.Tokens[i].Tok.is(tok::colon)) {
>> > Annotation.SpaceRequiredBefore =
>> > - Line.Tokens[0].Tok.isNot(tok::kw_case) && !IsObjCMethodDecl
>> > &&
>> > - (i != e - 1);
>> > + Line.Tokens[0].Tok.isNot(tok::kw_case) && !IsObjCMethodDecl
>> > &&
>> > + (i != e - 1);
>> > // Don't break at ':' if identifier before it can beak.
>> > if (IsObjCMethodDecl && Line.Tokens[i -
>> > 1].Tok.is(tok::identifier) &&
>> > Annotations[i - 1].CanBreakBefore)
>> > @@ -799,8 +800,7 @@
>> > return getPrecedence(Tok) > prec::Comma;
>> > }
>> >
>> > - TokenAnnotation::TokenType determineStarAmpUsage(unsigned Index,
>> > - bool IsRHS) {
>> > + TokenAnnotation::TokenType determineStarAmpUsage(unsigned Index, bool
>> > IsRHS) {
>> > if (Index == Annotations.size())
>> > return TokenAnnotation::TT_Unknown;
>> >
>> > @@ -866,8 +866,8 @@
>> > return false;
>> > if (Right.is(tok::amp) || Right.is(tok::star))
>> > return Left.isLiteral() ||
>> > - (Left.isNot(tok::star) && Left.isNot(tok::amp) &&
>> > - !Style.PointerAndReferenceBindToType);
>> > + (Left.isNot(tok::star) && Left.isNot(tok::amp) &&
>> > + !Style.PointerAndReferenceBindToType);
>> > if (Left.is(tok::amp) || Left.is(tok::star))
>> > return Right.isLiteral() || Style.PointerAndReferenceBindToType;
>> > if (Right.is(tok::star) && Left.is(tok::l_paren))
>> > @@ -889,9 +889,9 @@
>> > return false;
>> > if (Right.is(tok::l_paren)) {
>> > return Left.is(tok::kw_if) || Left.is(tok::kw_for) ||
>> > - Left.is(tok::kw_while) || Left.is(tok::kw_switch) ||
>> > - (Left.isNot(tok::identifier) && Left.isNot(tok::kw_sizeof) &&
>> > - Left.isNot(tok::kw_typeof));
>> > + Left.is(tok::kw_while) || Left.is(tok::kw_switch) ||
>> > + (Left.isNot(tok::identifier) && Left.isNot(tok::kw_sizeof)
>> > &&
>> > + Left.isNot(tok::kw_typeof));
>> > }
>> > return true;
>> > }
>> > @@ -901,11 +901,11 @@
>> > Right.Tok.is(tok::comment) || Right.Tok.is(tok::greater))
>> > return false;
>> > return (isBinaryOperator(Left) && Left.Tok.isNot(tok::lessless)) ||
>> > - Left.Tok.is(tok::comma) || Right.Tok.is(tok::lessless) ||
>> > - Right.Tok.is(tok::arrow) || Right.Tok.is(tok::period) ||
>> > - Right.Tok.is(tok::colon) || Left.Tok.is(tok::semi) ||
>> > - Left.Tok.is(tok::l_brace) ||
>> > - (Left.Tok.is(tok::l_paren) && !Right.Tok.is(tok::r_paren));
>> > + Left.Tok.is(tok::comma) || Right.Tok.is(tok::lessless) ||
>> > + Right.Tok.is(tok::arrow) || Right.Tok.is(tok::period) ||
>> > + Right.Tok.is(tok::colon) || Left.Tok.is(tok::semi) ||
>> > + Left.Tok.is(tok::l_brace) ||
>> > + (Left.Tok.is(tok::l_paren) && !Right.Tok.is(tok::r_paren));
>> > }
>> >
>> > const UnwrappedLine &Line;
>> >
>> > Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=171039&r1=171038&r2=171039&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
>> > +++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Mon Dec 24 10:51:15
>> > 2012
>> > @@ -25,9 +25,7 @@
>> > UnwrappedLineParser::UnwrappedLineParser(const FormatStyle &Style,
>> > FormatTokenSource &Tokens,
>> > UnwrappedLineConsumer
>> > &Callback)
>> > - : Style(Style),
>> > - Tokens(Tokens),
>> > - Callback(Callback) {
>> > + : Style(Style), Tokens(Tokens), Callback(Callback) {
>> > }
>> >
>> > bool UnwrappedLineParser::parse() {
>> >
>> > Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=171039&r1=171038&r2=171039&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
>> > +++ cfe/trunk/lib/Format/UnwrappedLineParser.h Mon Dec 24 10:51:15 2012
>> > @@ -123,4 +123,4 @@
>> > } // end namespace format
>> > } // end namespace clang
>> >
>> > -#endif // LLVM_CLANG_FORMAT_UNWRAPPED_LINE_PARSER_H
>> > +#endif // LLVM_CLANG_FORMAT_UNWRAPPED_LINE_PARSER_H
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
More information about the cfe-commits
mailing list