[Constraint solver] Remove a dubious hack introduced with SR-2505

Author: DougGregor

lib/Sema/CSSimplify.cpp

@@ -917,36 +917,6 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments(
   argsWithLabels.append(args.begin(), args.end());
   AnyFunctionType::relabelParams(argsWithLabels, argLabels);
 
-  // FIXME: Remove this. It's functionally identical to the real code
-  // path below, except for some behavioral differences in solution ranking
-  // that I don't understand.
-  if (params.size() == 1 &&
-      args.size() == 1 &&
-      params[0].getLabel().empty() &&
-      args[0].getLabel().empty() &&
-      !params[0].getParameterFlags().isInOut() &&
-      !args[0].getParameterFlags().isInOut() &&
-      params[0].getPlainType()->isAny()) {
-    auto argType = args[0].getPlainType();
-
-    // Disallow assignment of noescape function to parameter of type
-    // Any. Allowing this would allow these functions to escape.
-    if (auto *fnTy = argType->getAs<AnyFunctionType>()) {
-      if (fnTy->isNoEscape()) {
-        auto *loc = cs.getConstraintLocator(locator);
-        // Allow assigned of 'no-escape' function with recorded fix.
-        if (cs.shouldAttemptFixes()) {
-          if (!cs.recordFix(MarkExplicitlyEscaping::create(cs, loc)))
-            return cs.getTypeMatchSuccess();
-        }
-
-        return cs.getTypeMatchFailure(locator);
-      }
-    }
-
-    return cs.getTypeMatchSuccess();
-  }
-
   // Match up the call arguments to the parameters.
   SmallVector<ParamBinding, 4> parameterBindings;
   ArgumentFailureTracker listener(cs, parameterBindings, locator);

test/Constraints/closures.swift

@@ -416,11 +416,11 @@ func r20789423() {
 
 }
 
-// Make sure that behavior related to allowing trailing closures to match functions
-// with Any as a final parameter is the same after the changes made by SR-2505, namely:
-// that we continue to select function that does _not_ have Any as a final parameter in
-// presence of other possibilities.
-
+// In the example below, SR-2505 started preferring C_SR_2505.test(_:) over
+// test(it:). Prior to Swift 5.1, we emulated the old behavior. However,
+// that behavior is inconsistent with the typical approach of preferring
+// overloads from the concrete type over one from a protocol, so we removed
+// the hack.
 protocol SR_2505_Initable { init() }
 struct SR_2505_II : SR_2505_Initable {}
 
@@ -442,10 +442,9 @@ class C_SR_2505 : P_SR_2505 {
   }
 
   func call(_ c: C_SR_2505) -> Bool {
-    // Note: no diagnostic about capturing 'self', because this is a
-    // non-escaping closure -- that's how we know we have selected
-    // test(it:) and not test(_)
-    return c.test { o in test(o) }
+    // Note: the diagnostic about capturing 'self', indicates that we have
+    // selected test(_) rather than test(it:)
+    return c.test { o in test(o) } // expected-error{{call to method 'test' in closure requires explicit 'self.' to make capture semantics explicit}}
   }
 }