Merge pull request swiftlang#31810 from xedin/rdar-63230293-5.3

[5.3][ConstraintSystem] Detect and diagnose inability to infer type of clo…

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -245,6 +245,9 @@ ERROR(no_candidates_match_result_type,none,
       "no '%0' candidates produce the expected contextual result type %1",
       (StringRef, Type))
 
+ERROR(cannot_infer_closure_parameter_type,none,
+      "unable to infer type of a closure parameter %0 in the current context",
+       (StringRef))
 ERROR(cannot_infer_closure_type,none,
       "unable to infer closure type in the current context", ())
 ERROR(cannot_infer_closure_result_type,none,

lib/Sema/CSBindings.cpp

@@ -1090,27 +1090,25 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {
       // resolved and had to be bound to a placeholder "hole" type.
       cs.increaseScore(SK_Hole);
 
+      ConstraintFix *fix = nullptr;
       if (auto *GP = TypeVar->getImpl().getGenericParameter()) {
         auto path = dstLocator->getPath();
         // Drop `generic parameter` locator element so that all missing
         // generic parameters related to the same path can be coalesced later.
-        auto *fix = DefaultGenericArgument::create(
+        fix = DefaultGenericArgument::create(
             cs, GP,
             cs.getConstraintLocator(dstLocator->getAnchor(), path.drop_back()));
-        if (cs.recordFix(fix))
-          return true;
+      } else if (TypeVar->getImpl().isClosureParameterType()) {
+        fix = SpecifyClosureParameterType::create(cs, dstLocator);
       } else if (TypeVar->getImpl().isClosureResultType()) {
-        auto *fix = SpecifyClosureReturnType::create(
-            cs, TypeVar->getImpl().getLocator());
-        if (cs.recordFix(fix))
-          return true;
+        fix = SpecifyClosureReturnType::create(cs, dstLocator);
       } else if (srcLocator->getAnchor() &&
                  isa<ObjectLiteralExpr>(srcLocator->getAnchor())) {
-        auto *fix = SpecifyObjectLiteralTypeImport::create(
-            cs, TypeVar->getImpl().getLocator());
-        if (cs.recordFix(fix))
-          return true;
+        fix = SpecifyObjectLiteralTypeImport::create(cs, dstLocator);
       }
+
+      if (fix && cs.recordFix(fix))
+        return true;
     }
   }
 

lib/Sema/CSDiagnostics.cpp

@@ -41,6 +41,12 @@
 using namespace swift;
 using namespace constraints;
 
+static bool hasFixFor(const Solution &solution, ConstraintLocator *locator) {
+  return llvm::any_of(solution.Fixes, [&locator](const ConstraintFix *fix) {
+    return fix->getLocator() == locator;
+  });
+}
+
 FailureDiagnostic::~FailureDiagnostic() {}
 
 bool FailureDiagnostic::diagnose(bool asNote) {
@@ -6127,6 +6133,74 @@ bool MissingContextualBaseInMemberRefFailure::diagnoseAsError() {
   return true;
 }
 
+bool UnableToInferClosureParameterType::diagnoseAsError() {
+  auto *closure = castToExpr<ClosureExpr>(getRawAnchor());
+
+  // Let's check whether this closure is an argument to
+  // a call which couldn't be properly resolved e.g.
+  // missing  member or invalid contextual reference and
+  // if so let's not diagnose this problem because main
+  // issue here is inability to establish context for
+  // closure inference.
+  //
+  // TODO(diagnostics): Once we gain an ability to determine
+  // originating source of type holes this check could be
+  // significantly simplified.
+  {
+    auto &solution = getSolution();
+
+    // If there is a contextual mismatch associated with this
+    // closure, let's not diagnose any parameter type issues.
+    if (hasFixFor(solution, getConstraintLocator(
+                                closure, LocatorPathElt::ContextualType())))
+      return false;
+
+    if (auto *parentExpr = findParentExpr(closure)) {
+      while (parentExpr &&
+             (isa<TupleExpr>(parentExpr) || isa<ParenExpr>(parentExpr))) {
+        parentExpr = findParentExpr(parentExpr);
+      }
+
+      if (parentExpr) {
+        // Missing or invalid member reference in call.
+        if (auto *AE = dyn_cast<ApplyExpr>(parentExpr)) {
+          if (getType(AE->getFn())->isHole())
+            return false;
+        }
+
+        // Any fix anchored on parent expression makes it unnecessary
+        // to diagnose unability to infer parameter type because it's
+        // an indication that proper context couldn't be established to
+        // resolve the closure.
+        if (llvm::any_of(solution.Fixes,
+                         [&parentExpr](const ConstraintFix *fix) -> bool {
+                           return fix->getAnchor() == parentExpr;
+                         }))
+          return false;
+      }
+    }
+  }
+
+  auto paramIdx = getLocator()
+                      ->castLastElementTo<LocatorPathElt::TupleElement>()
+                      .getIndex();
+
+  auto *PD = closure->getParameters()->get(paramIdx);
+
+  llvm::SmallString<16> id;
+  llvm::raw_svector_ostream OS(id);
+
+  if (PD->isAnonClosureParam()) {
+    OS << "$" << paramIdx;
+  } else {
+    OS << "'" << PD->getParameterName() << "'";
+  }
+
+  auto loc = PD->isAnonClosureParam() ? getLoc() : PD->getLoc();
+  emitDiagnosticAt(loc, diag::cannot_infer_closure_parameter_type, OS.str());
+  return true;
+}
+
 bool UnableToInferClosureReturnType::diagnoseAsError() {
   auto *closure = castToExpr<ClosureExpr>(getRawAnchor());
 

lib/Sema/CSDiagnostics.h

@@ -1973,6 +1973,15 @@ class MissingContextualBaseInMemberRefFailure final : public FailureDiagnostic {
   bool diagnoseAsError();
 };
 
+class UnableToInferClosureParameterType final : public FailureDiagnostic {
+public:
+  UnableToInferClosureParameterType(const Solution &solution,
+                                    ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator) {}
+
+  bool diagnoseAsError();
+};
+
 class UnableToInferClosureReturnType final : public FailureDiagnostic {
 public:
   UnableToInferClosureReturnType(const Solution &solution,

lib/Sema/CSFix.cpp

@@ -22,6 +22,7 @@
 #include "ConstraintSystem.h"
 #include "OverloadChoice.h"
 #include "swift/AST/Expr.h"
+#include "swift/AST/ParameterList.h"
 #include "swift/AST/Type.h"
 #include "swift/AST/Types.h"
 #include "swift/Basic/SourceManager.h"
@@ -1226,6 +1227,38 @@ SpecifyBaseTypeForContextualMember *SpecifyBaseTypeForContextualMember::create(
       SpecifyBaseTypeForContextualMember(cs, member, locator);
 }
 
+std::string SpecifyClosureParameterType::getName() const {
+  std::string name;
+  llvm::raw_string_ostream OS(name);
+
+  auto *closure = cast<ClosureExpr>(getAnchor());
+  auto paramLoc =
+      getLocator()->castLastElementTo<LocatorPathElt::TupleElement>();
+
+  auto *PD = closure->getParameters()->get(paramLoc.getIndex());
+
+  OS << "specify type for parameter ";
+  if (PD->isAnonClosureParam()) {
+    OS << "$" << paramLoc.getIndex();
+  } else {
+    OS << "'" << PD->getParameterName() << "'";
+  }
+
+  return OS.str();
+}
+
+bool SpecifyClosureParameterType::diagnose(const Solution &solution,
+                                           bool asNote) const {
+  UnableToInferClosureParameterType failure(solution, getLocator());
+  return failure.diagnose(asNote);
+}
+
+SpecifyClosureParameterType *
+SpecifyClosureParameterType::create(ConstraintSystem &cs,
+                                    ConstraintLocator *locator) {
+  return new (cs.getAllocator()) SpecifyClosureParameterType(cs, locator);
+}
+
 bool SpecifyClosureReturnType::diagnose(const Solution &solution,
                                         bool asNote) const {
   UnableToInferClosureReturnType failure(solution, getLocator());

lib/Sema/CSFix.h

@@ -232,6 +232,10 @@ enum class FixKind : uint8_t {
   /// inferred and has to be specified explicitly.
   SpecifyBaseTypeForContextualMember,
 
+  /// Type of the closure parameter used in the body couldn't be inferred
+  /// and has to be specified explicitly.
+  SpecifyClosureParameterType,
+
   /// Closure return type has to be explicitly specified because it can't be
   /// inferred in current context e.g. because it's a multi-statement closure.
   SpecifyClosureReturnType,
@@ -251,7 +255,7 @@ enum class FixKind : uint8_t {
 
   /// A warning fix that allows a coercion to perform a force-cast.
   AllowCoercionToForceCast,
-  
+
   /// Allow key path root type mismatch when applying a key path that has a
   /// root type not convertible to the type of the base instance.
   AllowKeyPathRootTypeMismatch,
@@ -1706,6 +1710,19 @@ class SpecifyBaseTypeForContextualMember final : public ConstraintFix {
   create(ConstraintSystem &cs, DeclNameRef member, ConstraintLocator *locator);
 };
 
+class SpecifyClosureParameterType final : public ConstraintFix {
+  SpecifyClosureParameterType(ConstraintSystem &cs, ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::SpecifyClosureParameterType, locator) {}
+
+public:
+  std::string getName() const;
+
+  bool diagnose(const Solution &solution, bool asNote = false) const;
+
+  static SpecifyClosureParameterType *create(ConstraintSystem &cs,
+                                             ConstraintLocator *locator);
+};
+
 class SpecifyClosureReturnType final : public ConstraintFix {
   SpecifyClosureReturnType(ConstraintSystem &cs, ConstraintLocator *locator)
       : ConstraintFix(cs, FixKind::SpecifyClosureReturnType, locator) {}

lib/Sema/CSGen.cpp

@@ -2196,8 +2196,12 @@ namespace {
             auto declaredTy = param->getType();
             externalType = CS.openUnboundGenericType(declaredTy, paramLoc);
           } else {
+            // Let's allow parameters which haven't been explicitly typed
+            // to become holes by default, this helps in situations like
+            // `foo { a in }` where `foo` doesn't exist.
             externalType = CS.createTypeVariable(
-                paramLoc, TVO_CanBindToInOut | TVO_CanBindToNoEscape);
+                paramLoc,
+                TVO_CanBindToInOut | TVO_CanBindToNoEscape | TVO_CanBindToHole);
           }
 
           closureParams.push_back(param->toFunctionParam(externalType));

lib/Sema/CSSimplify.cpp

@@ -9603,6 +9603,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::AllowTupleSplatForSingleParameter:
   case FixKind::AllowInvalidUseOfTrailingClosure:
   case FixKind::AllowNonClassTypeToConvertToAnyObject:
+  case FixKind::SpecifyClosureParameterType:
   case FixKind::SpecifyClosureReturnType:
   case FixKind::AddQualifierToAccessTopLevelName:
     llvm_unreachable("handled elsewhere");

lib/Sema/ConstraintGraph.cpp

@@ -1126,6 +1126,10 @@ bool ConstraintGraph::contractEdges() {
     if (isParamBindingConstraint && tyvar1->getImpl().canBindToInOut()) {
       bool isNotContractable = true;
       if (auto bindings = CS.getPotentialBindings(tyvar1)) {
+        // Holes can't be contracted.
+        if (bindings.IsHole)
+          continue;
+
         for (auto &binding : bindings.Bindings) {
           auto type = binding.BindingType;
           isNotContractable = type.findIf([&](Type nestedType) -> bool {

lib/Sema/ConstraintSystem.h

@@ -300,6 +300,10 @@ class TypeVariableType::Implementation {
   /// Determine whether this type variable represents a closure type.
   bool isClosureType() const;
 
+  /// Determine whether this type variable represents one of the
+  /// parameter types associated with a closure.
+  bool isClosureParameterType() const;
+
   /// Determine whether this type variable represents a closure result type.
   bool isClosureResultType() const;