Merge pull request swiftlang#31809 from xedin/rdar-63230293

xedin · web-flow · commit 01dcc29117aa · 2020-05-15T09:51:08.000-07:00
[ConstraintSystem] Detect and diagnose inability to infer type of closure parameter(s)
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -250,6 +250,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,
diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp
@@ -1087,32 +1087,27 @@ 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() &&
                  isExpr<ObjectLiteralExpr>(srcLocator->getAnchor())) {
-        auto *fix = SpecifyObjectLiteralTypeImport::create(
-            cs, TypeVar->getImpl().getLocator());
-        if (cs.recordFix(fix))
-          return true;
+        fix = SpecifyObjectLiteralTypeImport::create(cs, dstLocator);
       } else if (srcLocator->isKeyPathRoot()) {
-        auto *fix =
-            SpecifyKeyPathRootType::create(cs, TypeVar->getImpl().getLocator());
-        if (cs.recordFix(fix))
-          return true;
+        fix = SpecifyKeyPathRootType::create(cs, dstLocator);
       }
+
+      if (fix && cs.recordFix(fix))
+        return true;
     }
   }
 
diff --git a/lib/Sema/CSDiagnostics.cpp b/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) {
@@ -6125,6 +6131,75 @@ 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.
+        ASTNode parentNode(parentExpr);
+        if (llvm::any_of(solution.Fixes,
+                         [&parentNode](const ConstraintFix *fix) -> bool {
+                           return fix->getAnchor() == parentNode;
+                         }))
+          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());
 
diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h
@@ -1971,6 +1971,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,
diff --git a/lib/Sema/CSFix.cpp b/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"
@@ -1229,6 +1230,38 @@ SpecifyBaseTypeForContextualMember *SpecifyBaseTypeForContextualMember::create(
       SpecifyBaseTypeForContextualMember(cs, member, locator);
 }
 
+std::string SpecifyClosureParameterType::getName() const {
+  std::string name;
+  llvm::raw_string_ostream OS(name);
+
+  auto *closure = castToExpr<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());
diff --git a/lib/Sema/CSFix.h b/lib/Sema/CSFix.h
@@ -234,6 +234,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,
@@ -253,7 +257,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,
@@ -1712,6 +1716,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) {}
diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp
@@ -2191,8 +2191,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));
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -9609,6 +9609,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");
diff --git a/lib/Sema/ConstraintGraph.cpp b/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 {
diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h
@@ -301,6 +301,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;
 
diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp
@@ -90,6 +90,14 @@ bool TypeVariableType::Implementation::isClosureType() const {
   return isExpr<ClosureExpr>(locator->getAnchor()) && locator->getPath().empty();
 }
 
+bool TypeVariableType::Implementation::isClosureParameterType() const {
+  if (!(locator && locator->getAnchor()))
+    return false;
+
+  return isExpr<ClosureExpr>(locator->getAnchor()) &&
+         locator->isLastElement<LocatorPathElt::TupleElement>();
+}
+
 bool TypeVariableType::Implementation::isClosureResultType() const {
   if (!(locator && locator->getAnchor()))
     return false;
diff --git a/test/Constraints/closures.swift b/test/Constraints/closures.swift
@@ -504,7 +504,7 @@ struct S_3520 {
 func sr3520_set_via_closure<S, T>(_ closure: (inout S, T) -> ()) {} // expected-note {{in call to function 'sr3520_set_via_closure'}}
 sr3520_set_via_closure({ $0.number1 = $1 })
 // expected-error@-1 {{generic parameter 'S' could not be inferred}}
-// expected-error@-2 {{generic parameter 'T' could not be inferred}}
+// expected-error@-2 {{unable to infer type of a closure parameter $1 in the current context}}
 
 // SR-3073: UnresolvedDotExpr in single expression closure
 
@@ -1017,3 +1017,21 @@ func overloaded_with_default_and_autoclosure<T>(b: Int = 0, c: @escaping () -> T
 
 overloaded_with_default_and_autoclosure { 42 } // Ok
 overloaded_with_default_and_autoclosure(42) // Ok
+
+// SR-12815 - `error: type of expression is ambiguous without more context` in many cases where methods are missing
+func sr12815() {
+  let _ = { a, b in }
+  // expected-error@-1 {{unable to infer type of a closure parameter 'a' in the current context}}
+  // expected-error@-2 {{unable to infer type of a closure parameter 'b' in the current context}}
+
+  _ = .a { b in } // expected-error {{cannot infer contextual base in reference to member 'a'}}
+
+  struct S {}
+
+  func test(s: S) {
+    S.doesntExist { b in } // expected-error {{type 'S' has no member 'doesntExist'}}
+    s.doesntExist { b in } // expected-error {{value of type 'S' has no member 'doesntExist'}}
+    s.doesntExist1 { v in } // expected-error {{value of type 'S' has no member 'doesntExist1'}}
+     .doesntExist2() { $0 }
+  }
+}
diff --git a/test/Constraints/diagnostics.swift b/test/Constraints/diagnostics.swift
@@ -255,8 +255,7 @@ struct Toe {
   let toenail: Nail // expected-error {{cannot find type 'Nail' in scope}}
 
   func clip() {
-    // TODO(diagnostics): Solver should stop once it has detected that `toenail` doesn't exist and report that.
-    toenail.inspect { x in // expected-error {{type of expression is ambiguous without more context}}
+    toenail.inspect { x in
       toenail.inspect { y in }
     }
   }
@@ -297,7 +296,7 @@ func r18800223(_ i : Int) {
 }
 
 // <rdar://problem/21883806> Bogus "'_' can only appear in a pattern or on the left side of an assignment" is back
-_ = { $0 }  // expected-error {{unable to infer closure type in the current context}}
+_ = { $0 }  // expected-error {{unable to infer type of a closure parameter $0 in the current context}}
 
 
 
diff --git a/test/Sema/diag_ambiguous_overloads.swift b/test/Sema/diag_ambiguous_overloads.swift
@@ -15,15 +15,15 @@ fe(.baz) // expected-error {{reference to member 'baz' cannot be resolved withou
 fe(.nope, .nyet) // expected-error {{type 'Int' has no member 'nope'}}
 // expected-error@-1 {{reference to member 'nyet' cannot be resolved without a contextual type}}
 
-func fg<T>(_ f: (T) -> T) -> Void {} // expected-note {{in call to function 'fg'}}
-fg({x in x}) // expected-error {{generic parameter 'T' could not be inferred}}
+func fg<T>(_ f: (T) -> T) -> Void {}
+fg({x in x}) // expected-error {{unable to infer type of a closure parameter 'x' in the current context}}
 
 
 struct S {
-  func f<T>(_ i: (T) -> T, _ j: Int) -> Void {} // expected-note {{in call to function 'f'}}
+  func f<T>(_ i: (T) -> T, _ j: Int) -> Void {}
   func f(_ d: (Double) -> Double) -> Void {}
   func test() -> Void {
-    f({x in x}, 2) // expected-error {{generic parameter 'T' could not be inferred}}
+    f({x in x}, 2) // expected-error {{unable to infer type of a closure parameter 'x' in the current context}}
   }
   
   func g<T>(_ a: T, _ b: Int) -> Void {}
diff --git a/test/decl/typealias/generic.swift b/test/decl/typealias/generic.swift
@@ -68,7 +68,7 @@ typealias E<T1, T2> = Int  // expected-note {{generic type 'E' declared here}}
 // expected-note@-1 {{'T1' declared as parameter to type 'E'}}
 // expected-note@-2 {{'T2' declared as parameter to type 'E'}}
 
-typealias F<T1, T2> = (T1) -> T2 // expected-note {{'T1' declared as parameter to type 'F'}}
+typealias F<T1, T2> = (T1) -> T2
 
 // Type alias of type alias.
 typealias G<S1, S2> = A<S1, S2>
@@ -94,7 +94,7 @@ let _ : D<Int, Int, Float> = D(a: 1, b: 2)
 
 let _ : F = { (a : Int) -> Int in a }  // Infer the types of F
 
-let _ : F = { a in a } // expected-error {{generic parameter 'T1' could not be inferred}}
+let _ : F = { a in a } // expected-error {{unable to infer type of a closure parameter 'a' in the current context}}
 
 _ = MyType(a: "foo", b: 42)
 _ = A(a: "foo", b: 42)
diff --git a/test/expr/closure/anonymous.swift b/test/expr/closure/anonymous.swift
diff --git a/test/expr/closure/basic.swift b/test/expr/closure/basic.swift
diff --git a/test/expr/unary/keypath/salvage-with-other-type-errors.swift b/test/expr/unary/keypath/salvage-with-other-type-errors.swift
diff --git a/validation-test/compiler_crashers_2_fixed/0186-rdar46497155.swift b/validation-test/compiler_crashers_2_fixed/0186-rdar46497155.swift
