Merge pull request #24403 from DougGregor/constraint-solver-trailing-closure-pruning

[Constraint solver] Reject trailing closures matching non-closure-parameters

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -1030,6 +1030,9 @@ ERROR(extra_argument_to_nullary_call,none,
       "argument passed to call that takes no arguments", ())
 ERROR(extra_trailing_closure_in_call,none,
       "extra trailing closure passed in call", ())
+ERROR(trailing_closure_bad_param,none,
+      "trailing closure passed to parameter of type %0 that does not "
+      "accept a closure", (Type))
 ERROR(no_accessible_initializers,none,
       "%0 cannot be constructed because it has no accessible initializers",
       (Type))

lib/Sema/CSDiag.cpp

@@ -3517,6 +3517,28 @@ class ArgumentMatcher : public MatchCallArgumentListener {
     return true;
   }
 
+  bool trailingClosureMismatch(unsigned paramIdx, unsigned argIdx) override {
+    Expr *arg = ArgExpr;
+
+    auto tuple = dyn_cast<TupleExpr>(ArgExpr);
+    if (tuple)
+      arg = tuple->getElement(argIdx);
+
+    auto &param = Parameters[paramIdx];
+    TC.diagnose(arg->getLoc(), diag::trailing_closure_bad_param,
+                param.getPlainType())
+      .highlight(arg->getSourceRange());
+
+    auto candidate = CandidateInfo[0];
+    if (candidate.getDecl())
+      TC.diagnose(candidate.getDecl(), diag::decl_declared_here,
+                  candidate.getDecl()->getFullName());
+
+    Diagnosed = true;
+
+    return true;
+  }
+
   bool diagnose() {
     // Use matchCallArguments to determine how close the argument list is (in
     // shape) to the specified candidates parameters.  This ignores the

lib/Sema/CSSimplify.cpp

@@ -54,6 +54,11 @@ bool MatchCallArgumentListener::relabelArguments(ArrayRef<Identifier> newNames){
   return true;
 }
 
+bool MatchCallArgumentListener::trailingClosureMismatch(
+    unsigned paramIdx, unsigned argIdx) {
+  return true;
+}
+
 /// Produce a score (smaller is better) comparing a parameter name and
 /// potentially-typo'd argument name.
 ///
@@ -205,6 +210,21 @@ static ConstraintSystem::TypeMatchOptions getDefaultDecompositionOptions(
   return flags | ConstraintSystem::TMF_GenerateConstraints;
 }
 
+/// Determine whether the given parameter can accept a trailing closure.
+static bool acceptsTrailingClosure(const AnyFunctionType::Param &param) {
+  Type paramTy = param.getPlainType();
+  if (!paramTy)
+    return true;
+
+  paramTy = paramTy->lookThroughAllOptionalTypes();
+  return paramTy->isTypeParameter() ||
+      paramTy->is<ArchetypeType>() ||
+      paramTy->is<AnyFunctionType>() ||
+      paramTy->isTypeVariableOrMember() ||
+      paramTy->is<UnresolvedType>() ||
+      paramTy->isAny();
+}
+
 // FIXME: This should return ConstraintSystem::TypeMatchResult instead
 //        to give more information to the solver about the failure.
 bool constraints::
@@ -425,6 +445,14 @@ matchCallArguments(ArrayRef<AnyFunctionType::Param> args,
 
   // If we have a trailing closure, it maps to the last parameter.
   if (hasTrailingClosure && numParams > 0) {
+    // If there is no suitable last parameter to accept the trailing closure,
+    // notify the listener and bail if we need to.
+    if (!acceptsTrailingClosure(params[numParams - 1])) {
+      if (listener.trailingClosureMismatch(numParams - 1, numArgs - 1))
+        return true;
+    }
+
+    // Claim the parameter/argument pair.
     claimedArgs[numArgs-1] = true;
     ++numClaimedArgs;
     parameterBindings[numParams-1].push_back(numArgs-1);

lib/Sema/CalleeCandidateInfo.cpp

@@ -355,6 +355,10 @@ CalleeCandidateInfo::ClosenessResultTy CalleeCandidateInfo::evaluateCloseness(
       result = CC_ArgumentLabelMismatch;
       return true;
     }
+    bool trailingClosureMismatch(unsigned paramIdx, unsigned argIdx) override {
+      result = CC_ArgumentMismatch;
+      return true;
+    }
   } listener;
 
   // Use matchCallArguments to determine how close the argument list is (in

lib/Sema/ConstraintSystem.h

@@ -3700,6 +3700,13 @@ class MatchCallArgumentListener {
   /// \returns true to indicate that this should cause a failure, false
   /// otherwise.
   virtual bool relabelArguments(ArrayRef<Identifier> newNames);
+
+  /// Indicates that the trailing closure argument at the given \c argIdx
+  /// cannot be passed to the last parameter at \c paramIdx.
+  ///
+  /// \returns true to indicate that this should cause a failure, false
+  /// otherwise.
+  virtual bool trailingClosureMismatch(unsigned paramIdx, unsigned argIdx);
 };
 
 /// Match the call arguments (as described by the given argument type) to

test/Constraints/diag_missing_arg.swift

@@ -35,19 +35,20 @@ trailingClosureSingle1() { 1 } // expected-error {{missing argument for paramete
 
 func trailingClosureSingle2(x: () -> Int, y: Int) {} // expected-note * {{here}}
 // FIXME: Bad diagnostics.
-trailingClosureSingle2 { 1 } // expected-error {{missing argument for parameter 'x' in call}} {{23-23=(x: <#() -> Int#>)}}
-trailingClosureSingle2() { 1 } // expected-error {{missing argument for parameter 'x' in call}} {{24-24=x: <#() -> Int#>}}
+trailingClosureSingle2 { 1 } // expected-error {{cannot convert value of type '() -> Int' to expected argument type '(x: () -> Int, y: Int)'}}
+trailingClosureSingle2() { 1 } // expected-error {{cannot convert value of type '() -> Int' to expected argument type '(x: () -> Int, y: Int)'}}
 
 func trailingClosureMulti1(x: Int, y: Int, z: () -> Int) {} // expected-note * {{here}}
 trailingClosureMulti1(y: 1) { 1 } // expected-error {{missing argument for parameter 'x' in call}} {{23-23=x: <#Int#>, }}
 trailingClosureMulti1(x: 1) { 1 } // expected-error {{missing argument for parameter 'y' in call}} {{27-27=, y: <#Int#>}}
 trailingClosureMulti1(x: 1, y: 1) // expected-error {{missing argument for parameter 'z' in call}} {{33-33=, z: <#() -> Int#>}}
 
 func trailingClosureMulti2(x: Int, y: () -> Int, z: Int) {} // expected-note * {{here}}
-trailingClosureMulti2 { 1 } // expected-error {{missing argument for parameter 'x' in call}} {{22-22=(x: <#Int#>)}}
+trailingClosureMulti2 { 1 } // expected-error {{cannot convert value of type '() -> Int' to expected argument type '(x: Int, y: () -> Int, z: Int)'}}
 // FIXME: Bad diagnostics.
-trailingClosureMulti2() { 1 } // expected-error {{missing argument for parameter 'x' in call}} {{23-23=x: <#Int#>}}
-trailingClosureMulti2(x: 1) { 1 } // expected-error {{missing argument for parameter 'y' in call}} {{27-27=, y: <#() -> Int#>}}
+trailingClosureMulti2() { 1 } // expected-error {{cannot convert value of type '() -> Int' to expected argument type '(x: Int, y: () -> Int, z: Int)'}}
+trailingClosureMulti2(x: 1) { 1 } // expected-error {{cannot invoke 'trailingClosureMulti2' with an argument list of type '(x: Int, @escaping () -> Int)'}}
+// expected-note@-1{{expected an argument list of type '(x: Int, y: () -> Int, z: Int)'}}
 
 func param2Func(x: Int, y: Int) {} // expected-note * {{here}}
 param2Func(x: 1) // expected-error {{missing argument for parameter 'y' in call}} {{16-16=, y: <#Int#>}}

test/Constraints/overload_filtering.swift

@@ -39,3 +39,14 @@ func testUnresolvedMember(i: Int) -> X {
   // CHECK-NEXT: introducing single enabled disjunction term {{.*}} bound to decl overload_filtering.(file).X.init(_:_:)
   return .init(i, i)
 }
+
+func trailing(x: Int = 0, y: () -> Void) { }
+func trailing(x: Int = 0, z: Float) { }
+
+func testTrailing() {
+  // CHECK: disabled disjunction term {{.*}} bound to decl overload_filtering.(file).trailing(x:z:)
+  trailing() { }
+
+  // CHECK: disabled disjunction term {{.*}} bound to decl overload_filtering.(file).trailing(x:z:)
+  trailing(x: 5) { }
+}

test/expr/closure/trailing.swift

@@ -132,8 +132,8 @@ func rdar17965209_test() {
 func limitXY(_ xy:Int, toGamut gamut: [Int]) {}
 let someInt = 0
 let intArray = [someInt]
-limitXY(someInt, toGamut: intArray) {}  // expected-error {{extra argument 'toGamut' in call}}
-
+limitXY(someInt, toGamut: intArray) {}  // expected-error{{cannot invoke 'limitXY' with an argument list of type '(Int, toGamut: [Int], @escaping () -> ())'}}
+// expected-note@-1{{expected an argument list of type '(Int, toGamut: [Int])'}}
 
 // <rdar://problem/23036383> QoI: Invalid trailing closures in stmt-conditions produce lowsy diagnostics
 func retBool(x: () -> Int) -> Bool {}