[Trailing closures] Bias toward the backward scan for ambiguities.

This approach, suggested by Xiaodi Wu, provides better source
compatibility for existing Swift code, by breaking ties in favor of the
existing Swift semantics. Each time the backward-scan rule is needed
(and differs from the forward-scan result), we will produce a warning
+ Fix-It to prepare for Swift 6 where the backward rule can be
removed.

Author: DougGregor

lib/Sema/CSRanking.cpp

@@ -49,8 +49,8 @@ void ConstraintSystem::increaseScore(ScoreKind kind, unsigned value) {
       log << "use of an unavailable declaration";
       break;
 
-    case SK_BackwardTrailingClosure:
-      llvm::errs() << "backward scan when matching a trailing closure";
+    case SK_ForwardTrailingClosure:
+      llvm::errs() << "forward scan when matching a trailing closure";
       break;
 
     case SK_Fix:

lib/Sema/CSSimplify.cpp

@@ -1228,13 +1228,13 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments(
         cs.getConstraintLocator(locator),
         callArgumentMatch->trailingClosureMatching);
 
-    // If we matched with the backward rule, increase the score for backward
-    // matches.
-    // FIXME: We shouldn't want this? Wait until later so we can diagnose
-    // appropriately.
-    if (callArgumentMatch->trailingClosureMatching
-            == TrailingClosureMatching::Backward)
-      cs.increaseScore(SK_BackwardTrailingClosure);
+    // If there was a disjunction because both forward and backward were
+    // possible, increase the score for forward matches to bias toward the
+    // (source-compatible) backward matches. The compiler will produce a
+    // warning for such code.
+    if (trailingClosureMatching &&
+        *trailingClosureMatching == TrailingClosureMatching::Forward)
+      cs.increaseScore(SK_ForwardTrailingClosure);
 
     // Take the parameter bindings we selected.
     parameterBindings = std::move(callArgumentMatch->parameterBindings);

lib/Sema/ConstraintSystem.h

@@ -660,8 +660,8 @@ enum ScoreKind {
   SK_Hole,
   /// A reference to an @unavailable declaration.
   SK_Unavailable,
-  /// A use of the "backward" scanning heuristic for trailing closures.
-  SK_BackwardTrailingClosure,
+  /// A use of the "forward" scan for trailing closures.
+  SK_ForwardTrailingClosure,
   /// A use of a disfavored overload.
   SK_DisfavoredOverload,
   /// An implicit force of an implicitly unwrapped optional value.

test/Constraints/diagnostics.swift

@@ -1260,8 +1260,7 @@ func rdar17170728() {
 
   let _ = [i, j, k].reduce(0 as Int?) {
     $0 && $1 ? $0 + $1 : ($0 ? $0 : ($1 ? $1 : nil))
-    // expected-error@-1 {{binary operator '+' cannot be applied to two 'Int?' operands}}
-    // expected-error@-2 4 {{optional type 'Int?' cannot be used as a boolean; test for '!= nil' instead}}
+    // expected-error@-1 {{binary operator '+' cannot be applied to two 'Bool' operands}}
   }
 }
 

test/Parse/multiple_trailing_closures.swift

@@ -68,14 +68,12 @@ func bar(_ s: S) {
   }
 }
 
-func multiple_trailing_with_defaults( // expected-note{{contains defaulted closure parameters 'animations' and 'completion'}}
+func multiple_trailing_with_defaults( // expected-note{{declared here}}
   duration: Int,
   animations: (() -> Void)? = nil,
   completion: (() -> Void)? = nil) {}
 
-multiple_trailing_with_defaults(duration: 42) {} // expected-warning{{unlabeled trailing closure argument matches}}
-// expected-note@-1{{'completion' to retain}}
-// expected-note@-2{{'animations' to silence this}}
+multiple_trailing_with_defaults(duration: 42) {} // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with 'completion' to suppress this warning}}
 
 multiple_trailing_with_defaults(duration: 42) {} completion: {}
 

test/expr/closure/trailing.swift

@@ -5,7 +5,7 @@ func takeFunc(_ f: (Int) -> Int) -> Int {}
 func takeValueAndFunc(_ value: Int, _ f: (Int) -> Int) {}
 func takeTwoFuncs(_ f: (Int) -> Int, _ g: (Int) -> Int) {}
 func takeFuncWithDefault(f : ((Int) -> Int)? = nil) {}
-func takeTwoFuncsWithDefaults(f1 : ((Int) -> Int)? = nil, f2 : ((String) -> String)? = nil) {} // expected-note{{contains defaulted closure parameters 'f1' and 'f2'}}
+func takeTwoFuncsWithDefaults(f1 : ((Int) -> Int)? = nil, f2 : ((String) -> String)? = nil) {}
 // expected-note@-1{{'takeTwoFuncsWithDefaults(f1:f2:)' declared here}}
 
 struct X {
@@ -107,8 +107,7 @@ func labeledArgumentAndTrailingClosure() {
 
   // Trailing closure binds to first parameter... unless only the second matches.
  takeTwoFuncsWithDefaults { "Hello, " + $0 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with 'f2' to suppress this warning}}
- takeTwoFuncsWithDefaults { $0 + 1 } // expected-warning{{rather than}}
-  // expected-note@-1 2{{label the argument}}
+ takeTwoFuncsWithDefaults { $0 + 1 }
   takeTwoFuncsWithDefaults(f1: {$0 + 1 })
 }
 

test/expr/postfix/call/forward_trailing_closure.swift

@@ -1,6 +1,6 @@
 // RUN: %target-typecheck-verify-swift -disable-fuzzy-forward-scan-trailing-closure-matching
 
-func forwardMatch1( // expected-note 5 {{contains defaulted}}
+func forwardMatch1(
   a: Int = 0, b: Int = 17, closure1: (Int) -> Int = { $0 }, c: Int = 42,
   numbers: Int..., closure2: ((Double) -> Double)? = nil,
   closure3: (String) -> String = { $0 + "!" }
@@ -11,29 +11,24 @@ func testForwardMatch1(i: Int, d: Double, s: String) {
   forwardMatch1()
 
   // Matching closure1
-  forwardMatch1 { _ in i } // expected-warning{{rather than}}
-  // expected-note@-1 2{{label the argument}}
+  forwardMatch1 { _ in i }
   forwardMatch1 { _ in i } closure2: { d + $0 }
   forwardMatch1 { _ in i } closure2: { d + $0 } closure3: { $0 + ", " + s + "!" }
   forwardMatch1 { _ in i } closure3: { $0 + ", " + s + "!" }
 
-  forwardMatch1(a: 5) { $0 + i } // expected-warning{{rather than}}
-  // expected-note@-1 2{{label the argument}}
+  forwardMatch1(a: 5) { $0 + i }
   forwardMatch1(a: 5) { $0 + i } closure2: { d + $0 }
   forwardMatch1(a: 5) { $0 + i } closure2: { d + $0 } closure3: { $0 + ", " + s + "!" }
   forwardMatch1(a: 5) { $0 + i } closure3: { $0 + ", " + s + "!" }
 
-  forwardMatch1(b: 1) { $0 + i } // expected-warning{{rather than}}
-  // expected-note@-1 2{{label the argument}}
+  forwardMatch1(b: 1) { $0 + i }
   forwardMatch1(b: 1) { $0 + i } closure2: { d + $0 }
   forwardMatch1(b: 1) { $0 + i } closure2: { d + $0 } closure3: { $0 + ", " + s + "!" }
   forwardMatch1(b: 1) { $0 + i } closure3: { $0 + ", " + s + "!" }
 
   // Matching closure2
-  forwardMatch1(closure1: { $0 + i }) { d + $0 } // expected-warning{{rather than}}
-  // expected-note@-1 2{{label the argument}}
-  forwardMatch1(closure1: { $0 + i }, numbers: 1, 2, 3) { d + $0 } // expected-warning{{rather than}}
-  // expected-note@-1 2{{label the argument}}
+  forwardMatch1(closure1: { $0 + i }) { d + $0 }
+  forwardMatch1(closure1: { $0 + i }, numbers: 1, 2, 3) { d + $0 }
   forwardMatch1(closure1: { $0 + i }, numbers: 1, 2, 3) { d + $0 } closure3: { $0 + ", " + s + "!" }
 
   // Matching closure3

test/expr/postfix/call/forward_trailing_closure_ambiguity.swift

@@ -1,7 +1,7 @@
 // RUN: %target-typecheck-verify-swift
 
 // Ambiguity when calling.
-func ambiguous1( // expected-note 3 {{'ambiguous1(x:a:y:b:z:c:)' contains defaulted closure parameters '}}
+func ambiguous1( // expected-note 3 {{declared here}}
 
   x: (Int) -> Int = { $0 },
   a: Int = 5,
@@ -12,24 +12,18 @@ func ambiguous1( // expected-note 3 {{'ambiguous1(x:a:y:b:z:c:)' contains defaul
 ) {}
 
 func testAmbiguous1() {
-  ambiguous1 { $0 } // expected-warning{{since Swift 5.3, unlabeled trailing closure argument matches parameter 'x' rather than parameter 'z'}}
-    // expected-note@-1{{label the argument with 'z' to retain the pre-Swift 5.3 behavior}}{{13-13=(z: }}{{20-20=)}}
-    // expected-note@-2{{label the argument with 'x' to silence this warning for Swift 5.3 and newer}}{{13-13=(x: }}{{20-20=)}}
+  ambiguous1 { $0 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with 'z' to suppress this warning}}{{13-13=(z: }}{{20-20=)}}
 
-  ambiguous1() { $0 } // expected-warning{{since Swift 5.3, unlabeled trailing closure argument matches parameter 'x' rather than parameter 'z'}}
-    // expected-note@-1{{label the argument with 'z' to retain the pre-Swift 5.3 behavior}}{{14-15=z: }}{{22-22=)}}
-    // expected-note@-2{{label the argument with 'x' to silence this warning for Swift 5.3 and newer}}{{14-15=x: }}{{22-22=)}}
+  ambiguous1() { $0 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with 'z' to suppress this warning}}{{14-15=z: }}{{22-22=)}}
 
-  ambiguous1(a: 3) { $0 } // expected-warning{{since Swift 5.3, unlabeled trailing closure argument matches parameter 'y' rather than parameter 'z'}}
-    // expected-note@-1{{label the argument with 'z' to retain the pre-Swift 5.3 behavior}}{{18-19=, z: }}{{26-26=)}}
-    // expected-note@-2{{label the argument with 'y' to silence this warning for Swift 5.3 and newer}}{{18-19=, y: }}{{26-26=)}}
+  ambiguous1(a: 3) { $0 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with 'z' to suppress this warning}}{{18-19=, z: }}{{26-26=)}}
 
   // No warning; this is matching the last parameter.
   ambiguous1(b: 3) { $0 }
 }
 
 // Ambiguity with two unlabeled arguments.
-func ambiguous2( // expected-note{{'ambiguous2(_:a:y:b:_:x:)' contains defaulted closure parameters '_' and '_'}}
+func ambiguous2( // expected-note{{declared here}}
   _: (Int) -> Int = { $0 },
   a: Int = 5,
   y: (Int) -> Int = { $0 },
@@ -39,11 +33,11 @@ func ambiguous2( // expected-note{{'ambiguous2(_:a:y:b:_:x:)' contains defaulted
 ) {}
 
 func testAmbiguous2() {
-    ambiguous2 { $0 } // expected-warning{{since Swift 5.3, unlabeled trailing closure argument matches earlier parameter '_' rather than later parameter with the same name}}
+    ambiguous2 { $0 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with '_' to suppress this warning}}{{15-15=(}}{{22-22=)}}
 }
 
 // Ambiguity with one unlabeled argument.
-func ambiguous3( // expected-note{{'ambiguous3(x:a:y:b:_:c:)' contains defaulted closure parameters 'x' and '_'}}
+func ambiguous3( // expected-note{{declared here}}
   x: (Int) -> Int = { $0 },
   a: Int = 5,
   y: (Int) -> Int = { $0 },
@@ -53,9 +47,7 @@ func ambiguous3( // expected-note{{'ambiguous3(x:a:y:b:_:c:)' contains defaulted
 ) {}
 
 func testAmbiguous3() {
-  ambiguous3 { $0 } // expected-warning{{since Swift 5.3, unlabeled trailing closure argument matches parameter 'x' rather than parameter '_'}}
-    // expected-note@-1{{label the argument with '_' to retain the pre-Swift 5.3 behavior}}{{13-13=(}}{{20-20=)}}
-    // expected-note@-2{{label the argument with 'x' to silence this warning for Swift 5.3 and newer}}{{13-13=(x: }}{{20-20=)}}
+  ambiguous3 { $0 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with '_' to suppress this warning}}{{13-13=(}}{{20-20=)}}
 }
 
 // Not ambiguous because of an arity mismatch that would lead to different

test/expr/postfix/call/forward_trailing_closure_fuzzy.swift

@@ -42,13 +42,11 @@ func testTrailingClosureEitherDirection() {
 }
 
 // Check that we resolve ambiguities when both directions can be matched.
-// expected-note@+1{{'trailingClosureBothDirections(f:g:)' contains defaulted closure parameters 'f' and 'g'}}
+// expected-note@+1{{declared here}}
 func trailingClosureBothDirections(
   f: (Int, Int) -> Int = { $0 + $1 }, g: (Int, Int) -> Int = { $0 - $1 }
 ) { }
-trailingClosureBothDirections { $0 * $1 } // expected-warning{{since Swift 5.3, unlabeled trailing closure argument matches parameter 'f' rather than parameter 'g'}}
-// expected-note@-1{{label the argument with 'g' to retain the pre-Swift 5.3 behavior}}
-// expected-note@-2{{label the argument with 'f' to silence this warning for Swift 5.3 and newer}}
+trailingClosureBothDirections { $0 * $1 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with 'g' to suppress this warning}}
 
 // Check an amusing quirk of the "backward" rule that allows the order of
 // arguments to be swapped.