Merge pull request swiftlang#84003 from hamishknight/minesweeper

Author: hamishknight

lib/Sema/CSBindings.cpp

@@ -2681,8 +2681,11 @@ bool TypeVarBindingProducer::computeNext() {
       // let's have it try `Void` as well because there is an
       // implicit conversion `() -> T` to `() -> Void` and this
       // helps to avoid creating a thunk to support it.
+      // Avoid doing this is we already have a hole binding since
+      // introducing Void will just cause local solution ambiguities.
       if (getLocator()->isLastElement<LocatorPathElt::ClosureResult>() &&
-          binding.Kind == AllowedBindingKind::Supertypes) {
+          binding.Kind == AllowedBindingKind::Supertypes &&
+          !binding.BindingType->isPlaceholder()) {
         auto voidType = CS.getASTContext().TheEmptyTupleType;
         addNewBinding(binding.withSameSource(voidType, BindingKind::Exact));
       }

lib/Sema/CSGen.cpp

@@ -1716,8 +1716,13 @@ namespace {
         CS.recordFix(
             IgnoreInvalidASTNode::create(CS, CS.getConstraintLocator(locator)));
 
-        return CS.createTypeVariable(CS.getConstraintLocator(repr),
-                                     TVO_CanBindToHole);
+        // Immediately bind the result to a hole since we know it's invalid and
+        // want it to propagate rather than allowing other type variables to
+        // become holes.
+        auto *tv = CS.createTypeVariable(CS.getConstraintLocator(repr),
+                                         TVO_CanBindToHole);
+        CS.recordTypeVariablesAsHoles(tv);
+        return tv;
       }
       // Diagnose top-level usages of placeholder types.
       if (auto *ty = dyn_cast<PlaceholderTypeRepr>(repr->getWithoutParens())) {
@@ -4908,16 +4913,27 @@ bool ConstraintSystem::generateConstraints(
         return convertTypeLocator;
       };
 
-      // Substitute type variables in for placeholder types.
-      convertType = convertType.transformRec([&](Type type) -> std::optional<Type> {
-        if (type->is<PlaceholderType>()) {
-          return Type(createTypeVariable(getLocator(type),
-                                         TVO_CanBindToNoEscape |
-                                             TVO_PrefersSubtypeBinding |
-                                             TVO_CanBindToHole));
-        }
-        return std::nullopt;
-      });
+      // If the contextual type has an error, we can't apply the solution.
+      // Record a fix for an invalid AST node.
+      if (convertType->hasError())
+        recordFix(IgnoreInvalidASTNode::create(*this, convertTypeLocator));
+
+      // Substitute type variables in for placeholder and error types.
+      convertType =
+          convertType.transformRec([&](Type type) -> std::optional<Type> {
+            if (!isa<PlaceholderType, ErrorType>(type.getPointer()))
+              return std::nullopt;
+
+            auto flags = TVO_CanBindToNoEscape | TVO_PrefersSubtypeBinding |
+                         TVO_CanBindToHole;
+            auto tv = Type(createTypeVariable(getLocator(type), flags));
+            // For ErrorTypes we want to eagerly bind to a hole since we
+            // know this is where the issue is.
+            if (isa<ErrorType>(type.getPointer())) {
+              recordTypeVariablesAsHoles(tv);
+            }
+            return tv;
+          });
 
       addContextualConversionConstraint(expr, convertType, ctp,
                                         convertTypeLocator);

lib/Sema/CSSimplify.cpp

@@ -5569,6 +5569,17 @@ bool ConstraintSystem::repairFailures(
     return true;
   };
 
+  // Propagate a hole from one type to another. This is useful for contextual
+  // types since type resolution forms a top-level ErrorType for types with
+  // nested errors, e.g `S<@error_type>` becomes `@error_type`. As such, when
+  // matching `S<$T0> == $T1` where `$T1` is a hole from a contextual type, we
+  // want to eagerly turn `$T0` into a hole since it's likely that `$T1` would
+  // have provided the contextual info for it.
+  auto tryPropagateHole = [&](Type from, Type to) {
+    if (from->isPlaceholder() && to->hasTypeVariable())
+      recordTypeVariablesAsHoles(to);
+  };
+
   if (path.empty()) {
     if (!anchor)
       return false;
@@ -6342,6 +6353,13 @@ bool ConstraintSystem::repairFailures(
 
   case ConstraintLocator::ClosureBody:
   case ConstraintLocator::ClosureResult: {
+    // If either type is a placeholder, consider this fixed, eagerly propagating
+    // a hole from the contextual type.
+    if (lhs->isPlaceholder() || rhs->isPlaceholder()) {
+      tryPropagateHole(rhs, lhs);
+      return true;
+    }
+
     if (repairByInsertingExplicitCall(lhs, rhs))
       break;
 
@@ -6361,9 +6379,12 @@ bool ConstraintSystem::repairFailures(
   }
 
   case ConstraintLocator::ContextualType: {
-    // If either type is a placeholder, consider this fixed
-    if (lhs->isPlaceholder() || rhs->isPlaceholder())
+    // If either type is a placeholder, consider this fixed, eagerly propagating
+    // a hole from the contextual type.
+    if (lhs->isPlaceholder() || rhs->isPlaceholder()) {
+      tryPropagateHole(rhs, lhs);
       return true;
+    }
 
     // If either side is not yet resolved, it's too early for this fix.
     if (lhs->isTypeVariableOrMember() || rhs->isTypeVariableOrMember())
@@ -7002,6 +7023,13 @@ bool ConstraintSystem::repairFailures(
   }
 
   case ConstraintLocator::CoercionOperand: {
+    // If either type is a placeholder, consider this fixed, eagerly propagating
+    // a hole from the contextual type.
+    if (lhs->isPlaceholder() || rhs->isPlaceholder()) {
+      tryPropagateHole(rhs, lhs);
+      return true;
+    }
+
     auto *coercion = castToExpr<CoerceExpr>(anchor);
 
     // Coercion from T.Type to T.Protocol.

lib/Sema/CSSyntacticElement.cpp

@@ -845,12 +845,6 @@ class SyntacticElementConstraintGenerator
         ContextualPattern::forPatternBindingDecl(patternBinding, index);
     Type patternType = TypeChecker::typeCheckPattern(contextualPattern);
 
-    // Fail early if pattern couldn't be type-checked.
-    if (!patternType || patternType->hasError()) {
-      hadError = true;
-      return;
-    }
-
     auto target = getTargetForPattern(patternBinding, index, patternType);
     if (!target) {
       hadError = true;

lib/Sema/TypeCheckConstraints.cpp

@@ -876,11 +876,6 @@ bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD,
       auto contextualPattern = ContextualPattern::forRawPattern(pattern, DC);
       patternType = typeCheckPattern(contextualPattern);
     }
-
-    if (patternType->hasError()) {
-      PBD->setInvalid();
-      return true;
-    }
   }
 
   bool hadError = TypeChecker::typeCheckBinding(pattern, init, DC, patternType,

lib/Sema/TypeCheckStmt.cpp

@@ -1075,7 +1075,7 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
     assert(TheFunc && "Should have bailed from pre-check if this is None");
 
     Type ResultTy = TheFunc->getBodyResultType();
-    if (!ResultTy || ResultTy->hasError())
+    if (!ResultTy)
       return nullptr;
 
     if (!RS->hasResult()) {

lib/Sema/TypeOfReference.cpp

@@ -273,16 +273,32 @@ class InferableTypeOpener final {
     return substTy;
   }
 
+  Type transformErrorType(ErrorType *errTy) {
+    // For ErrorTypes we want to eagerly bind to a hole since we know this is
+    // where the issue is.
+    auto *tv = createTypeVariable(cs.getConstraintLocator(locator));
+    cs.recordTypeVariablesAsHoles(tv);
+    return tv;
+  }
+
   Type transform(Type type) {
     if (!type)
       return type;
-    if (!type->hasUnboundGenericType() && !type->hasPlaceholder())
+    if (!type->hasUnboundGenericType() && !type->hasPlaceholder() &&
+        !type->hasError()) {
       return type;
-
+    }
+    // If the type has an error, we can't apply the solution. Record a fix for
+    // an invalid AST node.
+    if (type->hasError()) {
+      cs.recordFix(
+          IgnoreInvalidASTNode::create(cs, cs.getConstraintLocator(locator)));
+    }
     return type.transformRec([&](Type type) -> std::optional<Type> {
-      if (!type->hasUnboundGenericType() && !type->hasPlaceholder())
+      if (!type->hasUnboundGenericType() && !type->hasPlaceholder() &&
+          !type->hasError()) {
         return type;
-
+      }
       auto *tyPtr = type.getPointer();
       if (auto unbound = dyn_cast<UnboundGenericType>(tyPtr))
         return transformUnboundGenericType(unbound);
@@ -292,6 +308,8 @@ class InferableTypeOpener final {
         return transformBoundGenericType(genericTy);
       if (auto *aliasTy = dyn_cast<TypeAliasType>(tyPtr))
         return transformTypeAliasType(aliasTy);
+      if (auto *errTy = dyn_cast<ErrorType>(tyPtr))
+        return transformErrorType(errTy);
 
       return std::nullopt;
     });

test/ClangImporter/overlay.swift

@@ -19,3 +19,4 @@ Redeclaration.NSStringToNSString(AppKit.NSStringToNSString("abc")) // expected-w
 
 let viaStruct: Redeclaration.FooStruct1 = AppKit.FooStruct1()
 let forwardDecl: Redeclaration.Tribool = AppKit.Tribool() // expected-error {{no type named 'Tribool' in module 'Redeclaration'}}
+// expected-error@-1 {{missing argument for parameter #1 in call}}

test/Constraints/generics.swift

@@ -1088,3 +1088,28 @@ do {
       f(x, { $0 }, { _ in Task {} }) // expected-warning {{result of 'Task<E>' initializer is unused}}
   }
 }
+
+func testHolePropagation() {
+  struct S<T: P> {}
+  struct R {}
+
+  // The hole from the contextual type should propagate such that we don't
+  // complain about not being able to infer 'T'.
+  _ = { () -> S<R> in S() } // expected-error {{type 'R' does not conform to protocol 'P'}}
+  _ = { () -> S<R> in return S() } // expected-error {{type 'R' does not conform to protocol 'P'}}
+  _ = { () -> S<R> in (); return S() } // expected-error {{type 'R' does not conform to protocol 'P'}}
+
+  let _: () -> S<R> = { S() } // expected-error {{type 'R' does not conform to protocol 'P'}}
+  let _: () -> S<R> = { return S() } // expected-error {{type 'R' does not conform to protocol 'P'}}
+  let _: () -> S<R> = { (); return S() } // expected-error {{type 'R' does not conform to protocol 'P'}}
+
+  _ = { S() }() as S<R> // expected-error {{type 'R' does not conform to protocol 'P'}}
+  _ = { return S() }() as S<R> // expected-error {{type 'R' does not conform to protocol 'P'}}
+  _ = { (); return S() } as () -> S<R> // expected-error {{type 'R' does not conform to protocol 'P'}}
+
+  func makeT<T>() -> T {}
+
+  _ = { () -> (S<R>, Int) in (makeT(), 0) } // expected-error {{type 'R' does not conform to protocol 'P'}}
+  _ = { () -> (S<R>, Int) in return (makeT(), 0) } // expected-error {{type 'R' does not conform to protocol 'P'}}
+  _ = { () -> (S<R>, Int) in (); return (makeT(), 0) } // expected-error {{type 'R' does not conform to protocol 'P'}}
+}

test/Constraints/invalid_constraint_lookup.swift

@@ -27,4 +27,5 @@ protocol Collection : _Collection, Sequence {
 }
 func insertionSort<C: Mutable> (_ elements: inout C, i: C.Index) { // expected-error {{cannot find type 'Mutable' in scope}} expected-error {{'Index' is not a member type of type 'C'}}
   var x: C.Iterator.Element = elements[i] // expected-error {{'Iterator' is not a member type of type 'C'}}
+  // expected-error@-1 {{value of type 'C' has no subscripts}}
 }