Skip to content

Commit 503d2d8

Browse files
committed
[CSSolver] Use correct locator when matching function result types related to closures
Currently we always use 'FunctionResult' as a path element when matching function result types, but closure result type is allowed to be implicitly converted to Void, which means we need to be careful when to use 'FunctionResult' and 'ClosureResult'. Resolves: rdar://problem/37790062 (cherry picked from commit 20019be)
1 parent 2d0d2e9 commit 503d2d8

File tree

5 files changed

+111
-11
lines changed

5 files changed

+111
-11
lines changed

lib/Sema/CSRanking.cpp

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,14 +1027,32 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
10271027
// Compare the type variable bindings.
10281028
auto &tc = cs.getTypeChecker();
10291029
for (auto &binding : diff.typeBindings) {
1030+
auto type1 = binding.bindings[idx1];
1031+
auto type2 = binding.bindings[idx2];
1032+
1033+
auto &impl = binding.typeVar->getImpl();
1034+
1035+
if (auto *locator = impl.getLocator()) {
1036+
auto path = locator->getPath();
1037+
if (!path.empty() &&
1038+
path.back().getKind() == ConstraintLocator::ClosureResult) {
1039+
// Since we support `() -> T` to `() -> Void` and
1040+
// `() -> Never` to `() -> T` conversions, it's always
1041+
// preferable to pick `T` rather than `Never` with
1042+
// all else being equal.
1043+
if (type2->isUninhabited())
1044+
++score1;
1045+
1046+
if (type1->isUninhabited())
1047+
++score2;
1048+
}
1049+
}
1050+
10301051
// If the type variable isn't one for which we should be looking at the
10311052
// bindings, don't.
1032-
if (!binding.typeVar->getImpl().prefersSubtypeBinding())
1053+
if (!impl.prefersSubtypeBinding())
10331054
continue;
10341055

1035-
auto type1 = binding.bindings[idx1];
1036-
auto type2 = binding.bindings[idx2];
1037-
10381056
// If the types are equivalent, there's nothing more to do.
10391057
if (type1->isEqual(type2))
10401058
continue;

lib/Sema/CSSimplify.cpp

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,10 +1235,8 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
12351235
}
12361236

12371237
// Result type can be covariant (or equal).
1238-
return matchTypes(func1->getResult(), func2->getResult(), subKind,
1239-
subflags,
1240-
locator.withPathElement(
1241-
ConstraintLocator::FunctionResult));
1238+
return matchTypes(func1->getResult(), func2->getResult(), subKind, subflags,
1239+
locator.withPathElement(ConstraintLocator::FunctionResult));
12421240
}
12431241

12441242
ConstraintSystem::SolutionKind
@@ -1564,6 +1562,8 @@ ConstraintSystem::SolutionKind
15641562
ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
15651563
TypeMatchOptions flags,
15661564
ConstraintLocatorBuilder locator) {
1565+
auto origType1 = type1;
1566+
15671567
bool isArgumentTupleConversion
15681568
= kind == ConstraintKind::ArgumentTupleConversion ||
15691569
kind == ConstraintKind::OperatorArgumentTupleConversion;
@@ -2324,7 +2324,32 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
23242324
// Allow '() -> T' to '() -> ()' and '() -> Never' to '() -> T' for closure
23252325
// literals.
23262326
if (auto elt = locator.last()) {
2327-
if (elt->getKind() == ConstraintLocator::ClosureResult) {
2327+
auto isClosureResult = [&]() {
2328+
if (elt->getKind() == ConstraintLocator::ClosureResult)
2329+
return true;
2330+
2331+
// If constraint is matching function results where
2332+
// left-hand side is a 'closure result' we need to allow
2333+
// certain implicit conversions.
2334+
if (elt->getKind() != ConstraintLocator::FunctionResult)
2335+
return false;
2336+
2337+
if (auto *typeVar = origType1->getAs<TypeVariableType>()) {
2338+
auto *locator = typeVar->getImpl().getLocator();
2339+
if (!locator)
2340+
return false;
2341+
2342+
auto path = locator->getPath();
2343+
if (path.empty())
2344+
return false;
2345+
2346+
return path.back().getKind() == ConstraintLocator::ClosureResult;
2347+
}
2348+
2349+
return false;
2350+
};
2351+
2352+
if (isClosureResult()) {
23282353
if (concrete && kind >= ConstraintKind::Subtype &&
23292354
(type1->isUninhabited() || type2->isVoid())) {
23302355
increaseScore(SK_FunctionConversion);

test/Constraints/closures.swift

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,6 @@ func rdar33429010_2() {
630630
let iter = I_33429010()
631631
var acc: Int = 0 // expected-warning {{}}
632632
let _: Int = AnySequence { iter }.rdar33429010(into: acc, { $0 + $1 })
633-
// expected-warning@-1 {{result of operator '+' is unused}}
634633
let _: Int = AnySequence { iter }.rdar33429010(into: acc, { $0.rdar33429010_incr($1) })
635634
}
636635

@@ -652,3 +651,16 @@ func rdar36054961() {
652651
str.replaceSubrange(range, with: str[range].reversed())
653652
}])
654653
}
654+
655+
func rdar37790062() {
656+
struct S<T> {
657+
init(_ a: () -> T, _ b: () -> T) {}
658+
}
659+
660+
func foo() -> Int { return 42 }
661+
func bar() -> Void {}
662+
func baz() -> (String, Int) { return ("question", 42) }
663+
664+
_ = S({ foo() }, { bar() }) // Ok, should infer T to be 'Void'
665+
_ = S({ baz() }, { bar() }) // Ok, should infer T to be 'Void'
666+
}

test/Constraints/rdar37790062.swift

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
protocol A {
4+
associatedtype V
5+
associatedtype E: Error
6+
7+
init(value: V)
8+
init(error: E)
9+
10+
func foo<U>(value: (V) -> U, error: (E) -> U) -> U
11+
}
12+
13+
enum R<T, E: Error> : A {
14+
case foo(T)
15+
case bar(E)
16+
17+
init(value: T) { self = .foo(value) }
18+
init(error: E) { self = .bar(error) }
19+
20+
func foo<R>(value: (T) -> R, error: (E) -> R) -> R {
21+
fatalError()
22+
}
23+
}
24+
25+
protocol P {
26+
associatedtype V
27+
28+
@discardableResult
29+
func baz(callback: @escaping (V) -> Void) -> Self
30+
}
31+
32+
class C<V> : P {
33+
func baz(callback: @escaping (V) -> Void) -> Self { return self }
34+
}
35+
class D<T, E: Error> : C<R<T, E>> {
36+
init(fn: (_ ret: @escaping (V) -> Void) -> Void) {}
37+
}
38+
39+
extension A where V: P, V.V: A, E == V.V.E {
40+
func bar() -> D<V.V.V, V.V.E> {
41+
return D { complete in
42+
foo(value: { promise in promise.baz { result in } },
43+
error: { complete(R(error: $0)) })
44+
}
45+
}
46+
}

test/expr/closure/closures.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,6 @@ func rdar19179412() -> (Int) -> Int {
259259
func takesVoidFunc(_ f: () -> ()) {}
260260
var i: Int = 1
261261

262-
// expected-warning @+1 {{expression of type 'Int' is unused}}
263262
takesVoidFunc({i})
264263
// expected-warning @+1 {{expression of type 'Int' is unused}}
265264
var f1: () -> () = {i}

0 commit comments

Comments
 (0)