[NFC] Merge ExprCleaner and ExprCleanser

The distinction was made between these two because of rdar://25341015,
wherein we needed a diagnostic that could detect the shadowing of
global functions by calls in protocols that don't match any
of the protocol's members.

This used to function by hoping that we had an argument tuple type lying
around after type checking.  The expr cleaner would re-write that
type into the AST and we would look up based on it.  Now that we write
Type() into the AST on failure, we must instead type check the
component parts of the argument themselves to form a tuple
type to make the lookup.

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