Skip to content

Commit

Permalink
[clang-format/ObjC] Improve split priorities for ObjC methods
Browse files Browse the repository at this point in the history
Reduce penalty for aligning ObjC method arguments using the colon alignment as
this is the canonical way.

Trying to fit a whole expression into one line should not force other line
breaks (e.g. when ObjC method expression is a part of other expression).

llvm-svn: 336520
  • Loading branch information
jacek1727 committed Jul 9, 2018
1 parent e7c4b59 commit 6b475b7
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 4 deletions.
5 changes: 5 additions & 0 deletions clang/lib/Format/FormatToken.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,11 @@ struct FormatToken {
/// selector consist of.
unsigned ObjCSelectorNameParts = 0;

/// The 0-based index of the parameter/argument. For ObjC it is set
/// for the selector name token.
/// For now calculated only for ObjC.
unsigned ParameterIndex = 0;

/// Stores the number of required fake parentheses and the
/// corresponding operator precedence.
///
Expand Down
18 changes: 16 additions & 2 deletions clang/lib/Format/TokenAnnotator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,8 @@ class AnnotatingParser {
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) {
Expand Down Expand Up @@ -2142,8 +2144,20 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
// 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 the canonical way (still prefer fitting everything
// into one line if possible). Trying to fit a whole expression into one
// line should not force other line breaks (e.g. when ObjC method
// expression is a part of other expression).
Current->SplitPenalty = splitPenalty(Line, *Current, InFunctionDecl);
if (Style.Language == FormatStyle::LK_ObjC &&
Current->is(TT_SelectorName) && Current->ParameterIndex > 0) {
if (Current->ParameterIndex == 1)
Current->SplitPenalty += 5 * Current->BindingStrength;
} else {
Current->SplitPenalty += 20 * Current->BindingStrength;
}

Current = Current->Next;
}
Expand Down
21 changes: 19 additions & 2 deletions clang/unittests/Format/FormatTestObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,18 @@ TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
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(
Expand Down Expand Up @@ -754,11 +766,10 @@ TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
"[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];");
Expand Down Expand Up @@ -803,6 +814,12 @@ TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");

Style.ColumnLimit = 20;
verifyFormat("aaaaa = [a aa:aa\n"
" aa:aa];");
verifyFormat("aaaaaa = [aa aa:aa\n"
" aa:aa];");

Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Expand Down

0 comments on commit 6b475b7

Please sign in to comment.