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

[Nico Weber]

On Wed, Jan 23, 2013 at 8:58 AM, Daniel Jasper <djasper at google.com> wrote:

> Author: djasper
> Date: Wed Jan 23 10:58:21 2013
> New Revision: 173273
>
> URL: http://llvm.org/viewvc/llvm-project?rev=173273&view=rev
> Log:
> Don't try to align builder-type continuations on assignments.
>
> Before:
> int aaaa = aaaaa().aaaaa() // force break
>            .aaaaa();
> After:
> int aaaa = aaaaa().aaaaa() // force break
>     .aaaaa();
>

Since this pattern is explicitly detected already, should this try to align
on the '.'?


>
> The other indent is just wrong and confusing.
>
> 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=173273&r1=173272&r2=173273&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/Format.cpp (original)
> +++ cfe/trunk/lib/Format/Format.cpp Wed Jan 23 10:58:21 2013
> @@ -421,9 +421,9 @@
>
>    struct ParenState {
>      ParenState(unsigned Indent, unsigned LastSpace)
> -        : Indent(Indent), LastSpace(LastSpace), FirstLessLess(0),
> -          BreakBeforeClosingBrace(false), BreakAfterComma(false),
> -          HasMultiParameterLine(false) {}
> +        : Indent(Indent), LastSpace(LastSpace), AssignmentColumn(0),
> +          FirstLessLess(0), BreakBeforeClosingBrace(false),
> +          BreakAfterComma(false), HasMultiParameterLine(false) {}
>
>      /// \brief The position to which a specific parenthesis level needs
> to be
>      /// indented.
> @@ -436,6 +436,9 @@
>      ///                             OtherParameter));
>      unsigned LastSpace;
>
> +    /// \brief This is the column of the first token after an assignment.
> +    unsigned AssignmentColumn;
> +
>      /// \brief The position the first "<<" operator encountered on each
> level.
>      ///
>      /// Used to align "<<" operators. 0 if no such operator has been
> encountered
> @@ -457,6 +460,8 @@
>          return Indent < Other.Indent;
>        if (LastSpace != Other.LastSpace)
>          return LastSpace < Other.LastSpace;
> +      if (AssignmentColumn != Other.AssignmentColumn)
> +        return AssignmentColumn < Other.AssignmentColumn;
>        if (FirstLessLess != Other.FirstLessLess)
>          return FirstLessLess < Other.FirstLessLess;
>        if (BreakBeforeClosingBrace != Other.BreakBeforeClosingBrace)
> @@ -547,6 +552,9 @@
>          State.Column = State.ForLoopVariablePos;
>        } else if (State.NextToken->Parent->ClosesTemplateDeclaration) {
>          State.Column = State.Stack[ParenLevel].Indent - 4;
> +      } else if (Previous.Type == TT_BinaryOperator &&
> +                 State.Stack.back().AssignmentColumn != 0) {
> +        State.Column = State.Stack.back().AssignmentColumn;
>        } else {
>          State.Column = State.Stack[ParenLevel].Indent;
>        }
> @@ -587,7 +595,7 @@
>        if (RootToken.isNot(tok::kw_for) && ParenLevel == 0 &&
>            (getPrecedence(Previous) == prec::Assignment ||
>             Previous.is(tok::kw_return)))
> -        State.Stack[ParenLevel].Indent = State.Column + Spaces;
> +        State.Stack.back().AssignmentColumn = State.Column + Spaces;
>        if (Previous.is(tok::l_paren) || Previous.is(tok::l_brace) ||
>            State.NextToken->Parent->Type == TT_TemplateOpener)
>          State.Stack[ParenLevel].Indent = State.Column + Spaces;
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=173273&r1=173272&r2=173273&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Jan 23 10:58:21 2013
> @@ -1006,10 +1006,10 @@
>  TEST_F(FormatTest, FormatsBuilderPattern) {
>    verifyFormat(
>        "return llvm::StringSwitch<Reference::Kind>(name)\n"
> -      "       .StartsWith(\".eh_frame_hdr\", ORDER_EH_FRAMEHDR)\n"
> -      "       .StartsWith(\".eh_frame\",
> ORDER_EH_FRAME).StartsWith(\".init\", ORDER_INIT)\n"
> -      "       .StartsWith(\".fini\", ORDER_FINI).StartsWith(\".hash\",
> ORDER_HASH)\n"
> -      "       .Default(ORDER_TEXT);\n");
> +      "    .StartsWith(\".eh_frame_hdr\", ORDER_EH_FRAMEHDR)\n"
> +      "    .StartsWith(\".eh_frame\",
> ORDER_EH_FRAME).StartsWith(\".init\", ORDER_INIT)\n"
> +      "    .StartsWith(\".fini\", ORDER_FINI).StartsWith(\".hash\",
> ORDER_HASH)\n"
> +      "    .Default(ORDER_TEXT);\n");
>  }
>
>  TEST_F(FormatTest, DoesNotBreakTrailingAnnotation) {
> @@ -1042,6 +1042,10 @@
>    verifyFormat(
>        "CharSourceRange LineRange = CharSourceRange::getTokenRange(\n"
>        "    Line.Tokens.front().Tok.getLo(),
> Line.Tokens.back().Tok.getLoc());");
> +
> +  verifyFormat(
> +      "aaaaaaaaaaaaaaaaaaaaaaaaaa aaaa = aaaaaaaaaaaaaa(0).aaaa()\n"
> +      "    .aaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaa::aaaaaaaaaaaaaaaaaaaaa);");
>  }
>
>  TEST_F(FormatTest, AlignsAfterAssignments) {
>
>
> _______________________________________________
> 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/20130123/61ca20ae/attachment.html>