[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

Author: xedin

lib/Sema/CSRanking.cpp

@@ -1007,14 +1007,32 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
   // Compare the type variable bindings.
   auto &tc = cs.getTypeChecker();
   for (auto &binding : diff.typeBindings) {
+    auto type1 = binding.bindings[idx1];
+    auto type2 = binding.bindings[idx2];
+
+    auto &impl = binding.typeVar->getImpl();
+
+    if (auto *locator = impl.getLocator()) {
+      auto path = locator->getPath();
+      if (!path.empty() &&
+          path.back().getKind() == ConstraintLocator::ClosureResult) {
+        // Since we support `() -> T` to `() -> Void` and
+        // `() -> Never` to `() -> T` conversions, it's always
+        // preferable to pick `T` rather than `Never` with
+        // all else being equal.
+        if (type2->isUninhabited())
+          ++score1;
+
+        if (type1->isUninhabited())
+          ++score2;
+      }
+    }
+
     // If the type variable isn't one for which we should be looking at the
     // bindings, don't.
-    if (!binding.typeVar->getImpl().prefersSubtypeBinding())
+    if (!impl.prefersSubtypeBinding())
       continue;
 
-    auto type1 = binding.bindings[idx1];
-    auto type2 = binding.bindings[idx2];
-
     // If the types are equivalent, there's nothing more to do.
     if (type1->isEqual(type2))
       continue;

lib/Sema/CSSimplify.cpp

@@ -1212,10 +1212,8 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
     return result;
 
   // Result type can be covariant (or equal).
-  return matchTypes(func1->getResult(), func2->getResult(), subKind,
-                     subflags,
-                     locator.withPathElement(
-                       ConstraintLocator::FunctionResult));
+  return matchTypes(func1->getResult(), func2->getResult(), subKind, subflags,
+                    locator.withPathElement(ConstraintLocator::FunctionResult));
 }
 
 ConstraintSystem::TypeMatchResult
@@ -1501,6 +1499,8 @@ ConstraintSystem::TypeMatchResult
 ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
                              TypeMatchOptions flags,
                              ConstraintLocatorBuilder locator) {
+  auto origType1 = type1;
+
   bool isArgumentTupleConversion
           = kind == ConstraintKind::ArgumentTupleConversion ||
             kind == ConstraintKind::OperatorArgumentTupleConversion;
@@ -2243,7 +2243,32 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
   // Allow '() -> T' to '() -> ()' and '() -> Never' to '() -> T' for closure
   // literals.
   if (auto elt = locator.last()) {
-    if (elt->getKind() == ConstraintLocator::ClosureResult) {
+    auto isClosureResult = [&]() {
+      if (elt->getKind() == ConstraintLocator::ClosureResult)
+        return true;
+
+      // If constraint is matching function results where
+      // left-hand side is a 'closure result' we need to allow
+      // certain implicit conversions.
+      if (elt->getKind() != ConstraintLocator::FunctionResult)
+        return false;
+
+      if (auto *typeVar = origType1->getAs<TypeVariableType>()) {
+        auto *locator = typeVar->getImpl().getLocator();
+        if (!locator)
+          return false;
+
+        auto path = locator->getPath();
+        if (path.empty())
+          return false;
+
+        return path.back().getKind() == ConstraintLocator::ClosureResult;
+      }
+
+      return false;
+    };
+
+    if (isClosureResult()) {
       if (concrete && kind >= ConstraintKind::Subtype &&
           (type1->isUninhabited() || type2->isVoid())) {
         increaseScore(SK_FunctionConversion);

test/Constraints/closures.swift

@@ -630,7 +630,6 @@ func rdar33429010_2() {
   let iter = I_33429010()
   var acc: Int = 0 // expected-warning {{}}
   let _: Int = AnySequence { iter }.rdar33429010(into: acc, { $0 + $1 })
-  // expected-warning@-1 {{result of operator '+' is unused}}
   let _: Int = AnySequence { iter }.rdar33429010(into: acc, { $0.rdar33429010_incr($1) })
 }
 
@@ -652,3 +651,16 @@ func rdar36054961() {
      str.replaceSubrange(range, with: str[range].reversed())
   }])
 }
+
+func rdar37790062() {
+  struct S<T> {
+    init(_ a: () -> T, _ b: () -> T) {}
+  }
+
+  func foo() -> Int { return 42 }
+  func bar() -> Void {}
+  func baz() -> (String, Int) { return ("question", 42) }
+
+  _ = S({ foo() }, { bar() }) // Ok, should infer T to be 'Void'
+  _ = S({ baz() }, { bar() }) // Ok, should infer T to be 'Void'
+}

test/Constraints/rdar37790062.swift

@@ -0,0 +1,46 @@
+// RUN: %target-typecheck-verify-swift
+
+protocol A {
+  associatedtype V
+  associatedtype E: Error
+
+  init(value: V)
+  init(error: E)
+
+  func foo<U>(value: (V) -> U, error: (E) -> U) -> U
+}
+
+enum R<T, E: Error> : A {
+  case foo(T)
+  case bar(E)
+
+  init(value: T) { self = .foo(value) }
+  init(error: E) { self = .bar(error) }
+
+  func foo<R>(value: (T) -> R, error: (E) -> R) -> R {
+    fatalError()
+  }
+}
+
+protocol P {
+  associatedtype V
+
+  @discardableResult
+  func baz(callback: @escaping (V) -> Void) -> Self
+}
+
+class C<V> : P {
+  func baz(callback: @escaping (V) -> Void) -> Self { return self }
+}
+class D<T, E: Error> : C<R<T, E>> {
+  init(fn: (_ ret: @escaping (V) -> Void) -> Void) {}
+}
+
+extension A where V: P, V.V: A, E == V.V.E {
+  func bar() -> D<V.V.V, V.V.E> {
+    return D { complete in
+      foo(value: { promise in promise.baz { result in } },
+          error: { complete(R(error: $0)) })
+    }
+  }
+}

test/expr/closure/closures.swift

@@ -259,7 +259,6 @@ func rdar19179412() -> (Int) -> Int {
 func takesVoidFunc(_ f: () -> ()) {}
 var i: Int = 1
 
-// expected-warning @+1 {{expression of type 'Int' is unused}}
 takesVoidFunc({i})
 // expected-warning @+1 {{expression of type 'Int' is unused}}
 var f1: () -> () = {i}