From 96415b237e72b061a2188109936186200ceca414 Mon Sep 17 00:00:00 2001
From: Pavel Yaskevich <pyaskevich@apple.com>
Date: Thu, 19 Nov 2020 12:57:21 -0800
Subject: [PATCH] [CSBinding] Attempt to join any existing and viable bindings
 with new binding

Instead of attempting to join only the last recorded supertype
binding, let's attempt to join any previously recorded supertype
binding with an incoming one to make sure that set contains only
the most viable types.
---
 include/swift/Sema/ConstraintSystem.h |  8 ++--
 lib/Sema/CSBindings.cpp               | 53 ++++++++++++++++-----------
 2 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h
index 1a9b32a0ed536..7e144c4c3bb64 100644
--- a/include/swift/Sema/ConstraintSystem.h
+++ b/include/swift/Sema/ConstraintSystem.h
@@ -4706,6 +4706,11 @@ class ConstraintSystem {
       return {type, kind, BindingSource};
     }
 
+    /// Determine whether this binding could be a viable candidate
+    /// to be "joined" with some other binding. It has to be at least
+    /// a non-default r-value supertype binding with no type variables.
+    bool isViableForJoin() const;
+
     static PotentialBinding forHole(TypeVariableType *typeVar,
                                     ConstraintLocator *locator) {
       return {HoleType::get(typeVar->getASTContext(), typeVar),
@@ -4745,9 +4750,6 @@ class ConstraintSystem {
     /// Whether this type variable has literal bindings.
     LiteralBindingKind LiteralBinding = LiteralBindingKind::None;
 
-    /// Tracks the position of the last known supertype in the group.
-    Optional<unsigned> lastSupertypeIndex;
-
     /// A set of all not-yet-resolved type variables this type variable
     /// is a subtype of, supertype of or is equivalent to. This is used
     /// to determine ordering inside of a chain of subtypes to help infer
diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp
index 61dc593265c6e..de8a0c41e13d3 100644
--- a/lib/Sema/CSBindings.cpp
+++ b/lib/Sema/CSBindings.cpp
@@ -22,6 +22,17 @@
 using namespace swift;
 using namespace constraints;
 
+bool ConstraintSystem::PotentialBinding::isViableForJoin() const {
+  return Kind == AllowedBindingKind::Supertypes &&
+         !BindingType->hasLValueType() &&
+         !BindingType->hasUnresolvedType() &&
+         !BindingType->hasTypeVariable() &&
+         !BindingType->hasHole() &&
+         !BindingType->hasUnboundGenericType() &&
+         !hasDefaultedLiteralProtocol() &&
+         !isDefaultableBinding();
+}
+
 bool ConstraintSystem::PotentialBindings::isPotentiallyIncomplete() const {
   // Generic parameters are always potentially incomplete.
   if (isGenericParameter())
@@ -679,30 +690,30 @@ void ConstraintSystem::PotentialBindings::addPotentialBinding(
   // If this is a non-defaulted supertype binding,
   // check whether we can combine it with another
   // supertype binding by computing the 'join' of the types.
-  if (binding.Kind == AllowedBindingKind::Supertypes &&
-      !binding.BindingType->hasUnresolvedType() &&
-      !binding.BindingType->hasTypeVariable() &&
-      !binding.BindingType->hasHole() &&
-      !binding.BindingType->hasUnboundGenericType() &&
-      !binding.hasDefaultedLiteralProtocol() &&
-      !binding.isDefaultableBinding() && allowJoinMeet) {
-    if (lastSupertypeIndex) {
-      auto &lastBinding = Bindings[*lastSupertypeIndex];
-      auto lastType = lastBinding.BindingType->getWithoutSpecifierType();
-      auto bindingType = binding.BindingType->getWithoutSpecifierType();
-
-      auto join = Type::join(lastType, bindingType);
-      if (join && !(*join)->isAny() &&
-          (!(*join)->getOptionalObjectType()
-           || !(*join)->getOptionalObjectType()->isAny())) {
-        // Replace the last supertype binding with the join. We're done.
-        lastBinding.BindingType = *join;
-        return;
+  if (binding.isViableForJoin() && allowJoinMeet) {
+    bool joined = false;
+
+    auto isAcceptableJoin = [](Type type) {
+      return !type->isAny() && (!type->getOptionalObjectType() ||
+                                !type->getOptionalObjectType()->isAny());
+    };
+
+    for (auto &existingBinding : Bindings) {
+      if (!existingBinding.isViableForJoin())
+        continue;
+
+      auto join = Type::join(existingBinding.BindingType, binding.BindingType);
+
+      if (join && isAcceptableJoin(*join)) {
+        existingBinding.BindingType = *join;
+        joined = true;
       }
     }
 
-    // Record this as the most recent supertype index.
-    lastSupertypeIndex = Bindings.size();
+    // If new binding has been joined with at least one of existing
+    // bindings, there is no reason to include it into the set.
+    if (joined)
+      return;
   }
 
   if (auto *literalProtocol = binding.getDefaultedLiteralProtocol())