[ConstraintGraph] Disallow subtype constraint contractions

Presently subtype constraint is considered a candidate for contraction
via `shouldContractEdge` when left-hand side of the subtype constraint
is an inout type with type variable inside. Although it's intended to
be used only in combination with BindParam constraint assigned to the
same variable, that is actually never checked and contraction of subtype
constraint like that is invalid.

Resolves: rdar://problem/34333874

Author: xedin

lib/Sema/ConstraintGraph.cpp

@@ -641,13 +641,6 @@ static bool shouldContractEdge(ConstraintKind kind) {
   case ConstraintKind::BindParam:
   case ConstraintKind::BindToPointerType:
   case ConstraintKind::Equal:
-
-  // We currently only allow subtype contractions for the purpose of 
-  // parameter binding constraints.
-  // TODO: We do this because of how inout parameter bindings are handled
-  // for implicit closure parameters. We should consider adjusting our
-  // current approach to unlock more opportunities for subtype contractions.
-  case ConstraintKind::Subtype:
     return true;
 
   default:
@@ -671,12 +664,7 @@ static bool isStrictInoutSubtypeConstraint(Constraint *constraint) {
     t1 = tt->getElementType(0).getPointer();
   }
 
-  auto iot = t1->getAs<InOutType>();
-
-  if (!iot)
-    return false;
-
-  return !iot->getObjectType()->isTypeVariableOrMember();
+  return t1->is<InOutType>();
 }
 
 bool ConstraintGraph::contractEdges() {
@@ -698,14 +686,6 @@ bool ConstraintGraph::contractEdges() {
         auto t1 = constraint->getFirstType()->getDesugaredType();
         auto t2 = constraint->getSecondType()->getDesugaredType();
 
-        if (kind == ConstraintKind::Subtype) {
-          if (auto iot1 = t1->getAs<InOutType>()) {
-            t1 = iot1->getObjectType().getPointer();
-          } else {
-            continue;
-          }
-        }
-
         auto tyvar1 = t1->getAs<TypeVariableType>();
         auto tyvar2 = t2->getAs<TypeVariableType>();
 

validation-test/Sema/rdar34333874.swift

@@ -0,0 +1,42 @@
+// RUN: %target-run-simple-swift
+// REQUIRES: executable_test
+
+class C {
+  var a: Int = 0
+  var b: Int = 0
+}
+
+@inline(never)
+func foo<T>(_ item: T, update: (inout T) throws -> Void) rethrows -> T {
+  var this = item
+  try update(&this)
+  return this
+}
+
+// Test single statement closure because it's type-checked
+// together with the call to `foo`
+
+let rdar34333874_1 = foo(C()) {
+  $0.a = 42
+}
+
+// The multi-statement closure which is type-checked
+// separately from call to `foo`
+
+let rdar34333874_2 = foo(C()) {
+  $0.a = 42
+  $0.b = 0
+}
+
+print(rdar34333874_1)
+print(rdar34333874_2)
+
+// Example which avoids mutating fields of the class
+
+@inline(never)
+func bar(_ o : C) {
+  let _ = foo(o) { (item) in
+  }
+}
+
+bar(C())