jolesiak updated this revision to Diff 152043.
jolesiak added a comment.
- Add test
Repository:
rC Clang
https://reviews.llvm.org/D48352
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===================================================================
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -678,6 +678,18 @@
verifyFormat("[(id)foo bar:(id) ? baz : quux];");
verifyFormat("4 > 4 ? (id)a : (id)baz;");
+ unsigned PreviousColumnLimit = Style.ColumnLimit;
+ Style.ColumnLimit = 50;
+ // Instead of:
+ // bool a =
+ // ([object a:42] == 0 || [object a:42
+ // b:42] == 0);
+ verifyFormat("bool a = ([object a:42] == 0 ||\n"
+ " [object a:42 b:42] == 0);");
+ Style.ColumnLimit = PreviousColumnLimit;
+ verifyFormat("bool a = ([aaaaaaaa aaaaa] == aaaaaaaaaaaaaaaaa ||\n"
+ " [aaaaaaaa aaaaa] == aaaaaaaaaaaaaaaaaaaa);");
+
// This tests that the formatter doesn't break after "backing" but before ":",
// which would be at 80 columns.
verifyFormat(
@@ -754,11 +766,10 @@
"[self aaaaaaaaaaaaa:aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa,\n"
" aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa,\n"
" aaaaaaaaaaaaaaa, aaaaaaaaaaaaaaa];");
+
verifyFormat("[self // break\n"
" a:a\n"
" aaa:aaa];");
- verifyFormat("bool a = ([aaaaaaaa aaaaa] == aaaaaaaaaaaaaaaaa ||\n"
- " [aaaaaaaa aaaaa] == aaaaaaaaaaaaaaaaaaaa);");
// Formats pair-parameters.
verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
@@ -803,6 +814,53 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ Style.ColumnLimit = 20;
+ verifyFormat("aaaaa = [a aa:aa\n"
+ " aa:aa];");
+
+ // Message receiver taking multiple lines.
+ // Non-corner case.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] a:42 b:42];");
+ // Arguments just fit into one line.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaaaaaa:42 b:42];");
+ // Arguments just over a column limit.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaaaaaa:42\n"
+ " bb:42];");
+ // Arguments just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ " b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ " b:42\n"
+ " c:42];");
+ verifyFormat("[[obj aaaaaa:42\n"
+ " b:42]\n"
+ " cc:42\n"
+ " d:42];");
+
+ // Avoid breaking receiver expression.
+ Style.ColumnLimit = 30;
+ verifyFormat("fooooooo =\n"
+ " [[obj fooo] aaa:42\n"
+ " aaa:42];");
+ verifyFormat("[[[obj foo] bar] aa:42\n"
+ " bb:42\n"
+ " cc:42];");
+
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -515,11 +515,24 @@
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
+ // FirstObjCSelectorName is set when a colon is found. This does
+ // not work, however, when method has no parameters.
+ // Here, we set FirstObjCSelectorName when the end of the expression is
+ // reached, in case it was not set already.
+ if (!Contexts.back().FirstObjCSelectorName) {
+ FormatToken* Previous = CurrentToken->getPreviousNonComment();
+ if (Previous && Previous->is(TT_SelectorName)) {
+ Previous->ObjCSelectorNameParts = 1;
+ Contexts.back().FirstObjCSelectorName = Previous;
+ }
+ } else {
+ Left->ParameterCount =
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
+ }
+
if (Contexts.back().FirstObjCSelectorName) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
- Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
- Left->ParameterCount;
if (Left->BlockParameterCount > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
@@ -539,11 +552,6 @@
TT_DesignatedInitializerLSquare)) {
Left->Type = TT_ObjCMethodExpr;
StartsObjCMethodExpr = true;
- // ParameterCount might have been set to 1 before expression was
- // recognized as ObjCMethodExpr (as '1 + number of commas' formula is
- // used for other expression types). Parameter counter has to be,
- // therefore, reset to 0.
- Left->ParameterCount = 0;
Contexts.back().ColonIsObjCMethodExpr = true;
if (Parent && Parent->is(tok::r_paren))
// FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
@@ -617,12 +625,12 @@
}
void updateParameterCount(FormatToken *Left, FormatToken *Current) {
+ // For ObjC methods number of parameters is calculated differently as
+ // method declarations have a different structure (parameters are not inside
+ // parenthesis scope).
if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
++Left->BlockParameterCount;
- if (Left->Type == TT_ObjCMethodExpr) {
- if (Current->is(tok::colon))
- ++Left->ParameterCount;
- } else if (Current->is(tok::comma)) {
+ if (Current->is(tok::comma)) {
++Left->ParameterCount;
if (!Left->Role)
Left->Role.reset(new CommaSeparatedList(Style));
@@ -712,6 +720,10 @@
Contexts.back().LongestObjCSelectorName)
Contexts.back().LongestObjCSelectorName =
Tok->Previous->ColumnWidth;
+
+ Tok->Previous->ParameterIndex =
+ Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
+ ++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
}
} else if (Contexts.back().ColonIsForRangeExpr) {
Tok->Type = TT_RangeBasedForLoopColon;
@@ -2125,11 +2137,26 @@
if (Current->is(TT_CtorInitializerColon))
InFunctionDecl = false;
- // FIXME: Only calculate this if CanBreakBefore is true once static
- // initializers etc. are sorted out.
- // FIXME: Move magic numbers to a better place.
- Current->SplitPenalty = 20 * Current->BindingStrength +
- splitPenalty(Line, *Current, InFunctionDecl);
+ // Reduce penalty for aligning ObjC method arguments using the colon
+ // alignment as this is a canonical way (still prefer fitting everything
+ // into one line if possible) and trying to fit a whole epression into one
+ // line should not force other line breaks (e.g. when ObjC method
+ // expression is a part of other expression).
+ if (Style.Language == FormatStyle::LK_ObjC &&
+ Current->is(TT_SelectorName) && Current->ParameterIndex > 0) {
+ if (Current->ParameterIndex == 1) {
+ Current->SplitPenalty = 5 * Current->BindingStrength +
+ splitPenalty(Line, *Current, InFunctionDecl);
+ } else {
+ Current->SplitPenalty = splitPenalty(Line, *Current, InFunctionDecl);
+ }
+ } else {
+ // FIXME: Only calculate this if CanBreakBefore is true once static
+ // initializers etc. are sorted out.
+ // FIXME: Move magic numbers to a better place.
+ Current->SplitPenalty = 20 * Current->BindingStrength +
+ splitPenalty(Line, *Current, InFunctionDecl);
+ }
Current = Current->Next;
}
Index: lib/Format/FormatToken.h
===================================================================
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -243,10 +243,16 @@
/// e.g. because several of them are block-type.
unsigned LongestObjCSelectorName = 0;
- /// How many parts ObjC selector have (i.e. how many parameters method
- /// has).
+ /// If this is the first ObjC selector name in an ObjC method
+ /// definition or call, this contains the number of parts that whole selector
+ /// consist of.
unsigned ObjCSelectorNameParts = 0;
+ /// What is the 0-based index of the parameter/argument. For ObjC it is set
+ /// for the selector name.
+ /// For now calculated only for ObjC.
+ unsigned ParameterIndex = 0;
+
/// Stores the number of required fake parentheses and the
/// corresponding operator precedence.
///
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -321,6 +321,9 @@
State.Stack.back().NoLineBreakInOperand)
return false;
+ if (Previous.is(tok::l_square) && Previous.is(TT_ObjCMethodExpr))
+ return false;
+
return !State.Stack.back().NoLineBreak;
}
@@ -1394,6 +1397,29 @@
(Current.is(tok::greater) && Current.is(TT_DictLiteral))))
State.Stack.pop_back();
+ // Reevaluate whether ObjC message arguments fit into one line.
+ // If a receiver spans multiple lines, e.g.:
+ // [[object block:^{
+ // return 42;
+ // }] a:42 b:42];
+ // BreakBeforeParameter is calculated based on an incorrect assumption
+ // (it is checked whether the whole expression fits into one line without
+ // considering a line break inside a message receiver).
+ // We check whether arguements fit after receiver scope closer (into the same
+ // line).
+ if (Current.MatchingParen && Current.MatchingParen->Previous) {
+ const FormatToken &CurrentScopeOpener = *Current.MatchingParen->Previous;
+ if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+ CurrentScopeOpener.MatchingParen) {
+ int NecessarySpaceInLine =
+ getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+ CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+ if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+ Style.ColumnLimit)
+ State.Stack.back().BreakBeforeParameter = false;
+ }
+ }
+
if (Current.is(tok::r_square)) {
// If this ends the array subscript expr, reset the corresponding value.
const FormatToken *NextNonComment = Current.getNextNonComment();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
