Fold ConstraintSolver::coerceToRValue() into TypeChecker::coerceToRValue()

ConstraintSolver::coerceToRValue() missed a bunch of cases where we
should be dealing with lvalues, e.g., tuples, "try" expressions, and
so on. Extend TypeChecker::coerceToRValue() to deal with the
expression type side-tables and ConstraintSolver::coerceToRValue() to
it.

Fixes rdar://problem/32700180, a crash-on-valid.

Author: DougGregor

lib/Sema/CSApply.cpp

@@ -7285,32 +7285,15 @@ namespace {
 } // end anonymous namespace
 
 Expr *ConstraintSystem::coerceToRValue(Expr *expr) {
-  // Can't load from an inout value.
-  if (auto *iot = getType(expr)->getAs<InOutType>()) {
-    // Emit a fixit if we can find the & expression that turned this into an
-    // inout.
-    auto &tc = getTypeChecker();
-    if (auto addrOf =
-        dyn_cast<InOutExpr>(expr->getSemanticsProvidingExpr())) {
-      tc.diagnose(expr->getLoc(), diag::load_of_explicit_lvalue,
-                  iot->getObjectType())
-        .fixItRemove(SourceRange(addrOf->getLoc()));
-      return coerceToRValue(addrOf->getSubExpr());
-    } else {
-      tc.diagnose(expr->getLoc(), diag::load_of_explicit_lvalue,
-                  iot->getObjectType());
-      return expr;
-    }
-  }
-
-  // If we already have an rvalue, we're done, otherwise emit a load.
-  if (auto lvalueTy = getType(expr)->getAs<LValueType>()) {
-    propagateLValueAccessKind(expr, AccessKind::Read);
-    return cacheType(new (getASTContext())
-                         LoadExpr(expr, lvalueTy->getObjectType()));
-  }
-
-  return expr;
+  auto &tc = getTypeChecker();
+  return tc.coerceToRValue(expr,
+                           [&](Expr *expr) {
+                             return getType(expr);
+                           },
+                           [&](Expr *expr, Type type) {
+                             expr->setType(type);
+                             cacheType(expr);
+                           });
 }
 
 /// Emit the fixes computed as part of the solution, returning true if we were

lib/Sema/TypeCheckConstraints.cpp

@@ -2856,52 +2856,64 @@ bool TypeChecker::isSubstitutableFor(Type type, ArchetypeType *archetype,
   return true;
 }
 
-Expr *TypeChecker::coerceToRValue(Expr *expr) {
+Expr *TypeChecker::coerceToRValue(Expr *expr,
+                               llvm::function_ref<Type(Expr *)> getType,
+                               llvm::function_ref<void(Expr *, Type)> setType) {
+  Type exprTy = getType(expr);
+
   // If expr has no type, just assume it's the right expr.
-  if (!expr->getType())
+  if (!exprTy)
     return expr;
-  
-  Type exprTy = expr->getType();
-  
+
   // If the type is already materializable, then we're already done.
   if (!exprTy->hasLValueType())
     return expr;
 
   // Load lvalues.
   if (auto lvalue = exprTy->getAs<LValueType>()) {
     expr->propagateLValueAccessKind(AccessKind::Read);
-    return new (Context) LoadExpr(expr, lvalue->getObjectType());
+    auto result = new (Context) LoadExpr(expr, lvalue->getObjectType());
+    setType(result, lvalue->getObjectType());
+    return result;
   }
 
   // Walk into parenthesized expressions to update the subexpression.
   if (auto paren = dyn_cast<IdentityExpr>(expr)) {
-    auto sub = coerceToRValue(paren->getSubExpr());
+    auto sub =  coerceToRValue(paren->getSubExpr(), getType, setType);
     paren->setSubExpr(sub);
-    paren->setType(sub->getType());
+    setType(paren, getType(sub));
     return paren;
   }
 
   // Walk into 'try' and 'try!' expressions to update the subexpression.
   if (auto tryExpr = dyn_cast<AnyTryExpr>(expr)) {
-    auto sub = coerceToRValue(tryExpr->getSubExpr());
+    auto sub = coerceToRValue(tryExpr->getSubExpr(), getType, setType);
     tryExpr->setSubExpr(sub);
-    if (isa<OptionalTryExpr>(tryExpr) && !sub->getType()->hasError())
-      tryExpr->setType(OptionalType::get(sub->getType()));
+    if (isa<OptionalTryExpr>(tryExpr) && !getType(sub)->hasError())
+      setType(tryExpr, OptionalType::get(getType(sub)));
     else
-      tryExpr->setType(sub->getType());
+      setType(tryExpr, getType(sub));
     return tryExpr;
   }
 
+  // Can't load from an inout value.
+  if (auto inoutExpr = dyn_cast<InOutExpr>(expr)) {
+    diagnose(expr->getLoc(), diag::load_of_explicit_lvalue,
+             exprTy->castTo<InOutType>()->getObjectType())
+      .fixItRemove(SourceRange(inoutExpr->getLoc()));
+    return coerceToRValue(inoutExpr->getSubExpr(), getType, setType);
+  }
+
   // Walk into tuples to update the subexpressions.
   if (auto tuple = dyn_cast<TupleExpr>(expr)) {
     bool anyChanged = false;
     for (auto &elt : tuple->getElements()) {
       // Materialize the element.
-      auto oldType = elt->getType();
-      elt = coerceToRValue(elt);
+      auto oldType = getType(elt);
+      elt = coerceToRValue(elt, getType, setType);
 
       // If the type changed at all, make a note of it.
-      if (elt->getType().getPointer() != oldType.getPointer()) {
+      if (getType(elt).getPointer() != oldType.getPointer()) {
         anyChanged = true;
       }
     }
@@ -2911,11 +2923,11 @@ Expr *TypeChecker::coerceToRValue(Expr *expr) {
       SmallVector<TupleTypeElt, 4> elements;
       elements.reserve(tuple->getElements().size());
       for (unsigned i = 0, n = tuple->getNumElements(); i != n; ++i) {
-        Type type = tuple->getElement(i)->getType();
+        Type type = getType(tuple->getElement(i));
         Identifier name = tuple->getElementName(i);
         elements.push_back(TupleTypeElt(type, name));
       }
-      tuple->setType(TupleType::get(elements, Context));
+      setType(tuple, TupleType::get(elements, Context));
     }
 
     return tuple;

lib/Sema/TypeChecker.h

@@ -1747,7 +1747,13 @@ class TypeChecker final : public LazyResolver {
 
   /// \brief Coerce the given expression to materializable type, if it
   /// isn't already.
-  Expr *coerceToRValue(Expr *expr);
+  Expr *coerceToRValue(Expr *expr,
+                       llvm::function_ref<Type(Expr *)> getType
+                         = [](Expr *expr) { return expr->getType(); },
+                       llvm::function_ref<void(Expr *, Type)> setType
+                         = [](Expr *expr, Type type) {
+                           expr->setType(type);
+                         });
 
   /// Require that the library intrinsics for working with Optional<T>
   /// exist.

validation-test/compiler_crashers_2_fixed/0106-rdar32700180.swift

@@ -0,0 +1,19 @@
+// RUN: %target-swift-frontend %s -emit-ir
+// REQUIRES: objc_interop
+
+func f(_: AnyObject?) { }
+
+class C {
+  private var a: Int
+  private var b: Int
+
+  func test() {
+    f((self.a, self.b) as AnyObject)
+  }
+
+  init() {
+    a = 0
+    b = 0
+  }
+}
+