<div dir="ltr">On Mon, Oct 7, 2013 at 10:11 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: djasper<br>
Date: Tue Oct 8 00:11:18 2013<br>
New Revision: 192168<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=192168&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=192168&view=rev</a><br>
Log:<br>
clang-format: Improve constructor initializer linewrapping.<br>
<br>
Specifically make ConstructorInitializerAllOnOneLineOrOnePerLine work<br>
nicely with BreakConstructorInitializersBeforeComma.<br>
<br>
This fixes <a href="http://llvm.org/PR17395" target="_blank">llvm.org/PR17395</a>.<br>
<br>
Modified:<br>
cfe/trunk/lib/Format/ContinuationIndenter.cpp<br>
cfe/trunk/lib/Format/TokenAnnotator.cpp<br>
cfe/trunk/unittests/Format/FormatTest.cpp<br>
<br>
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=192168&r1=192167&r2=192168&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=192168&r1=192167&r2=192168&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)<br>
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Tue Oct 8 00:11:18 2013<br>
@@ -44,6 +44,18 @@ static bool startsSegmentOfBuilderTypeCa<br>
return Tok.isMemberAccess() && Tok.Previous && Tok.Previous->closesScope();<br>
}<br>
<br>
+// Returns \c true if \c Current starts a new parameter.<br>
+static bool startsNextParameter(const FormatToken &Current,<br>
+ const FormatStyle &Style) {<br>
+ const FormatToken &Previous = *Current.Previous;<br>
+ if (Current.Type == TT_CtorInitializerComma &&<br>
+ Style.BreakConstructorInitializersBeforeComma)<br>
+ return true;<br>
+ return Previous.is(tok::comma) && !Current.isTrailingComment() &&<br>
+ (Previous.Type != TT_CtorInitializerComma ||<br>
+ !Style.BreakConstructorInitializersBeforeComma);<br></blockquote><div><br></div><div>Isn't that last part redundant, given the first 'if'?</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+}<br>
+<br>
ContinuationIndenter::ContinuationIndenter(const FormatStyle &Style,<br>
SourceManager &SourceMgr,<br>
WhitespaceManager &Whitespaces,<br>
@@ -113,15 +125,9 @@ bool ContinuationIndenter::mustBreak(con<br>
return true;<br>
if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)<br>
return true;<br>
- if (Style.BreakConstructorInitializersBeforeComma) {<br>
- if (Previous.Type == TT_CtorInitializerComma)<br>
- return false;<br>
- if (Current.Type == TT_CtorInitializerComma)<br>
- return true;<br>
- }<br>
- if ((Previous.isOneOf(tok::comma, tok::semi) || Current.is(tok::question) ||<br>
- (Current.Type == TT_ConditionalExpr &&<br>
- !(Current.is(tok::colon) && Previous.is(tok::question)))) &&<br>
+ if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) ||<br>
+ Current.is(tok::question) ||<br>
+ (Current.Type == TT_ConditionalExpr && Previous.isNot(tok::question))) &&<br>
State.Stack.back().BreakBeforeParameter && !Current.isTrailingComment() &&<br>
!Current.isOneOf(tok::r_paren, tok::r_brace))<br>
return true;<br>
@@ -250,8 +256,7 @@ void ContinuationIndenter::addTokenOnCur<br>
if (Previous.opensScope() && Previous.Type != TT_ObjCMethodExpr &&<br>
Current.Type != TT_LineComment)<br>
State.Stack.back().Indent = State.Column + Spaces;<br>
- if (Previous.is(tok::comma) && !Current.isTrailingComment() &&<br>
- State.Stack.back().AvoidBinPacking)<br>
+ if (State.Stack.back().AvoidBinPacking && startsNextParameter(Current, Style))<br>
State.Stack.back().NoLineBreak = true;<br>
if (startsSegmentOfBuilderTypeCall(Current))<br>
State.Stack.back().ContainsUnwrappedBuilder = true;<br>
<br>
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=192168&r1=192167&r2=192168&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=192168&r1=192167&r2=192168&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)<br>
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Oct 8 00:11:18 2013<br>
@@ -1366,7 +1366,8 @@ bool TokenAnnotator::mustBreakBefore(con<br>
// FIXME: Fix horrible hack of using BindingStrength to find top-level <>.<br>
return true;<br>
} else if (Right.Type == TT_CtorInitializerComma &&<br>
- Style.BreakConstructorInitializersBeforeComma) {<br>
+ Style.BreakConstructorInitializersBeforeComma &&<br>
+ !Style.ConstructorInitializerAllOnOneLineOrOnePerLine) {<br>
return true;<br>
} else if (Right.Previous->BlockKind == BK_Block &&<br>
Right.Previous->isNot(tok::r_brace) &&<br>
@@ -1450,6 +1451,9 @@ bool TokenAnnotator::canBreakBefore(cons<br>
if (Left.Type == TT_CtorInitializerComma &&<br>
Style.BreakConstructorInitializersBeforeComma)<br>
return false;<br>
+ if (Right.Type == TT_CtorInitializerComma &&<br>
+ Style.BreakConstructorInitializersBeforeComma)<br>
+ return true;<br>
if (Right.isBinaryOperator() && Style.BreakBeforeBinaryOperators)<br>
return true;<br>
if (Left.is(tok::greater) && Right.is(tok::greater) &&<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=192168&r1=192167&r2=192168&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=192168&r1=192167&r2=192168&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)<br>
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Oct 8 00:11:18 2013<br>
@@ -6587,6 +6587,20 @@ TEST_F(FormatTest, ConstructorInitialize<br>
", b(b)\n"<br>
", c(c) {}",<br>
Style);<br>
+<br>
+ Style.ConstructorInitializerAllOnOneLineOrOnePerLine = true;<br>
+ Style.ConstructorInitializerIndentWidth = 4;<br>
+ verifyFormat(<br>
+ "SomeClass::Constructor()\n"<br>
+ " : aaaaaaaa(aaaaaaaa), aaaaaaaa(aaaaaaaa), aaaaaaaa(aaaaaaaa) {}",<br>
+ Style);<br>
+ Style.ConstructorInitializerIndentWidth = 4;<br>
+ Style.ColumnLimit = 60;<br>
+ verifyFormat("SomeClass::Constructor()\n"<br>
+ " : aaaaaaaa(aaaaaaaa)\n"<br>
+ " , aaaaaaaa(aaaaaaaa)\n"<br>
+ " , aaaaaaaa(aaaaaaaa) {}",<br>
+ Style);<br>
}<br>
<br>
TEST_F(FormatTest, FormatsWithWebKitStyle) {<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>