Merge pull request swiftlang#15069 from apple/revert-14846-rdar-37790062-alt

Revert "[CSSolver] Use correct locator when matching function result types re…"

Author: xedin

lib/SILGen/SILGenPoly.cpp

@@ -1573,9 +1573,6 @@ class ResultPlanner {
       ///
       /// Valid: reabstraction info, InnerResult, OuterResult.
       ReabstractDirectToDirect,
-
-      /// Ignore the next direct inner result, since the outer is 'Void'.
-      IgnoreDirectResult,
     };
 
     Operation(Kind kind) : TheKind(kind) {}
@@ -1720,24 +1717,6 @@ class ResultPlanner {
                                     SILResultInfo outerResult,
                                     SILValue optOuterResultAddr);
 
-  void planIgnoredResult(AbstractionPattern innerOrigType,
-                         CanType innerSubstType, PlanData &planData) {
-    if (innerOrigType.isTuple()) {
-      auto innerSubstTuple = cast<TupleType>(innerSubstType);
-      for (unsigned i = 0, n = innerSubstTuple->getNumElements(); i != n; ++i)
-        planIgnoredResult(innerOrigType.getTupleElementType(i),
-                          innerSubstTuple.getElementType(i), planData);
-      return;
-    }
-
-    auto innerResult = claimNextInnerResult(planData);
-    if (innerResult.isFormalIndirect() &&
-        SGF.silConv.isSILIndirect(innerResult))
-      (void)addInnerIndirectResultTemporary(planData, innerResult);
-    else
-      addIgnoreDirectResult();
-  }
-
   /// Claim the next inner result from the plan data.
   SILResultInfo claimNextInnerResult(PlanData &data) {
     return claimNext(data.InnerResults);
@@ -1887,10 +1866,6 @@ class ResultPlanner {
     op.OuterOrigType = outerOrigType;
     op.OuterSubstType = outerSubstType;
   }
-
-  void addIgnoreDirectResult() {
-    (void)addOperation(Operation::IgnoreDirectResult);
-  }
 };
 
 } // end anonymous namespace
@@ -1901,13 +1876,6 @@ void ResultPlanner::plan(AbstractionPattern innerOrigType,
                          AbstractionPattern outerOrigType,
                          CanType outerSubstType,
                          PlanData &planData) {
-  // Conversion from `() -> T` to `() -> Void` is allowed when
-  // the argument is a closure.
-  if (!innerSubstType->isVoid() && outerSubstType->isVoid()) {
-    planIgnoredResult(innerOrigType, innerSubstType, planData);
-    return;
-  }
-
   // The substituted types must match up in tuple-ness and arity.
   assert(
       isa<TupleType>(innerSubstType) == isa<TupleType>(outerSubstType) ||
@@ -2616,10 +2584,6 @@ void ResultPlanner::execute(ArrayRef<SILValue> innerDirectResults,
     case Operation::InjectOptionalIndirect:
       SGF.B.createInjectEnumAddr(Loc, op.OuterResultAddr, op.SomeDecl);
       continue;
-
-    case Operation::IgnoreDirectResult:
-      (void)claimNext(innerDirectResults);
-      continue;
     }
     llvm_unreachable("bad operation kind");
   }

lib/Sema/CSRanking.cpp

@@ -1007,32 +1007,14 @@ 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 (!impl.prefersSubtypeBinding())
+    if (!binding.typeVar->getImpl().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,8 +1212,10 @@ 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
@@ -1499,8 +1501,6 @@ 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,32 +2243,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
   // Allow '() -> T' to '() -> ()' and '() -> Never' to '() -> T' for closure
   // literals.
   if (auto elt = locator.last()) {
-    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 (elt->getKind() == ConstraintLocator::ClosureResult) {
       if (concrete && kind >= ConstraintKind::Subtype &&
           (type1->isUninhabited() || type2->isVoid())) {
         increaseScore(SK_FunctionConversion);

test/Constraints/closures.swift

@@ -630,6 +630,7 @@ 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) })
 }
 
@@ -651,38 +652,3 @@ func rdar36054961() {
      str.replaceSubrange(range, with: str[range].reversed())
   }])
 }
-
-protocol P_37790062 {
-  associatedtype T
-  var elt: T { get }
-}
-
-func rdar37790062() {
-  struct S<T> {
-    init(_ a: () -> T, _ b: () -> T) {}
-  }
-
-  class C1 : P_37790062 {
-    typealias T = Int
-    var elt: T { return 42 }
-  }
-
-  class C2 : P_37790062 {
-    typealias T = (String, Int, Void)
-    var elt: T { return ("question", 42, ()) }
-  }
-
-  func foo() -> Int { return 42 }
-  func bar() -> Void {}
-  func baz() -> (String, Int) { return ("question", 42) }
-  func bzz<T>(_ a: T) -> T { return a }
-  func faz<T: P_37790062>(_ a: T) -> T.T { return a.elt }
-
-  _ = S({ foo() }, { bar() }) // Ok, should infer T to be 'Void'
-  _ = S({ baz() }, { bar() }) // Ok, should infer T to be 'Void'
-  _ = S({ bzz(("question", 42)) }, { bar() }) // Ok
-  _ = S({ bzz(String.self) }, { bar() }) // Ok
-  _ = S({ bzz(((), (()))) }, { bar() }) // Ok
-  _ = S({ bzz(C1()) }, { bar() }) // Ok
-  _ = S({ faz(C2()) }, { bar() }) // Ok
-}

test/Constraints/rdar37790062.swift

@@ -24,15 +24,13 @@ public func test(_ dict: NSDictionary) {
 // popq   %rbp  ;<== Blocks the handshake from objc_autoreleaseReturnValue
 // jmp    0x01ec20 ; symbol stub for: objc_retainAutoreleasedReturnValue
 
-// CHECK-LABEL: define {{.*}}swiftcc %TSo12NSEnumeratorC* @"$S34objc_retainAutoreleasedReturnValue4testyySo12NSDictionaryCFSo12NSEnumeratorCADXEfU_"(%TSo12NSDictionaryC*)
+// CHECK-LABEL: define {{.*}}swiftcc void @"$S34objc_retainAutoreleasedReturnValue4testyySo12NSDictionaryCFyADXEfU_"(%TSo12NSDictionaryC*)
 // CHECK: entry:
 // CHECK:   call {{.*}}@objc_msgSend
 // CHECK:   notail call i8* @objc_retainAutoreleasedReturnValue
-// CHECK:   ret %TSo12NSEnumeratorC*
+// CHECK:   ret void
 
-// CHECK-LABEL: define {{.*}}swiftcc void @"$SSo12NSDictionaryCSo12NSEnumeratorCIgxo_ABIegx_TR"(%TSo12NSDictionaryC*, i8*, %swift.opaque*)
-
-// OPT-LABEL: define {{.*}}swiftcc void @"$S34objc_retainAutoreleasedReturnValue10useClosureyySo12NSDictionaryC_yADXEtF06$SSo12h44CSo12NSEnumeratorCIgxo_ABIegx_TR049$S34objc_bcD42Value4testyySo12a6CFSo12B8CADXEfU_Tf3npf_nTf1nc_nTf4g_n"(%TSo12NSDictionaryC*)
+// OPT-LABEL: define {{.*}}swiftcc void @"$S34objc_retainAutoreleasedReturnValue4testyySo12NSDictionaryCFyADXEfU_"(%TSo12NSDictionaryC*)
 // OPT: entry:
 // OPT:   call {{.*}}@objc_msgSend
 // OPT:   notail call i8* @objc_retainAutoreleasedReturnValue

test/IRGen/objc_retainAutoreleasedReturnValue.swift

@@ -259,6 +259,7 @@ 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}