Merge pull request #13109 from xedin/rdar-35541153

[CSBindings] Avoid binding type variables to collection types directly

Author: xedin

lib/Sema/CSBindings.cpp

@@ -278,6 +278,7 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) {
 
         result.foundLiteralBinding(constraint->getProtocol());
         result.addPotentialBinding({defaultType, AllowedBindingKind::Subtypes,
+                                    constraint->getKind(),
                                     constraint->getProtocol()});
         continue;
       }
@@ -305,6 +306,7 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) {
         result.foundLiteralBinding(constraint->getProtocol());
         exactTypes.insert(defaultType->getCanonicalType());
         result.addPotentialBinding({defaultType, AllowedBindingKind::Subtypes,
+                                    constraint->getKind(),
                                     constraint->getProtocol()});
       }
 
@@ -481,11 +483,11 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) {
     }
 
     if (exactTypes.insert(type->getCanonicalType()).second)
-      result.addPotentialBinding({type, kind, None},
+      result.addPotentialBinding({type, kind, constraint->getKind()},
                                  /*allowJoinMeet=*/!adjustedIUO);
     if (alternateType &&
         exactTypes.insert(alternateType->getCanonicalType()).second)
-      result.addPotentialBinding({alternateType, kind, None},
+      result.addPotentialBinding({alternateType, kind, constraint->getKind()},
                                  /*allowJoinMeet=*/false);
   }
 
@@ -566,8 +568,9 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) {
       continue;
 
     ++result.NumDefaultableBindings;
-    result.addPotentialBinding(
-        {type, AllowedBindingKind::Exact, None, constraint->getLocator()});
+    result.addPotentialBinding({type, AllowedBindingKind::Exact,
+                                constraint->getKind(), None,
+                                constraint->getLocator()});
   }
 
   // Determine if the bindings only constrain the type variable from above with

lib/Sema/CSSolver.cpp

@@ -650,6 +650,21 @@ bool ConstraintSystem::tryTypeVariableBindings(
         }
         type = openUnboundGenericType(type, typeVar->getImpl().getLocator());
         type = type->reconstituteSugar(/*recursive=*/false);
+      } else if ((binding.BindingSource == ConstraintKind::ArgumentConversion ||
+                  binding.BindingSource ==
+                      ConstraintKind::ArgumentTupleConversion) &&
+                 !type->hasTypeVariable() && isCollectionType(type)) {
+        // If the type binding comes from the argument conversion, let's
+        // instead of binding collection types directly let's try to
+        // bind using temporary type variables substituted for element
+        // types, that's going to ensure that subtype relationship is
+        // always preserved.
+        auto *BGT = type->castTo<BoundGenericType>();
+        auto UGT = UnboundGenericType::get(BGT->getDecl(), BGT->getParent(),
+                                           BGT->getASTContext());
+
+        type = openUnboundGenericType(UGT, typeVar->getImpl().getLocator());
+        type = type->reconstituteSugar(/*recursive=*/false);
       }
 
       // FIXME: We want the locator that indicates where the binding came
@@ -697,7 +712,8 @@ bool ConstraintSystem::tryTypeVariableBindings(
           = *((*binding.DefaultedProtocol)->getKnownProtocolKind());
         for (auto altType : getAlternativeLiteralTypes(knownKind)) {
           if (exploredTypes.insert(altType->getCanonicalType()).second)
-            newBindings.push_back({altType, AllowedBindingKind::Subtypes, 
+            newBindings.push_back({altType, AllowedBindingKind::Subtypes,
+                                   binding.BindingSource,
                                    binding.DefaultedProtocol});
         }
       }
@@ -710,7 +726,7 @@ bool ConstraintSystem::tryTypeVariableBindings(
         // Try lvalue qualification in addition to rvalue qualification.
         auto subtype = LValueType::get(type);
         if (exploredTypes.insert(subtype->getCanonicalType()).second)
-          newBindings.push_back({subtype, binding.Kind, None});
+          newBindings.push_back({subtype, binding.Kind, binding.BindingSource});
       }
 
       if (binding.Kind == AllowedBindingKind::Subtypes) {
@@ -719,7 +735,8 @@ bool ConstraintSystem::tryTypeVariableBindings(
           if (scalarIdx >= 0) {
             auto eltType = tupleTy->getElementType(scalarIdx);
             if (exploredTypes.insert(eltType->getCanonicalType()).second)
-              newBindings.push_back({eltType, binding.Kind, None});
+              newBindings.push_back(
+                  {eltType, binding.Kind, binding.BindingSource});
           }
         }
 
@@ -733,10 +750,12 @@ bool ConstraintSystem::tryTypeVariableBindings(
             if (auto otherTypeVar = objTy->getAs<TypeVariableType>()) {
               if (typeVar->getImpl().canBindToLValue() ==
                   otherTypeVar->getImpl().canBindToLValue()) {
-                newBindings.push_back({objTy, binding.Kind, None});
+                newBindings.push_back(
+                    {objTy, binding.Kind, binding.BindingSource});
               }
             } else {
-              newBindings.push_back({objTy, binding.Kind, None});
+              newBindings.push_back(
+                  {objTy, binding.Kind, binding.BindingSource});
             }
           }
         }
@@ -753,7 +772,11 @@ bool ConstraintSystem::tryTypeVariableBindings(
 
         // If we haven't seen this supertype, add it.
         if (exploredTypes.insert((*simpleSuper)->getCanonicalType()).second)
-          newBindings.push_back({*simpleSuper, binding.Kind, None});
+          newBindings.push_back({
+              *simpleSuper,
+              binding.Kind,
+              binding.BindingSource,
+          });
       }
     }
 

lib/Sema/ConstraintSystem.cpp

@@ -577,6 +577,18 @@ Optional<Type> ConstraintSystem::isSetType(Type type) {
   return None;
 }
 
+bool ConstraintSystem::isCollectionType(Type type) {
+  auto &ctx = type->getASTContext();
+  if (auto *structType = type->getAs<BoundGenericStructType>()) {
+    auto *decl = structType->getDecl();
+    if (decl == ctx.getArrayDecl() || decl == ctx.getDictionaryDecl() ||
+        decl == ctx.getSetDecl())
+      return true;
+  }
+
+  return false;
+}
+
 bool ConstraintSystem::isAnyHashableType(Type type) {
   if (auto tv = type->getAs<TypeVariableType>()) {
     auto fixedType = getFixedType(tv);

lib/Sema/ConstraintSystem.h

@@ -1998,6 +1998,9 @@ class ConstraintSystem {
   /// element type of the set.
   static Optional<Type> isSetType(Type t);
 
+  /// Determine if the type in question is one of the known collection types.
+  static bool isCollectionType(Type t);
+
   /// \brief Determine if the type in question is AnyHashable.
   bool isAnyHashableType(Type t);
 
@@ -2552,6 +2555,9 @@ class ConstraintSystem {
     /// The kind of bindings permitted.
     AllowedBindingKind Kind;
 
+    /// The kind of the constraint this binding came from.
+    ConstraintKind BindingSource;
+
     /// The defaulted protocol associated with this binding.
     Optional<ProtocolDecl *> DefaultedProtocol;
 
@@ -2560,9 +2566,11 @@ class ConstraintSystem {
     ConstraintLocator *DefaultableBinding = nullptr;
 
     PotentialBinding(Type type, AllowedBindingKind kind,
+                     ConstraintKind bindingSource,
                      Optional<ProtocolDecl *> defaultedProtocol = None,
                      ConstraintLocator *defaultableBinding = nullptr)
-        : BindingType(type), Kind(kind), DefaultedProtocol(defaultedProtocol),
+        : BindingType(type), Kind(kind), BindingSource(bindingSource),
+          DefaultedProtocol(defaultedProtocol),
           DefaultableBinding(defaultableBinding) {}
 
     bool isDefaultableBinding() const { return DefaultableBinding != nullptr; }

test/Constraints/generics.swift

@@ -528,3 +528,16 @@ do {
     _ = { baz($0) }(construct_generic { d }) // Ok
   }
 }
+
+// rdar://problem/35541153 - Generic parameter inference bug
+
+func rdar35541153() {
+  func foo<U: Equatable, V: Equatable, C: Collection>(_ c: C) where C.Element == (U, V) {}
+  func bar<K: Equatable, V, C: Collection>(_ c: C, _ k: K, _ v: V) where C.Element == (K, V) {}
+
+  let x: [(a: Int, b: Int)] = []
+  let y: [(k: String, v: Int)] = []
+
+  foo(x) // Ok
+  bar(y, "ultimate question", 42) // Ok
+}