Merge pull request #70777 from xedin/diagnostic-improvements-for-closures

xedin · web-flow · commit 4c604b479b30 · 2024-01-10T11:13:49.000-08:00
[ConstraintSystem] Closure type-checking improvements
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -388,6 +388,9 @@ ERROR(cannot_convert_argument_value_for_swift_func,none,
 NOTE(candidate_has_invalid_argument_at_position,none,
      "candidate expects %select{|in-out }2value of type %0 for parameter #%1 (got %3)",
      (Type, unsigned, bool, Type))
+NOTE(candidate_has_invalid_closure_at_position,none,
+     "closure passed to parameter of type %0 that does not "
+     "accept a closure", (Type))
 
 ERROR(cannot_convert_array_to_variadic,none,
       "cannot pass array of type %0 as variadic arguments of type %1",
@@ -1547,9 +1550,9 @@ ERROR(extra_argument_to_nullary_call,none,
 ERROR(extra_trailing_closure_in_call,none,
       "extra trailing closure passed in "
       "%select{call|subscript|macro expansion}0", (unsigned))
-ERROR(trailing_closure_bad_param,none,
-      "trailing closure passed to parameter of type %0 that does not "
-      "accept a closure", (Type))
+ERROR(closure_bad_param,none,
+      "%select{|trailing }1closure passed to parameter of type %0 that does not "
+      "accept a closure", (Type, bool))
 WARNING(unlabeled_trailing_closure_deprecated,Deprecation,
         "backward matching of the unlabeled trailing closure is deprecated; label the argument with %0 to suppress this warning",
         (Identifier))
diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp
@@ -5580,6 +5580,12 @@ bool MissingArgumentsFailure::isMisplacedMissingArgument(
   auto argType = solution.simplifyType(solution.getType(unaryArg));
   auto paramType = fnType->getParams()[1].getPlainType();
 
+  if (isExpr<ClosureExpr>(unaryArg) && argType->is<UnresolvedType>()) {
+    auto unwrappedParamTy = paramType->lookThroughAllOptionalTypes();
+    if (unwrappedParamTy->is<FunctionType>() || unwrappedParamTy->isAny())
+      return true;
+  }
+
   return TypeChecker::isConvertibleTo(argType, paramType, solution.getDC());
 }
 
@@ -7292,7 +7298,7 @@ bool ArgumentMismatchFailure::diagnoseAsError() {
   if (diagnoseAttemptedRegexBuilder())
     return true;
 
-  if (diagnoseTrailingClosureMismatch())
+  if (diagnoseClosureMismatch())
     return true;
 
   if (diagnoseKeyPathAsFunctionResultMismatch())
@@ -7346,10 +7352,15 @@ bool ArgumentMismatchFailure::diagnoseAsError() {
 bool ArgumentMismatchFailure::diagnoseAsNote() {
   auto *locator = getLocator();
   if (auto *callee = getCallee()) {
-    emitDiagnosticAt(callee, diag::candidate_has_invalid_argument_at_position,
-                     getToType(), getParamPosition(),
-                     locator->isLastElement<LocatorPathElt::LValueConversion>(),
-                     getFromType());
+    if (isExpr<ClosureExpr>(getAnchor())) {
+      emitDiagnosticAt(callee, diag::candidate_has_invalid_closure_at_position,
+                       getToType());
+    } else {
+      emitDiagnosticAt(callee, diag::candidate_has_invalid_argument_at_position,
+                       getToType(), getParamPosition(),
+                       locator->isLastElement<LocatorPathElt::LValueConversion>(),
+                       getFromType());
+    }
     return true;
   }
 
@@ -7633,15 +7644,16 @@ bool ArgumentMismatchFailure::diagnoseAttemptedRegexBuilder() const {
   return true;
 }
 
-bool ArgumentMismatchFailure::diagnoseTrailingClosureMismatch() const {
-  if (!Info.isTrailingClosure())
+bool ArgumentMismatchFailure::diagnoseClosureMismatch() const {
+  if (!isExpr<ClosureExpr>(getAnchor()))
     return false;
 
   auto paramType = getToType();
   if (paramType->lookThroughAllOptionalTypes()->is<AnyFunctionType>())
     return false;
 
-  emitDiagnostic(diag::trailing_closure_bad_param, paramType)
+  emitDiagnostic(diag::closure_bad_param, paramType,
+                 Info.isTrailingClosure())
       .highlight(getSourceRange());
 
   if (auto overload = getCalleeOverloadChoiceIfAvailable(getLocator())) {
diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h
@@ -2120,9 +2120,9 @@ class ArgumentMismatchFailure : public ContextualFailure {
   /// or now deprecated `init(initialValue:)`.
   bool diagnosePropertyWrapperMismatch() const;
 
-  /// Tailored diagnostics for argument mismatches associated with trailing
+  /// Tailored diagnostics for argument mismatches associated with (trailing)
   /// closures being passed to non-closure parameters.
-  bool diagnoseTrailingClosureMismatch() const;
+  bool diagnoseClosureMismatch() const;
 
   /// Tailored key path as function diagnostics for argument mismatches where
   /// argument is a keypath expression that has a root type that matches a
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -11554,6 +11554,28 @@ bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar,
     return true;
   };
 
+  // If contextual type is not a function type or `Any` and this
+  // closure is used as an argument, let's skip resolution.
+  //
+  // Doing so improves performance if closure is passed as an argument
+  // to a (heavily) overloaded declaration, avoid unrelated errors,
+  // propagate holes, and record a more impactful fix.
+  if (!contextualType->isTypeVariableOrMember() &&
+      !(contextualType->is<FunctionType>() || contextualType->isAny()) &&
+      locator.endsWith<LocatorPathElt::ApplyArgToParam>()) {
+    if (!shouldAttemptFixes())
+      return false;
+
+    assignFixedType(typeVar, PlaceholderType::get(getASTContext(), typeVar),
+                    closureLocator);
+    recordTypeVariablesAsHoles(inferredClosureType);
+
+    return !recordFix(
+        AllowArgumentMismatch::create(*this, typeVar, contextualType,
+                                      getConstraintLocator(locator)),
+        /*impact=*/15);
+  }
+
   // Determine whether a result builder will be applied.
   auto resultBuilderType = getOpenedResultBuilderTypeFor(*this, locator);
 
diff --git a/test/Constraints/closures.swift b/test/Constraints/closures.swift
@@ -1153,7 +1153,7 @@ struct R_76250381<Result, Failure: Error> {
 func rdar77022842(argA: Bool? = nil, argB: Bool? = nil) {
   if let a = argA ?? false, if let b = argB ?? {
     // expected-error@-1 {{initializer for conditional binding must have Optional type, not 'Bool'}}
-    // expected-error@-2 {{cannot convert value of type '() -> ()' to expected argument type 'Bool?'}}
+    // expected-error@-2 {{closure passed to parameter of type 'Bool?' that does not accept a closure}}
     // expected-error@-3 {{cannot convert value of type 'Void' to expected condition type 'Bool'}}
   } // expected-error {{expected '{' after 'if' condition}}
 }
@@ -1247,3 +1247,14 @@ do {
 
 
 }
+
+do {
+  func test(_: Int, _: Int) {}
+  // expected-note@-1 {{closure passed to parameter of type 'Int' that does not accept a closure}}
+  func test(_: Int, _: String) {}
+  // expected-note@-1 {{closure passed to parameter of type 'String' that does not accept a closure}}
+
+  test(42) { // expected-error {{no exact matches in call to local function 'test'}}
+    print($0)
+  }
+}
diff --git a/test/Sema/property_wrapper_parameter_invalid.swift b/test/Sema/property_wrapper_parameter_invalid.swift
@@ -262,12 +262,12 @@ struct ProjectionWrapper<Value> {
 
 func testInvalidWrapperInference() {
   struct S<V> {
-    static func test(_ keyPath: KeyPath<V, String>) {} // expected-note {{'test' declared here}}
+    static func test(_ keyPath: KeyPath<V, String>) {} // expected-note 2 {{'test' declared here}}
   }
 
   // expected-error@+1 {{trailing closure passed to parameter of type 'KeyPath<Int, String>' that does not accept a closure}}
   S<Int>.test { $value in }
-  // expected-error@+1 {{cannot convert value of type '(_) -> ()' to expected argument type 'KeyPath<Int, String>'}}
+  // expected-error@+1 {{closure passed to parameter of type 'KeyPath<Int, String>' that does not accept a closure}}
   S<Int>.test({ $value in })
 
   func testGenericClosure<T>(_ closure: T) {}
diff --git a/test/expr/closure/closures.swift b/test/expr/closure/closures.swift
@@ -42,9 +42,9 @@ func funcdecl5(_ a: Int, _ y: Int) {
   func6({($0) + $0})    // // expected-error {{contextual closure type '(Int, Int) -> Int' expects 2 arguments, but 1 was used in closure body}}
 
 
-  var testfunc : ((), Int) -> Int  // expected-note {{'testfunc' declared here}}
+  var testfunc : ((), Int) -> Int  // expected-note 2 {{'testfunc' declared here}}
   testfunc({$0+1})  // expected-error {{missing argument for parameter #2 in call}}
-  // expected-error@-1 {{cannot convert value of type '(Int) -> Int' to expected argument type '()'}}
+  // expected-error@-1 {{closure passed to parameter of type '()' that does not accept a closure}}
 
   funcdecl5(1, 2) // recursion.
 
