Looking at the added / changed test cases that currently does not help much / would be harmful. I think what we'll need to do is change the formatter to actually try multiple indents on the same line break and then assign different penalties to them. This might help with many of these things. However, we don't do this yet.
On Wed, Jan 23, 2013 at 11:10 PM, Nico Weber <[email protected]> wrote: > On Wed, Jan 23, 2013 at 8:58 AM, Daniel Jasper <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
