Merge pull request swiftlang#28111 from DougGregor/minor-cs-cleanups

Author: swift-ci

lib/PrintAsObjC/ModuleContentsWriter.h

@@ -35,4 +35,5 @@ void printModuleContentsAsObjC(raw_ostream &os,
 
 } // end namespace swift
 
-#endif
+#endif
+

lib/Sema/BuilderTransform.cpp

@@ -53,7 +53,7 @@ class BuilderClosureVisitor
   Expr *buildCallIfWanted(SourceLoc loc,
                           Identifier fnName, ArrayRef<Expr *> args,
                           ArrayRef<Identifier> argLabels,
-                          bool allowOneWay) {
+                          bool oneWay) {
     if (!wantExpr)
       return nullptr;
 
@@ -87,7 +87,7 @@ class BuilderClosureVisitor
                                     /*trailing closure*/ nullptr,
                                     /*implicit*/true);
 
-    if (allowOneWay) {
+    if (oneWay) {
       // Form a one-way constraint to prevent backward propagation.
       result = new (ctx) OneWayExpr(result);
     }
@@ -173,10 +173,10 @@ class BuilderClosureVisitor
         if (builderSupports(ctx.Id_buildExpression)) {
           expr = buildCallIfWanted(expr->getLoc(), ctx.Id_buildExpression,
                                    { expr }, { Identifier() },
-                                   /*allowOneWay=*/true);
-        } else {
-          expr = new (ctx) OneWayExpr(expr);
+                                   /*oneWay=*/false);
         }
+
+        expr = new (ctx) OneWayExpr(expr);
       }
 
       expressions.push_back(expr);
@@ -186,7 +186,7 @@ class BuilderClosureVisitor
     return buildCallIfWanted(braceStmt->getStartLoc(),
                              ctx.Id_buildBlock, expressions,
                              /*argLabels=*/{ },
-                             /*allowOneWay=*/true);
+                             /*oneWay=*/true);
   }
 
   Expr *visitReturnStmt(ReturnStmt *stmt) {
@@ -212,7 +212,7 @@ class BuilderClosureVisitor
       return nullptr;
 
     return buildCallIfWanted(doStmt->getStartLoc(), ctx.Id_buildDo, arg,
-                             /*argLabels=*/{ }, /*allowOneWay=*/true);
+                             /*argLabels=*/{ }, /*oneWay=*/true);
   }
 
   CONTROL_FLOW_STMT(Yield)
@@ -299,7 +299,7 @@ class BuilderClosureVisitor
       chainExpr = buildCallIfWanted(ifStmt->getStartLoc(),
                                     ctx.Id_buildIf, chainExpr,
                                     /*argLabels=*/{ },
-                                    /*allowOneWay=*/true);
+                                    /*oneWay=*/true);
     } else {
       // Form a one-way constraint to prevent backward propagation.
       chainExpr = new (ctx) OneWayExpr(chainExpr);
@@ -423,7 +423,7 @@ class BuilderClosureVisitor
       operand = buildCallIfWanted(operand->getStartLoc(),
                                   ctx.Id_buildEither, operand,
                                   {isSecond ? ctx.Id_second : ctx.Id_first},
-                                  /*allowOneWay=*/false);
+                                  /*oneWay=*/false);
     }
 
     // Inject into Optional if required.  We'll be adding the call to

lib/Sema/CSApply.cpp

@@ -3566,7 +3566,7 @@ namespace {
         // Convert the subexpression.
         Expr *sub = expr->getSubExpr();
 
-        cs.setExprTypes(sub);
+        solution.setExprTypes(sub);
 
         if (tc.convertToType(sub, toType, cs.DC))
           return nullptr;
@@ -4697,7 +4697,7 @@ namespace {
       return result;
     }
 
-    void finalize(Expr *&result) {
+    void finalize() {
       assert(ExprStack.empty());
       assert(OpenedExistentials.empty());
 
@@ -4722,9 +4722,6 @@ namespace {
           .fixItInsert(cast->getStartLoc(), "(")
           .fixItInsertAfter(cast->getEndLoc(), ")");
       }
-      
-      // Set the final types on the expression.
-      cs.setExprTypes(result);
     }
 
     /// Diagnose an optional injection that is probably not what the
@@ -5603,7 +5600,7 @@ ClosureExpr *ExprRewriter::coerceClosureExprToVoid(ClosureExpr *closureExpr) {
       cs.setType(singleExpr,
                  cs.getType(singleExpr)->getWithoutSpecifierType());
 
-    cs.setExprTypes(singleExpr);
+    solution.setExprTypes(singleExpr);
     TypeChecker::checkIgnoredExpr(singleExpr);
 
     SmallVector<ASTNode, 2> elements;
@@ -5642,7 +5639,7 @@ ClosureExpr *ExprRewriter::coerceClosureExprFromNever(ClosureExpr *closureExpr)
     auto returnStmt = cast<ReturnStmt>(member.get<Stmt *>());
     auto singleExpr = returnStmt->getResult();
 
-    cs.setExprTypes(singleExpr);
+    solution.setExprTypes(singleExpr);
     TypeChecker::checkIgnoredExpr(singleExpr);
 
     SmallVector<ASTNode, 1> elements;
@@ -7038,7 +7035,7 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, ConcreteDeclRef callee,
     cs.setType(apply, fnType->getResult());
     apply->setIsSuper(isSuper);
 
-    cs.setExprTypes(apply);
+    solution.setExprTypes(apply);
     Expr *result = TypeChecker::substituteInputSugarTypeForResult(apply);
     cs.cacheExprTypes(result);
 
@@ -7339,7 +7336,7 @@ namespace {
         } else {
           // For other closures, type-check the body once we've finished with
           // the expression.
-          cs.setExprTypes(closure);
+          Rewriter.solution.setExprTypes(closure);
           ClosuresToTypeCheck.push_back(closure);
         }
 
@@ -7560,8 +7557,8 @@ Expr *ConstraintSystem::applySolution(Solution &solution, Expr *expr,
                                    getConstraintLocator(expr));
   }
 
-  if (result)
-    rewriter.finalize(result);
+  solution.setExprTypes(result);
+  rewriter.finalize();
 
   return result;
 }
@@ -7575,6 +7572,59 @@ Expr *Solution::coerceToType(Expr *expr, Type toType,
   if (!result)
     return nullptr;
 
-  rewriter.finalize(result);
+  setExprTypes(result);
+  rewriter.finalize();
   return result;
 }
+
+namespace {
+class SetExprTypes : public ASTWalker {
+  const Solution &solution;
+
+public:
+  explicit SetExprTypes(const Solution &solution)
+      : solution(solution) {}
+
+  Expr *walkToExprPost(Expr *expr) override {
+    auto &cs = solution.getConstraintSystem();
+    auto exprType = cs.getType(expr);
+    exprType = solution.simplifyType(exprType);
+    // assert((!expr->getType() || expr->getType()->isEqual(exprType)) &&
+    //       "Mismatched types!");
+    assert(!exprType->hasTypeVariable() &&
+           "Should not write type variable into expression!");
+    expr->setType(exprType);
+
+    if (auto kp = dyn_cast<KeyPathExpr>(expr)) {
+      for (auto i : indices(kp->getComponents())) {
+        Type componentType;
+        if (cs.hasType(kp, i)) {
+          componentType = solution.simplifyType(cs.getType(kp, i));
+          assert(!componentType->hasTypeVariable() &&
+                 "Should not write type variable into key-path component");
+        }
+
+        kp->getMutableComponents()[i].setComponentType(componentType);
+      }
+    }
+
+    return expr;
+  }
+
+  /// Ignore statements.
+  std::pair<bool, Stmt *> walkToStmtPre(Stmt *stmt) override {
+    return { false, stmt };
+  }
+
+  /// Ignore declarations.
+  bool walkToDeclPre(Decl *decl) override { return false; }
+};
+}
+
+void Solution::setExprTypes(Expr *expr) const {
+  if (!expr)
+    return;
+
+  SetExprTypes SET(*this);
+  expr->walk(SET);
+}

lib/Sema/ConstraintSystem.h

@@ -741,6 +741,8 @@ class Solution {
     return None;
   }
 
+  void setExprTypes(Expr *expr) const;
+
   SWIFT_DEBUG_DUMP;
 
   /// Dump this solution.
@@ -1508,58 +1510,7 @@ class ConstraintSystem {
     bool walkToDeclPre(Decl *decl) override { return false; }
   };
 
-  class SetExprTypes : public ASTWalker {
-    Expr *RootExpr;
-    ConstraintSystem &CS;
-    bool ExcludeRoot;
-
-  public:
-    SetExprTypes(Expr *expr, ConstraintSystem &cs, bool excludeRoot)
-        : RootExpr(expr), CS(cs), ExcludeRoot(excludeRoot) {}
-
-    Expr *walkToExprPost(Expr *expr) override {
-      if (ExcludeRoot && expr == RootExpr)
-        return expr;
-
-      //assert((!expr->getType() || CS.getType(expr)->isEqual(expr->getType()))
-      //       && "Mismatched types!");
-      assert(!CS.getType(expr)->hasTypeVariable() &&
-             "Should not write type variable into expression!");
-      expr->setType(CS.getType(expr));
-
-      if (auto kp = dyn_cast<KeyPathExpr>(expr)) {
-        for (auto i : indices(kp->getComponents())) {
-          Type componentType;
-          if (CS.hasType(kp, i))
-            componentType = CS.getType(kp, i);
-          kp->getMutableComponents()[i].setComponentType(componentType);
-        }
-      }
-
-      return expr;
-    }
-
-    /// Ignore statements.
-    std::pair<bool, Stmt *> walkToStmtPre(Stmt *stmt) override {
-      return { false, stmt };
-    }
-
-    /// Ignore declarations.
-    bool walkToDeclPre(Decl *decl) override { return false; }
-  };
-
 public:
-
-  void setExprTypes(Expr *expr) {
-    SetExprTypes SET(expr, *this, /* excludeRoot = */ false);
-    expr->walk(SET);
-  }
-
-  void setSubExprTypes(Expr *expr) {
-    SetExprTypes SET(expr, *this, /* excludeRoot = */ true);
-    expr->walk(SET);
-  }
-
   /// Cache the types of the given expression and all subexpressions.
   void cacheExprTypes(Expr *expr) {
     bool excludeRoot = false;

lib/Sema/TypeCheckConstraints.cpp

@@ -2980,7 +2980,7 @@ bool TypeChecker::typeCheckForEachBinding(DeclContext *dc, ForEachStmt *stmt) {
       Stmt->setPattern(pattern);
       Stmt->setSequence(expr);
 
-      cs.setExprTypes(expr);
+      solution.setExprTypes(expr);
       return expr;
     }
   };
@@ -3464,7 +3464,7 @@ bool TypeChecker::convertToType(Expr *&expr, Type type, DeclContext *dc,
     return true;
   }
 
-  cs.setExprTypes(expr);
+  solution.setExprTypes(expr);
 
   if (getLangOpts().DebugConstraintSolver) {
     auto &log = Context.TypeCheckerDebug->getStream();