Fix some LValue type checking issues exposed by key paths.

We had an inconsistency in the handling of ConstraintKind::Equal in that
we would take
  $T1 Equal $T2
where $T2 was previously bound to a type, and bind the RValue type of
$T2's type to $T1.  That does not allow for us to later attempt to bind
the LValue type of that type to $T1 (as might happen in simplifying an
OptionalObject constraint).

Instead, if $T1 can be bound to an LValue and $T2 is not an LValue,
we'll defer simplifying the Equal constraint until after $T1 is bound
through some other type variable binding or constraint simplification.

Fixes rdar://problem/31724272.

Author: rudkx

lib/Sema/CSSimplify.cpp

@@ -1418,17 +1418,30 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
       }
 
       // Provide a fixed type for the type variable.
-      bool wantRvalue = kind == ConstraintKind::Equal;
       if (typeVar1) {
         // Simplify the right-hand type and perform the "occurs" check.
         typeVar1 = getRepresentative(typeVar1);
         type2 = simplifyType(type2, flags);
         if (typeVarOccursInType(typeVar1, type2))
           return formUnsolvedResult();
 
-        // If we want an rvalue, get the rvalue.
-        if (wantRvalue)
-          type2 = type2->getRValueType();
+        // Equal constraints allow mixed LValue/RValue bindings.
+        if (kind == ConstraintKind::Equal) {
+          // If we could bind an LValue to the type variable, but the
+          // type that is already bound is not an LValue, defer
+          // simplifying the constraint since we may come along at a
+          // later time and attempt to bind the LValue type to this
+          // type variable.
+          if (typeVar1->getImpl().canBindToLValue()) {
+            if (!type2->isLValueType()) {
+              return formUnsolvedResult();
+            }
+          } else {
+            // If the type variable does not allow LValue bindings,
+            // get the RValue type.
+            type2 = type2->getRValueType();
+          }
+        }
 
         // If the left-hand type variable cannot bind to an lvalue,
         // but we still have an lvalue, fail.
@@ -1467,9 +1480,23 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
       if (typeVarOccursInType(typeVar2, type1))
         return formUnsolvedResult();
 
-      // If we want an rvalue, get the rvalue.
-      if (wantRvalue)
-        type1 = type1->getRValueType();
+      // Equal constraints allow mixed LValue/RValue bindings.
+      if (kind == ConstraintKind::Equal) {
+        // If we could bind an LValue to the type variable, but the
+        // type that is already bound is not an LValue, defer
+        // simplifying the constraint since we may come along at a
+        // later time and attempt to bind the LValue type to this
+        // type variable.
+        if (typeVar2->getImpl().canBindToLValue()) {
+          if (!type1->isLValueType()) {
+            return formUnsolvedResult();
+          }
+        } else {
+          // If the type variable does not allow LValue bindings,
+          // get the RValue type.
+          type1 = type1->getRValueType();
+        }
+      }
 
       if (!typeVar2->getImpl().canBindToLValue() &&
           type1->isLValueType()) {

test/expr/unary/keypath/keypath.swift

@@ -17,6 +17,7 @@ struct A: Hashable {
 
   var property: Prop
   var optProperty: Prop?
+  let optLetProperty: Prop?
 
   subscript(sub: Sub) -> A { get { return self } set { } }
 
@@ -88,9 +89,7 @@ func testKeyPath(sub: Sub, optSub: OptSub, x: Int) {
   let _: ReferenceWritableKeyPath<A, Prop> = \.property
 
   // FIXME crash let _: PartialKeyPath<A> = \[sub]
-  // FIXME should resolve: expected-error@+1{{}}
   let _: KeyPath<A, A> = \.[sub]
-  // FIXME should resolve: expected-error@+1{{}}
   let _: WritableKeyPath<A, A> = \.[sub]
   // expected-error@+1{{ambiguous}} (need to improve diagnostic)
   let _: ReferenceWritableKeyPath<A, A> = \.[sub]
@@ -109,8 +108,8 @@ func testKeyPath(sub: Sub, optSub: OptSub, x: Int) {
   // expected-error@+1{{cannot convert}}
   let _: ReferenceWritableKeyPath<A, A?> = \.optProperty?[sub]
 
-  // FIXME should resolve: expected-error@+1{{}}
   let _: KeyPath<A, Prop> = \.optProperty!
+  let _: KeyPath<A, Prop> = \.optLetProperty!
   let _: KeyPath<A, Prop?> = \.property[optSub]?.optProperty!
   let _: KeyPath<A, A?> = \.property[optSub]?.optProperty![sub]