Merge pull request #35029 from xedin/bindings-finalize-refactoring

[ConstraintSystem] Make binding producer responsible for adjustments to the binding set

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -5817,14 +5817,20 @@ class TypeVarBindingProducer : public BindingProducer<TypeVariableBinding> {
   llvm::SmallPtrSet<CanType, 4> ExploredTypes;
   llvm::SmallPtrSet<TypeBase *, 4> BoundTypes;
 
+  /// Determines whether this type variable has a
+  /// `ExpressibleByNilLiteral` requirement which
+  /// means that bindings have to either conform
+  /// to that protocol or be wrapped in an optional.
+  bool CanBeNil;
+
 public:
   using Element = TypeVariableBinding;
 
   TypeVarBindingProducer(ConstraintSystem &cs,
-                         ConstraintSystem::PotentialBindings &bindings)
-      : BindingProducer(cs, bindings.TypeVar->getImpl().getLocator()),
-        TypeVar(bindings.TypeVar),
-        Bindings(bindings.Bindings.begin(), bindings.Bindings.end()) {}
+                         ConstraintSystem::PotentialBindings &bindings);
+
+  /// Retrieve a set of bindings available in the current state.
+  ArrayRef<Binding> getCurrentBindings() const { return Bindings; }
 
   Optional<Element> operator()() override {
     // Once we reach the end of the current bindings
@@ -5846,6 +5852,10 @@ class TypeVarBindingProducer : public BindingProducer<TypeVariableBinding> {
   /// \returns true if some new bindings were sucessfully computed,
   /// false otherwise.
   bool computeNext();
+
+  /// 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;
 };
 
 /// Iterator over disjunction choices, makes it

lib/Sema/CSBindings.cpp

@@ -547,41 +547,6 @@ void ConstraintSystem::PotentialBindings::finalize(
   inferTransitiveBindings(cs, existingTypes, inferredBindings);
   inferDefaultTypes(cs, existingTypes);
 
-  // Adjust optionality of existing bindings based on presence of
-  // `ExpressibleByNilLiteral` requirement.
-  if (llvm::any_of(Protocols, [](Constraint *constraint) {
-        auto *protocol = constraint->getProtocol();
-        return protocol->isSpecificProtocol(
-            KnownProtocolKind::ExpressibleByNilLiteral);
-      })) {
-    for (auto &binding : Bindings) {
-      bool wrapInOptional = false;
-      if (binding.Kind == AllowedBindingKind::Supertypes) {
-        auto type = binding.BindingType->getRValueType();
-        // If the type doesn't conform to ExpressibleByNilLiteral,
-        // produce an optional of that type as a potential binding. We
-        // overwrite the binding in place because the non-optional type
-        // will fail to type-check against the nil-literal conformance.
-        bool conformsToExprByNilLiteral = false;
-        if (auto *nominalBindingDecl = type->getAnyNominal()) {
-          SmallVector<ProtocolConformance *, 2> conformances;
-          conformsToExprByNilLiteral = nominalBindingDecl->lookupConformance(
-              cs.DC->getParentModule(),
-              cs.getASTContext().getProtocol(
-                  KnownProtocolKind::ExpressibleByNilLiteral),
-              conformances);
-        }
-        wrapInOptional = !conformsToExprByNilLiteral;
-      } else if (binding.isDefaultableBinding() &&
-                 binding.BindingType->isAny()) {
-        wrapInOptional = true;
-      }
-
-      if (wrapInOptional)
-        binding.BindingType = OptionalType::get(binding.BindingType);
-    }
-  }
-
   // If there are no bindings, typeVar may be a hole.
   if (cs.shouldAttemptFixes() && Bindings.empty() &&
       TypeVar->getImpl().canBindToHole()) {
@@ -601,21 +566,6 @@ void ConstraintSystem::PotentialBindings::finalize(
 
     addPotentialBinding(PotentialBinding::forHole(TypeVar, locator));
   }
-
-  // Let's always consider `Any` to be a last resort binding because
-  // it's always better to infer concrete type and erase it if required
-  // by the context.
-  if (Bindings.size() > 1) {
-    auto AnyTypePos =
-        llvm::find_if(Bindings, [](const PotentialBinding &binding) {
-          return binding.BindingType->isAny() &&
-                 !binding.isDefaultableBinding();
-        });
-
-    if (AnyTypePos != Bindings.end()) {
-      std::rotate(AnyTypePos, AnyTypePos + 1, Bindings.end());
-    }
-  }
 }
 
 Optional<ConstraintSystem::PotentialBindings>

lib/Sema/CSStep.cpp

@@ -446,13 +446,15 @@ void TypeVariableStep::setup() {
     PO.PrintTypesForDebugging = true;
     auto &log = getDebugLogger();
 
+    auto initialBindings = Producer.getCurrentBindings();
     log << "Initial bindings: ";
-    interleave(InitialBindings.begin(), InitialBindings.end(),
-               [&](const Binding &binding) {
-                 log << TypeVar->getString(PO)
-                     << " := " << binding.BindingType->getString(PO);
-               },
-               [&log] { log << ", "; });
+    interleave(
+        initialBindings.begin(), initialBindings.end(),
+        [&](const Binding &binding) {
+          log << TypeVar->getString(PO)
+              << " := " << binding.BindingType->getString(PO);
+        },
+        [&log] { log << ", "; });
 
     log << '\n';
   }

lib/Sema/CSStep.h

@@ -479,9 +479,9 @@ class ComponentStep final : public SolverStep {
 template <typename P> class BindingStep : public SolverStep {
   using Scope = ConstraintSystem::SolverScope;
 
+protected:
   P Producer;
 
-protected:
   /// Indicates whether any of the attempted bindings
   /// produced a solution.
   bool AnySolved = false;
@@ -569,10 +569,6 @@ class TypeVariableStep final : public BindingStep<TypeVarBindingProducer> {
   using Binding = ConstraintSystem::PotentialBinding;
 
   TypeVariableType *TypeVar;
-  // A set of the initial bindings to consider, which is
-  // also a source of follow-up "computed" bindings such
-  // as supertypes, defaults etc.
-  SmallVector<Binding, 4> InitialBindings;
 
   /// Indicates whether source of one of the previously
   /// attempted bindings was a literal constraint. This
@@ -583,8 +579,7 @@ class TypeVariableStep final : public BindingStep<TypeVarBindingProducer> {
 public:
   TypeVariableStep(ConstraintSystem &cs, BindingContainer &bindings,
                    SmallVectorImpl<Solution> &solutions)
-      : BindingStep(cs, {cs, bindings}, solutions), TypeVar(bindings.TypeVar),
-        InitialBindings(bindings.Bindings.begin(), bindings.Bindings.end()) {}
+      : BindingStep(cs, {cs, bindings}, solutions), TypeVar(bindings.TypeVar) {}
 
   void setup() override;
 
@@ -593,8 +588,7 @@ class TypeVariableStep final : public BindingStep<TypeVarBindingProducer> {
   void print(llvm::raw_ostream &Out) override {
     PrintOptions PO;
     PO.PrintTypesForDebugging = true;
-    Out << "TypeVariableStep for " << TypeVar->getString(PO) << " with #"
-        << InitialBindings.size() << " initial bindings\n";
+    Out << "TypeVariableStep for " << TypeVar->getString(PO) << '\n';
   }
 
 protected:

lib/Sema/ConstraintSystem.cpp

@@ -5330,3 +5330,67 @@ bool ConstraintSystem::isReadOnlyKeyPathComponent(
 
   return false;
 }
+
+TypeVarBindingProducer::TypeVarBindingProducer(
+    ConstraintSystem &cs, ConstraintSystem::PotentialBindings &bindings)
+    : BindingProducer(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);
+      })) {
+  // A binding to `Any` which should always be considered as a last resort.
+  Optional<Binding> Any;
+
+  for (const auto &binding : bindings.Bindings) {
+    auto type = binding.BindingType;
+
+    // Adjust optionality of existing bindings based on presence of
+    // `ExpressibleByNilLiteral` requirement.
+    if (requiresOptionalAdjustment(binding)) {
+      Bindings.push_back(binding.withType(OptionalType::get(type)));
+    } else if (type->isAny()) {
+      Any.emplace(binding);
+    } else {
+      Bindings.push_back(binding);
+    }
+  }
+
+  // Let's always consider `Any` to be a last resort binding because
+  // it's always better to infer concrete type and erase it if required
+  // by the context.
+  if (Any) {
+    Bindings.push_back(*Any);
+  }
+}
+
+bool TypeVarBindingProducer::requiresOptionalAdjustment(
+    const Binding &binding) const {
+  // If type variable can't be `nil` then adjustment is
+  // not required.
+  if (!CanBeNil)
+    return false;
+
+  if (binding.Kind == BindingKind::Supertypes) {
+    auto type = binding.BindingType->getRValueType();
+    // If the type doesn't conform to ExpressibleByNilLiteral,
+    // produce an optional of that type as a potential binding. We
+    // overwrite the binding in place because the non-optional type
+    // will fail to type-check against the nil-literal conformance.
+    bool conformsToExprByNilLiteral = false;
+    if (auto *nominalBindingDecl = type->getAnyNominal()) {
+      SmallVector<ProtocolConformance *, 2> conformances;
+      conformsToExprByNilLiteral = nominalBindingDecl->lookupConformance(
+          CS.DC->getParentModule(),
+          CS.getASTContext().getProtocol(
+              KnownProtocolKind::ExpressibleByNilLiteral),
+          conformances);
+    }
+    return !conformsToExprByNilLiteral;
+  } else if (binding.isDefaultableBinding() && binding.BindingType->isAny()) {
+    return true;
+  }
+
+  return false;
+}