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

Nico Weber thakis at chromium.org
Mon Jan 14 06:51:12 PST 2013


This isn't allowed in chromium:
http://dev.chromium.org/developers/coding-style#Code_Formatting (line
2) Can you make this optional?

(The original design document mentioned something about the formatter
trying to pick up preferred style from the file automatically – is
this still planned?)

Thanks,
Nico

On Jan 14, 2013 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.
>
> 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




More information about the cfe-commits mailing list