Merge pull request swiftlang#28712 from hborla/function-parameter-mismatch-diagnostics

[ConstraintSystem] Port function parameter type mismatch diagnostics.

Author: hborla

lib/Sema/CSDiag.cpp

@@ -249,8 +249,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
       Optional<std::function<bool(ArrayRef<OverloadChoice>)>> callback = None,
       bool includeInaccessibleMembers = true);
 
-  bool diagnoseTrailingClosureErrors(ApplyExpr *expr);
-
   bool diagnoseSubscriptErrors(SubscriptExpr *SE, bool performingSet);
 
   bool visitExpr(Expr *E);
@@ -1710,113 +1708,6 @@ static bool diagnoseClosureExplicitParameterMismatch(
   return false;
 }
 
-bool FailureDiagnosis::diagnoseTrailingClosureErrors(ApplyExpr *callExpr) {
-  if (!callExpr->hasTrailingClosure())
-    return false;
-
-  auto *fnExpr = callExpr->getFn();
-  auto *argExpr = callExpr->getArg();
-
-  ClosureExpr *closureExpr = nullptr;
-  if (auto *PE = dyn_cast<ParenExpr>(argExpr)) {
-    closureExpr = dyn_cast<ClosureExpr>(PE->getSubExpr());
-  } else {
-    return false;
-  }
-
-  if (!closureExpr)
-    return false;
-
-  class CallResultListener : public ExprTypeCheckListener {
-    Type expectedResultType;
-
-  public:
-    explicit CallResultListener(Type resultType)
-        : expectedResultType(resultType) {}
-
-    bool builtConstraints(ConstraintSystem &cs, Expr *expr) override {
-      if (!expectedResultType)
-        return false;
-
-      auto resultType = cs.getType(expr);
-      auto *locator = cs.getConstraintLocator(expr);
-
-      // Since we know that this is trailing closure, format of the
-      // type could be like this - ((Input) -> Result) -> ClosureResult
-      // which we can leverage to create specific conversion for
-      // result type of the call itself, this might help us gain
-      // some valuable contextual information.
-      if (auto *fnType = resultType->getAs<AnyFunctionType>()) {
-        cs.addConstraint(ConstraintKind::Conversion, fnType->getResult(),
-                         expectedResultType, locator);
-      } else if (auto *typeVar = resultType->getAs<TypeVariableType>()) {
-        auto tv = cs.createTypeVariable(cs.getConstraintLocator(expr),
-                                        TVO_CanBindToLValue |
-                                        TVO_PrefersSubtypeBinding |
-                                        TVO_CanBindToNoEscape);
-
-        auto extInfo = FunctionType::ExtInfo().withThrows();
-
-        FunctionType::Param tvParam(tv);
-        auto fTy = FunctionType::get({tvParam}, expectedResultType, extInfo);
-
-        // Add a conversion constraint between the types.
-        cs.addConstraint(ConstraintKind::Conversion, typeVar, fTy, locator,
-                         /*isFavored*/ true);
-      }
-
-      return false;
-    }
-  };
-
-  SmallPtrSet<TypeBase *, 4> possibleTypes;
-  auto currentType = CS.simplifyType(CS.getType(fnExpr));
-
-  // If current type has type variables or unresolved types
-  // let's try to re-typecheck it to see if we can get some
-  // more information about what is going on.
-  if (currentType->hasTypeVariable() || currentType->hasUnresolvedType()) {
-    auto contextualType = CS.getContextualType();
-    CallResultListener listener(contextualType);
-    getPossibleTypesOfExpressionWithoutApplying(
-        fnExpr, CS.DC, possibleTypes, FreeTypeVariableBinding::UnresolvedType,
-        &listener);
-
-    // Looks like there is there a contextual mismatch
-    // related to function type, let's try to diagnose it.
-    if (possibleTypes.empty() && contextualType &&
-        !contextualType->hasUnresolvedType())
-      return diagnoseContextualConversionError(callExpr, contextualType,
-                                               CS.getContextualTypePurpose());
-  } else {
-    possibleTypes.insert(currentType.getPointer());
-  }
-
-  for (Type type : possibleTypes) {
-    auto *fnType = type->getAs<AnyFunctionType>();
-    if (!fnType)
-      continue;
-
-    auto params = fnType->getParams();
-    if (params.size() != 1)
-      return false;
-
-    Type paramType = params.front().getOldType();
-    if (auto paramFnType = paramType->getAs<AnyFunctionType>()) {
-      auto closureType = CS.getType(closureExpr);
-      if (auto *argFnType = closureType->getAs<AnyFunctionType>()) {
-        auto *params = closureExpr->getParameters();
-        auto loc = params ? params->getStartLoc() : closureExpr->getStartLoc();
-        if (diagnoseClosureExplicitParameterMismatch(
-                CS, loc, argFnType->getParams(), paramFnType->getParams()))
-          return true;
-      }
-    }
-  }
-
-  return false;
-}
-
 /// Check if there failure associated with expression is related
 /// to given contextual type.
 bool FailureDiagnosis::diagnoseCallContextualConversionErrors(
@@ -1910,13 +1801,6 @@ static bool isViableOverloadSet(const CalleeCandidateInfo &CCI,
 }
 
 bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
-  // If this call involves trailing closure as an argument,
-  // let's treat it specially, because re-typecheck of the
-  // either function or arguments might results in diagnosing
-  // of the unrelated problems due to luck of context.
-  if (diagnoseTrailingClosureErrors(callExpr))
-    return true;
-
   if (diagnoseCallContextualConversionErrors(callExpr, CS.getContextualType(),
                                              CS.getContextualTypePurpose()))
     return true;

lib/Sema/CSDiagnostics.cpp

@@ -2826,6 +2826,16 @@ bool TupleContextualFailure::diagnoseAsError() {
   return true;
 }
 
+bool FunctionTypeMismatch::diagnoseAsError() {
+  auto purpose = getContextualTypePurpose();
+  auto diagnostic = getDiagnosticFor(purpose, /*forProtocol=*/false);
+  if (!diagnostic)
+    return false;
+
+  emitDiagnostic(getAnchor()->getLoc(), *diagnostic, getFromType(), getToType());
+  return true;
+}
+
 bool AutoClosureForwardingFailure::diagnoseAsError() {
   auto *loc = getLocator();
   auto last = loc->castLastElementTo<LocatorPathElt::ApplyArgToParam>();

lib/Sema/CSDiagnostics.h

@@ -852,6 +852,23 @@ class TupleContextualFailure final : public ContextualFailure {
   }
 };
 
+class FunctionTypeMismatch final : public ContextualFailure {
+  /// Indices of the parameters whose types do not match.
+  llvm::SmallVector<unsigned, 4> Indices;
+
+public:
+  FunctionTypeMismatch(ConstraintSystem &cs, ContextualTypePurpose purpose,
+                         Type lhs, Type rhs, llvm::ArrayRef<unsigned> indices,
+                         ConstraintLocator *locator)
+      : ContextualFailure(cs, purpose, lhs, rhs, locator),
+        Indices(indices.begin(), indices.end()) {
+    std::sort(Indices.begin(), Indices.end());
+    assert(getFromType()->is<AnyFunctionType>() && getToType()->is<AnyFunctionType>());
+  }
+
+  bool diagnoseAsError() override;
+};
+
 /// Diagnose situations when @autoclosure argument is passed to @autoclosure
 /// parameter directly without calling it first.
 class AutoClosureForwardingFailure final : public FailureDiagnostic {

lib/Sema/CSFix.cpp

@@ -255,6 +255,35 @@ ContextualMismatch *ContextualMismatch::create(ConstraintSystem &cs, Type lhs,
   return new (cs.getAllocator()) ContextualMismatch(cs, lhs, rhs, locator);
 }
 
+/// Computes the contextual type information for a type mismatch of a
+/// component in a structural type (tuple or function type).
+///
+/// \returns A tuple containing the contextual type purpose, the source type,
+/// and the contextual type.
+static Optional<std::tuple<ContextualTypePurpose, Type, Type>>
+getStructuralTypeContext(ConstraintSystem &cs, ConstraintLocator *locator) {
+  if (auto contextualType = cs.getContextualType()) {
+    if (auto *anchor = simplifyLocatorToAnchor(locator))
+      return std::make_tuple(cs.getContextualTypePurpose(),
+                             cs.getType(anchor),
+                             contextualType);
+  } else if (auto argApplyInfo = cs.getFunctionArgApplyInfo(locator)) {
+    return std::make_tuple(CTP_CallArgument,
+                           argApplyInfo->getArgType(),
+                           argApplyInfo->getParamType());
+  } else if (auto *coerceExpr = dyn_cast<CoerceExpr>(locator->getAnchor())) {
+    return std::make_tuple(CTP_CoerceOperand,
+                           cs.getType(coerceExpr->getSubExpr()),
+                           cs.getType(coerceExpr));
+  } else if (auto *assignExpr = dyn_cast<AssignExpr>(locator->getAnchor())) {
+    return std::make_tuple(CTP_AssignSource,
+                           cs.getType(assignExpr->getSrc()),
+                           cs.getType(assignExpr->getDest()));
+  }
+
+  return None;
+}
+
 bool AllowTupleTypeMismatch::coalesceAndDiagnose(
     ArrayRef<ConstraintFix *> fixes, bool asNote) const {
   llvm::SmallVector<unsigned, 4> indices;
@@ -270,31 +299,16 @@ bool AllowTupleTypeMismatch::coalesceAndDiagnose(
 
   auto &cs = getConstraintSystem();
   auto *locator = getLocator();
-  auto purpose = cs.getContextualTypePurpose();
+  ContextualTypePurpose purpose;
   Type fromType;
   Type toType;
 
   if (getFromType()->is<TupleType>() && getToType()->is<TupleType>()) {
+    purpose = cs.getContextualTypePurpose();
     fromType = getFromType();
     toType = getToType();
-  } else if (auto contextualType = cs.getContextualType()) {
-    auto *tupleExpr = simplifyLocatorToAnchor(locator);
-    if (!tupleExpr)
-      return false;
-    fromType = cs.getType(tupleExpr);
-    toType = contextualType;
-  } else if (auto argApplyInfo = cs.getFunctionArgApplyInfo(locator)) {
-    purpose = CTP_CallArgument;
-    fromType = argApplyInfo->getArgType();
-    toType = argApplyInfo->getParamType();
-  } else if (auto *coerceExpr = dyn_cast<CoerceExpr>(locator->getAnchor())) {
-    purpose = CTP_CoerceOperand;
-    fromType = cs.getType(coerceExpr->getSubExpr());
-    toType = cs.getType(coerceExpr);
-  } else if (auto *assignExpr = dyn_cast<AssignExpr>(locator->getAnchor())) {
-    purpose = CTP_AssignSource;
-    fromType = cs.getType(assignExpr->getSrc());
-    toType = cs.getType(assignExpr->getDest());
+  } else if (auto contextualTypeInfo = getStructuralTypeContext(cs, locator)) {
+    std::tie(purpose, fromType, toType) = *contextualTypeInfo;
   } else {
     return false;
   }
@@ -315,6 +329,41 @@ AllowTupleTypeMismatch::create(ConstraintSystem &cs, Type lhs, Type rhs,
       AllowTupleTypeMismatch(cs, lhs, rhs, locator, index);
 }
 
+bool AllowFunctionTypeMismatch::coalesceAndDiagnose(
+    ArrayRef<ConstraintFix *> fixes, bool asNote) const {
+  llvm::SmallVector<unsigned, 4> indices{ParamIndex};
+
+  for (auto fix : fixes) {
+    if (auto *fnFix = fix->getAs<AllowFunctionTypeMismatch>())
+      indices.push_back(fnFix->ParamIndex);
+  }
+
+  auto &cs = getConstraintSystem();
+  auto *locator = getLocator();
+  ContextualTypePurpose purpose;
+  Type fromType;
+  Type toType;
+
+  auto contextualTypeInfo = getStructuralTypeContext(cs, locator);
+  if (!contextualTypeInfo)
+    return false;
+
+  std::tie(purpose, fromType, toType) = *contextualTypeInfo;
+  FunctionTypeMismatch failure(cs, purpose, fromType, toType, indices, locator);
+  return failure.diagnose(asNote);
+}
+
+bool AllowFunctionTypeMismatch::diagnose(bool asNote) const {
+  return coalesceAndDiagnose({}, asNote);
+}
+
+AllowFunctionTypeMismatch *
+AllowFunctionTypeMismatch::create(ConstraintSystem &cs, Type lhs, Type rhs,
+                                  ConstraintLocator *locator, unsigned index) {
+  return new (cs.getAllocator())
+      AllowFunctionTypeMismatch(cs, lhs, rhs, locator, index);
+}
+
 bool GenericArgumentsMismatch::diagnose(bool asNote) const {
   auto &cs = getConstraintSystem();
   GenericArgumentsMismatchFailure failure(cs, getFromType(), getToType(),

lib/Sema/CSFix.h

@@ -145,6 +145,10 @@ enum class FixKind : uint8_t {
   /// types.
   AllowTupleTypeMismatch,
 
+  /// Allow a function type to be destructured with mismatched parameter types
+  /// or return type.
+  AllowFunctionTypeMismatch,
+
   /// Allow an invalid member access on a value of protocol type as if
   /// that protocol type were a generic constraint requiring conformance
   /// to that protocol.
@@ -987,6 +991,35 @@ class AllowTupleTypeMismatch final : public ContextualMismatch {
   bool diagnose(bool asNote = false) const override;
 };
 
+class AllowFunctionTypeMismatch final : public ContextualMismatch {
+  /// The index of the parameter where the type mismatch occurred.
+  unsigned ParamIndex;
+
+  AllowFunctionTypeMismatch(ConstraintSystem &cs, Type lhs, Type rhs,
+                            ConstraintLocator *locator, unsigned index)
+      : ContextualMismatch(cs, FixKind::AllowFunctionTypeMismatch, lhs, rhs,
+                           locator), ParamIndex(index) {}
+
+public:
+  static AllowFunctionTypeMismatch *create(ConstraintSystem &cs, Type lhs,
+                                           Type rhs, ConstraintLocator *locator,
+                                           unsigned index);
+
+  static bool classof(const ConstraintFix *fix) {
+    return fix->getKind() == FixKind::AllowFunctionTypeMismatch;
+  }
+
+  std::string getName() const override {
+    return "allow function type mismatch";
+  }
+
+  bool coalesceAndDiagnose(ArrayRef<ConstraintFix *> secondaryFixes,
+                           bool asNote = false) const override;
+
+  bool diagnose(bool asNote = false) const override;
+};
+
+
 class AllowMutatingMemberOnRValueBase final : public AllowInvalidMemberRef {
   AllowMutatingMemberOnRValueBase(ConstraintSystem &cs, Type baseType,
                                   ValueDecl *member, DeclNameRef name,

lib/Sema/CSSimplify.cpp

@@ -3543,19 +3543,21 @@ bool ConstraintSystem::repairFailures(
 
     // Drop the `tuple element` locator element so that all tuple element
     // mismatches within the same tuple type can be coalesced later.
+    auto index = elt.getAs<LocatorPathElt::TupleElement>()->getIndex();
     path.pop_back();
     auto *tupleLocator = getConstraintLocator(locator.getAnchor(), path);
 
-    // TODO: Add a tailored fix for function parameter type mismatches.
     // Let this fail if it's a contextual mismatch with sequence element types,
     // as there's a special fix for that.
-    if (tupleLocator->isLastElement<LocatorPathElt::FunctionArgument>() ||
-        tupleLocator->isLastElement<LocatorPathElt::SequenceElementType>())
+    if (tupleLocator->isLastElement<LocatorPathElt::SequenceElementType>())
       break;
 
-    auto index = elt.getAs<LocatorPathElt::TupleElement>()->getIndex();
-    auto *fix =
-        AllowTupleTypeMismatch::create(*this, lhs, rhs, tupleLocator, index);
+    ConstraintFix *fix;
+    if (tupleLocator->isLastElement<LocatorPathElt::FunctionArgument>()) {
+      fix = AllowFunctionTypeMismatch::create(*this, lhs, rhs, tupleLocator, index);
+    } else {
+      fix = AllowTupleTypeMismatch::create(*this, lhs, rhs, tupleLocator, index);
+    }
     conversionsOrFixes.push_back(fix);
     break;
   }
@@ -8533,6 +8535,12 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
     return matchTupleTypes(matchingType, smaller, matchKind, subflags, locator);
   }
 
+  case FixKind::AllowFunctionTypeMismatch: {
+    if (recordFix(fix, /*impact=*/5))
+      return SolutionKind::Error;
+    return SolutionKind::Solved;
+  }
+
   case FixKind::TreatEphemeralAsNonEphemeral: {
     auto *theFix = static_cast<TreatEphemeralAsNonEphemeral *>(fix);
     // If we have a non-ephemeral locator for an ephemeral conversion, make a

test/Constraints/trailing_closures_objc.swift

@@ -16,7 +16,7 @@ func foo(options: [AVMediaSelectionOption]) {
 
 func rdar28004686(a: [IndexPath]) {
   _ = a.sorted { (lhs: NSIndexPath, rhs: NSIndexPath) -> Bool in true }
-  // expected-error@-1 {{'NSIndexPath' is not convertible to 'IndexPath'}}
+  // expected-error@-1 {{cannot convert value of type '(NSIndexPath, NSIndexPath) -> Bool' to expected argument type '(IndexPath, IndexPath) throws -> Bool'}}
 }
 
 class Test: NSObject {

test/expr/closure/closures.swift

@@ -72,7 +72,7 @@ func funcdecl5(_ a: Int, _ y: Int) {
   func6({a,b in 4.0 })  // expected-error {{cannot convert value of type 'Double' to closure result type 'Int'}}
 
   // TODO: This diagnostic can be improved: rdar://22128205
-  func6({(a : Float, b) in 4 }) // expected-error {{cannot convert value of type '(Float, _) -> Int' to expected argument type '(Int, Int) -> Int'}}
+  func6({(a : Float, b) in 4 }) // expected-error {{cannot convert value of type '(Float, Int) -> Int' to expected argument type '(Int, Int) -> Int'}}