[CSBindings] Store the originator type variable in a transitive binding

Instead of using a flag in `addBinding`, let's record what type variable
does the binding belong to in `PotentialBinding` itself. This is going
to help remove bindings introduced transitively when the inference is
done lazily.

Author: xedin

include/swift/Sema/CSBindings.h

@@ -84,14 +84,20 @@ struct PotentialBinding {
   /// because they are synthetic, they have a locator instead.
   PointerUnion<Constraint *, ConstraintLocator *> BindingSource;
 
+  /// When the binding is transferred through a subtype chain, this
+  /// marks a type variable for which it was originally inferred.
+  TypeVariableType *Originator;
+
   PotentialBinding(Type type, AllowedBindingKind kind,
-                   PointerUnion<Constraint *, ConstraintLocator *> source)
-      : BindingType(type), Kind(kind), BindingSource(source) {}
+                   PointerUnion<Constraint *, ConstraintLocator *> source,
+                   TypeVariableType *originator)
+      : BindingType(type), Kind(kind), BindingSource(source),
+        Originator(originator) {}
 
   PotentialBinding(Type type, AllowedBindingKind kind, Constraint *source)
       : PotentialBinding(
-            type, kind,
-            PointerUnion<Constraint *, ConstraintLocator *>(source)) {}
+            type, kind, PointerUnion<Constraint *, ConstraintLocator *>(source),
+            /*originator=*/nullptr) {}
 
   bool isDefaultableBinding() const {
     if (auto *constraint = BindingSource.dyn_cast<Constraint *>())
@@ -124,13 +130,20 @@ struct PotentialBinding {
   Constraint *getSource() const { return cast<Constraint *>(BindingSource); }
 
   PotentialBinding withType(Type type) const {
-    return {type, Kind, BindingSource};
+    return {type, Kind, BindingSource, Originator};
   }
 
   PotentialBinding withSameSource(Type type, AllowedBindingKind kind) const {
-    return {type, kind, BindingSource};
+    return {type, kind, BindingSource, Originator};
+  }
+
+  PotentialBinding asTransitiveFrom(TypeVariableType *originator) const {
+    ASSERT(originator);
+    return {BindingType, Kind, BindingSource, originator};
   }
 
+  bool isTransitive() const { return bool(Originator); }
+
   /// 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.
@@ -140,12 +153,13 @@ struct PotentialBinding {
                                   ConstraintLocator *locator) {
     return {PlaceholderType::get(typeVar->getASTContext(), typeVar),
             AllowedBindingKind::Exact,
-            /*source=*/locator};
+            /*source=*/locator, /*originator=*/nullptr};
   }
 
   static PotentialBinding forPlaceholder(Type placeholderTy) {
     return {placeholderTy, AllowedBindingKind::Exact,
-            PointerUnion<Constraint *, ConstraintLocator *>()};
+            PointerUnion<Constraint *, ConstraintLocator *>(),
+            /*originator=*/nullptr};
   }
 
   void print(llvm::raw_ostream &out, const PrintOptions &PO) const;
@@ -473,7 +487,7 @@ class BindingSet {
   /// \param isTransitive Indicates whether this binding has been
   /// acquired through transitive inference and requires extra
   /// checking.
-  bool isViable(PotentialBinding &binding, bool isTransitive);
+  bool isViable(PotentialBinding &binding);
 
   /// Determine whether this set has any "viable" (or non-hole) bindings.
   ///
@@ -602,10 +616,7 @@ class BindingSet {
   /// Add a new binding to the set.
   ///
   /// \param binding The binding to add.
-  /// \param isTransitive Indicates whether this binding has been
-  /// acquired through transitive inference and requires validity
-  /// checking.
-  void addBinding(PotentialBinding binding, bool isTransitive);
+  void addBinding(PotentialBinding binding);
 
   void addLiteralRequirement(Constraint *literal);
 

include/swift/Sema/CSTrail.h

@@ -145,6 +145,7 @@ class SolverTrail {
         /// of a PotentialBinding.
         Type BindingType;
         PointerUnion<Constraint *, ConstraintLocator *> BindingSource;
+        TypeVariableType *Originator;
       } Binding;
 
       ConstraintFix *TheFix;

lib/Sema/CSBindings.cpp

@@ -43,7 +43,7 @@ BindingSet::BindingSet(ConstraintSystem &CS, TypeVariableType *TypeVar,
     : CS(CS), TypeVar(TypeVar), Info(info) {
 
   for (const auto &binding : info.Bindings)
-    addBinding(binding, /*isTransitive=*/false);
+    addBinding(binding);
 
   for (auto *constraint : info.Constraints) {
     switch (constraint->getKind()) {
@@ -596,7 +596,7 @@ void BindingSet::inferTransitiveKeyPathBindings() {
 
                 // Copy the bindings over to the root.
                 for (const auto &binding : bindings.Bindings)
-                  addBinding(binding, /*isTransitive=*/true);
+                  addBinding(binding.asTransitiveFrom(contextualRootVar));
 
                 // Make a note that the key path root is transitively adjacent
                 // to contextual root type variable and all of its variables.
@@ -606,9 +606,9 @@ void BindingSet::inferTransitiveKeyPathBindings() {
                                     bindings.AdjacentVars.end());
               }
             } else {
-              addBinding(
-                  binding.withSameSource(inferredRootTy, AllowedBindingKind::Exact),
-                  /*isTransitive=*/true);
+              auto newBinding = binding.withSameSource(
+                  inferredRootTy, AllowedBindingKind::Exact);
+              addBinding(newBinding.asTransitiveFrom(keyPathTy));
             }
           }
         }
@@ -679,8 +679,9 @@ void BindingSet::inferTransitiveSupertypeBindings() {
       if (ConstraintSystem::typeVarOccursInType(TypeVar, type))
         continue;
 
-      addBinding(binding.withSameSource(type, AllowedBindingKind::Supertypes),
-                 /*isTransitive=*/true);
+      auto newBinding =
+          binding.withSameSource(type, AllowedBindingKind::Supertypes);
+      addBinding(newBinding.asTransitiveFrom(entry.first));
     }
   }
 }
@@ -713,8 +714,7 @@ void BindingSet::inferTransitiveUnresolvedMemberRefBindings() {
                   continue;
             }
 
-            addBinding({protocolTy, AllowedBindingKind::Exact, constraint},
-                       /*isTransitive=*/false);
+            addBinding({protocolTy, AllowedBindingKind::Exact, constraint});
           }
         }
       }
@@ -829,8 +829,8 @@ bool BindingSet::finalizeKeyPathBindings() {
           // better diagnostics.
           auto keyPathTy = getKeyPathType(ctx, *capability, rootTy,
                                           CS.getKeyPathValueType(keyPath));
-          updatedBindings.insert(
-              {keyPathTy, AllowedBindingKind::Exact, locator});
+          updatedBindings.insert({keyPathTy, AllowedBindingKind::Exact, locator,
+                                  /*originator=*/nullptr});
         } else if (CS.shouldAttemptFixes()) {
           auto fixedRootTy = CS.getFixedType(rootTy);
           // If key path is structurally correct and has a resolved root
@@ -889,11 +889,11 @@ void BindingSet::finalizeUnresolvedMemberChainResult() {
   }
 }
 
-void BindingSet::addBinding(PotentialBinding binding, bool isTransitive) {
+void BindingSet::addBinding(PotentialBinding binding) {
   if (Bindings.count(binding))
     return;
 
-  if (!isViable(binding, isTransitive))
+  if (!isViable(binding))
     return;
 
   SmallPtrSet<TypeVariableType *, 4> referencedTypeVars;
@@ -957,14 +957,26 @@ void BindingSet::addBinding(PotentialBinding binding, bool isTransitive) {
     for (auto existingBinding = Bindings.begin();
          existingBinding != Bindings.end();) {
       if (existingBinding->isViableForJoin()) {
-        auto join =
+        auto joinType =
             Type::join(existingBinding->BindingType, binding.BindingType);
 
-        if (join && isAcceptableJoin(*join)) {
+        if (joinType && isAcceptableJoin(*joinType)) {
           // Result of the join has to use new binding because it refers
           // to the constraint that triggered the join that replaced the
           // existing binding.
-          joined.push_back(binding.withType(*join));
+          //
+          // For "join" to be transitive, both bindings have to be as
+          // well, otherwise we consider it a refinement of a direct
+          // binding.
+          auto *origintor =
+              binding.isTransitive() && existingBinding->isTransitive()
+                  ? binding.Originator
+                  : nullptr;
+
+          PotentialBinding join(*joinType, binding.Kind, binding.BindingSource,
+                                origintor);
+
+          joined.push_back(join);
           // Remove existing binding from the set.
           // It has to be re-introduced later, since its type has been changed.
           existingBinding = Bindings.erase(existingBinding);
@@ -1448,15 +1460,15 @@ static bool hasConversions(Type type) {
            type->is<BuiltinType>() || type->is<ArchetypeType>());
 }
 
-bool BindingSet::isViable(PotentialBinding &binding, bool isTransitive) {
+bool BindingSet::isViable(PotentialBinding &binding) {
   // Prevent against checking against the same opened nominal type
   // over and over again. Doing so means redundant work in the best
   // case. In the worst case, we'll produce lots of duplicate solutions
   // for this constraint system, which is problematic for overload
   // resolution.
   auto type = binding.BindingType;
 
-  if (isTransitive && !checkTypeOfBinding(TypeVar, type))
+  if (binding.isTransitive() && !checkTypeOfBinding(TypeVar, type))
     return false;
 
   auto *NTD = type->getAnyNominal();

lib/Sema/CSTrail.cpp

@@ -334,6 +334,7 @@ SolverTrail::Change::RetractedBinding(TypeVariableType *typeVar,
   result.Binding.TypeVar = typeVar;
   result.Binding.BindingType = binding.BindingType;
   result.Binding.BindingSource = binding.BindingSource;
+  result.Binding.Originator = binding.Originator;
   result.Options = unsigned(binding.Kind);
 
   return result;
@@ -563,9 +564,8 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const {
     break;
 
   case ChangeKind::RetractedBinding: {
-    PotentialBinding binding(Binding.BindingType,
-                             AllowedBindingKind(Options),
-                             Binding.BindingSource);
+    PotentialBinding binding(Binding.BindingType, AllowedBindingKind(Options),
+                             Binding.BindingSource, Binding.Originator);
 
     auto &bindings = cg[BindingRelation.TypeVar].getPotentialBindings();
     bindings.Bindings.push_back(binding);