[cfe-commits] r172413 - in /cfe/trunk: lib/Format/Format.cpp unittests/Format/FormatTest.cpp

Daniel Jasper djasper at google.com
Mon Jan 14 07:43:58 PST 2013


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).

Douglas: Should I set this to break or no break for the default LLVM Style?


On Mon, Jan 14, 2013 at 3:52 PM, Douglas Gregor <dgregor at apple.com> wrote:

>
> On Jan 14, 2013, at 6:14 AM, Daniel Jasper <djasper at google.com> wrote:
>
> > Author: djasper
> > Date: Mon Jan 14 08:14:23 2013
> > New Revision: 172413
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=172413&view=rev
> > Log:
> > Put short if statements on a single line.
> >
> > Before: if (a)
> >          return;
> > After:  if (a) return;
> >
> > Not yet sure, whether this is always desired, but we can add options and
> > make this a style parameter as we go along.
>
> I'd prefer this to be a style parameter…
>
>         - Doug
>
> > Modified:
> >    cfe/trunk/lib/Format/Format.cpp
> >    cfe/trunk/unittests/Format/FormatTest.cpp
> >
> > Modified: cfe/trunk/lib/Format/Format.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=172413&r1=172412&r2=172413&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Format/Format.cpp (original)
> > +++ cfe/trunk/lib/Format/Format.cpp Mon Jan 14 08:14:23 2013
> > @@ -69,12 +69,9 @@
> >         CanBreakBefore(false), MustBreakBefore(false),
> >         ClosesTemplateDeclaration(false), Parent(NULL) {}
> >
> > -  bool is(tok::TokenKind Kind) const {
> > -    return FormatTok.Tok.is(Kind);
> > -  }
> > -  bool isNot(tok::TokenKind Kind) const {
> > -    return FormatTok.Tok.isNot(Kind);
> > -  }
> > +  bool is(tok::TokenKind Kind) const { return FormatTok.Tok.is(Kind); }
> > +  bool isNot(tok::TokenKind Kind) const { return
> FormatTok.Tok.isNot(Kind); }
> > +
> >   bool isObjCAtKeyword(tok::ObjCKeywordKind Kind) const {
> >     return FormatTok.Tok.isObjCAtKeyword(Kind);
> >   }
> > @@ -185,8 +182,8 @@
> > /// \p RootToken fits into \p Limit columns on a single line.
> > ///
> > /// If true, sets \p Length to the required length.
> > -static bool fitsIntoLimit(const AnnotatedToken &RootToken,
> > -                          unsigned Limit, unsigned* Length = 0) {
> > +static bool fitsIntoLimit(const AnnotatedToken &RootToken, unsigned
> Limit,
> > +                          unsigned *Length = 0) {
> >   unsigned Columns = RootToken.FormatTok.TokenLength;
> >   const AnnotatedToken *Tok = &RootToken;
> >   while (!Tok->Children.empty()) {
> > @@ -781,8 +778,8 @@
> >           Tok->Type = TT_ObjCMethodExpr;
> >         break;
> >       case tok::l_paren: {
> > -        bool ParensWereObjCReturnType =
> > -            Tok->Parent && Tok->Parent->Type == TT_ObjCMethodSpecifier;
> > +        bool ParensWereObjCReturnType = Tok->Parent &&
> Tok->Parent->Type ==
> > +                                        TT_ObjCMethodSpecifier;
> >         if (!parseParens())
> >           return false;
> >         if (CurrentToken != NULL && CurrentToken->is(tok::colon)) {
> > @@ -1347,8 +1344,8 @@
> >
> > class Formatter : public UnwrappedLineConsumer {
> > public:
> > -  Formatter(DiagnosticsEngine &Diag, const FormatStyle &Style,
> > -            Lexer &Lex, SourceManager &SourceMgr,
> > +  Formatter(DiagnosticsEngine &Diag, const FormatStyle &Style, Lexer
> &Lex,
> > +            SourceManager &SourceMgr,
> >             const std::vector<CharSourceRange> &Ranges)
> >       : Diag(Diag), Style(Style), Lex(Lex), SourceMgr(SourceMgr),
> >         Ranges(Ranges) {}
> > @@ -1407,25 +1404,46 @@
> >     // Check whether the UnwrappedLine can be put onto a single line. If
> >     // so, this is bound to be the optimal solution (by definition) and
> we
> >     // don't need to analyze the entire solution space.
> > -    unsigned LengthLine1 = 0;
> > -    if (!fitsIntoLimit(I->First, Limit, &LengthLine1))
> > +    unsigned Length = 0;
> > +    if (!fitsIntoLimit(I->First, Limit, &Length))
> >       return false;
> > +    Limit -= Length + 1; // One space.
> > +    if (I + 1 == E)
> > +      return true;
> > +
> > +    if (I->Last->is(tok::l_brace)) {
> > +      tryMergeSimpleBlock(I, E, Limit);
> > +    } else if (I->First.is(tok::kw_if)) {
> > +      tryMergeSimpleIf(I, E, Limit);
> > +    }
> > +    return true;
> > +  }
> >
> > +  void tryMergeSimpleIf(std::vector<AnnotatedLine>::iterator &I,
> > +                        std::vector<AnnotatedLine>::iterator E,
> > +                        unsigned Limit) {
> > +    AnnotatedLine &Line = *I;
> > +    if (!fitsIntoLimit((I + 1)->First, Limit))
> > +      return;
> > +    if ((I + 1)->First.is(tok::kw_if) || (I + 1)->First.Type ==
> TT_LineComment)
> > +      return;
> > +    // Only inline simple if's (no nested if or else).
> > +    if (I + 2 != E && (I + 2)->First.is(tok::kw_else))
> > +      return;
> > +    join(Line, *(++I));
> > +  }
> > +
> > +  void tryMergeSimpleBlock(std::vector<AnnotatedLine>::iterator &I,
> > +                        std::vector<AnnotatedLine>::iterator E,
> > +                        unsigned Limit){
> >     // Check that we still have three lines and they fit into the limit.
> > -    if (I + 1 == E || I + 2 == E)
> > -      return true;
> > -    unsigned LengthLine2 = 0;
> > -    unsigned LengthLine3 = 0;
> > -    if (!fitsIntoLimit((I + 1)->First, Limit, &LengthLine2) ||
> > -        !fitsIntoLimit((I + 2)->First, Limit, &LengthLine3))
> > -      return true;
> > -    if (LengthLine1 + LengthLine2 + LengthLine3 + 2 > Limit) // Two
> spaces.
> > -      return true;
> > +    if (I + 2 == E || !nextTwoLinesFitInto(I, Limit))
> > +      return;
> >
> >     // First, check that the current line allows merging. This is the
> case if
> >     // we're not in a control flow statement and the last token is an
> opening
> >     // brace.
> > -    AnnotatedLine& Line = *I;
> > +    AnnotatedLine &Line = *I;
> >     bool AllowedTokens =
> >         Line.First.isNot(tok::kw_if) && Line.First.isNot(tok::kw_while)
> &&
> >         Line.First.isNot(tok::kw_do) && Line.First.isNot(tok::r_brace) &&
> > @@ -1434,17 +1452,17 @@
> >         // This gets rid of all ObjC @ keywords and methods.
> >         Line.First.isNot(tok::at) && Line.First.isNot(tok::minus) &&
> >         Line.First.isNot(tok::plus);
> > -    if (Line.Last->isNot(tok::l_brace) || !AllowedTokens)
> > -      return true;
> > +    if (!AllowedTokens)
> > +      return;
> >
> >     // Second, check that the next line does not contain any braces - if
> it
> >     // does, readability declines when putting it into a single line.
> >     const AnnotatedToken *Tok = &(I + 1)->First;
> >     if ((I + 1)->Last->Type == TT_LineComment || Tok->MustBreakBefore)
> > -      return true;
> > +      return;
> >     do {
> >       if (Tok->is(tok::l_brace) || Tok->is(tok::r_brace))
> > -        return true;
> > +        return;
> >       Tok = Tok->Children.empty() ? NULL : &Tok->Children.back();
> >     } while (Tok != NULL);
> >
> > @@ -1452,7 +1470,7 @@
> >     Tok = &(I + 2)->First;
> >     if (!Tok->Children.empty() || Tok->isNot(tok::r_brace) ||
> >         Tok->MustBreakBefore)
> > -      return true;
> > +      return;
> >
> >     // If the merged line fits, we use that instead and skip the next
> two lines.
> >     Line.Last->Children.push_back((I + 1)->First);
> > @@ -1464,7 +1482,16 @@
> >     join(Line, *(I + 1));
> >     join(Line, *(I + 2));
> >     I += 2;
> > -    return true;
> > +  }
> > +
> > +  bool nextTwoLinesFitInto(std::vector<AnnotatedLine>::iterator I,
> > +                           unsigned Limit) {
> > +    unsigned LengthLine1 = 0;
> > +    unsigned LengthLine2 = 0;
> > +    if (!fitsIntoLimit((I + 1)->First, Limit, &LengthLine1) ||
> > +        !fitsIntoLimit((I + 2)->First, Limit, &LengthLine2))
> > +      return false;
> > +    return LengthLine1 + LengthLine2 + 1 <= Limit; // One space.
> >   }
> >
> >   void join(AnnotatedLine &A, const AnnotatedLine &B) {
> >
> > Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=172413&r1=172412&r2=172413&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> > +++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jan 14 08:14:23 2013
> > @@ -130,12 +130,14 @@
> >
> //===----------------------------------------------------------------------===//
> >
> > TEST_F(FormatTest, FormatIfWithoutCompountStatement) {
> > -  verifyFormat("if (true)\n  f();\ng();");
> > -  verifyFormat("if (a)\n  if (b)\n    if (c)\n      g();\nh();");
> > +  verifyFormat("if (true) f();\ng();");
> > +  verifyFormat("if (a)\n  if (b)\n    if (c) g();\nh();");
> >   verifyFormat("if (a)\n  if (b) {\n    f();\n  }\ng();");
> >   verifyFormat("if (a)\n"
> >                "  // comment\n"
> >                "  f();");
> > +  verifyFormat("if (a) return;", getLLVMStyleWithColumns(14));
> > +  verifyFormat("if (a)\n  return;", getLLVMStyleWithColumns(13));
> > }
> >
> > TEST_F(FormatTest, ParseIfElse) {
> > @@ -152,8 +154,7 @@
> >   verifyFormat("if (true)\n"
> >                "  if (true)\n"
> >                "    if (true) {\n"
> > -               "      if (true)\n"
> > -               "        f();\n"
> > +               "      if (true) f();\n"
> >                "    } else {\n"
> >                "      g();\n"
> >                "    }\n"
> > @@ -1454,8 +1455,7 @@
> >
> >   verifyFormat("@implementation Foo\n"
> >                "+ (id)init {\n"
> > -               "  if (true)\n"
> > -               "    return nil;\n"
> > +               "  if (true) return nil;\n"
> >                "}\n"
> >                "// Look, a comment!\n"
> >                "- (int)answerWith:(int)i {\n"
> >
> >
> > _______________________________________________
> > 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/20130114/1fc7fbfb/attachment.html>


More information about the cfe-commits mailing list