r197980 - clang-format: (WebKit) Disallow 1-line constructors with initializers.

[Manuel Klimek]

On Tue, Dec 24, 2013 at 2:31 PM, Daniel Jasper <djasper at google.com> wrote:

> Author: djasper
> Date: Tue Dec 24 07:31:25 2013
> New Revision: 197980
>
> URL: http://llvm.org/viewvc/llvm-project?rev=197980&view=rev
> Log:
> clang-format: (WebKit) Disallow 1-line constructors with initializers.
>
> Before:
>   Constructor() : a(a) {}
>
> After:
>   Constructor()
>       : a(a)
>   {
>   }
>
> This style guide is pretty precise about this.
>
> 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=197980&r1=197979&r2=197980&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/Format.cpp (original)
> +++ cfe/trunk/lib/Format/Format.cpp Tue Dec 24 07:31:25 2013
> @@ -480,7 +480,7 @@ public:
>                             SmallVectorImpl<AnnotatedLine
> *>::const_iterator I,
>                             SmallVectorImpl<AnnotatedLine
> *>::const_iterator E) {
>      // We can never merge stuff if there are trailing line comments.
> -    AnnotatedLine *TheLine = *I;
> +    const AnnotatedLine *TheLine = *I;
>      if (TheLine->Last->Type == TT_LineComment)
>        return 0;
>
> @@ -498,7 +498,8 @@ public:
>      if (I + 1 == E || I[1]->Type == LT_Invalid)
>        return 0;
>
> -    if (TheLine->Last->Type == TT_FunctionLBrace) {
> +    if (TheLine->Last->Type == TT_FunctionLBrace &&
> +        TheLine->First != TheLine->Last) {
>        return Style.AllowShortFunctionsOnASingleLine
>                   ? tryMergeSimpleBlock(I, E, Limit)
>                   : 0;
> @@ -510,9 +511,11 @@ public:
>      }
>      if (I[1]->First->Type == TT_FunctionLBrace &&
>          Style.BreakBeforeBraces != FormatStyle::BS_Attach) {
> -      // Reduce the column limit by the number of spaces we need to insert
> -      // around braces.
> -      Limit = Limit > 3 ? Limit - 3 : 0;
> +      // Check for Limit <= 2 to accomodate for the " {".
>

So why don't we need to account for the full " { "?


> +      if (Limit <= 2 || (Style.ColumnLimit == 0 &&
> containsMustBreak(TheLine)))
> +        return 0;
> +      Limit -= 2;
> +
>        unsigned MergedLines = 0;
>        if (Style.AllowShortFunctionsOnASingleLine) {
>          MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);
> @@ -641,6 +644,14 @@ private:
>      return 1 + I[1]->Last->TotalLength + 1 + I[2]->Last->TotalLength <=
> Limit;
>    }
>
> +  bool containsMustBreak(const AnnotatedLine *Line) {
> +    for (const FormatToken *Tok = Line->First; Tok; Tok = Tok->Next) {
> +      if (Tok->MustBreakBefore)
> +        return true;
> +    }
> +    return false;
> +  }
> +
>    const FormatStyle &Style;
>  };
>
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=197980&r1=197979&r2=197980&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Dec 24 07:31:25 2013
> @@ -6997,6 +6997,16 @@ TEST_F(FormatTest, AllmanBraceBreaking)
>                 "}\n",
>                 BreakBeforeBrace);
>
> +  BreakBeforeBrace.ColumnLimit = 19;
> +  verifyFormat("void f() { int i; }", BreakBeforeBrace);
> +  BreakBeforeBrace.ColumnLimit = 18;
> +  verifyFormat("void f()\n"
> +               "{\n"
> +               "  int i;\n"
> +               "}",
> +               BreakBeforeBrace);
> +  BreakBeforeBrace.ColumnLimit = 80;
> +
>    FormatStyle BreakBeforeBraceShortIfs = BreakBeforeBrace;
>    BreakBeforeBraceShortIfs.AllowShortIfStatementsOnASingleLine = true;
>    BreakBeforeBraceShortIfs.AllowShortLoopsOnASingleLine = true;
> @@ -7716,16 +7726,26 @@ TEST_F(FormatTest, FormatsWithWebKitStyl
>                 "    :
> aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
>                 "    , aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaa, // break\n"
>                 "                               aaaaaaaaaaaaaa)\n"
> -               "    , aaaaaaaaaaaaaaaaaaaaaaa() {}",
> +               "    , aaaaaaaaaaaaaaaaaaaaaaa()\n"
> +               "{\n"
> +               "}",
>                 Style);
>    verifyFormat("SomeClass::Constructor()\n"
> -               "    : a(a) {}",
> +               "    : a(a)\n"
> +               "{\n"
> +               "}",
>                 Style);
> +  EXPECT_EQ("SomeClass::Constructor()\n"
> +            "    : a(a)\n"
> +            "{\n"
> +            "}",
> +            format("SomeClass::Constructor():a(a){}", Style));
>    verifyFormat("SomeClass::Constructor()\n"
>                 "    : a(a)\n"
>                 "    , b(b)\n"
> -               "    , c(c) {}",
> -               Style);
> +               "    , c(c)\n"
> +               "{\n"
> +               "}", Style);
>    verifyFormat("SomeClass::Constructor()\n"
>                 "    : a(a)\n"
>                 "{\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/20131227/94df5574/attachment.html>