Merge pull request #65366 from xedin/issue-65360

[CSSolver] Fix a crash in diagnostics related to pattern matching

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -3337,6 +3337,13 @@ class ConstraintSystem {
   void recordPotentialHole(TypeVariableType *typeVar);
   void recordAnyTypeVarAsPotentialHole(Type type);
 
+  /// Record all unbound type variables that occur in the given type
+  /// as being bound to "hole" type represented by \c PlaceholderType
+  /// in this constraint system.
+  ///
+  /// \param type The type on which to holeify.
+  void recordTypeVariablesAsHoles(Type type);
+
   void recordMatchCallArgumentResult(ConstraintLocator *locator,
                                      MatchCallArgumentResult result);
 

lib/Sema/BuilderTransform.cpp

@@ -1174,13 +1174,7 @@ ConstraintSystem::matchResultBuilder(AnyFunctionRef fn, Type builderType,
 
       if (auto *closure =
               getAsExpr<ClosureExpr>(fn.getAbstractClosureExpr())) {
-        auto closureTy = getClosureType(closure);
-        simplifyType(closureTy).visit([&](Type componentTy) {
-          if (auto *typeVar = componentTy->getAs<TypeVariableType>()) {
-            assignFixedType(typeVar,
-                            PlaceholderType::get(getASTContext(), typeVar));
-          }
-        });
+        recordTypeVariablesAsHoles(getClosureType(closure));
       }
 
       return getTypeMatchSuccess();

lib/Sema/CSGen.cpp

@@ -2768,8 +2768,9 @@ namespace {
         // correct handling of patterns like `_ as Foo` where `_` would
         // get a type of `Foo` but `is` pattern enclosing it could still be
         // inferred from enclosing context.
-        auto isType = CS.createTypeVariable(CS.getConstraintLocator(pattern),
-                                            TVO_CanBindToNoEscape);
+        auto isType =
+            CS.createTypeVariable(CS.getConstraintLocator(pattern),
+                                  TVO_CanBindToNoEscape | TVO_CanBindToHole);
         CS.addConstraint(
             ConstraintKind::Conversion, subPatternType, isType,
             locator.withPathElement(LocatorPathElt::PatternMatch(pattern)));
@@ -2919,8 +2920,8 @@ namespace {
         // TODO: we could try harder here, e.g. for enum elements to provide the
         // enum type.
         return setType(
-            CS.createTypeVariable(
-              CS.getConstraintLocator(locator), TVO_CanBindToNoEscape));
+            CS.createTypeVariable(CS.getConstraintLocator(locator),
+                                  TVO_CanBindToNoEscape | TVO_CanBindToHole));
       }
 
       llvm_unreachable("Unhandled pattern kind");

lib/Sema/CSSimplify.cpp

@@ -6159,12 +6159,7 @@ bool ConstraintSystem::repairFailures(
     // 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));
-        }
-      });
+      recordTypeVariablesAsHoles(rhs);
     }
 
     conversionsOrFixes.push_back(CollectionElementContextualMismatch::create(
@@ -10402,8 +10397,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
             // equality instead of argument application constraint, so allowing
             // them to bind member could mean missing valid hole positions in
             // the pattern.
-            assignFixedType(memberTypeVar, PlaceholderType::get(getASTContext(),
-                                                                memberTypeVar));
+            recordTypeVariablesAsHoles(memberTypeVar);
           } else {
             recordPotentialHole(memberTypeVar);
           }
@@ -14008,8 +14002,24 @@ void ConstraintSystem::recordAnyTypeVarAsPotentialHole(Type type) {
     return;
 
   type.visit([&](Type type) {
-    if (auto *typeVar = type->getAs<TypeVariableType>())
+    if (auto *typeVar = type->getAs<TypeVariableType>()) {
       typeVar->getImpl().enableCanBindToHole(getSavedBindings());
+    }
+  });
+}
+
+void ConstraintSystem::recordTypeVariablesAsHoles(Type type) {
+  type.visit([&](Type componentTy) {
+    if (auto *typeVar = componentTy->getAs<TypeVariableType>()) {
+      // Ignore bound type variables. This can happen if a type variable
+      // occurs in multiple positions and/or  if type hasn't been fully
+      // simplified before this call.
+      if (typeVar->getImpl().hasRepresentativeOrFixed())
+        return;
+
+      assignFixedType(typeVar,
+                      PlaceholderType::get(getASTContext(), typeVar));
+    }
   });
 }
 

lib/Sema/CSStep.cpp

@@ -1103,13 +1103,6 @@ void ConjunctionStep::SolverSnapshot::applySolution(const Solution &solution) {
   if (score.Data[SK_Fix] == 0)
     return;
 
-  auto holeify = [&](Type componentTy) {
-    if (auto *typeVar = componentTy->getAs<TypeVariableType>()) {
-      CS.assignFixedType(
-          typeVar, PlaceholderType::get(CS.getASTContext(), typeVar));
-    }
-  };
-
   // If this conjunction represents a closure and inference
   // has failed, let's bind all of unresolved type variables
   // in its interface type to holes to avoid extraneous
@@ -1118,14 +1111,13 @@ void ConjunctionStep::SolverSnapshot::applySolution(const Solution &solution) {
   if (locator->directlyAt<ClosureExpr>()) {
     auto closureTy =
         CS.getClosureType(castToExpr<ClosureExpr>(locator->getAnchor()));
-
-    CS.simplifyType(closureTy).visit(holeify);
+    CS.recordTypeVariablesAsHoles(closureTy);
   }
 
   // Same for a SingleValueStmtExpr, turn any unresolved type variables present
   // in its type into holes.
   if (locator->isForSingleValueStmtConjunction()) {
     auto *SVE = castToExpr<SingleValueStmtExpr>(locator->getAnchor());
-    CS.simplifyType(CS.getType(SVE)).visit(holeify);
+    CS.recordTypeVariablesAsHoles(CS.getType(SVE));
   }
 }

test/expr/closure/multi_statement.swift

@@ -336,9 +336,10 @@ func test_unknown_refs_in_tilde_operator() {
   enum E {
   }
 
-  let _: (E) -> Void = { // expected-error {{unable to infer closure type in the current context}}
+  let _: (E) -> Void = {
     if case .test(unknown) = $0 {
       // expected-error@-1 2 {{cannot find 'unknown' in scope}}
+      // expected-error@-2 {{type 'E' has no member 'test'}}
     }
   }
 }

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

@@ -0,0 +1,22 @@
+// RUN: %target-typecheck-verify-swift
+
+var a: Any?
+
+let _: () -> Void = {
+  for case (is Int)? in [a] {}
+  if case (is Int, is Int) = a {} // expected-error {{cannot convert value of type 'Any?' to specified type '(_, _)'}}
+}
+
+let _: () -> Void = {
+  for case (0)? in [a] {}
+  if case (0, 0) = a {}
+  // expected-error@-1 {{cannot convert value of type 'Any?' to specified type '(_, _)}}
+}
+
+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?]')}}
+}

validation-test/Sema/type_checker_crashers_fixed/rdar92327807.swift

@@ -10,7 +10,7 @@ func test(result: MyEnum, optResult: MyEnum?) {
   }
 
   if let .co(42) = result { // expected-error {{pattern matching in a condition requires the 'case' keyword}}
-    // expected-error@-1 {{type of expression is ambiguous without more context}}
+    // expected-error@-1 {{type 'MyEnum' has no member 'co'}}
   }
 
   if let .co = optResult { // expected-error {{pattern matching in a condition requires the 'case' keyword}}