Merge pull request #61796 from xedin/rdar-100343275

[CSSimplify] Bind for-in patterns to holes if element type is `Any`

Author: xedin

lib/Sema/CSDiagnostics.cpp

@@ -5774,13 +5774,42 @@ bool CollectionElementContextualFailure::diagnoseAsError() {
   auto eltType = getFromType();
   auto contextualType = getToType();
 
+  auto diagnoseSingleElement = [&](Diag<Type, Type> msg, Type eltType,
+                                   Type contextualType) {
+    auto diagnostic = emitDiagnostic(msg, eltType, contextualType);
+    (void)trySequenceSubsequenceFixIts(diagnostic);
+  };
+
   auto diagnoseAllOccurrences = [&](Diag<Type, Type> diagnostic) {
     assert(AffectedElements.size() > 1);
     for (auto *element : AffectedElements) {
       emitDiagnosticAt(element->getLoc(), diagnostic, eltType, contextualType);
     }
   };
 
+  if (locator->isForSequenceElementType()) {
+    auto purpose = FailureDiagnostic::getContextualTypePurpose(getAnchor());
+    // If this is a conversion failure related to binding of `for-each`
+    // statement it has to be diagnosed as pattern match if there are
+    // holes present in the contextual type.
+    if ((purpose == ContextualTypePurpose::CTP_ForEachStmt ||
+         purpose == ContextualTypePurpose::CTP_ForEachSequence) &&
+        contextualType->hasUnresolvedType()) {
+      auto diagnostic = emitDiagnostic(
+          (contextualType->is<TupleType>() && !eltType->is<TupleType>())
+              ? diag::cannot_match_expr_tuple_pattern_with_nontuple_value
+              : diag::cannot_match_unresolved_expr_pattern_with_value,
+          eltType);
+      (void)trySequenceSubsequenceFixIts(diagnostic);
+    } else {
+      diagnoseSingleElement(contextualType->isExistentialType()
+                                ? diag::cannot_convert_sequence_element_protocol
+                                : diag::cannot_convert_sequence_element_value,
+                            eltType, contextualType);
+    }
+    return true;
+  }
+
   auto isFixedToDictionary = [&](ArrayExpr *anchor) {
     return llvm::any_of(getSolution().Fixes, [&](ConstraintFix *fix) {
       auto *fixAnchor = getAsExpr<ArrayExpr>(fix->getAnchor());
@@ -5790,16 +5819,16 @@ bool CollectionElementContextualFailure::diagnoseAsError() {
   };
 
   bool treatAsDictionary = false;
-  Optional<InFlightDiagnostic> diagnostic;
   if (auto *AE = getAsExpr<ArrayExpr>(anchor)) {
     if (!(treatAsDictionary = isFixedToDictionary(AE))) {
       if (AffectedElements.size() > 1) {
         diagnoseAllOccurrences(diag::cannot_convert_array_element);
         return true;
       }
 
-      diagnostic.emplace(emitDiagnostic(diag::cannot_convert_array_element,
-                                        eltType, contextualType));
+      diagnoseSingleElement(diag::cannot_convert_array_element, eltType,
+                            contextualType);
+      return true;
     }
   }
 
@@ -5812,9 +5841,9 @@ bool CollectionElementContextualFailure::diagnoseAsError() {
         return true;
       }
 
-      diagnostic.emplace(emitDiagnostic(diag::cannot_convert_dict_key, eltType,
-                                        contextualType));
-      break;
+      diagnoseSingleElement(diag::cannot_convert_dict_key, eltType,
+                            contextualType);
+      return true;
     }
 
     case 1: { // value
@@ -5823,43 +5852,17 @@ bool CollectionElementContextualFailure::diagnoseAsError() {
         return true;
       }
 
-      diagnostic.emplace(emitDiagnostic(diag::cannot_convert_dict_value,
-                                        eltType, contextualType));
-      break;
+      diagnoseSingleElement(diag::cannot_convert_dict_value, eltType,
+                            contextualType);
+      return true;
     }
 
     default:
       break;
     }
   }
 
-  if (locator->isForSequenceElementType()) {
-    auto purpose = FailureDiagnostic::getContextualTypePurpose(getAnchor());
-    // If this is a conversion failure related to binding of `for-each`
-    // statement it has to be diagnosed as pattern match if there are
-    // holes present in the contextual type.
-    if ((purpose == ContextualTypePurpose::CTP_ForEachStmt ||
-         purpose == ContextualTypePurpose::CTP_ForEachSequence) &&
-        contextualType->hasUnresolvedType()) {
-      diagnostic.emplace(emitDiagnostic(
-          (contextualType->is<TupleType>() && !eltType->is<TupleType>())
-              ? diag::cannot_match_expr_tuple_pattern_with_nontuple_value
-              : diag::cannot_match_unresolved_expr_pattern_with_value,
-          eltType));
-    } else {
-      diagnostic.emplace(
-          emitDiagnostic(contextualType->isExistentialType()
-                             ? diag::cannot_convert_sequence_element_protocol
-                             : diag::cannot_convert_sequence_element_value,
-                         eltType, contextualType));
-    }
-  }
-
-  if (!diagnostic)
-    return false;
-
-  (void)trySequenceSubsequenceFixIts(*diagnostic);
-  return true;
+  return false;
 }
 
 bool MissingContextualConformanceFailure::diagnoseAsError() {

lib/Sema/CSSimplify.cpp

@@ -6011,6 +6011,12 @@ bool ConstraintSystem::repairFailures(
   }
 
   case ConstraintLocator::SequenceElementType: {
+    if (lhs->isPlaceholder() || rhs->isPlaceholder()) {
+      recordAnyTypeVarAsPotentialHole(lhs);
+      recordAnyTypeVarAsPotentialHole(rhs);
+      return true;
+    }
+
     // This is going to be diagnosed as `missing conformance`,
     // so no need to create duplicate fixes.
     if (rhs->isExistentialType())
@@ -6022,6 +6028,21 @@ 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()) {
+      rhs.visit([&](Type type) {
+        if (auto *typeVar = type->getAs<TypeVariableType>()) {
+          assignFixedType(typeVar,
+                          PlaceholderType::get(getASTContext(), typeVar));
+        }
+      });
+    }
+
     conversionsOrFixes.push_back(CollectionElementContextualMismatch::create(
         *this, lhs, rhs, getConstraintLocator(locator)));
     break;
@@ -13829,6 +13850,14 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
         impact = 10;
     }
 
+    // Increase impact of invalid conversions to `Any` and `AnyHashable`
+    // associated with collection elements (i.e. for-in sequence element)
+    // because it means that other side is structurally incompatible.
+    if (fix->getKind() == FixKind::IgnoreCollectionElementContextualMismatch) {
+      if (type2->isAny() || type2->isAnyHashable())
+        ++impact;
+    }
+
     if (recordFix(fix, impact))
       return SolutionKind::Error;
 

lib/Sema/CSStep.h

@@ -632,6 +632,14 @@ class TypeVariableStep final : public BindingStep<TypeVarBindingProducer> {
   bool attempt(const TypeVariableBinding &choice) override;
 
   bool shouldSkip(const TypeVariableBinding &choice) const override {
+    // Let's always attempt types inferred from "defaultable" constraints
+    // in diagnostic mode. This allows the solver to attempt i.e. `Any`
+    // for collection literals and produce better diagnostics for for-in
+    // statements like `for (x, y, z) in [] { ... }` when pattern type
+    // could not be inferred.
+    if (CS.shouldAttemptFixes())
+      return false;
+
     // If this is a defaultable binding and we have found solutions,
     // don't explore the default binding.
     return AnySolved && choice.isDefaultable();

test/stmt/foreach.swift

@@ -262,3 +262,9 @@ do {
   for (x, y) in (0, 0) {}
   // expected-error@-1 {{for-in loop requires '(Int, Int)' to conform to 'Sequence'}}
 }
+
+// rdar://100343275 - Sema is accepting incorrect code which leads to a crash in SILGen
+do {
+  for (x, y, z) in [] { // expected-error {{tuple pattern cannot match values of non-tuple type 'Any'}}
+  }
+}