[cfe-commits] r171039 - in /cfe/trunk: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h
Daniel Jasper
djasper at google.com
Tue Dec 25 02:10:59 PST 2012
On Mon, Dec 24, 2012 at 9:02 PM, Matt Beaumont-Gay <matthewbg at google.com>wrote:
> Comments from the peanut gallery...
>
> 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.
> >
> > 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 is a little dubious.
>
I agree. This has been introduced with the operator precedence patch. The
reasons I have not addressed it yet:
a) It is a reasonably rare case.
b) It is not terribly bad (I am not aware that it contradicts a style
guide).
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)?
} else if (aaaaaaaa ==
bbbbbbbb) ...
} else if (aaaaaaaa ==
bbbbbbbb) ...
} else if (
aaaaaaaa == bbbbbbbb) ...
(This might need a parallel Font).
> 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);
>
> This is nice!
>
> > } 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121225/6e462719/attachment.html>
More information about the cfe-commits
mailing list