r177521 - Reduce penalty for breaks after "(" for functions with parameters.

Daniel Jasper djasper at google.com
Wed Mar 20 12:53:24 PDT 2013


Well, the general rule we are trying to follow is to try and keep inner
expressions together if possible (we have to split at two points
between/before parameters of the function call, but there is no need to
also split the addition).

I think in the example, the individual parameters to the function call are
easier to see. I don't feel all that strongly, but we have had requests
from multiple users.


On Wed, Mar 20, 2013 at 7:34 PM, Jordan Rose <jordan_rose at apple.com> wrote:

> Really? I'm not sure this is an improvement. The dangling left-paren looks
> stranger to me than the wrapped expression...although I would have indented
> the wrapped expression one more level.
>
>
> On Mar 20, 2013, at 6:53 , Daniel Jasper <djasper at google.com> wrote:
>
> > Author: djasper
> > Date: Wed Mar 20 08:53:11 2013
> > New Revision: 177521
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=177521&view=rev
> > Log:
> > Reduce penalty for breaks after "(" for functions with parameters.
> >
> > Before:
> >  aaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
> >                    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
> >                    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);
> > After:
> >  aaaaaaaaaaaaaaaaa(
> >      aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
> >      aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);
> >
> > Modified:
> >    cfe/trunk/lib/Format/Format.cpp
> >    cfe/trunk/lib/Format/TokenAnnotator.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=177521&r1=177520&r2=177521&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Format/Format.cpp (original)
> > +++ cfe/trunk/lib/Format/Format.cpp Wed Mar 20 08:53:11 2013
> > @@ -372,8 +372,8 @@ private:
> >   void alignComments(comment_iterator I, comment_iterator E, unsigned
> Column) {
> >     while (I != E) {
> >       unsigned Spaces = I->Spaces + Column - I->MinColumn;
> > -      storeReplacement(I->Tok, std::string(I->NewLines, '\n') +
> > -                               std::string(Spaces, ' '));
> > +      storeReplacement(
> > +          I->Tok, std::string(I->NewLines, '\n') + std::string(Spaces,
> ' '));
> >       ++I;
> >     }
> >   }
> >
> > Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=177521&r1=177520&r2=177521&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
> > +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Mar 20 08:53:11 2013
> > @@ -937,7 +937,7 @@ unsigned TokenAnnotator::splitPenalty(co
> >     return 20;
> >
> >   if (opensScope(Left))
> > -    return 20;
> > +    return Left.ParameterCount > 1 ? prec::Comma : 20;
> >
> >   if (Right.is(tok::lessless)) {
> >     if (Left.is(tok::string_literal)) {
> >
> > Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=177521&r1=177520&r2=177521&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> > +++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Mar 20 08:53:11 2013
> > @@ -1485,6 +1485,10 @@ TEST_F(FormatTest, BreaksDesireably) {
> >   verifyFormat(
> >       "aaaaaa(aaa, new
> Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
> >       "                aaaaaaaaaaaaaaaaaaaaaaaaaaaaa));");
> > +  verifyFormat(
> > +      "aaaaaaaaaaaaaaaaa(\n"
> > +      "    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"
> > +      "    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);");
> >
> >   // This test case breaks on an incorrect memoization, i.e. an
> optimization not
> >   // taking into account the StopAt value.
> > @@ -1677,7 +1681,7 @@ TEST_F(FormatTest, BreaksConditionalExpr
> >       "                                                    :
> aaaaaaaaaaaaa);");
> >   verifyFormat(
> >       "aaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaa,\n"
> > -      "                   aaaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaa\n"
> > +      "                   aaaaaaaaaaaaaaaa ?
> aaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
> >       "                                    :
> aaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"
> >       "                   aaaaaaaaaaaaa);");
> >   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\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/20130320/3658d8a3/attachment.html>


More information about the cfe-commits mailing list