Merge pull request swiftlang#35044 from xedin/refactor-hole-binding

[CSBindings] Let producer record a binding for a hole

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -4725,6 +4725,9 @@ class ConstraintSystem {
     using BindingScore =
         std::tuple<bool, bool, bool, bool, bool, unsigned char, int>;
 
+    /// The constraint system this type variable and its bindings belong to.
+    ConstraintSystem &CS;
+
     TypeVariableType *TypeVar;
 
     /// The set of potential bindings.
@@ -4764,10 +4767,13 @@ class ConstraintSystem {
     llvm::SmallMapVector<TypeVariableType *, Constraint *, 4> SupertypeOf;
     llvm::SmallMapVector<TypeVariableType *, Constraint *, 4> EquivalentTo;
 
-    PotentialBindings(TypeVariableType *typeVar) : TypeVar(typeVar) {}
+    PotentialBindings(ConstraintSystem &cs, TypeVariableType *typeVar)
+        : CS(cs), TypeVar(typeVar) {}
 
     /// Determine whether the set of bindings is non-empty.
-    explicit operator bool() const { return !Bindings.empty(); }
+    explicit operator bool() const {
+      return !Bindings.empty() || isDirectHole();
+    }
 
     /// Determine whether attempting this type variable should be
     /// delayed until the rest of the constraint system is considered
@@ -4791,17 +4797,34 @@ class ConstraintSystem {
     /// `bind param` are present in the system.
     bool isPotentiallyIncomplete() const;
 
-    /// If there is only one binding and it's to a hole type, consider
-    /// this type variable to be a hole in a constraint system regardless
-    /// of where hole type originated.
+    /// If this type variable doesn't have any viable bindings, or
+    /// if there is only one binding and it's a hole type, consider
+    /// this type variable to be a hole in a constraint system
+    /// regardless of where hole type originated.
     bool isHole() const {
+      if (isDirectHole())
+        return true;
+
       if (Bindings.size() != 1)
         return false;
 
-      auto &binding = Bindings.front();
+      const auto &binding = Bindings.front();
       return binding.BindingType->is<HoleType>();
     }
 
+    /// Determines whether the only possible binding for this type variable
+    /// would be a hole type. This is different from `isHole` method because
+    /// type variable could also acquire a hole type transitively if one
+    /// of the type variables in its subtype/equivalence chain has been
+    /// bound to a hole type.
+    bool isDirectHole() const {
+      // Direct holes are only allowed in "diagnostic mode".
+      if (!CS.shouldAttemptFixes())
+        return false;
+
+      return Bindings.empty() && TypeVar->getImpl().canBindToHole();
+    }
+
     /// Determine if the bindings only constrain the type variable from above
     /// with an existential type; such a binding is not very helpful because
     /// it's impossible to enumerate the existential type's subtypes.
@@ -4816,22 +4839,26 @@ class ConstraintSystem {
     }
 
     unsigned getNumDefaultableBindings() const {
-      return llvm::count_if(Bindings, [](const PotentialBinding &binding) {
-        return binding.isDefaultableBinding();
-      });
+      return isDirectHole()
+                 ? 1
+                 : llvm::count_if(Bindings,
+                                  [](const PotentialBinding &binding) {
+                                    return binding.isDefaultableBinding();
+                                  });
     }
 
     static BindingScore formBindingScore(const PotentialBindings &b) {
       auto numDefaults = b.getNumDefaultableBindings();
-      auto hasNoDefaultableBindings = b.Bindings.size() > numDefaults;
+      auto numNonDefaultableBindings =
+          b.isDirectHole() ? 0 : b.Bindings.size() - numDefaults;
 
       return std::make_tuple(b.isHole(),
-                             !hasNoDefaultableBindings,
+                             numNonDefaultableBindings == 0,
                              b.isDelayed(),
                              b.isSubtypeOfExistentialType(),
                              b.involvesTypeVariables(),
                              static_cast<unsigned char>(b.LiteralBinding),
-                             -(b.Bindings.size() - numDefaults));
+                             -numNonDefaultableBindings);
     }
 
     /// Compare two sets of bindings, where \c x < y indicates that
@@ -4847,8 +4874,10 @@ class ConstraintSystem {
       if (yScore < xScore)
         return false;
 
-      auto xDefaults = x.Bindings.size() + std::get<6>(xScore);
-      auto yDefaults = y.Bindings.size() + std::get<6>(yScore);
+      auto xDefaults =
+          x.isDirectHole() ? 1 : x.Bindings.size() + std::get<6>(xScore);
+      auto yDefaults =
+          y.isDirectHole() ? 1 : y.Bindings.size() + std::get<6>(yScore);
 
       // If there is a difference in number of default types,
       // prioritize bindings with fewer of them.
@@ -4972,6 +5001,8 @@ class ConstraintSystem {
     void dump(llvm::raw_ostream &out,
               unsigned indent = 0) const LLVM_ATTRIBUTE_USED {
       out.indent(indent);
+      if (isDirectHole())
+        out << "hole ";
       if (isPotentiallyIncomplete())
         out << "potentially_incomplete ";
       if (isDelayed())
@@ -5826,8 +5857,7 @@ class TypeVarBindingProducer : public BindingProducer<TypeVariableBinding> {
 public:
   using Element = TypeVariableBinding;
 
-  TypeVarBindingProducer(ConstraintSystem &cs,
-                         ConstraintSystem::PotentialBindings &bindings);
+  TypeVarBindingProducer(ConstraintSystem::PotentialBindings &bindings);
 
   /// Retrieve a set of bindings available in the current state.
   ArrayRef<Binding> getCurrentBindings() const { return Bindings; }

lib/Sema/CSBindings.cpp

@@ -546,26 +546,6 @@ void ConstraintSystem::PotentialBindings::finalize(
   inferTransitiveProtocolRequirements(cs, inferredBindings);
   inferTransitiveBindings(cs, existingTypes, inferredBindings);
   inferDefaultTypes(cs, existingTypes);
-
-  // If there are no bindings, typeVar may be a hole.
-  if (cs.shouldAttemptFixes() && Bindings.empty() &&
-      TypeVar->getImpl().canBindToHole()) {
-    // If the base of the unresolved member reference like `.foo`
-    // couldn't be resolved we'd want to bind it to a hole at the
-    // very last moment possible, just like generic parameters.
-    auto *locator = TypeVar->getImpl().getLocator();
-
-    // If this type variable is associated with a code completion token
-    // and it failed to infer any bindings let's adjust hole's locator
-    // to point to a code completion token to avoid attempting to "fix"
-    // this problem since its rooted in the fact that constraint system
-    // is under-constrained.
-    if (AssociatedCodeCompletionToken) {
-      locator = cs.getConstraintLocator(AssociatedCodeCompletionToken);
-    }
-
-    addPotentialBinding(PotentialBinding::forHole(TypeVar, locator));
-  }
 }
 
 Optional<ConstraintSystem::PotentialBindings>
@@ -595,7 +575,7 @@ ConstraintSystem::determineBestBindings() {
     if (shouldAttemptFixes() && typeVar->getImpl().canBindToHole())
       return true;
 
-    return !bindings.Bindings.empty() || !bindings.Defaults.empty() ||
+    return bindings || !bindings.Defaults.empty() ||
            llvm::any_of(bindings.Protocols, [&](Constraint *constraint) {
              return bool(
                  TypeChecker::getDefaultType(constraint->getProtocol(), DC));
@@ -635,7 +615,7 @@ ConstraintSystem::determineBestBindings() {
     // If these are the first bindings, or they are better than what
     // we saw before, use them instead.
     if (!bestBindings || bindings < *bestBindings)
-      bestBindings = bindings;
+      bestBindings.emplace(bindings);
   }
 
   return bestBindings;
@@ -783,7 +763,7 @@ ConstraintSystem::inferBindingsFor(TypeVariableType *typeVar, bool finalize) {
          "not a representative");
   assert(!typeVar->getImpl().getFixedType(nullptr) && "has a fixed type");
 
-  PotentialBindings bindings(typeVar);
+  PotentialBindings bindings(*this, typeVar);
 
   // Gather the constraints associated with this type variable.
   auto constraints = CG.gatherConstraints(
@@ -796,7 +776,7 @@ ConstraintSystem::inferBindingsFor(TypeVariableType *typeVar, bool finalize) {
 
     // Upon inference failure let's produce an empty set of bindings.
     if (failed)
-      return {typeVar};
+      return {*this, typeVar};
   }
 
   if (finalize) {

lib/Sema/CSSolver.cpp

@@ -2287,7 +2287,8 @@ void DisjunctionChoice::propagateConversionInfo(ConstraintSystem &cs) const {
     return;
 
   auto bindings = cs.inferBindingsFor(typeVar);
-  if (bindings.involvesTypeVariables() || bindings.Bindings.size() != 1)
+  if (bindings.isHole() || bindings.involvesTypeVariables() ||
+      bindings.Bindings.size() != 1)
     return;
 
   auto conversionType = bindings.Bindings[0].BindingType;

lib/Sema/CSStep.cpp

@@ -342,7 +342,7 @@ StepResult ComponentStep::take(bool prevFailed) {
       (!disjunction || bestBindings->favoredOverDisjunction(disjunction))) {
     // Produce a type variable step.
     return suspend(
-        std::make_unique<TypeVariableStep>(CS, *bestBindings, Solutions));
+        std::make_unique<TypeVariableStep>(*bestBindings, Solutions));
   } else if (disjunction) {
     // Produce a disjunction step.
     return suspend(

lib/Sema/CSStep.h

@@ -577,9 +577,10 @@ class TypeVariableStep final : public BindingStep<TypeVarBindingProducer> {
   bool SawFirstLiteralConstraint = false;
 
 public:
-  TypeVariableStep(ConstraintSystem &cs, BindingContainer &bindings,
+  TypeVariableStep(BindingContainer &bindings,
                    SmallVectorImpl<Solution> &solutions)
-      : BindingStep(cs, {cs, bindings}, solutions), TypeVar(bindings.TypeVar) {}
+      : BindingStep(bindings.CS, {bindings}, solutions),
+        TypeVar(bindings.TypeVar) {}
 
   void setup() override;
 

lib/Sema/ConstraintSystem.cpp

@@ -5332,14 +5332,29 @@ bool ConstraintSystem::isReadOnlyKeyPathComponent(
 }
 
 TypeVarBindingProducer::TypeVarBindingProducer(
-    ConstraintSystem &cs, ConstraintSystem::PotentialBindings &bindings)
-    : BindingProducer(cs, bindings.TypeVar->getImpl().getLocator()),
+    ConstraintSystem::PotentialBindings &bindings)
+    : BindingProducer(bindings.CS, bindings.TypeVar->getImpl().getLocator()),
       TypeVar(bindings.TypeVar),
       CanBeNil(llvm::any_of(bindings.Protocols, [](Constraint *constraint) {
         auto *protocol = constraint->getProtocol();
         return protocol->isSpecificProtocol(
             KnownProtocolKind::ExpressibleByNilLiteral);
       })) {
+  if (bindings.isDirectHole()) {
+    auto *locator = getLocator();
+    // If this type variable is associated with a code completion token
+    // and it failed to infer any bindings let's adjust hole's locator
+    // to point to a code completion token to avoid attempting to "fix"
+    // this problem since its rooted in the fact that constraint system
+    // is under-constrained.
+    if (bindings.AssociatedCodeCompletionToken) {
+      locator = CS.getConstraintLocator(bindings.AssociatedCodeCompletionToken);
+    }
+
+    Bindings.push_back(Binding::forHole(TypeVar, locator));
+    return;
+  }
+
   // A binding to `Any` which should always be considered as a last resort.
   Optional<Binding> Any;