Merge pull request #28986 from DougGregor/for-each-pattern-cleanup

[Type checker] Move for-each pattern checking logic into the solver.

Author: DougGregor

lib/Sema/TypeCheckConstraints.cpp

@@ -2892,13 +2892,15 @@ bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD,
   return hadError;
 }
 
-auto TypeChecker::typeCheckForEachBinding(
-    DeclContext *dc, ForEachStmt *stmt) -> Optional<ForEachBinding> {
+bool TypeChecker::typeCheckForEachBinding(DeclContext *dc, ForEachStmt *stmt) {
   /// Type checking listener for for-each binding.
   class BindingListener : public ExprTypeCheckListener {
     /// The for-each statement.
     ForEachStmt *Stmt;
 
+    /// The declaration context in which this for-each statement resides.
+    DeclContext *DC;
+
     /// The locator we're using.
     ConstraintLocator *Locator;
 
@@ -2927,7 +2929,8 @@ auto TypeChecker::typeCheckForEachBinding(
     Type IteratorType;
 
   public:
-    explicit BindingListener(ForEachStmt *stmt) : Stmt(stmt) { }
+    explicit BindingListener(ForEachStmt *stmt, DeclContext *dc)
+        : Stmt(stmt), DC(dc) { }
 
     bool builtConstraints(ConstraintSystem &cs, Expr *expr) override {
       // Save the locator we're using for the expression.
@@ -2957,6 +2960,25 @@ auto TypeChecker::typeCheckForEachBinding(
       auto elementLocator = cs.getConstraintLocator(
           ContextualLocator, ConstraintLocator::SequenceElementType);
 
+      // Check the element pattern.
+      ASTContext &ctx = cs.getASTContext();
+      if (auto *P = TypeChecker::resolvePattern(Stmt->getPattern(), DC,
+                                                /*isStmtCondition*/false)) {
+        Stmt->setPattern(P);
+      } else {
+        Stmt->getPattern()->setType(ErrorType::get(ctx));
+        return true;
+      }
+
+      TypeResolutionOptions options(TypeResolverContext::InExpression);
+      options |= TypeResolutionFlags::AllowUnspecifiedTypes;
+      options |= TypeResolutionFlags::AllowUnboundGenerics;
+      if (TypeChecker::typeCheckPattern(Stmt->getPattern(), DC, options)) {
+        // FIXME: Handle errors better.
+        Stmt->getPattern()->setType(ErrorType::get(ctx));
+        return true;
+      }
+
       // Collect constraints from the element pattern.
       auto pattern = Stmt->getPattern();
       InitType = cs.generateConstraints(pattern, elementLocator);
@@ -2985,13 +3007,12 @@ auto TypeChecker::typeCheckForEachBinding(
       }
 
       // Reference the makeIterator witness.
-      ASTContext &ctx = cs.getASTContext();
       FuncDecl *makeIterator = ctx.getSequenceMakeIterator();
       Type makeIteratorType =
           cs.createTypeVariable(Locator, TVO_CanBindToNoEscape);
       cs.addValueWitnessConstraint(
           LValueType::get(SequenceType), makeIterator,
-          makeIteratorType, cs.DC, FunctionRefKind::Compound,
+          makeIteratorType, DC, FunctionRefKind::Compound,
           ContextualLocator);
 
       Stmt->setSequence(expr);
@@ -3001,6 +3022,7 @@ auto TypeChecker::typeCheckForEachBinding(
     Expr *appliedSolution(Solution &solution, Expr *expr) override {
       // Figure out what types the constraints decided on.
       auto &cs = solution.getConstraintSystem();
+      ASTContext &ctx = cs.getASTContext();
       InitType = solution.simplifyType(InitType);
       SequenceType = solution.simplifyType(SequenceType);
       ElementType = solution.simplifyType(ElementType);
@@ -3012,17 +3034,17 @@ auto TypeChecker::typeCheckForEachBinding(
 
       cs.cacheExprTypes(expr);
       Stmt->setSequence(expr);
+      solution.setExprTypes(expr);
 
       // Apply the solution to the iteration pattern as well.
       Pattern *pattern = Stmt->getPattern();
       TypeResolutionOptions options(TypeResolverContext::ForEachStmt);
       options |= TypeResolutionFlags::OverrideType;
       if (TypeChecker::coercePatternToType(pattern,
-                                           TypeResolution::forContextual(cs.DC),
+                                           TypeResolution::forContextual(DC),
                                            InitType, options)) {
         return nullptr;
       }
-
       Stmt->setPattern(pattern);
 
       // Get the conformance of the sequence type to the Sequence protocol.
@@ -3032,24 +3054,79 @@ auto TypeChecker::typeCheckForEachBinding(
              "Couldn't find sequence conformance");
       Stmt->setSequenceConformance(SequenceConformance);
 
-      solution.setExprTypes(expr);
-      return expr;
-    }
+      // Check the filtering condition.
+      // FIXME: This should be pulled into the constraint system itself.
+      if (auto *Where = Stmt->getWhere()) {
+        if (!TypeChecker::typeCheckCondition(Where, DC))
+          Stmt->setWhere(Where);
+      }
 
-    ForEachBinding getBinding() const {
-      return { SequenceType, SequenceConformance, IteratorType, ElementType };
+      // Invoke iterator() to get an iterator from the sequence.
+      VarDecl *iterator;
+      Type nextResultType = OptionalType::get(ElementType);
+      {
+        // Create a local variable to capture the iterator.
+        std::string name;
+        if (auto np = dyn_cast_or_null<NamedPattern>(Stmt->getPattern()))
+          name = "$"+np->getBoundName().str().str();
+        name += "$generator";
+
+        iterator = new (ctx) VarDecl(
+            /*IsStatic*/ false, VarDecl::Introducer::Var,
+            /*IsCaptureList*/ false, Stmt->getInLoc(),
+            ctx.getIdentifier(name), DC);
+        iterator->setInterfaceType(IteratorType->mapTypeOutOfContext());
+        iterator->setImplicit();
+        Stmt->setIteratorVar(iterator);
+
+        auto genPat = new (ctx) NamedPattern(iterator);
+        genPat->setImplicit();
+
+        // TODO: test/DebugInfo/iteration.swift requires this extra info to
+        // be around.
+        PatternBindingDecl::createImplicit(
+            ctx, StaticSpellingKind::None, genPat,
+            new (ctx) OpaqueValueExpr(Stmt->getInLoc(), nextResultType),
+            DC, /*VarLoc*/ Stmt->getForLoc());
+      }
+
+      // Create the iterator variable.
+      auto *varRef = TypeChecker::buildCheckedRefExpr(
+          iterator, DC, DeclNameLoc(Stmt->getInLoc()), /*implicit*/ true);
+      if (varRef)
+        Stmt->setIteratorVarRef(varRef);
+
+      // Convert that Optional<Element> value to the type of the pattern.
+      auto optPatternType = OptionalType::get(Stmt->getPattern()->getType());
+      if (!optPatternType->isEqual(nextResultType)) {
+        OpaqueValueExpr *elementExpr =
+            new (ctx) OpaqueValueExpr(Stmt->getInLoc(), nextResultType,
+                                      /*isPlaceholder=*/true);
+        Expr *convertElementExpr = elementExpr;
+        if (TypeChecker::typeCheckExpression(
+                convertElementExpr, DC,
+                TypeLoc::withoutLoc(optPatternType),
+                CTP_CoerceOperand).isNull()) {
+          return nullptr;
+        }
+        elementExpr->setIsPlaceholder(false);
+        Stmt->setElementExpr(elementExpr);
+        Stmt->setConvertElementExpr(convertElementExpr);
+      }
+
+      return expr;
     }
   };
 
-  BindingListener listener(stmt);
+  BindingListener listener(stmt, dc);
   Expr *seq = stmt->getSequence();
   assert(seq && "type-checking an uninitialized for-each statement?");
 
   // Type-check the for-each loop sequence and element pattern.
   auto resultTy = TypeChecker::typeCheckExpression(seq, dc, &listener);
   if (!resultTy)
-    return None;
-  return listener.getBinding();
+    return true;
+  return false;
 }
 
 bool TypeChecker::typeCheckCondition(Expr *&expr, DeclContext *dc) {

lib/Sema/TypeCheckPattern.cpp

@@ -741,23 +741,7 @@ bool TypeChecker::typeCheckPattern(Pattern *P, DeclContext *dc,
   case PatternKind::Typed: {
     auto resolution = TypeResolution::forContextual(dc);
     TypedPattern *TP = cast<TypedPattern>(P);
-    bool hadError = validateTypedPattern(resolution, TP, options);
-
-    // If we have unbound generic types, don't apply them below; instead,
-    // the caller will call typeCheckBinding() later.
-    if (P->getType()->hasUnboundGenericType())
-      return hadError;
-
-    Pattern *subPattern = TP->getSubPattern();
-    if (TypeChecker::coercePatternToType(subPattern, resolution, P->getType(),
-                                         options|TypeResolutionFlags::FromNonInferredPattern,
-                                         TP->getTypeLoc()))
-      hadError = true;
-    else {
-      TP->setSubPattern(subPattern);
-      TP->setType(subPattern->getType());
-    }
-    return hadError;
+    return validateTypedPattern(resolution, TP, options);
   }
 
   // A wildcard or name pattern cannot appear by itself in a context

lib/Sema/TypeCheckStmt.cpp

@@ -731,108 +731,8 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
   }
 
   Stmt *visitForEachStmt(ForEachStmt *S) {
-    TypeResolutionOptions options(TypeResolverContext::InExpression);
-    options |= TypeResolutionFlags::AllowUnspecifiedTypes;
-    options |= TypeResolutionFlags::AllowUnboundGenerics;
-
-    if (auto *P = TypeChecker::resolvePattern(S->getPattern(), DC,
-                                              /*isStmtCondition*/false)) {
-      S->setPattern(P);
-    } else {
-      S->getPattern()->setType(ErrorType::get(getASTContext()));
-      return nullptr;
-    }
-
-    if (TypeChecker::typeCheckPattern(S->getPattern(), DC, options)) {
-      // FIXME: Handle errors better.
-      S->getPattern()->setType(ErrorType::get(getASTContext()));
-      return nullptr;
-    }
-
-    auto binding = TypeChecker::typeCheckForEachBinding(DC, S);
-    if (!binding)
-      return nullptr;
-
-    if (auto *Where = S->getWhere()) {
-      if (TypeChecker::typeCheckCondition(Where, DC))
-        return nullptr;
-      S->setWhere(Where);
-    }
-
-
-    // Retrieve the 'Sequence' protocol.
-    ProtocolDecl *sequenceProto = TypeChecker::getProtocol(
-        getASTContext(), S->getForLoc(), KnownProtocolKind::Sequence);
-    if (!sequenceProto) {
-      return nullptr;
-    }
-
-    // Retrieve the 'Iterator' protocol.
-    ProtocolDecl *iteratorProto = TypeChecker::getProtocol(
-        getASTContext(), S->getForLoc(), KnownProtocolKind::IteratorProtocol);
-    if (!iteratorProto) {
+    if (TypeChecker::typeCheckForEachBinding(DC, S))
       return nullptr;
-    }
-
-    // Invoke iterator() to get an iterator from the sequence.
-    Type iteratorTy = binding->iteratorType;
-    VarDecl *iterator;
-    Type nextResultType = OptionalType::get(binding->elementType);
-    {
-      // Create a local variable to capture the iterator.
-      std::string name;
-      if (auto np = dyn_cast_or_null<NamedPattern>(S->getPattern()))
-        name = "$"+np->getBoundName().str().str();
-      name += "$generator";
-
-      iterator = new (getASTContext()) VarDecl(
-          /*IsStatic*/ false, VarDecl::Introducer::Var,
-          /*IsCaptureList*/ false, S->getInLoc(),
-          getASTContext().getIdentifier(name), DC);
-      iterator->setInterfaceType(iteratorTy->mapTypeOutOfContext());
-      iterator->setImplicit();
-      S->setIteratorVar(iterator);
-
-      auto genPat = new (getASTContext()) NamedPattern(iterator);
-      genPat->setImplicit();
-
-      // TODO: test/DebugInfo/iteration.swift requires this extra info to
-      // be around.
-      PatternBindingDecl::createImplicit(
-          getASTContext(), StaticSpellingKind::None, genPat,
-          new (getASTContext()) OpaqueValueExpr(S->getInLoc(), nextResultType),
-          DC, /*VarLoc*/ S->getForLoc());
-    }
-
-    // Working with iterators requires Optional.
-    if (TypeChecker::requireOptionalIntrinsics(getASTContext(), S->getForLoc()))
-      return nullptr;
-
-    // Create the iterator variable.
-    auto *varRef = TypeChecker::buildCheckedRefExpr(iterator, DC,
-                                                    DeclNameLoc(S->getInLoc()),
-                                                    /*implicit*/ true);
-    if (!varRef)
-      return nullptr;
-    S->setIteratorVarRef(varRef);
-
-    // Convert that Optional<Element> value to the type of the pattern.
-    auto optPatternType = OptionalType::get(S->getPattern()->getType());
-    if (!optPatternType->isEqual(nextResultType)) {
-      OpaqueValueExpr *elementExpr =
-          new (getASTContext()) OpaqueValueExpr(S->getInLoc(), nextResultType,
-                                                /*isPlaceholder=*/true);
-      Expr *convertElementExpr = elementExpr;
-      if (TypeChecker::typeCheckExpression(
-              convertElementExpr, DC,
-              TypeLoc::withoutLoc(optPatternType),
-              CTP_CoerceOperand).isNull()) {
-        return nullptr;
-      }
-      elementExpr->setIsPlaceholder(false);
-      S->setElementExpr(elementExpr);
-      S->setConvertElementExpr(convertElementExpr);
-    }
 
     // Type-check the body of the loop.
     AddLabeledStmt loopNest(*this, S);

lib/Sema/TypeCheckStorage.cpp

@@ -251,6 +251,15 @@ PatternBindingEntryRequest::evaluate(Evaluator &eval,
       binding->diagnose(diag::inferred_opaque_type,
                         binding->getInit(entryNumber)->getType());
     }
+  } else {
+    // Coerce the pattern to the computed type.
+    auto resolution = TypeResolution::forContextual(binding->getDeclContext());
+    if (TypeChecker::coercePatternToType(pattern, resolution,
+                                         pattern->getType(), options)) {
+      binding->setInvalid();
+      pattern->setType(ErrorType::get(Context));
+      return &pbe;
+    }
   }
 
   // If the pattern binding appears in a type or library file context, then

lib/Sema/TypeChecker.h

@@ -1032,19 +1032,10 @@ class TypeChecker final {
   static bool typeCheckBinding(Pattern *&P, Expr *&Init, DeclContext *DC);
   static bool typeCheckPatternBinding(PatternBindingDecl *PBD, unsigned patternNumber);
 
-  /// Information about a type-checked for-each binding.
-  struct ForEachBinding {
-    Type sequenceType;
-    ProtocolConformanceRef sequenceConformance;
-    Type iteratorType;
-    Type elementType;
-  };
-
   /// Type-check a for-each loop's pattern binding and sequence together.
   ///
-  /// \returns the binding, if successful.
-  static Optional<ForEachBinding> typeCheckForEachBinding(
-      DeclContext *dc, ForEachStmt *stmt);
+  /// \returns true if a failure occurred.
+  static bool typeCheckForEachBinding(DeclContext *dc, ForEachStmt *stmt);
 
   /// Compute the set of captures for the given function or closure.
   static void computeCaptures(AnyFunctionRef AFR);

test/decl/var/variables.swift

@@ -107,6 +107,7 @@ func test21057425() -> (Int, Int) {
 
 // rdar://problem/21081340
 func test21081340() {
+  func foo() { }
   let (x: a, y: b): () = foo() // expected-error{{tuple pattern has the wrong length for tuple type '()'}}
 }