[CS] Improve handling of holes for Named/AnyPatterns

Rather than eagerly binding them to holes if the
sequence element type ends up being Any, let's
record the CollectionElementContextualMismatch fix,
and then if the patterns end up becoming holes,
skip penalizing them if we know the fix was
recorded. This avoids prematurely turning type
variables for ExprPatterns into holes, which
should be able to get better bindings from the
expression provided. Also this means we'll apply
the logic to non-Any sequence types, which
previously we would give a confusing diagnostic
to.

Author: hamishknight

include/swift/Sema/ConstraintLocator.h

@@ -18,11 +18,12 @@
 #ifndef SWIFT_SEMA_CONSTRAINTLOCATOR_H
 #define SWIFT_SEMA_CONSTRAINTLOCATOR_H
 
-#include "swift/Basic/Debug.h"
-#include "swift/Basic/LLVM.h"
 #include "swift/AST/ASTNode.h"
 #include "swift/AST/Type.h"
 #include "swift/AST/Types.h"
+#include "swift/Basic/Debug.h"
+#include "swift/Basic/LLVM.h"
+#include "swift/Basic/NullablePtr.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/PointerIntPair.h"
@@ -313,6 +314,10 @@ class ConstraintLocator : public llvm::FoldingSetNode {
   /// branch, and if so, the kind of branch.
   Optional<SingleValueStmtBranchKind> isForSingleValueStmtBranch() const;
 
+  /// If the locator in question is for a pattern match, returns the pattern,
+  /// otherwise \c nullptr.
+  NullablePtr<Pattern> getPatternMatch() const;
+
   /// Returns true if \p locator is ending with either of the following
   ///  - Member
   ///  - Member -> KeyPathDynamicMember

lib/Sema/CSBindings.cpp

@@ -2203,16 +2203,25 @@ TypeVariableBinding::fixForHole(ConstraintSystem &cs) const {
     return std::make_pair(fix, /*impact=*/(unsigned)10);
   }
 
-  if (auto pattern = getAsPattern(dstLocator->getAnchor())) {
-    if (dstLocator->getPath().size() == 1 &&
-        dstLocator->isLastElement<LocatorPathElt::PatternDecl>()) {
+  if (auto pattern = dstLocator->getPatternMatch()) {
+    if (dstLocator->isLastElement<LocatorPathElt::PatternDecl>()) {
+      // If this is the pattern in a for loop, and we have a mismatch of the
+      // element type, then we don't have any useful contextual information
+      // for the pattern, and can just bind to a hole without needing to penalize
+      // the solution further.
+      auto *seqLoc = cs.getConstraintLocator(
+          dstLocator->getAnchor(), ConstraintLocator::SequenceElementType);
+      if (cs.hasFixFor(seqLoc,
+                       FixKind::IgnoreCollectionElementContextualMismatch)) {
+        return None;
+      }
       // Not being able to infer the type of a variable in a pattern binding
       // decl is more dramatic than anything that could happen inside the
       // expression because we want to preferrably point the diagnostic to a
       // part of the expression that caused us to be unable to infer the
       // variable's type.
       ConstraintFix *fix =
-          IgnoreUnresolvedPatternVar::create(cs, pattern, dstLocator);
+          IgnoreUnresolvedPatternVar::create(cs, pattern.get(), dstLocator);
       return std::make_pair(fix, /*impact=*/(unsigned)100);
     }
   }

lib/Sema/CSGen.cpp

@@ -2471,6 +2471,9 @@ namespace {
                      : nullptr;
         };
 
+        auto matchLoc =
+            locator.withPathElement(LocatorPathElt::PatternMatch(pattern));
+
         // Always prefer a contextual type when it's available.
         if (externalPatternType) {
           type = externalPatternType;
@@ -2479,8 +2482,8 @@ namespace {
           type = CS.getType(initializer)->getRValueType();
         } else {
           type = CS.createTypeVariable(
-              CS.getConstraintLocator(pattern,
-                                      ConstraintLocator::AnyPatternDecl),
+              CS.getConstraintLocator(matchLoc,
+                                      LocatorPathElt::AnyPatternDecl()),
               TVO_CanBindToNoEscape | TVO_CanBindToHole);
         }
         return setType(type);
@@ -2515,9 +2518,12 @@ namespace {
           }
         }
 
+        auto matchLoc =
+            locator.withPathElement(LocatorPathElt::PatternMatch(pattern));
+
         if (!varType) {
           varType = CS.createTypeVariable(
-              CS.getConstraintLocator(pattern,
+              CS.getConstraintLocator(matchLoc,
                                       LocatorPathElt::NamedPatternDecl()),
               TVO_CanBindToNoEscape | TVO_CanBindToHole);
 

lib/Sema/CSSimplify.cpp

@@ -6228,16 +6228,6 @@ bool ConstraintSystem::repairFailures(
     // `Int` vs. `(_, _)`.
     recordAnyTypeVarAsPotentialHole(rhs);
 
-    // If the element type is `Any` i.e. `for (x, y) in [] { ... }`
-    // it would never match and the pattern (`rhs` = `(x, y)`)
-    // doesn't have any other source of contextual information,
-    // so instead of waiting for elements to become holes with an
-    // unrelated fixes, let's proactively bind all of the pattern
-    // elemnts to holes here.
-    if (lhs->isAny()) {
-      recordTypeVariablesAsHoles(rhs);
-    }
-
     conversionsOrFixes.push_back(CollectionElementContextualMismatch::create(
         *this, lhs, rhs, getConstraintLocator(locator)));
     break;

lib/Sema/ConstraintLocator.cpp

@@ -669,6 +669,14 @@ ConstraintLocator::isForSingleValueStmtBranch() const {
   return SingleValueStmtBranchKind::Regular;
 }
 
+NullablePtr<Pattern> ConstraintLocator::getPatternMatch() const {
+  auto matchElt = findLast<LocatorPathElt::PatternMatch>();
+  if (!matchElt)
+    return nullptr;
+
+  return matchElt->getPattern();
+}
+
 bool ConstraintLocator::isMemberRef() const {
   if (isLastElement<LocatorPathElt::Member>()) {
     return true;

test/stmt/foreach.swift

@@ -268,3 +268,8 @@ do {
   for (x, y, z) in [] { // expected-error {{tuple pattern cannot match values of non-tuple type 'Any'}}
   }
 }
+
+do {
+  // https://github.com/apple/swift/issues/65650 - Make sure we say 'String', not 'Any'.
+  for (x, y) in [""] {} // expected-error {{tuple pattern cannot match values of non-tuple type 'String'}}
+}

validation-test/Sema/type_checker_crashers_fixed/issue-65360.swift

@@ -16,7 +16,5 @@ let _: () -> Void = {
 let _: () -> Void = {
   for case (0)? in [a] {}
   for case (0, 0) in [a] {}
-  // expected-error@-1 {{conflicting arguments to generic parameter 'Elements' ('[Any]' vs. '[Any?]')}}
-  // expected-error@-2 {{conflicting arguments to generic parameter 'Element' ('Any' vs. 'Any?')}}
-  // expected-error@-3 {{conflicting arguments to generic parameter 'Self' ('[Any]' vs. '[Any?]')}}
+  // expected-error@-1 {{cannot convert value of type 'Any?' to expected element type '(_, _)'}}
 }