[CSBindings] Don't generate bindings for defaults

Let's keep defaults separate from direct and transitive bindings,
that would make it easier to handle them in incremental model.

Instead of generating bindings for defaults and adding to the main
set, let's allow producer to choose what to do with them once type
variable has been picked for attempting.

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -4741,7 +4741,7 @@ class ConstraintSystem {
     Optional<llvm::SmallPtrSet<Constraint *, 4>> TransitiveProtocols;
 
     /// The set of constraints which would be used to infer default types.
-    llvm::TinyPtrVector<Constraint *> Defaults;
+    llvm::SmallDenseMap<CanType, Constraint *, 2> Defaults;
 
     /// The set of constraints which delay attempting this type variable.
     llvm::TinyPtrVector<Constraint *> DelayedBy;
@@ -4772,7 +4772,7 @@ class ConstraintSystem {
 
     /// Determine whether the set of bindings is non-empty.
     explicit operator bool() const {
-      return !Bindings.empty() || isDirectHole();
+      return !Bindings.empty() || !Defaults.empty() || isDirectHole();
     }
 
     /// Determine whether attempting this type variable should be
@@ -4822,7 +4822,8 @@ class ConstraintSystem {
       if (!CS.shouldAttemptFixes())
         return false;
 
-      return Bindings.empty() && TypeVar->getImpl().canBindToHole();
+      return Bindings.empty() && Defaults.empty() &&
+             TypeVar->getImpl().canBindToHole();
     }
 
     /// Determine if the bindings only constrain the type variable from above
@@ -4838,19 +4839,38 @@ class ConstraintSystem {
       });
     }
 
-    unsigned getNumDefaultableBindings() const {
-      return isDirectHole()
-                 ? 1
-                 : llvm::count_if(Bindings,
-                                  [](const PotentialBinding &binding) {
-                                    return binding.isDefaultableBinding();
-                                  });
+    unsigned getNumViableDefaultableBindings() const {
+      if (isDirectHole())
+        return 1;
+
+      // We might want to consider adding this as a field to `PotentialBindings`
+      // and collect this information as new bindings become available but,
+      // since we are moving towards incremental model, which implies that
+      // `PotentialBindings` would stay alive for a long time, it's not
+      // immediately clear whether storage overhead of keeping this set just
+      // for ranking is worth it.
+      llvm::SmallPtrSet<CanType, 4> discoveredTypes;
+      for (const auto &binding : Bindings)
+        discoveredTypes.insert(binding.BindingType->getCanonicalType());
+
+      return llvm::count_if(
+          Defaults, [&](const std::pair<CanType, Constraint *> &def) {
+            return def.second->getKind() == ConstraintKind::Defaultable &&
+                   discoveredTypes.count(def.first) == 0;
+          });
     }
 
     static BindingScore formBindingScore(const PotentialBindings &b) {
-      auto numDefaults = b.getNumDefaultableBindings();
+      // If there are no bindings available but this type
+      // variable represents a closure - let's consider it
+      // as having a single non-default binding - that would
+      // be a type inferred based on context.
+      // It's considered to be non-default for purposes of
+      // ranking because we'd like to prioritize resolving
+      // closures to gain more information from their bodies.
       auto numNonDefaultableBindings =
-          b.isDirectHole() ? 0 : b.Bindings.size() - numDefaults;
+          !b.Bindings.empty() ? b.Bindings.size()
+                              : b.TypeVar->getImpl().isClosureType() ? 1 : 0;
 
       return std::make_tuple(b.isHole(),
                              numNonDefaultableBindings == 0,
@@ -4874,10 +4894,8 @@ class ConstraintSystem {
       if (yScore < xScore)
         return false;
 
-      auto xDefaults =
-          x.isDirectHole() ? 1 : x.Bindings.size() + std::get<6>(xScore);
-      auto yDefaults =
-          y.isDirectHole() ? 1 : y.Bindings.size() + std::get<6>(yScore);
+      auto xDefaults = x.getNumViableDefaultableBindings();
+      auto yDefaults = y.getNumViableDefaultableBindings();
 
       // If there is a difference in number of default types,
       // prioritize bindings with fewer of them.
@@ -4922,6 +4940,8 @@ class ConstraintSystem {
       }
     }
 
+    void addDefault(Constraint *constraint);
+
     /// Add a potential binding to the list of bindings,
     /// coalescing supertype bounds when we are able to compute the meet.
     void addPotentialBinding(PotentialBinding binding,
@@ -5014,34 +5034,46 @@ class ConstraintSystem {
       if (involvesTypeVariables())
         out << "involves_type_vars ";
 
-      auto numDefaultable = getNumDefaultableBindings();
+      auto numDefaultable = getNumViableDefaultableBindings();
       if (numDefaultable > 0)
         out << "#defaultable_bindings=" << numDefaultable << " ";
 
       PrintOptions PO;
       PO.PrintTypesForDebugging = true;
+
+      auto printBinding = [&](const PotentialBinding &binding) {
+        auto type = binding.BindingType;
+        switch (binding.Kind) {
+        case AllowedBindingKind::Exact:
+          break;
+
+        case AllowedBindingKind::Subtypes:
+          out << "(subtypes of) ";
+          break;
+
+        case AllowedBindingKind::Supertypes:
+          out << "(supertypes of) ";
+          break;
+        }
+        if (auto *literal = binding.getDefaultedLiteralProtocol())
+          out << "(default from " << literal->getName() << ") ";
+        out << type.getString(PO);
+      };
+
       out << "bindings={";
-      interleave(Bindings,
-                 [&](const PotentialBinding &binding) {
-                   auto type = binding.BindingType;
-                   switch (binding.Kind) {
-                   case AllowedBindingKind::Exact:
-                     break;
-
-                   case AllowedBindingKind::Subtypes:
-                     out << "(subtypes of) ";
-                     break;
-
-                   case AllowedBindingKind::Supertypes:
-                     out << "(supertypes of) ";
-                     break;
-                   }
-                   if (auto *literal = binding.getDefaultedLiteralProtocol())
-                     out << "(default from " << literal->getName() << ") ";
-                   out << type.getString(PO);
-                 },
-                 [&]() { out << "; "; });
+      interleave(Bindings, printBinding, [&]() { out << "; "; });
       out << "}";
+
+      if (!Defaults.empty()) {
+        out << " defaults={";
+        for (const auto &entry : Defaults) {
+          auto *constraint = entry.second;
+          PotentialBinding binding{constraint->getSecondType(),
+                                   AllowedBindingKind::Exact, constraint};
+          printBinding(binding);
+        }
+        out << "}";
+      }
     }
 
     void dump(ConstraintSystem *cs,
@@ -5839,6 +5871,9 @@ class TypeVarBindingProducer : public BindingProducer<TypeVariableBinding> {
 
   TypeVariableType *TypeVar;
   llvm::SmallVector<Binding, 8> Bindings;
+  /// The set of defaults to attempt once producer
+  /// runs out of direct & transitive bindings.
+  llvm::SmallVector<Constraint *, 4> DelayedDefaults;
 
   // The index pointing to the offset in the bindings
   // generator is currently at, `numTries` represents
@@ -5886,6 +5921,8 @@ class TypeVarBindingProducer : public BindingProducer<TypeVariableBinding> {
   /// Check whether binding type is required to either conform to
   /// `ExpressibleByNilLiteral` protocol or be wrapped into an optional type.
   bool requiresOptionalAdjustment(const Binding &binding) const;
+
+  Binding getDefaultBinding(Constraint *constraint) const;
 };
 
 /// Iterator over disjunction choices, makes it

lib/Sema/CSBindings.cpp

@@ -319,7 +319,8 @@ void ConstraintSystem::PotentialBindings::inferTransitiveBindings(
                   });
 
     // Infer transitive defaults.
-    llvm::copy(bindings.Defaults, std::back_inserter(Defaults));
+    for (const auto &def : bindings.Defaults)
+      addDefault(def.second);
 
     // TODO: We shouldn't need this in the future.
     if (entry.second->getKind() != ConstraintKind::Subtype)
@@ -513,24 +514,6 @@ void ConstraintSystem::PotentialBindings::inferDefaultTypes(
                              : AllowedBindingKind::Supertypes,
                          constraint});
   }
-
-  /// Add defaultable constraints.
-  for (auto *constraint : Defaults) {
-    Type type = constraint->getSecondType();
-    if (!existingTypes.insert(type->getCanonicalType()).second)
-      continue;
-
-    if (constraint->getKind() == ConstraintKind::DefaultClosureType) {
-      // If there are no other possible bindings for this closure
-      // let's default it to the type inferred from its parameters/body,
-      // otherwise we should only attempt contextual types as a
-      // top-level closure type.
-      if (!Bindings.empty())
-        continue;
-    }
-
-    addPotentialBinding({type, AllowedBindingKind::Exact, constraint});
-  }
 }
 
 void ConstraintSystem::PotentialBindings::finalize(
@@ -575,7 +558,7 @@ ConstraintSystem::determineBestBindings() {
     if (shouldAttemptFixes() && typeVar->getImpl().canBindToHole())
       return true;
 
-    return bindings || !bindings.Defaults.empty() ||
+    return bindings ||
            llvm::any_of(bindings.Protocols, [&](Constraint *constraint) {
              return bool(
                  TypeChecker::getDefaultType(constraint->getProtocol(), DC));
@@ -654,6 +637,11 @@ findInferableTypeVars(Type type,
   type.walk(Walker(typeVars));
 }
 
+void ConstraintSystem::PotentialBindings::addDefault(Constraint *constraint) {
+  auto defaultTy = constraint->getSecondType();
+  Defaults.insert({defaultTy->getCanonicalType(), constraint});
+}
+
 void ConstraintSystem::PotentialBindings::addPotentialBinding(
     PotentialBinding binding, bool allowJoinMeet) {
   assert(!binding.BindingType->is<ErrorType>());
@@ -1094,7 +1082,7 @@ bool ConstraintSystem::PotentialBindings::infer(
     // Do these in a separate pass.
     if (cs.getFixedTypeRecursive(constraint->getFirstType(), true)
             ->getAs<TypeVariableType>() == TypeVar) {
-      Defaults.push_back(constraint);
+      addDefault(constraint);
     }
     break;
 
@@ -1358,6 +1346,22 @@ bool TypeVarBindingProducer::computeNext() {
     }
   }
 
+  if (NumTries == 0) {
+    // Add defaultable constraints (if any).
+    for (auto *constraint : DelayedDefaults) {
+      if (constraint->getKind() == ConstraintKind::DefaultClosureType) {
+        // If there are no other possible bindings for this closure
+        // let's default it to the type inferred from its parameters/body,
+        // otherwise we should only attempt contextual types as a
+        // top-level closure type.
+        if (!ExploredTypes.empty())
+          continue;
+      }
+
+      addNewBinding(getDefaultBinding(constraint));
+    }
+  }
+
   if (newBindings.empty())
     return false;
 

lib/Sema/ConstraintSystem.cpp

@@ -5378,6 +5378,24 @@ TypeVarBindingProducer::TypeVarBindingProducer(
   if (Any) {
     Bindings.push_back(*Any);
   }
+
+  {
+    bool noBindings = Bindings.empty();
+
+    for (const auto &entry : bindings.Defaults) {
+      auto *constraint = entry.second;
+      if (noBindings) {
+        // If there are no direct or transitive bindings to attempt
+        // let's add defaults to the list right away.
+        Bindings.push_back(getDefaultBinding(constraint));
+      } else {
+        // Otherwise let's delay attempting default bindings
+        // until all of the direct & transitive bindings and
+        // their derivatives have been attempted.
+        DelayedDefaults.push_back(constraint);
+      }
+    }
+  }
 }
 
 bool TypeVarBindingProducer::requiresOptionalAdjustment(
@@ -5409,3 +5427,15 @@ bool TypeVarBindingProducer::requiresOptionalAdjustment(
 
   return false;
 }
+
+ConstraintSystem::PotentialBinding
+TypeVarBindingProducer::getDefaultBinding(Constraint *constraint) const {
+  assert(constraint->getKind() == ConstraintKind::Defaultable ||
+         constraint->getKind() == ConstraintKind::DefaultClosureType);
+
+  auto type = constraint->getSecondType();
+  Binding binding{type, BindingKind::Exact, constraint};
+  return requiresOptionalAdjustment(binding)
+             ? binding.withType(OptionalType::get(type))
+             : binding;
+}