Merge pull request #23125 from DougGregor/constraint-solver-remove-sr-2505-hack

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

Author: DougGregor

CHANGELOG.md

@@ -26,6 +26,15 @@ Swift 5.1
 
 * Key path expressions can now include references to tuple elements.
 
+* Single-parameter functions accepting values of type `Any` are no
+  longer preferred over other functions.
+
+```swift
+func foo(_: Any) { print("Any") }
+func foo<T>(_: T) { print("T") }
+foo(0) // prints "Any" in Swift < 5.1, "T" in Swift 5.1
+```
+
 Swift 5.0
 ---------
 

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/ClangImporter/objc_parse.swift

@@ -178,7 +178,7 @@ func keyedSubscripting(_ b: B, idx: A, a: A) {
   dict[NSString()] = a
   let value = dict[NSString()]
 
-  dict[nil] = a // expected-error {{cannot assign value of type 'A' to type 'Any?'}}
+  dict[nil] = a // expected-error {{ambiguous subscript with base type 'NSMutableDictionary' and index type '_'}}
   let q = dict[nil]  // expected-error {{ambiguous subscript}}
   _ = q
 }

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}}
   }
 }
 

validation-test/Sema/OverridesAndOverloads.swift

@@ -103,10 +103,8 @@ Overrides.test("covariant argument override, struct to protocol") {
   }
 
   // FIXME: https://bugs.swift.org/browse/SR-731
-  expectFailure {
-    Derived().foo(P1ImplS1())
-    expectEqual("Derived.foo(P1)", which)
-  }
+  Derived().foo(P1ImplS1())
+  expectEqual("Base.foo(P1ImplS1)", which)
 
   Derived().foo(P1xImplS1())
   expectEqual("Derived.foo(P1)", which)
@@ -321,14 +319,13 @@ Overloads.test("generic methods are worse than non-generic") {
     func foo(_: C1) { which = "foo(C1)" }
     func foo(_: Any) { which = "foo(Any)" }
     func foo<T>(_: T) { which = "foo(T)" }
-    // It is not possible to call foo<T>(T).  foo(Any) always wins.
 
     func bar(_: C1) { which = "bar(C1)" }
     func bar<T>(_: T) { which = "bar(T)" }
   }
 
   Base().foo(C1());     expectEqual("foo(C1)", which)
-  Base().foo(Token1()); expectEqual("foo(Any)", which)
+  Base().foo(Token1()); expectEqual("foo(T)", which)
 
   Base().bar(C1());     expectEqual("bar(C1)", which)
   Base().bar(Token1()); expectEqual("bar(T)", which)

validation-test/Sema/type_checker_perf/fast/rdar29358447.swift.gyb

@@ -1,4 +1,4 @@
-// RUN: %scale-test --begin 0 --end 25000 --step 1000 --typecheck --select incrementConstraintsPerContractionCounter %s -Xfrontend=-solver-disable-shrink -Xfrontend=-disable-constraint-solver-performance-hacks -Xfrontend=-solver-enable-operator-designated-types
+// RUN: %scale-test --begin 0 --end 10000 --step 1000 --typecheck --select incrementConstraintsPerContractionCounter %s -Xfrontend=-solver-disable-shrink -Xfrontend=-disable-constraint-solver-performance-hacks -Xfrontend=-solver-enable-operator-designated-types
 // REQUIRES: OS=macosx
 // REQUIRES: asserts