Merge pull request swiftlang#35098 from xedin/dont-precompute-default-bindings

xedin · web-flow · commit 3e1b01ac7aa3 · 2020-12-17T00:33:37.000-08:00
[CSBindings] Don't generate bindings for defaults
diff --git a/include/swift/Sema/ConstraintSystem.h b/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,45 @@ 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;
+
+      auto numDefaultable = llvm::count_if(
+          Defaults, [](const std::pair<CanType, Constraint *> &entry) {
+            return entry.second->getKind() == ConstraintKind::Defaultable;
+          });
+
+      // Short-circuit unviable checks if there are no defaultable bindings.
+      if (numDefaultable == 0)
+        return 0;
+
+      // Defaultable constraint is unviable if its type is covered by
+      // an existing direct or transitive binding.
+      auto unviable =
+          llvm::count_if(Bindings, [&](const PotentialBinding &binding) {
+            auto type = binding.BindingType->getCanonicalType();
+            auto def = Defaults.find(type);
+            return def != Defaults.end()
+                       ? def->second->getKind() == ConstraintKind::Defaultable
+                       : false;
+          });
+
+      assert(numDefaultable >= unviable);
+      return numDefaultable - unviable;
     }
 
     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 +4901,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 +4947,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 +5041,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 +5878,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 +5928,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
diff --git a/lib/Sema/CSBindings.cpp b/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>());
@@ -1025,7 +1013,8 @@ bool ConstraintSystem::PotentialBindings::infer(
         // type from context e.g. parameter type of a function call),
         // we need to test type with and without l-value after
         // delaying bindings for as long as possible.
-        if (isExpr<ForceValueExpr>(anchor) && !type->is<LValueType>()) {
+        if (isExpr<ForceValueExpr>(anchor) &&
+            TypeVar->getImpl().canBindToLValue() && !type->is<LValueType>()) {
           addPotentialBinding(binding->withType(LValueType::get(type)));
           DelayedBy.push_back(constraint);
         }
@@ -1094,7 +1083,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 +1347,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;
 
diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp
@@ -5372,6 +5372,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(
@@ -5403,3 +5421,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;
+}
