Merge pull request #11047 from CodaFi/who-cleans-the-cleanser

[NFC] Merge ExprCleaner and ExprCleanser

Author: CodaFi

lib/Sema/CSDiag.cpp

@@ -4635,14 +4635,27 @@ static bool diagnoseImplicitSelfErrors(Expr *fnExpr, Expr *argExpr,
   // If argument wasn't properly type-checked, let's retry without changing AST.
   if (!argType || argType->hasUnresolvedType() || argType->hasTypeVariable() ||
       argType->hasTypeParameter()) {
-    // Let's type check argument expression without any contextual information.
-    ConcreteDeclRef ref = nullptr;
-    auto typeResult =
-        TC.getTypeOfExpressionWithoutApplying(argExpr, CS.DC, ref);
-    if (!typeResult.hasValue())
+    auto *argTuple = dyn_cast<TupleExpr>(argExpr);
+    if (!argTuple) {
+      // Bail out if we don't have a well-formed argument list.
       return false;
+    }
+    
+    // Let's type check individual argument expressions without any
+    // contextual information to try to recover an argument type that
+    // matches what the user actually wrote instead of what the typechecker
+    // expects.
+    SmallVector<TupleTypeElt, 4> elts;
+    for (auto *el : argTuple->getElements()) {
+      ConcreteDeclRef ref = nullptr;
+      auto typeResult =
+        TC.getTypeOfExpressionWithoutApplying(el, CS.DC, ref);
+      if (!typeResult.hasValue())
+        return false;
+      elts.push_back(typeResult.getValue());
+    }
 
-    argType = typeResult.getValue();
+    argType = TupleType::get(elts, CS.getASTContext());
   }
 
   auto typeKind = argType->getKind();
@@ -8758,6 +8771,10 @@ static void noteArchetypeSource(const TypeLoc &loc, ArchetypeType *archetype,
     // `Pair<Any, Any>`.
     // Right now we only handle this when the type that's at fault is the
     // top-level type passed to this function.
+    if (loc.getType().isNull()) {
+      return;
+    }
+    
     ArrayRef<Type> genericArgs;
     if (auto *boundGenericTy = loc.getType()->getAs<BoundGenericType>()) {
       if (boundGenericTy->getDecl() == FoundDecl)

lib/Sema/ConstraintSystem.h

@@ -3034,54 +3034,73 @@ ForcedCheckedCastExpr *findForcedDowncast(ASTContext &ctx, Expr *expr);
 /// their type variables, and we don't want pointers into the original AST to
 /// dereference these now-dangling types.
 class ExprCleaner {
-  llvm::SmallDenseMap<Expr *, Type> Exprs;
-  llvm::SmallDenseMap<TypeLoc *, Type> TypeLocs;
-  llvm::SmallDenseMap<Pattern *, Type> Patterns;
+  llvm::SmallVector<Expr*,4> Exprs;
+  llvm::SmallVector<TypeLoc*, 4> TypeLocs;
+  llvm::SmallVector<Pattern*, 4> Patterns;
+  llvm::SmallVector<VarDecl*, 4> Vars;
 public:
 
   ExprCleaner(Expr *E) {
-    struct ExprCleanserImpl : public ASTWalker {
+    struct ExprCleanerImpl : public ASTWalker {
       ExprCleaner *TS;
-      ExprCleanserImpl(ExprCleaner *TS) : TS(TS) {}
+      ExprCleanerImpl(ExprCleaner *TS) : TS(TS) {}
 
       std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
-        TS->Exprs.insert({ expr, expr->getType() });
+        TS->Exprs.push_back(expr);
         return { true, expr };
       }
 
       bool walkToTypeLocPre(TypeLoc &TL) override {
-        TS->TypeLocs.insert({ &TL, TL.getType() });
+        TS->TypeLocs.push_back(&TL);
         return true;
       }
 
       std::pair<bool, Pattern*> walkToPatternPre(Pattern *P) override {
-        TS->Patterns.insert({ P, P->hasType() ? P->getType() : Type() });
+        TS->Patterns.push_back(P);
         return { true, P };
       }
 
+      bool walkToDeclPre(Decl *D) override {
+        if (auto VD = dyn_cast<VarDecl>(D))
+          TS->Vars.push_back(VD);
+
+        return true;
+      }
+
       // Don't walk into statements.  This handles the BraceStmt in
       // non-single-expr closures, so we don't walk into their body.
       std::pair<bool, Stmt *> walkToStmtPre(Stmt *S) override {
         return { false, S };
       }
     };
 
-    E->walk(ExprCleanserImpl(this));
+    E->walk(ExprCleanerImpl(this));
   }
 
   ~ExprCleaner() {
     // Check each of the expression nodes to verify that there are no type
     // variables hanging out.  If so, just nuke the type.
     for (auto E : Exprs) {
-      E.getFirst()->setType(E.getSecond());
+      if (E->getType() && E->getType()->hasTypeVariable())
+        E->setType(Type());
     }
 
     for (auto TL : TypeLocs) {
-      TL.getFirst()->setType(TL.getSecond(), false);
+      if (TL->getTypeRepr() && TL->getType() &&
+          TL->getType()->hasTypeVariable())
+        TL->setType(Type(), false);
     }
 
     for (auto P : Patterns) {
-      P.getFirst()->setType(P.getSecond());
+      if (P->hasType() && P->getType()->hasTypeVariable())
+        P->setType(Type());
+    }
+
+    for (auto VD : Vars) {
+      if (VD->hasType() && VD->getType()->hasTypeVariable()) {
+        VD->setType(Type());
+        VD->setInterfaceType(Type());
+      }
     }
   }
 };

lib/Sema/TypeCheckConstraints.cpp

@@ -1689,87 +1689,6 @@ solveForExpression(Expr *&expr, DeclContext *dc, Type convertType,
   return false;
 }
 
-namespace {
-  /// ExprCleanser - This class is used by typeCheckExpression to ensure that in
-  /// no situation will an expr node be left with a dangling type variable stuck
-  /// to it.  Often type checking will create new AST nodes and replace old ones
-  /// (e.g. by turning an UnresolvedDotExpr into a MemberRefExpr).  These nodes
-  /// might be left with pointers into the temporary constraint system through
-  /// their type variables, and we don't want pointers into the original AST to
-  /// dereference these now-dangling types.
-  class ExprCleanser {
-    llvm::SmallVector<Expr*,4> Exprs;
-    llvm::SmallVector<TypeLoc*, 4> TypeLocs;
-    llvm::SmallVector<Pattern*, 4> Patterns;
-    llvm::SmallVector<VarDecl*, 4> Vars;
-  public:
-
-    ExprCleanser(Expr *E) {
-      struct ExprCleanserImpl : public ASTWalker {
-        ExprCleanser *TS;
-        ExprCleanserImpl(ExprCleanser *TS) : TS(TS) {}
-
-        std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
-          TS->Exprs.push_back(expr);
-          return { true, expr };
-        }
-
-        bool walkToTypeLocPre(TypeLoc &TL) override {
-          TS->TypeLocs.push_back(&TL);
-          return true;
-        }
-
-        std::pair<bool, Pattern*> walkToPatternPre(Pattern *P) override {
-          TS->Patterns.push_back(P);
-          return { true, P };
-        }
-
-        bool walkToDeclPre(Decl *D) override {
-          if (auto VD = dyn_cast<VarDecl>(D))
-            TS->Vars.push_back(VD);
-
-          return true;
-        }
-
-        // Don't walk into statements.  This handles the BraceStmt in
-        // non-single-expr closures, so we don't walk into their body.
-        std::pair<bool, Stmt *> walkToStmtPre(Stmt *S) override {
-          return { false, S };
-        }
-      };
-
-      E->walk(ExprCleanserImpl(this));
-    }
-
-    ~ExprCleanser() {
-      // Check each of the expression nodes to verify that there are no type
-      // variables hanging out.  If so, just nuke the type.
-      for (auto E : Exprs) {
-        if (E->getType() && E->getType()->hasTypeVariable())
-          E->setType(Type());
-      }
-
-      for (auto TL : TypeLocs) {
-        if (TL->getTypeRepr() && TL->getType() &&
-            TL->getType()->hasTypeVariable())
-          TL->setType(Type(), false);
-      }
-
-      for (auto P : Patterns) {
-        if (P->hasType() && P->getType()->hasTypeVariable())
-          P->setType(Type());
-      }
-
-      for (auto VD : Vars) {
-        if (VD->hasType() && VD->getType()->hasTypeVariable()) {
-          VD->setType(Type());
-          VD->setInterfaceType(Type());
-        }
-      }
-    }
-  };
-} // end anonymous namespace
-
 #pragma mark High-level entry points
 Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
                                       TypeLoc convertType,
@@ -1791,7 +1710,7 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
   ConstraintSystem cs(*this, dc, csOptions);
   cs.baseCS = baseCS;
   CleanupIllFormedExpressionRAII cleanup(Context, expr);
-  ExprCleanser cleanup2(expr);
+  ExprCleaner cleanup2(expr);
 
   // Verify that a purpose was specified if a convertType was.  Note that it is
   // ok to have a purpose without a convertType (which is used for call