[CSBindings] Avoid inferring duplicate bindings from supertypes

If there is `subtype` relationship between two type variables
binding inference algorithm attempts to infer supertype bindings
transitively, but it doesn't check whether some of the new types
are already included in the set, which leads to duplicate solutions.

Author: xedin

lib/Sema/CSBindings.cpp

@@ -21,6 +21,66 @@
 using namespace swift;
 using namespace constraints;
 
+void ConstraintSystem::inferTransitiveSupertypeBindings(
+    const llvm::SmallDenseMap<TypeVariableType *, PotentialBindings>
+        &inferredBindings,
+    PotentialBindings &bindings) {
+  auto *typeVar = bindings.TypeVar;
+
+  llvm::SmallVector<Constraint *, 4> subtypeOf;
+  // First, let's collect all of the `subtype` constraints associated
+  // with this type variable.
+  llvm::copy_if(bindings.Sources, std::back_inserter(subtypeOf),
+                [&](const Constraint *constraint) -> bool {
+                  if (constraint->getKind() != ConstraintKind::Subtype)
+                    return false;
+
+                  auto rhs = simplifyType(constraint->getSecondType());
+                  return rhs->getAs<TypeVariableType>() == typeVar;
+                });
+
+  if (subtypeOf.empty())
+    return;
+
+  // We need to make sure that there are no duplicate bindings in the
+  // set, other we'll produce multiple identical solutions.
+  llvm::SmallPtrSet<CanType, 4> existingTypes;
+  for (const auto &binding : bindings.Bindings)
+    existingTypes.insert(binding.BindingType->getCanonicalType());
+
+  for (auto *constraint : subtypeOf) {
+    auto *tv =
+        simplifyType(constraint->getFirstType())->getAs<TypeVariableType>();
+    if (!tv)
+      continue;
+
+    auto relatedBindings = inferredBindings.find(tv);
+    if (relatedBindings == inferredBindings.end())
+      continue;
+
+    for (auto &binding : relatedBindings->getSecond().Bindings) {
+      // We need the binding kind for the potential binding to
+      // either be Exact or Supertypes in order for it to make sense
+      // to add Supertype bindings based on the relationship between
+      // our type variables.
+      if (binding.Kind != AllowedBindingKind::Exact &&
+          binding.Kind != AllowedBindingKind::Supertypes)
+        continue;
+
+      auto type = binding.BindingType;
+
+      if (!existingTypes.insert(type->getCanonicalType()).second)
+        continue;
+
+      if (ConstraintSystem::typeVarOccursInType(typeVar, type))
+        continue;
+
+      bindings.addPotentialBinding(
+          binding.withSameSource(type, AllowedBindingKind::Supertypes));
+    }
+  }
+}
+
 Optional<ConstraintSystem::PotentialBindings>
 ConstraintSystem::determineBestBindings() {
   // Look for potential type variable bindings.
@@ -44,46 +104,8 @@ ConstraintSystem::determineBestBindings() {
       continue;
 
     auto &bindings = cachedBindings->getSecond();
-    // All of the relevant relational constraints associated with
-    // current type variable should be recored by its potential bindings.
-    for (auto *constraint : bindings.Sources) {
-      if (constraint->getKind() != ConstraintKind::Subtype)
-        continue;
-
-      auto lhs = simplifyType(constraint->getFirstType());
-      auto rhs = simplifyType(constraint->getSecondType());
-
-      // We are only interested in 'subtype' constraints which have
-      // type variable on the left-hand side.
-      if (rhs->getAs<TypeVariableType>() != typeVar)
-        continue;
-
-      auto *tv = lhs->getAs<TypeVariableType>();
-      if (!tv)
-        continue;
-
-      auto relatedBindings = cache.find(tv);
-      if (relatedBindings == cache.end())
-        continue;
-
-      for (auto &binding : relatedBindings->getSecond().Bindings) {
-        // We need the binding kind for the potential binding to
-        // either be Exact or Supertypes in order for it to make sense
-        // to add Supertype bindings based on the relationship between
-        // our type variables.
-        if (binding.Kind != AllowedBindingKind::Exact &&
-            binding.Kind != AllowedBindingKind::Supertypes)
-          continue;
-
-        auto type = binding.BindingType;
 
-        if (ConstraintSystem::typeVarOccursInType(typeVar, type))
-          continue;
-
-        bindings.addPotentialBinding(
-            binding.withSameSource(type, AllowedBindingKind::Supertypes));
-      }
-    }
+    inferTransitiveSupertypeBindings(cache, bindings);
 
     if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) {
       auto &log = getASTContext().TypeCheckerDebug->getStream();

lib/Sema/ConstraintSystem.h

@@ -3761,6 +3761,22 @@ class ConstraintSystem {
       bool &addOptionalSupertypeBindings) const;
   PotentialBindings getPotentialBindings(TypeVariableType *typeVar) const;
 
+  /// Detect `subtype` relationship between two type variables and
+  /// attempt to infer supertype bindings transitively e.g.
+  ///
+  /// Given A <: T1 <: T2 transitively A <: T2
+  ///
+  /// Which gives us a new (superclass A) binding for T2 as well as T1.
+  ///
+  /// \param inferredBindings The set of all bindings inferred for type
+  /// variables in the workset.
+  /// \param bindings The type variable we aim to infer new supertype
+  /// bindings for.
+  void inferTransitiveSupertypeBindings(
+      const llvm::SmallDenseMap<TypeVariableType *, PotentialBindings>
+          &inferredBindings,
+      PotentialBindings &bindings);
+
 private:
   /// Add a constraint to the constraint system.
   SolutionKind addConstraintImpl(ConstraintKind kind, Type first, Type second,