[Diagnostics] Transfer previously resolved types directly from expressions

While trying to diagnose the problem with previously type-checked
sub-expression, use its type-checked variant as a source of type
information, instead of transferring from its original constraint system,
because if expression was type-checked successfully it would
have all of the required information in AST, and that doesn't
rely on associated constraint system being present.

Resolves: rdar://problem/42056741

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.
@@ -2457,7 +2450,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/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.
@@ -783,7 +777,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;
 
@@ -2315,13 +2309,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];
   }
 

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

@@ -0,0 +1,25 @@
+// 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^#)
+  }
+}