<div dir="ltr">On Tue, Apr 2, 2013 at 4:33 PM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: djasper<br>
Date: Tue Apr 2 09:33:13 2013<br>
New Revision: 178542<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=178542&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=178542&view=rev</a><br>
Log:<br>
Fix some inconsistent use of indentation.<br>
<br>
Basically we have always special-cased the top-level statement of an<br>
unwrapped line (the one with ParenLevel == 0) and that lead to several<br>
inconsistencies. All added tests were formatted in a strange way, for<br>
example:<br>
<br>
Before:<br>
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<br>
.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<br>
.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<br>
.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa();<br>
if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<br>
.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<br>
.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<br>
.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa()) {<br>
}<br>
<br>
After:<br>
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<br>
.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<br>
.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<br>
.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa();<br>
if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<br>
.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<br>
.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<br>
.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa()) {<br>
}<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=178542&r1=178541&r2=178542&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=178542&r1=178541&r2=178542&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/Format.cpp (original)<br>
+++ cfe/trunk/lib/Format/Format.cpp Tue Apr 2 09:33:13 2013<br>
@@ -485,7 +485,7 @@ public:<br>
State.Column = FirstIndent;<br>
State.NextToken = &RootToken;<br>
State.Stack.push_back(<br>
- ParenState(FirstIndent + 4, FirstIndent, !Style.BinPackParameters,<br>
+ ParenState(FirstIndent, FirstIndent, !Style.BinPackParameters,<br>
/*HasMultiParameterLine=*/ false));<br>
State.VariablePos = 0;<br>
State.LineContainsContinuedForLoopSection = false;<br>
@@ -536,7 +536,8 @@ private:<br>
BreakBeforeClosingBrace(false), QuestionColumn(0),<br>
AvoidBinPacking(AvoidBinPacking), BreakBeforeParameter(false),<br>
HasMultiParameterLine(HasMultiParameterLine), ColonPos(0),<br>
- StartOfFunctionCall(0) {}<br>
+ StartOfFunctionCall(0), NestedNameSpecifierContinuation(0),<br>
+ CallContinuation(0) {}<br>
<br>
/// \brief The position to which a specific parenthesis level needs to be<br>
/// indented.<br>
@@ -582,6 +583,14 @@ private:<br>
/// \brief The start of the most recent function in a builder-type call.<br>
unsigned StartOfFunctionCall;<br>
<br>
+ /// \brief If a nested name specifier was broken over multiple lines, this<br>
+ /// contains the start column of the second line. Otherwise 0.<br>
+ unsigned NestedNameSpecifierContinuation;<br>
+<br>
+ /// \brief If a call expression was broken over multiple lines, this<br>
+ /// contains the start column of the second line. Otherwise 0.<br>
+ unsigned CallContinuation;<br>
+<br>
bool operator<(const ParenState &Other) const {<br>
if (Indent != Other.Indent)<br>
return Indent < Other.Indent;<br>
@@ -603,6 +612,12 @@ private:<br>
return ColonPos < Other.ColonPos;<br>
if (StartOfFunctionCall != Other.StartOfFunctionCall)<br>
return StartOfFunctionCall < Other.StartOfFunctionCall;<br>
+ if (NestedNameSpecifierContinuation !=<br>
+ Other.NestedNameSpecifierContinuation)<br>
+ return NestedNameSpecifierContinuation <<br>
+ Other.NestedNameSpecifierContinuation;<br>
+ if (CallContinuation != Other.CallContinuation)<br>
+ return CallContinuation < Other.CallContinuation;<br>
return false;<br>
}<br>
};<br>
@@ -682,6 +697,8 @@ private:<br>
return 0;<br>
}<br>
<br>
+ unsigned IndentedFurther =<br></blockquote><div><br></div><div style>I'd call it "ContinuationIndent"</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ std::max(State.Stack.back().LastSpace, State.Stack.back().Indent) + 4;<br>
if (Newline) {<br>
unsigned WhitespaceStartColumn = State.Column;<br>
if (Current.is(tok::r_brace)) {<br>
@@ -693,15 +710,16 @@ private:<br>
} else if (Current.is(tok::lessless) &&<br>
State.Stack.back().FirstLessLess != 0) {<br>
State.Column = State.Stack.back().FirstLessLess;<br>
- } else if (State.ParenLevel != 0 &&<br>
- (Previous.isOneOf(tok::equal, tok::coloncolon) ||<br>
- Current.isOneOf(tok::period, tok::arrow, tok::question) ||<br>
- isComparison(Previous))) {<br>
- // Indent and extra 4 spaces after if we know the current expression is<br>
- // continued. Don't do that on the top level, as we already indent 4<br>
- // there.<br>
- State.Column = std::max(State.Stack.back().LastSpace,<br>
- State.Stack.back().Indent) + 4;<br>
+ } else if (Previous.is(tok::coloncolon)) {<br></blockquote><div><br></div><div style>I think this is easy to understand for me as I saw you scribbling on the whiteboard, but I'd add a comment like // Continuation of a broken nested name specifier.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ State.Column = IndentedFurther;<br>
+ if (State.Stack.back().NestedNameSpecifierContinuation == 0)<br>
+ State.Stack.back().NestedNameSpecifierContinuation = State.Column;<br>
+ State.Column = State.Stack.back().NestedNameSpecifierContinuation;<br></blockquote><div><br></div><div style>You really wanted to save lines here :) I'd unfold this into two trivially to understand cases. Then you also don't need the comment, as the first line I hit will say "NestedNameSpecifierContinuation".</div>
<div style> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ } else if (Current.isOneOf(tok::period, tok::arrow)) {<br>
+ State.Column = IndentedFurther;<br>
+ if (State.Stack.back().CallContinuation == 0)<br>
+ State.Stack.back().CallContinuation = State.Column;<br>
+ State.Column = State.Stack.back().CallContinuation; </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
} else if (Current.Type == TT_ConditionalExpr) {<br>
State.Column = State.Stack.back().QuestionColumn;<br>
} else if (Previous.is(tok::comma) && State.VariablePos != 0 &&<br>
@@ -710,7 +728,7 @@ private:<br>
State.Column = State.VariablePos;<br>
} else if (Previous.ClosesTemplateDeclaration ||<br>
(Current.Type == TT_StartOfName && State.ParenLevel == 0)) {<br>
- State.Column = State.Stack.back().Indent - 4;<br>
+ State.Column = State.Stack.back().Indent;<br>
} else if (Current.Type == TT_ObjCSelectorName) {<br>
if (State.Stack.back().ColonPos > Current.FormatTok.TokenLength) {<br>
State.Column =<br>
@@ -720,11 +738,15 @@ private:<br>
State.Stack.back().ColonPos =<br>
State.Column + Current.FormatTok.TokenLength;<br>
}<br>
- } else if (Previous.Type == TT_ObjCMethodExpr ||<br>
- Current.Type == TT_StartOfName) {<br>
- State.Column = State.Stack.back().Indent + 4;<br>
+ } else if (Current.Type == TT_StartOfName || Current.is(tok::question) ||<br>
+ Previous.is(tok::equal) || isComparison(Previous) ||<br>
+ Previous.Type == TT_ObjCMethodExpr) {<br>
+ // Indent and extra 4 spaces if the current expression is continued.<br></blockquote><div><br></div><div style>s/and/an/</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ State.Column = IndentedFurther;<br>
} else {<br>
State.Column = State.Stack.back().Indent;<br>
+ if (State.Column == FirstIndent)<br></blockquote><div><br></div><div style>Can you please explain this in a comment.</div><div style> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ State.Column += 4;<br>
}<br>
<br>
if (Current.is(tok::question))<br>
@@ -845,6 +867,8 @@ private:<br>
State.Stack.back().AvoidBinPacking = true;<br>
State.Stack.back().BreakBeforeParameter = false;<br>
}<br>
+ if (Current.Type == TT_ObjCMethodSpecifier)<br>
+ State.Stack.back().Indent += 4;<br></blockquote><div><br></div><div style>... and this.</div><div style> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
// Insert scopes created by fake parenthesis.<br>
for (unsigned i = 0, e = Current.FakeLParens; i != e; ++i) {<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=178542&r1=178541&r2=178542&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=178542&r1=178541&r2=178542&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)<br>
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Apr 2 09:33:13 2013<br>
@@ -1670,7 +1670,7 @@ TEST_F(FormatTest, FormatsBuilderPattern<br>
" .Default(ORDER_TEXT);\n");<br>
<br>
verifyFormat("return aaaaaaaaaaaaaaaaa->aaaaa().aaaaaaaaaaaaa().aaaaaa() <\n"<br>
- " aaaaaaaaaaaaaaaaaaa->aaaaa().aaaaaaaaaaaaa().aaaaaa();");<br>
+ " aaaaaaaaaaaaaaa->aaaaa().aaaaaaaaaaaaa().aaaaaa();");<br>
verifyFormat(<br>
"aaaaaaa->aaaaaaa\n"<br>
" ->aaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"<br>
@@ -1765,6 +1765,12 @@ TEST_F(FormatTest, AlignsAfterReturn) {<br>
verifyFormat(<br>
"return (aaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaa +\n"<br>
" aaaaaaaaaaaaaaaaaaaaaaaaa);");<br>
+ verifyFormat(<br>
+ "return aaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa >=\n"<br>
+ " aaaaaaaaaaaaaaaaaaaaaa();");<br>
+ verifyFormat(<br>
+ "return (aaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa >=\n"<br>
+ " aaaaaaaaaaaaaaaaaaaaaa());");<br>
}<br>
<br>
TEST_F(FormatTest, BreaksConditionalExpressions) {<br>
@@ -1799,6 +1805,10 @@ TEST_F(FormatTest, BreaksConditionalExpr<br>
verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"<br>
" ? aaaaaaaaaaaaaaaaaaaaaaaaaaa\n"<br>
" : aaaaaaaaaaaaaaaaaaaaaaaaaaa;");<br>
+ verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaa =\n"<br>
+ " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"<br>
+ " ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"<br>
+ " : aaaaaaaaaaaaaaaa;");<br>
verifyFormat(<br>
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa == aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"<br>
" ? aaaaaaaaaaaaaaa\n"<br>
@@ -1837,7 +1847,7 @@ TEST_F(FormatTest, DeclarationsOfMultipl<br>
verifyFormat("bool aaaaaaaaaaaaaaaaaaaaaaaaa =\n"<br>
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa),\n"<br>
" bbbbbbbbbbbbbbbbbbbbbbbbb =\n"<br>
- " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(bbbbbbbbbbbbbbbb);");<br>
+ " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(bbbbbbbbbbbbbbbb);");<br>
verifyFormat(<br>
"bool aaaaaaaaaaaaaaaaaaaaa =\n"<br>
" bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb && cccccccccccccccccccccccccccc,\n"<br>
@@ -1920,6 +1930,10 @@ TEST_F(FormatTest, AlignsPipes) {<br>
" << \"eeeeeeeeeeeeeeeee = \" << eeeeeeeeeeeeeeeee;");<br>
verifyFormat("llvm::outs() << aaaaaaaaaaaaaaaaaaaaaaaa << \"=\"\n"<br>
" << bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb;");<br>
+<br>
+ verifyFormat(<br>
+ "llvm::errs() << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"<br>
+ " .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa();");<br>
}<br>
<br>
TEST_F(FormatTest, UnderstandsEquals) {<br>
@@ -1973,6 +1987,11 @@ TEST_F(FormatTest, WrapsAtFunctionCallsI<br>
" .aaaaaaaaaaaaaaa(\n"<br>
" aa(aaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"<br>
" aaaaaaaaaaaaaaaaaaaaaaaaaaa));");<br>
+ verifyFormat("if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"<br>
+ " .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"<br>
+ " .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"<br>
+ " .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa()) {\n"<br>
+ "}");<br>
<br>
// Here, it is not necessary to wrap at "." or "->".<br>
verifyFormat("if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaa) ||\n"<br>
@@ -2068,6 +2087,10 @@ TEST_F(FormatTest, WrapsAtNestedNameSpec<br>
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"<br>
" aaaaaaaaaaaaaaaaaaaaa);",<br>
getLLVMStyleWithColumns(74));<br>
+<br>
+ verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa::\n"<br>
+ " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"<br>
+ " .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa();");<br>
}<br>
<br>
TEST_F(FormatTest, UnderstandsTemplateParameters) {<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">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><br></div></div>