r194214 - clang-format: Improve ObjC variadic and binary expression parameters.

[Nico Weber]

Awesome, thanks! PR16185 comment 2 isn't quite perfect yet though as far as
I can tell.

In a block, I get (I deleted some characters from the function names):

{
  popup_wdow_.reset(
      [[RenderWidgetPopupWindow alloc] iniithContentRect:NSMakRet(
          origin_global.x, origin_global.y, pos.width(), pos.height())

 syeMask:NSBorderlessWindowMask

 bking:NSBackingStoreBuffered
                                                     der:NO]);
}

while this is probably worse than (manually-aligned)

{
  popup_wdow_.reset(
      [[RenderWidgetPopupWindow alloc]
          iniithContentRect:NSMakRet(origin_global.x, origin_global.y,
                                     pos.width(), pos.height())
                    syeMask:NSBorderlessWindowMask
                      bking:NSBackingStoreBuffered
                        der:NO]);
}

If I omit the outer block, I get

popup_window_.reset([[RenderWidgetPopupWindow alloc]
    initWithContentRect:
        NSMakeRect(origin_global.x, origin_global.y, pos.width(),
pos.height())
              styleMask:NSBorderlessWindowMask
                backing:NSBackingStoreBuffered
                  defer:NO]);

but (manually-produced)

popup_window_.reset([[RenderWidgetPopupWindow alloc]
    initWithContentRect:
        NSMakeRect(origin_global.x, origin_global.y, pos.width(),
pos.height())
              styleMask:NSBorderlessWindowMask
                backing:NSBackingStoreBuffered
                  defer:NO]);

is probably nicer.


On Thu, Nov 7, 2013 at 11:23 AM, Daniel Jasper <djasper at google.com> wrote:

> Author: djasper
> Date: Thu Nov  7 13:23:49 2013
> New Revision: 194214
>
> URL: http://llvm.org/viewvc/llvm-project?rev=194214&view=rev
> Log:
> clang-format: Improve ObjC variadic and binary expression parameters.
>
> Before:
>   [self aaaaaaaaaaaaaaa:aaaaaaaaaaaaaaa | aaaaaaaaaaaaaaa |
> aaaaaaaaaaaaaaa |
>                            aaaaaaaaaaaaaaa | aaaaaaaaaaaaaaa |
> aaaaaaaaaaaaaaa |
>                            aaaaaaaaaaaaaaa | aaaaaaaaaaaaaaa];
>   [self aaaaaaaaaaaaaaa:aaaaaaaaaaaaaaa,
>       aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa,
>       aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa];
>
> After:
>   [self aaaaaaaaaaaaaaa:aaaaaaaaaaaaaaa | aaaaaaaaaaaaaaa |
> aaaaaaaaaaaaaaa |
>                         aaaaaaaaaaaaaaa | aaaaaaaaaaaaaaa |
> aaaaaaaaaaaaaaa |
>                         aaaaaaaaaaaaaaa | aaaaaaaaaaaaaaa];
>   [self aaaaaaaaaaaaaaa:aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa,
>                         aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa,
>                         aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa];
>
> This addresses llvm.org/PR15349 and llvm.org/PR16185.
>
> Modified:
>     cfe/trunk/lib/Format/ContinuationIndenter.cpp
>     cfe/trunk/lib/Format/TokenAnnotator.cpp
>     cfe/trunk/unittests/Format/FormatTest.cpp
>
> Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=194214&r1=194213&r2=194214&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
> +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Nov  7 13:23:49 2013
> @@ -499,7 +499,8 @@ unsigned ContinuationIndenter::moveState
>    // is special cased.
>    bool SkipFirstExtraIndent =
>        (Previous && (Previous->opensScope() ||
> Previous->is(tok::kw_return) ||
> -                    Previous->getPrecedence() == prec::Assignment));
> +                    Previous->getPrecedence() == prec::Assignment ||
> +                    Previous->Type == TT_ObjCMethodExpr));
>    for (SmallVectorImpl<prec::Level>::const_reverse_iterator
>             I = Current.FakeLParens.rbegin(),
>             E = Current.FakeLParens.rend();
>
> Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=194214&r1=194213&r2=194214&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
> +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Nov  7 13:23:49 2013
> @@ -871,8 +871,11 @@ public:
>
>    /// \brief Parse expressions with the given operatore precedence.
>    void parse(int Precedence = 0) {
> -    // Skip 'return' as it is not part of a binary expression.
> -    while (Current && Current->is(tok::kw_return))
> +    // Skip 'return' and ObjC selector colons as they are not part of a
> binary
> +    // expression.
> +    while (Current &&
> +           (Current->is(tok::kw_return) ||
> +            (Current->is(tok::colon) && Current->Type ==
> TT_ObjCMethodExpr)))
>        next();
>
>      if (Current == NULL || Precedence > PrecedenceArrowAndPeriod)
> @@ -944,12 +947,11 @@ private:
>      if (Current) {
>        if (Current->Type == TT_ConditionalExpr)
>          return prec::Conditional;
> -      else if (Current->is(tok::semi) || Current->Type ==
> TT_InlineASMColon)
> +      else if (Current->is(tok::semi) || Current->Type ==
> TT_InlineASMColon ||
> +               Current->Type == TT_ObjCSelectorName)
>          return 0;
>        else if (Current->Type == TT_BinaryOperator ||
> Current->is(tok::comma))
>          return Current->getPrecedence();
> -      else if (Current->Type == TT_ObjCSelectorName)
> -        return prec::Assignment;
>        else if (Current->isOneOf(tok::period, tok::arrow))
>          return PrecedenceArrowAndPeriod;
>      }
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=194214&r1=194213&r2=194214&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Nov  7 13:23:49 2013
> @@ -5373,10 +5373,18 @@ TEST_F(FormatTest, FormatObjCMethodExpr)
>                 "    aaaaaaaaaa:bbbbbbbbbbbbbbbbb\n"
>                 "         aaaaa:bbbbbbbbbbb + bbbbbbbbbbbb\n"
>                 "          aaaa:bbb];");
> +  verifyFormat(
> +      "[self aaaaaaaaaa:aaaaaaaaaaaaaaa | aaaaaaaaaaaaaaa |
> aaaaaaaaaaaaaaa |\n"
> +      "                 aaaaaaaaaaaaaaa | aaaaaaaaaaaaaaa |
> aaaaaaaaaaaaaaa |\n"
> +      "                 aaaaaaaaaaaaaaa | aaaaaaaaaaaaaaa];");
>
>    // Variadic parameters.
>    verifyFormat(
>        "NSArray *myStrings = [NSArray stringarray:@\"a\", @\"b\", nil];");
> +  verifyFormat(
> +      "[self aaaaaaaaaaaaa:aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa,
> aaaaaaaaaaaaaaa,\n"
> +      "                    aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa,
> aaaaaaaaaaaaaaa,\n"
> +      "                    aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa];");
>  }
>
>  TEST_F(FormatTest, ObjCAt) {
>
>
> _______________________________________________
> 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/20131107/2fae6745/attachment.html>