<div dir="ltr">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.</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jan 23, 2013 at 11:10 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Wed, Jan 23, 2013 at 8:58 AM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br>
</div><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: djasper<br>
Date: Wed Jan 23 10:58:21 2013<br>
New Revision: 173273<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=173273&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=173273&view=rev</a><br>
Log:<br>
Don't try to align builder-type continuations on assignments.<br>
<br>
Before:<br>
int aaaa = aaaaa().aaaaa() // force break<br>
.aaaaa();<br>
After:<br>
int aaaa = aaaaa().aaaaa() // force break<br>
.aaaaa();<br></blockquote><div><br></div></div><div>Since this pattern is explicitly detected already, should this try to align on the '.'?</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
The other indent is just wrong and confusing.<br>
<br>
Modified:<br>
cfe/trunk/lib/Format/Format.cpp<br>
cfe/trunk/unittests/Format/FormatTest.cpp<br>
<br>
Modified: cfe/trunk/lib/Format/Format.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=173273&r1=173272&r2=173273&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=173273&r1=173272&r2=173273&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/Format.cpp (original)<br>
+++ cfe/trunk/lib/Format/Format.cpp Wed Jan 23 10:58:21 2013<br>
@@ -421,9 +421,9 @@<br>
<br>
struct ParenState {<br>
ParenState(unsigned Indent, unsigned LastSpace)<br>
- : Indent(Indent), LastSpace(LastSpace), FirstLessLess(0),<br>
- BreakBeforeClosingBrace(false), BreakAfterComma(false),<br>
- HasMultiParameterLine(false) {}<br>
+ : Indent(Indent), LastSpace(LastSpace), AssignmentColumn(0),<br>
+ FirstLessLess(0), BreakBeforeClosingBrace(false),<br>
+ BreakAfterComma(false), HasMultiParameterLine(false) {}<br>
<br>
/// \brief The position to which a specific parenthesis level needs to be<br>
/// indented.<br>
@@ -436,6 +436,9 @@<br>
/// OtherParameter));<br>
unsigned LastSpace;<br>
<br>
+ /// \brief This is the column of the first token after an assignment.<br>
+ unsigned AssignmentColumn;<br>
+<br>
/// \brief The position the first "<<" operator encountered on each level.<br>
///<br>
/// Used to align "<<" operators. 0 if no such operator has been encountered<br>
@@ -457,6 +460,8 @@<br>
return Indent < Other.Indent;<br>
if (LastSpace != Other.LastSpace)<br>
return LastSpace < Other.LastSpace;<br>
+ if (AssignmentColumn != Other.AssignmentColumn)<br>
+ return AssignmentColumn < Other.AssignmentColumn;<br>
if (FirstLessLess != Other.FirstLessLess)<br>
return FirstLessLess < Other.FirstLessLess;<br>
if (BreakBeforeClosingBrace != Other.BreakBeforeClosingBrace)<br>
@@ -547,6 +552,9 @@<br>
State.Column = State.ForLoopVariablePos;<br>
} else if (State.NextToken->Parent->ClosesTemplateDeclaration) {<br>
State.Column = State.Stack[ParenLevel].Indent - 4;<br>
+ } else if (Previous.Type == TT_BinaryOperator &&<br>
+ State.Stack.back().AssignmentColumn != 0) {<br>
+ State.Column = State.Stack.back().AssignmentColumn;<br>
} else {<br>
State.Column = State.Stack[ParenLevel].Indent;<br>
}<br>
@@ -587,7 +595,7 @@<br>
if (RootToken.isNot(tok::kw_for) && ParenLevel == 0 &&<br>
(getPrecedence(Previous) == prec::Assignment ||<br>
Previous.is(tok::kw_return)))<br>
- State.Stack[ParenLevel].Indent = State.Column + Spaces;<br>
+ State.Stack.back().AssignmentColumn = State.Column + Spaces;<br>
if (Previous.is(tok::l_paren) || Previous.is(tok::l_brace) ||<br>
State.NextToken->Parent->Type == TT_TemplateOpener)<br>
State.Stack[ParenLevel].Indent = State.Column + Spaces;<br>
<br>
Modified: cfe/trunk/unittests/Format/FormatTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=173273&r1=173272&r2=173273&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=173273&r1=173272&r2=173273&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)<br>
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Jan 23 10:58:21 2013<br>
@@ -1006,10 +1006,10 @@<br>
TEST_F(FormatTest, FormatsBuilderPattern) {<br>
verifyFormat(<br>
"return llvm::StringSwitch<Reference::Kind>(name)\n"<br>
- " .StartsWith(\".eh_frame_hdr\", ORDER_EH_FRAMEHDR)\n"<br>
- " .StartsWith(\".eh_frame\", ORDER_EH_FRAME).StartsWith(\".init\", ORDER_INIT)\n"<br>
- " .StartsWith(\".fini\", ORDER_FINI).StartsWith(\".hash\", ORDER_HASH)\n"<br>
- " .Default(ORDER_TEXT);\n");<br>
+ " .StartsWith(\".eh_frame_hdr\", ORDER_EH_FRAMEHDR)\n"<br>
+ " .StartsWith(\".eh_frame\", ORDER_EH_FRAME).StartsWith(\".init\", ORDER_INIT)\n"<br>
+ " .StartsWith(\".fini\", ORDER_FINI).StartsWith(\".hash\", ORDER_HASH)\n"<br>
+ " .Default(ORDER_TEXT);\n");<br>
}<br>
<br>
TEST_F(FormatTest, DoesNotBreakTrailingAnnotation) {<br>
@@ -1042,6 +1042,10 @@<br>
verifyFormat(<br>
"CharSourceRange LineRange = CharSourceRange::getTokenRange(\n"<br>
" Line.Tokens.front().Tok.getLo(), Line.Tokens.back().Tok.getLoc());");<br>
+<br>
+ verifyFormat(<br>
+ "aaaaaaaaaaaaaaaaaaaaaaaaaa aaaa = aaaaaaaaaaaaaa(0).aaaa()\n"<br>
+ " .aaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaa::aaaaaaaaaaaaaaaaaaaaa);");<br>
}<br>
<br>
TEST_F(FormatTest, AlignsAfterAssignments) {<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br>
</blockquote></div><br></div>