[CS] Allow bidirectional inference for ExprPatterns

Rather than using a conjunction, just form the
constraints for the match expression right away.

Author: hamishknight

include/swift/Sema/ConstraintSystem.h

@@ -4329,27 +4329,22 @@ class ConstraintSystem {
   /// \returns \c true if constraint generation failed, \c false otherwise
   bool generateConstraints(SingleValueStmtExpr *E);
 
-  /// Generate constraints for an array of ExprPatterns, forming a conjunction
-  /// that solves each expression in turn.
-  void generateConstraints(ArrayRef<ExprPattern *> exprPatterns,
-                           ConstraintLocatorBuilder locator);
-
   /// Generate constraints for the given (unchecked) expression.
   ///
   /// \returns a possibly-sanitized expression, or null if an error occurred.
   [[nodiscard]]
   Expr *generateConstraints(Expr *E, DeclContext *dc,
                             bool isInputExpression = true);
 
-  /// Generate constraints for binding the given pattern to the
-  /// value of the given expression.
+  /// Generate constraints for a given pattern.
   ///
-  /// \returns a possibly-sanitized initializer, or null if an error occurred.
+  /// \returns The type of the pattern, or \c None if a failure occured.
   [[nodiscard]]
-  Type generateConstraints(Pattern *P, ConstraintLocatorBuilder locator,
-                           bool bindPatternVarsOneWay,
-                           PatternBindingDecl *patternBinding,
-                           unsigned patternIndex);
+  Optional<Type> generateConstraints(Pattern *P,
+                                     ConstraintLocatorBuilder locator,
+                                     bool bindPatternVarsOneWay,
+                                     PatternBindingDecl *patternBinding,
+                                     unsigned patternIndex);
 
   /// Generate constraints for a statement condition.
   ///

lib/Sema/CSGen.cpp

@@ -2382,7 +2382,7 @@ namespace {
     /// for the types of each variable declared within the pattern, along
     /// with a one-way constraint binding that to the type to which the
     /// variable will be ascribed or inferred.
-    Type getTypeForPattern(
+    Optional<Type> getTypeForPattern(
        Pattern *pattern, ConstraintLocatorBuilder locator,
        bool bindPatternVarsOneWay,
        PatternBindingDecl *patternBinding = nullptr,
@@ -2406,14 +2406,21 @@ namespace {
             locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
             bindPatternVarsOneWay);
 
-        return setType(ParenType::get(CS.getASTContext(), underlyingType));
+        if (!underlyingType)
+          return None;
+
+        return setType(ParenType::get(CS.getASTContext(), *underlyingType));
       }
       case PatternKind::Binding: {
         auto *subPattern = cast<BindingPattern>(pattern)->getSubPattern();
         auto type = getTypeForPattern(subPattern, locator,
                                       bindPatternVarsOneWay);
+
+        if (!type)
+          return None;
+
         // Var doesn't affect the type.
-        return setType(type);
+        return setType(*type);
       }
       case PatternKind::Any: {
         Type type;
@@ -2587,6 +2594,9 @@ namespace {
 
         Type type = TypeChecker::typeCheckPattern(contextualPattern);
 
+        if (!type)
+          return None;
+
         // Look through reference storage types.
         type = type->getReferenceStorageReferent();
 
@@ -2598,16 +2608,19 @@ namespace {
         auto *subPattern = cast<TypedPattern>(pattern)->getSubPattern();
         // Determine the subpattern type. It will be convertible to the
         // ascribed type.
-        Type subPatternType = getTypeForPattern(
+        auto subPatternType = getTypeForPattern(
             subPattern,
             locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
             bindPatternVarsOneWay);
 
+        if (!subPatternType)
+          return None;
+
         // NOTE: The order here is important! Pattern matching equality is
         // not symmetric (we need to fix that either by using a different
         // constraint, or actually making it symmetric).
         CS.addConstraint(
-            ConstraintKind::Equal, openedType, subPatternType,
+            ConstraintKind::Equal, openedType, *subPatternType,
             locator.withPathElement(LocatorPathElt::PatternMatch(pattern)));
 
         // FIXME [OPAQUE SUPPORT]: the distinction between where we want opaque
@@ -2627,12 +2640,15 @@ namespace {
           auto &tupleElt = tuplePat->getElement(i);
 
           auto *eltPattern = tupleElt.getPattern();
-          Type eltTy = getTypeForPattern(
+          auto eltTy = getTypeForPattern(
               eltPattern,
               locator.withPathElement(LocatorPathElt::PatternMatch(eltPattern)),
               bindPatternVarsOneWay);
 
-          tupleTypeElts.push_back(TupleTypeElt(eltTy, tupleElt.getLabel()));
+          if (!eltTy)
+            return None;
+
+          tupleTypeElts.push_back(TupleTypeElt(*eltTy, tupleElt.getLabel()));
         }
 
         return setType(TupleType::get(tupleTypeElts, CS.getASTContext()));
@@ -2641,12 +2657,15 @@ namespace {
       case PatternKind::OptionalSome: {
         auto *subPattern = cast<OptionalSomePattern>(pattern)->getSubPattern();
         // The subpattern must have optional type.
-        Type subPatternType = getTypeForPattern(
+        auto subPatternType = getTypeForPattern(
             subPattern,
             locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
             bindPatternVarsOneWay);
 
-        return setType(OptionalType::get(subPatternType));
+        if (!subPatternType)
+          return None;
+
+        return setType(OptionalType::get(*subPatternType));
       }
 
       case PatternKind::Is: {
@@ -2675,12 +2694,14 @@ namespace {
               subPattern,
               locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
               bindPatternVarsOneWay);
+          if (!subPatternType)
+            return None;
 
           // NOTE: The order here is important! Pattern matching equality is
           // not symmetric (we need to fix that either by using a different
           // constraint, or actually making it symmetric).
           CS.addConstraint(
-              ConstraintKind::Equal, castType, subPatternType,
+              ConstraintKind::Equal, castType, *subPatternType,
               locator.withPathElement(LocatorPathElt::PatternMatch(pattern)));
         }
         return setType(isType);
@@ -2741,6 +2762,9 @@ namespace {
                 TypeResolverContext::InExpression, patternMatchLoc);
           }();
 
+          if (!parentType)
+            return None;
+
           // Perform member lookup into the parent's metatype.
           Type parentMetaType = MetatypeType::get(parentType);
           CS.addValueMemberConstraint(
@@ -2770,13 +2794,16 @@ namespace {
           // When there is a subpattern, the member will have function type,
           // and we're matching the type of that subpattern to the parameter
           // types.
-          Type subPatternType = getTypeForPattern(
+          auto subPatternType = getTypeForPattern(
               subPattern,
               locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)),
               bindPatternVarsOneWay);
 
+          if (!subPatternType)
+            return None;
+
           SmallVector<AnyFunctionType::Param, 4> params;
-          decomposeTuple(subPatternType, params);
+          decomposeTuple(*subPatternType, params);
 
           // Remove parameter labels; they aren't used when matching cases,
           // but outright conflicts will be checked during coercion.
@@ -2811,10 +2838,24 @@ namespace {
       }
 
       case PatternKind::Expr: {
-        // We generate constraints for ExprPatterns in a separate pass. For
-        // now, just create a type variable.
-        return setType(CS.createTypeVariable(CS.getConstraintLocator(locator),
-                                             TVO_CanBindToNoEscape));
+        auto *EP = cast<ExprPattern>(pattern);
+        Type patternTy = CS.createTypeVariable(CS.getConstraintLocator(locator),
+                                               TVO_CanBindToNoEscape);
+
+        auto target = SyntacticElementTarget::forExprPattern(EP);
+
+        if (CS.preCheckTarget(target, /*replaceInvalidRefWithErrors=*/true,
+                              /*leaveClosureBodyUnchecked=*/false)) {
+          return None;
+        }
+        CS.setType(EP->getMatchVar(), patternTy);
+
+        if (CS.generateConstraints(target))
+          return None;
+
+        CS.setTargetFor(EP, target);
+        CS.setExprPatternFor(EP->getSubExpr(), EP);
+        return setType(patternTy);
       }
       }
 
@@ -4197,10 +4238,14 @@ static bool generateInitPatternConstraints(ConstraintSystem &cs,
 
   Type patternType;
   if (auto pattern = target.getInitializationPattern()) {
-    patternType = cs.generateConstraints(
+    auto ty = cs.generateConstraints(
         pattern, locator, target.shouldBindPatternVarsOneWay(),
         target.getInitializationPatternBindingDecl(),
         target.getInitializationPatternBindingIndex());
+    if (!ty)
+      return true;
+
+    patternType = *ty;
   } else {
     patternType = cs.createTypeVariable(locator, TVO_CanBindToNoEscape);
   }
@@ -4359,7 +4404,7 @@ generateForEachStmtConstraints(ConstraintSystem &cs,
   // Collect constraints from the element pattern.
   auto elementLocator = cs.getConstraintLocator(
       sequenceExpr, ConstraintLocator::SequenceElementType);
-  Type initType =
+  auto initType =
       cs.generateConstraints(pattern, elementLocator,
                              target.shouldBindPatternVarsOneWay(), nullptr, 0);
   if (!initType)
@@ -4378,7 +4423,7 @@ generateForEachStmtConstraints(ConstraintSystem &cs,
     // resolving `optional object` constraint which is sometimes too eager.
     cs.addConstraint(ConstraintKind::Conversion, nextType,
                      OptionalType::get(elementType), elementTypeLoc);
-    cs.addConstraint(ConstraintKind::Conversion, elementType, initType,
+    cs.addConstraint(ConstraintKind::Conversion, elementType, *initType,
                      elementLocator);
   }
 
@@ -4404,7 +4449,7 @@ generateForEachStmtConstraints(ConstraintSystem &cs,
 
   // Populate all of the information for a for-each loop.
   forEachStmtInfo.elementType = elementType;
-  forEachStmtInfo.initType = initType;
+  forEachStmtInfo.initType = *initType;
   target.setPattern(pattern);
   target.getForEachStmtInfo() = forEachStmtInfo;
   return target;
@@ -4584,7 +4629,7 @@ bool ConstraintSystem::generateConstraints(
 
       // Generate constraints to bind all of the internal declarations
       // and verify the pattern.
-      Type patternType = generateConstraints(
+      auto patternType = generateConstraints(
           pattern, locator, /*shouldBindPatternVarsOneWay*/ true,
           target.getPatternBindingOfUninitializedVar(),
           target.getIndexOfUninitializedVar());
@@ -4613,25 +4658,13 @@ Expr *ConstraintSystem::generateConstraints(
   return generateConstraintsFor(*this, expr, dc);
 }
 
-Type ConstraintSystem::generateConstraints(
+Optional<Type> ConstraintSystem::generateConstraints(
     Pattern *pattern, ConstraintLocatorBuilder locator,
     bool bindPatternVarsOneWay, PatternBindingDecl *patternBinding,
     unsigned patternIndex) {
   ConstraintGenerator cg(*this, nullptr);
-  auto ty = cg.getTypeForPattern(pattern, locator, bindPatternVarsOneWay,
-                                 patternBinding, patternIndex);
-  assert(ty);
-
-  // Gather the ExprPatterns, and form a conjunction for their expressions.
-  SmallVector<ExprPattern *, 4> exprPatterns;
-  pattern->forEachNode([&](Pattern *P) {
-    if (auto *EP = dyn_cast<ExprPattern>(P))
-      exprPatterns.push_back(EP);
-  });
-  if (!exprPatterns.empty())
-    generateConstraints(exprPatterns, getConstraintLocator(pattern));
-
-  return ty;
+  return cg.getTypeForPattern(pattern, locator, bindPatternVarsOneWay,
+                              patternBinding, patternIndex);
 }
 
 bool ConstraintSystem::generateConstraints(StmtCondition condition,
@@ -4713,14 +4746,16 @@ bool ConstraintSystem::generateConstraints(
     // Generate constraints for the pattern, including one-way bindings for
     // any variables that show up in this pattern, because those variables
     // can be referenced in the guard expressions and the body.
-    Type patternType = generateConstraints(
+    auto patternType = generateConstraints(
         pattern, locator, /* bindPatternVarsOneWay=*/true,
         /*patternBinding=*/nullptr, /*patternBindingIndex=*/0);
+    if (!patternType)
+      return true;
 
     // Convert the subject type to the pattern, which establishes the
     // bindings.
     addConstraint(
-        ConstraintKind::Conversion, subjectType, patternType, locator);
+        ConstraintKind::Conversion, subjectType, *patternType, locator);
 
     // Generate constraints for the guard expression, if there is one.
     Expr *guardExpr = caseLabelItem.getGuardExpr();

lib/Sema/CSSimplify.cpp

@@ -5937,6 +5937,16 @@ bool ConstraintSystem::repairFailures(
     if (repairByConstructingRawRepresentableType(lhs, rhs))
       break;
 
+    // If this is for an initialization, but we don't have a contextual type,
+    // then this is a pattern match of something that isn't a TypedPattern. As
+    // such, this is more like a regular conversion than a contextual type
+    // conversion, and type mismatches ought be diagnosed elsewhere (e.g for an
+    // ExprPattern, we should diagnose an argument mismatch).
+    if (purpose == CTP_Initialization &&
+        getContextualTypeLoc(anchor).isNull()) {
+      break;
+    }
+
     conversionsOrFixes.push_back(IgnoreContextualType::create(
         *this, lhs, rhs, getConstraintLocator(locator)));
     break;