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(); 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/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: 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"