Merge pull request #17947 from xedin/rdar-42056741

[Diagnostics] Transfer previously resolved types directly from expressions

Author: xedin

lib/Sema/CSDiag.cpp

@@ -2356,22 +2356,15 @@ Expr *FailureDiagnosis::typeCheckChildIndependently(
   // telling us that it knows what it is doing, then believe it.
   if (!options.contains(TCC_ForceRecheck)) {
     if (CS.TC.isExprBeingDiagnosed(subExpr)) {
-      auto exprAndCS = CS.TC.getExprBeingDiagnosed(subExpr);
-      auto *savedExpr = exprAndCS.first;
+      auto *savedExpr = CS.TC.getExprBeingDiagnosed(subExpr);
       if (subExpr == savedExpr)
         return subExpr;
 
-      auto *oldCS = exprAndCS.second;
-
-      // The types on the result might have already been cached into
-      // another CS, but likely not this one.
-      if (oldCS != &CS)
-        CS.transferExprTypes(oldCS, savedExpr);
-
+      CS.cacheExprTypes(savedExpr);
       return savedExpr;
     }
 
-    CS.TC.addExprForDiagnosis(subExpr, std::make_pair(subExpr, &CS));
+    CS.TC.addExprForDiagnosis(subExpr, subExpr);
   }
 
   // Validate contextual type before trying to use it.
@@ -2448,7 +2441,8 @@ Expr *FailureDiagnosis::typeCheckChildIndependently(
     SavedTypeData.restore();
   }
 
-  CS.TC.addExprForDiagnosis(preCheckedExpr, std::make_pair(subExpr, &CS));
+  if (preCheckedExpr != subExpr)
+    CS.TC.addExprForDiagnosis(preCheckedExpr, subExpr);
 
   return subExpr;
 }

lib/Sema/ConstraintSystem.h

@@ -1335,30 +1335,6 @@ class ConstraintSystem {
     bool walkToDeclPre(Decl *decl) override { return false; }
   };
 
-  class TransferExprTypes : public ASTWalker {
-    ConstraintSystem &toCS;
-    ConstraintSystem &fromCS;
-
-  public:
-    TransferExprTypes(ConstraintSystem &toCS, ConstraintSystem &fromCS)
-        : toCS(toCS), fromCS(fromCS) {}
-
-    Expr *walkToExprPost(Expr *expr) override {
-      if (fromCS.hasType(expr))
-        toCS.setType(expr, fromCS.getType(expr));
-
-      return expr;
-    }
-
-    /// \brief Ignore statements.
-    std::pair<bool, Stmt *> walkToStmtPre(Stmt *stmt) override {
-      return { false, stmt };
-    }
-
-    /// \brief Ignore declarations.
-    bool walkToDeclPre(Decl *decl) override { return false; }
-  };
-
 public:
 
   void setExprTypes(Expr *expr) {
@@ -1384,10 +1360,6 @@ class ConstraintSystem {
     expr->walk(CacheExprTypes(expr, *this, excludeRoot));
   }
 
-  void transferExprTypes(ConstraintSystem *oldCS, Expr *expr) {
-    expr->walk(TransferExprTypes(*this, *oldCS));
-  }
-
   /// \brief The current solver state.
   ///
   /// This will be non-null when we're actively solving the constraint

lib/Sema/TypeChecker.h

@@ -57,12 +57,6 @@ namespace constraints {
 using ConformanceMap =
     llvm::DenseMap<SubstitutableType *, SmallVector<ProtocolConformance *, 2>>;
 
-/// \brief Used for recursive lookups into an expr that is already
-/// being type-checked and the constraint system in which its type is
-/// stored.
-using ExprAndConstraintSystem =
-    std::pair<Expr *, constraints::ConstraintSystem *>;
-
 /// Special-case type checking semantics for certain declarations.
 enum class DeclTypeCheckingSemantics {
   /// A normal declaration.
@@ -779,7 +773,7 @@ class TypeChecker final : public LazyResolver {
   Optional<NominalTypeDecl *> ArrayDecl;
 
   /// The set of expressions currently being analyzed for failures.
-  llvm::DenseMap<Expr*, ExprAndConstraintSystem> DiagnosedExprs;
+  llvm::DenseMap<Expr*, Expr*> DiagnosedExprs;
 
   ModuleDecl *StdlibModule = nullptr;
 
@@ -2311,13 +2305,13 @@ class TypeChecker final : public LazyResolver {
   void checkInitializerErrorHandling(Initializer *I, Expr *E);
   void checkEnumElementErrorHandling(EnumElementDecl *D);
 
-  void addExprForDiagnosis(Expr *E1, ExprAndConstraintSystem Result) {
+  void addExprForDiagnosis(Expr *E1, Expr *Result) {
     DiagnosedExprs[E1] = Result;
   }
   bool isExprBeingDiagnosed(Expr *E) {
     return DiagnosedExprs.count(E);
   }
-  ExprAndConstraintSystem getExprBeingDiagnosed(Expr *E) {
+  Expr *getExprBeingDiagnosed(Expr *E) {
     return DiagnosedExprs[E];
   }
 

test/Constraints/rdar42056741.swift

@@ -0,0 +1,23 @@
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -verify %s
+// REQUIRES: objc_interop
+
+import Foundation
+
+class A {
+  static var `default` = A()
+
+  func foo(arg: String) -> Bool {
+    return false
+  }
+
+  func foo(arg: String, _ flag: UnsafeMutablePointer<ObjCBool>?) -> Bool {
+    return true
+  }
+}
+
+class B {
+  var bar: Bool = false
+  func baz() {
+    bar = A.default.foo(arg: self.) // expected-error {{expected member name following '.'}}
+  }
+}

validation-test/IDE/crashers_2_fixed/0020-rdar42056741.swift

@@ -0,0 +1,23 @@
+// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -code-completion-token=COMPLETE -source-filename=%s
+// REQUIRES: objc_interop
+
+import Foundation
+
+class A {
+  static var `default` = A()
+
+  func foo(arg: String) -> Bool {
+    return false
+  }
+
+  func foo(arg: String, _ flag: UnsafeMutablePointer<ObjCBool>?) -> Bool {
+    return true
+  }
+}
+
+class B {
+  var bar: Bool = false
+  func baz() {
+    bar = A.default.foo(arg: self.#^COMPLETE^#)
+  }
+}