Merge pull request #38117 from LucianoPAlmeida/SR14720-escaping

[SR-14720][Sema] Improve assign escapeness mismatch diagnostic for function type parameters

Author: LucianoPAlmeida

include/swift/AST/DiagnosticsSema.def

@@ -3890,6 +3890,12 @@ ERROR(converting_noattrfunc_to_type,none,
       "converting %select{non-escaping|non-concurrent function}0 value to %1 "
       "may %select{allow it to escape|introduce data races}0",
       (unsigned, Type))
+NOTE(escape_expected_at_parameter_position,none,
+     "parameter #%0 expects escaping value of type %1",
+     (unsigned, Type))
+NOTE(add_explicit_escaping,none,
+     "add explicit @escaping to function parameter #%0",
+     (unsigned))
 
 ERROR(converting_func_loses_global_actor,none,
       "converting function value of type %0 to %1 loses global actor %2",

lib/Sema/CSDiagnostics.cpp

@@ -905,6 +905,94 @@ bool AttributedFuncToTypeConversionFailure::diagnoseAsError() {
   return true;
 }
 
+static VarDecl *getDestinationVarDecl(AssignExpr *AE,
+                                      const Solution &solution) {
+  ConstraintLocator *locator = nullptr;
+  if (auto *URDE = dyn_cast<UnresolvedDotExpr>(AE->getDest())) {
+    locator = solution.getConstraintLocator(URDE, {ConstraintLocator::Member});
+  } else if (auto *declRef = dyn_cast<DeclRefExpr>(AE->getDest())) {
+    locator = solution.getConstraintLocator(declRef);
+  }
+  if (!locator)
+    return nullptr;
+
+  auto overload = solution.getOverloadChoiceIfAvailable(locator);
+  if (!overload)
+    return nullptr;
+
+  return dyn_cast_or_null<VarDecl>(overload->choice.getDecl());
+}
+
+bool AttributedFuncToTypeConversionFailure::
+    diagnoseFunctionParameterEscapenessMismatch(AssignExpr *AE) const {
+  auto loc = getLocator();
+  if (attributeKind != Escaping)
+    return false;
+
+  if (!loc->findLast<LocatorPathElt::FunctionArgument>())
+    return false;
+
+  auto destType = getType(AE->getDest())->lookThroughAllOptionalTypes();
+  auto destFnType = destType->castTo<FunctionType>();
+  auto sourceType = getType(AE->getSrc())->lookThroughAllOptionalTypes();
+
+  // The tuple locator element will give us the exact parameter mismatch
+  // position.
+  auto tupleElt = loc->getLastElementAs<LocatorPathElt::TupleElement>();
+  auto mismatchPosition = tupleElt ? tupleElt->getIndex() : 0;
+  auto param = destFnType->getParams()[mismatchPosition];
+
+  emitDiagnostic(diag::cannot_convert_assign, sourceType, destType);
+  emitDiagnosticAt(AE->getDest()->getLoc(),
+                   diag::escape_expected_at_parameter_position,
+                   mismatchPosition, param.getParameterType());
+
+  auto &solution = getSolution();
+  auto decl = getDestinationVarDecl(AE, solution);
+  // We couldn't find a declaration to add an extra note with a fix-it but
+  // the main diagnostic was already covered.
+  if (!decl)
+    return true;
+
+  auto declRepr = decl->getTypeReprOrParentPatternTypeRepr();
+  class TopLevelFuncReprFinder : public ASTWalker {
+    bool walkToTypeReprPre(TypeRepr *TR) override {
+      FnRepr = dyn_cast<FunctionTypeRepr>(TR);
+      return FnRepr == nullptr;
+    }
+
+  public:
+    FunctionTypeRepr *FnRepr;
+    TopLevelFuncReprFinder() : FnRepr(nullptr) {}
+  };
+
+  // Look to find top-level function repr that maybe inside optional
+  // representations.
+  TopLevelFuncReprFinder fnFinder;
+  declRepr->walk(fnFinder);
+
+  auto declFnRepr = fnFinder.FnRepr;
+  if (!declFnRepr)
+    return true;
+
+  auto note = emitDiagnosticAt(decl->getLoc(), diag::add_explicit_escaping,
+                               mismatchPosition);
+  auto argsRepr = declFnRepr->getArgsTypeRepr();
+  auto argRepr = argsRepr->getElement(mismatchPosition).Type;
+  if (!param.isAutoClosure()) {
+    note.fixItInsert(argRepr->getStartLoc(), "@escaping ");
+  } else {
+    auto attrRepr = dyn_cast<AttributedTypeRepr>(argRepr);
+    if (attrRepr) {
+      auto autoclosureEndLoc = Lexer::getLocForEndOfToken(
+          getASTContext().SourceMgr,
+          attrRepr->getAttrs().getLoc(TAK_autoclosure));
+      note.fixItInsertAfter(autoclosureEndLoc, " @escaping");
+    }
+  }
+  return true;
+}
+
 bool AttributedFuncToTypeConversionFailure::diagnoseParameterUse() const {
   auto convertTo = getToType();
   // If the other side is not a function, we have common case diagnostics
@@ -961,6 +1049,11 @@ bool AttributedFuncToTypeConversionFailure::diagnoseParameterUse() const {
       diagnostic = diag::passing_noattrfunc_to_attrfunc;
     }
   } else if (auto *AE = getAsExpr<AssignExpr>(getRawAnchor())) {
+    // Attempt to diagnose escape/non-escape mismatch in function
+    // parameter position.
+    if (diagnoseFunctionParameterEscapenessMismatch(AE))
+      return true;
+
     if (auto *DRE = dyn_cast<DeclRefExpr>(AE->getSrc())) {
       PD = dyn_cast<ParamDecl>(DRE->getDecl());
       diagnostic = diag::assigning_noattrfunc_to_attrfunc;

lib/Sema/CSDiagnostics.h

@@ -740,6 +740,11 @@ class AttributedFuncToTypeConversionFailure final : public ContextualFailure {
   /// argument, or trying to assign it to a variable which expects @escaping
   /// or @Sendable function.
   bool diagnoseParameterUse() const;
+
+  /// Emit a tailored diagnostic for a no-escape/espace mismatch for function
+  /// arguments where the mismatch has to take into account that a
+  /// function type subtype relation in the parameter position is contravariant.
+  bool diagnoseFunctionParameterEscapenessMismatch(AssignExpr *) const;
 };
 
 /// Diagnose failure where a global actor attribute is dropped when

test/Constraints/function_conversion.swift

@@ -57,9 +57,10 @@ func twoFns(_ f: (Int) -> Int, _ g: @escaping (Int) -> Int) {
 takesAny(consumeNoEscape)
 takesAny(consumeEscaping)
 
-var noEscapeParam: ((Int) -> Int) -> () = consumeNoEscape
+var noEscapeParam: ((Int) -> Int) -> () = consumeNoEscape // expected-note {{add explicit @escaping to function parameter}}{{21-21=@escaping }}
 var escapingParam: (@escaping (Int) -> Int) -> () = consumeEscaping
-noEscapeParam = escapingParam // expected-error {{converting non-escaping value to '(Int) -> Int' may allow it to escape}}
+noEscapeParam = escapingParam // expected-error {{cannot assign value of type '(@escaping (Int) -> Int) -> ()' to type '((Int) -> Int) -> ()'}}
+// expected-note@-1{{parameter #0 expects escaping value of type '(Int) -> Int'}}
 
 escapingParam = takesAny
 noEscapeParam = takesAny // expected-error {{converting non-escaping value to 'Any' may allow it to escape}}

test/attr/attr_escaping.swift

@@ -146,15 +146,15 @@ func takesNoEscapeFunction(fn: () -> ()) { // expected-note {{parameter 'fn' is
 
 
 class FooClass {
-  var stored : Optional<(()->Int)->Void> = nil
+  var stored : Optional<(()->Int)->Void> = nil  // expected-note {{add explicit @escaping to function parameter #0}} {{26-26=@escaping }}
   var computed : (()->Int)->Void {
     get { return stored! }
     set(newValue) { stored = newValue } // ok
   }
   var computedEscaping : (@escaping ()->Int)->Void {
     get { return stored! }
-    set(newValue) { stored = newValue } // expected-error{{assigning non-escaping parameter 'newValue' to an @escaping closure}}
-    // expected-note@-1 {{parameter 'newValue' is implicitly non-escaping}}
+    set(newValue) { stored = newValue } // expected-error{{cannot assign value of type '(@escaping () -> Int) -> Void' to type '(() -> Int) -> Void'}}
+    // expected-note@-1{{parameter #0 expects escaping value of type '() -> Int'}}
   }
 }
 
@@ -230,3 +230,40 @@ extension SR_9760 {
 
 // SR-9178
 func foo<T>(_ x: @escaping T) {} // expected-error 1{{@escaping attribute only applies to function types}}
+
+// SR-14720
+var global: ((() -> Void) -> Void)? = nil // expected-note {{add explicit @escaping to function parameter #0}} {{15-15=@escaping }}
+
+class SR14720 {
+  let ok: (@escaping () -> Void) -> Void // OK
+  let callback: (() -> Void) -> Void // expected-note {{add explicit @escaping to function parameter #0}} {{18-18=@escaping }}
+  let callback1: (() -> Void, () -> Void) -> Void // expected-note {{add explicit @escaping to function parameter #1}} {{31-31=@escaping }} 
+  let callbackAuto: (@autoclosure () -> Void) -> Void // expected-note {{add explicit @escaping to function parameter #0}} {{34-34= @escaping}}
+  let callbackOpt: ((() -> Void) -> Void)? // expected-note{{add explicit @escaping to function parameter #0}} {{22-22=@escaping }}
+
+  init(f: @escaping (@escaping() -> Void) -> Void) {
+    self.callback = f // expected-error{{cannot assign value of type '(@escaping () -> Void) -> Void' to type '(() -> Void) -> Void'}}
+    // expected-note@-1{{parameter #0 expects escaping value of type '() -> Void'}}
+    self.ok = f // Ok
+  }
+
+  init(af: @escaping (@escaping() -> Void) -> Void) {
+    self.callbackOpt = af // expected-error{{cannot assign value of type '(@escaping () -> Void) -> Void' to type '(() -> Void) -> Void'}}
+    // expected-note@-1{{parameter #0 expects escaping value of type '() -> Void'}}  
+  }
+
+  init(ag: @escaping (@escaping() -> Void) -> Void) {
+    global = ag // expected-error{{cannot assign value of type '(@escaping () -> Void) -> Void' to type '(() -> Void) -> Void'}}
+    // expected-note@-1{{parameter #0 expects escaping value of type '() -> Void'}}
+  }
+
+  init(a: @escaping (@escaping () -> Void) -> Void) {
+    self.callbackAuto = a // expected-error{{cannot assign value of type '(@escaping () -> Void) -> Void' to type '(@autoclosure () -> Void) -> Void'}}
+    // expected-note@-1{{parameter #0 expects escaping value of type '() -> Void'}}
+  }
+
+  init(f: @escaping (() -> Void, @escaping() -> Void) -> Void) {
+    self.callback1 = f // expected-error{{cannot assign value of type '(() -> Void, @escaping () -> Void) -> Void' to type '(() -> Void, () -> Void) -> Void'}}
+    // expected-note@-1{{parameter #1 expects escaping value of type '() -> Void'}}
+  }
+}