jolesiak created this revision.
Herald added a subscriber: cfe-commits.
1. Fix counting parameters/arguments for ObjC.
2. Fix split priorities for ObjC methods.
3. Fix breaking after square bracket starting ObjC method expression.
4. Fix putting ObjC method arguments into one line when they fit.
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,16 @@
verifyFormat("[(id)foo bar:(id) ? baz : quux];");
verifyFormat("4 > 4 ? (id)a : (id)baz;");
+ unsigned PreviousColumnLimit = Style.ColumnLimit;
+ Style.ColumnLimit = 50;
+ verifyFormat("bool a = ([object a:42] == 0 ||\n"
+ " [object a:42 b:42] == 0);");
+ // Instead of:
+ // bool a =
+ // ([object a:42] == 0 || [object a:42
+ // b:42] == 0);
+ Style.ColumnLimit = PreviousColumnLimit;
+
// This tests that the formatter doesn't break after "backing" but before ":",
// which would be at 80 columns.
verifyFormat(
@@ -803,6 +813,50 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ // Message receiver taking multiple lines.
+ Style.ColumnLimit = 20;
+ // 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
