Skip to content

Commit 42bb252

Browse files
committed
[Overload resolution] Prefer functions with fewer defaulted/variadic arguments.
When comparing two functions for overload resolution, break apart the parameter lists to compare individual parameters rather than comparing the tuples. This allows us to prefer functions with fewer arguments to ones with more, defaulted or variadic arguments. That preference was already encoded in the constraint optimizer, which led to some strange behavior where the preference was expressed for function calls but not for calls to initializers. Fixes rdar://problem/24128153. The standard library change tweaks the anachronistic, unavailable "print" variants somewhat. The only behavior change here is a slight regression for cases like: print(a: 1, b: 2) where we used to produce a diagnostic: Please wrap your tuple argument in parentheses: 'print((...))' but we now get: argument labels '(a:, b:)' do not match any available overloads However, this regression will happen at some point *anyway*, if SE-0029 (or anything else that removes the implicit tuple splat operation) goes through.
1 parent 151dd3b commit 42bb252

File tree

6 files changed

+83
-51
lines changed

6 files changed

+83
-51
lines changed

lib/Sema/CSRanking.cpp

Lines changed: 44 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -469,19 +469,6 @@ static bool isProtocolExtensionAsSpecializedAs(TypeChecker &tc,
469469
return cs.solveSingle().hasValue();
470470
}
471471

472-
/// Count the number of default arguments in the primary clause.
473-
static unsigned countDefaultArguments(AbstractFunctionDecl *func) {
474-
auto paramList = func->getParameterList(func->getImplicitSelfDecl() != 0);
475-
476-
unsigned count = 0;
477-
for (auto elt : *paramList) {
478-
if (elt->isDefaultArgument())
479-
++count;
480-
}
481-
482-
return count;
483-
}
484-
485472
/// \brief Determine whether the first declaration is as "specialized" as
486473
/// the second declaration.
487474
///
@@ -646,6 +633,7 @@ static bool isDeclAsSpecializedAs(TypeChecker &tc, DeclContext *dc,
646633
break;
647634
}
648635

636+
bool fewerEffectiveParameters = false;
649637
switch (checkKind) {
650638
case CheckAll:
651639
// Check whether the first type is a subtype of the second.
@@ -660,40 +648,60 @@ static bool isDeclAsSpecializedAs(TypeChecker &tc, DeclContext *dc,
660648
// second type's inputs, i.e., can we forward the arguments?
661649
auto funcTy1 = openedType1->castTo<FunctionType>();
662650
auto funcTy2 = openedType2->castTo<FunctionType>();
663-
cs.addConstraint(ConstraintKind::Subtype,
664-
funcTy1->getInput(),
665-
funcTy2->getInput(),
666-
locator);
651+
SmallVector<CallArgParam, 4> params1 =
652+
decomposeArgParamType(funcTy1->getInput());
653+
SmallVector<CallArgParam, 4> params2 =
654+
decomposeArgParamType(funcTy2->getInput());
655+
656+
unsigned numParams1 = params1.size();
657+
unsigned numParams2 = params2.size();
658+
if (numParams1 > numParams2) return false;
659+
660+
for (unsigned i = 0; i != numParams2; ++i) {
661+
// If there is no corresponding argument in the first
662+
// parameter list...
663+
if (i >= numParams1) {
664+
// We need either a default argument or a variadic
665+
// argument for the first declaration to be more
666+
// specialized.
667+
if (!params2[i].HasDefaultArgument && !params2[i].Variadic)
668+
return false;
669+
670+
fewerEffectiveParameters = true;
671+
continue;
672+
}
673+
674+
// Labels must match.
675+
if (params1[i].Label != params2[i].Label) return false;
676+
677+
// If one parameter is variadic and the other is not...
678+
if (params1[i].Variadic != params2[i].Variadic) {
679+
// If the first parameter is the variadic one, it's not
680+
// more specialized.
681+
if (params1[i].Variadic) return false;
682+
683+
fewerEffectiveParameters = true;
684+
}
685+
686+
// Check whether the first parameter is a subtype of the second.
687+
cs.addConstraint(ConstraintKind::Subtype,
688+
params1[i].Ty, params2[i].Ty, locator);
689+
}
690+
667691
break;
668692
}
669693
}
670694

671695
// Solve the system.
672696
auto solution = cs.solveSingle(FreeTypeVariableBinding::Allow);
673697

674-
675698
// Ban value-to-optional conversions.
676699
if (solution && solution->getFixedScore().Data[SK_ValueToOptional] == 0)
677700
return true;
678701

679-
// Between two imported functions/methods, prefer one with fewer
680-
// defaulted arguments.
681-
//
682-
// FIXME: This is a total hack; we should be comparing based on the
683-
// number of default arguments at were actually used. It's only safe to
684-
// do this because the count will be zero except when under the
685-
// experimental InferDefaultArguments mode of the Clang importer.
686-
if (isa<AbstractFunctionDecl>(decl1) &&
687-
isa<AbstractFunctionDecl>(decl2) &&
688-
decl1->getClangDecl() &&
689-
decl2->getClangDecl()) {
690-
unsigned defArgsIn1
691-
= countDefaultArguments(cast<AbstractFunctionDecl>(decl1));
692-
unsigned defArgsIn2
693-
= countDefaultArguments(cast<AbstractFunctionDecl>(decl2));
694-
if (defArgsIn1 < defArgsIn2)
695-
return true;
696-
}
702+
// If the first function has fewer effective parameters than the
703+
// second, it is more specialized.
704+
if (fewerEffectiveParameters) return true;
697705

698706
return false;
699707
};

stdlib/public/core/Print.swift

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -154,15 +154,10 @@ internal func _debugPrint<Target: OutputStreamType>(
154154
//===----------------------------------------------------------------------===//
155155
//===--- Migration Aids ---------------------------------------------------===//
156156

157-
@available(*, unavailable, message="Please wrap your tuple argument in parentheses: 'print((...))'")
158-
public func print<T>(_: T) {}
159-
@available(*, unavailable, message="Please wrap your tuple argument in parentheses: 'debugPrint((...))'")
160-
public func debugPrint<T>(_: T) {}
161-
162157
@available(*, unavailable, message="Please use 'terminator: \"\"' instead of 'appendNewline: false': 'print((...), terminator: \"\")'")
163-
public func print<T>(_: T, appendNewline: Bool) {}
158+
public func print<T>(_: T, appendNewline: Bool = true) {}
164159
@available(*, unavailable, message="Please use 'terminator: \"\"' instead of 'appendNewline: false': 'debugPrint((...), terminator: \"\")'")
165-
public func debugPrint<T>(_: T, appendNewline: Bool) {}
160+
public func debugPrint<T>(_: T, appendNewline: Bool = true) {}
166161

167162

168163
//===--- FIXME: Not working due to <rdar://22101775> ----------------------===//
@@ -172,10 +167,10 @@ public func print<T>(_: T, inout _: OutputStreamType) {}
172167
public func debugPrint<T>(_: T, inout _: OutputStreamType) {}
173168

174169
@available(*, unavailable, message="Please use 'terminator: \"\"' instead of 'appendNewline: false' and use the 'toStream' label for the target stream: 'print((...), terminator: \"\", toStream: &...)'")
175-
public func print<T>(_: T, inout _: OutputStreamType, appendNewline: Bool) {}
170+
public func print<T>(_: T, inout _: OutputStreamType, appendNewline: Bool = true) {}
176171
@available(*, unavailable, message="Please use 'terminator: \"\"' instead of 'appendNewline: false' and use the 'toStream' label for the target stream: 'debugPrint((...), terminator: \"\", toStream: &...)'")
177172
public func debugPrint<T>(
178-
_: T, inout _: OutputStreamType, appendNewline: Bool
173+
_: T, inout _: OutputStreamType, appendNewline: Bool = true
179174
) {}
180175
//===----------------------------------------------------------------------===//
181176
//===----------------------------------------------------------------------===//

test/1_stdlib/PrintDiagnostics.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ print(3, &stream) // expected-error{{'&' used with non-inout argument of type 'A
99
debugPrint(3, &stream) // expected-error{{'&' used with non-inout argument of type 'Any'}}
1010
print(3, &stream, appendNewline: false) // expected-error {{cannot pass immutable value as inout argument: implicit conversion from 'String' to 'OutputStreamType' requires a temporary}}
1111
debugPrint(3, &stream, appendNewline: false) // expected-error {{cannot pass immutable value as inout argument: implicit conversion from 'String' to 'OutputStreamType' requires a temporary}}
12-
print(4, quack: 5) // expected-error {{'print' is unavailable: Please wrap your tuple argument in parentheses: 'print((...))'}}
12+
print(4, quack: 5) // expected-error {{argument labels '(_:, quack:)' do not match any available overloads}}
13+
// expected-note@-1{{overloads for 'print' exist with these partially matching parameter lists: (Any..., separator: String, terminator: String), (T, appendNewline: Bool), (T, inout OutputStreamType), (T, inout OutputStreamType, appendNewline: Bool)}}

test/Constraints/diagnostics.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -282,9 +282,10 @@ func rdar21784170() {
282282
}
283283

284284
// <rdar://problem/21829141> BOGUS: unexpected trailing closure
285-
func expect<T, U>(_: T) -> (U.Type) -> () { return { ty in () } } // expected-note{{found this candidate}}
286-
func expect<T, U>(_: T, _: Int = 1) -> (U.Type) -> () { return { ty in () } } // expected-note{{found this candidate}}
287-
expect(Optional(3))(Optional<Int>.self) // expected-error{{ambiguous use of 'expect'}}
285+
func expect<T, U>(_: T) -> (U.Type) -> Int { return { a in 0 } }
286+
func expect<T, U>(_: T, _: Int = 1) -> (U.Type) -> String { return { a in "String" } }
287+
let expectType1 = expect(Optional(3))(Optional<Int>.self)
288+
let expectType1Check: Int = expectType1
288289

289290
// <rdar://problem/19804707> Swift Enum Scoping Oddity
290291
func rdar19804707() {

test/Constraints/function.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,7 @@ class A {
6767
func a(text:String, something:Int?=nil) {
6868
}
6969
}
70-
A().a(text:"sometext") // expected-error {{argument labels '(text:)' do not match any available overloads}}
71-
// expected-note @-1 {{overloads for 'a' exist with these partially matching parameter lists: (String), (String, something: Int?)}}
70+
A().a(text:"sometext") // expected-error{{extraneous argument label 'text:' in call}}{{7-12=}}
7271

7372

7473
// <rdar://problem/22451001> QoI: incorrect diagnostic when argument to print has the wrong type

test/Constraints/overload.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,3 +133,31 @@ func test_contextual_result() {
133133
return overloaded_identity() // expected-error {{no 'overloaded_identity' candidates produce the expected contextual result type '()'}}
134134
// expected-note @-1 {{overloads for 'overloaded_identity' exist with these result types: Int, Float}}
135135
}
136+
137+
// rdar://problem/24128153
138+
struct X0 {
139+
init(_ i: Any.Type) { }
140+
init?(_ i: Any.Type, _ names: String...) { }
141+
}
142+
143+
let x0 = X0(Int.self)
144+
let x0check: X0 = x0 // okay: chooses first initializer
145+
146+
struct X1 {
147+
init?(_ i: Any.Type) { }
148+
init(_ i: Any.Type, _ names: String...) { }
149+
}
150+
151+
let x1 = X1(Int.self)
152+
let x1check: X1 = x1 // expected-error{{value of optional type 'X1?' not unwrapped; did you mean to use '!' or '?'?}}
153+
154+
155+
struct X2 {
156+
init?(_ i: Any.Type) { }
157+
init(_ i: Any.Type, a: Int = 0) { }
158+
init(_ i: Any.Type, a: Int = 0, b: Int = 0) { }
159+
init(_ i: Any.Type, a: Int = 0, c: Int = 0) { }
160+
}
161+
162+
let x2 = X2(Int.self)
163+
let x2check: X2 = x2 // expected-error{{value of optional type 'X2?' not unwrapped; did you mean to use '!' or '?'?}}

0 commit comments

Comments
 (0)