Merge pull request swiftlang#34988 from xedin/on-demand-fully-bound

[CSBindings] Instead of using a flag track constraints which delay type variable

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -4740,9 +4740,8 @@ class ConstraintSystem {
     /// The set of constraints which would be used to infer default types.
     llvm::TinyPtrVector<Constraint *> Defaults;
 
-    /// Whether these bindings should be delayed until the rest of the
-    /// constraint system is considered "fully bound".
-    bool FullyBound = false;
+    /// The set of constraints which delay attempting this type variable.
+    llvm::TinyPtrVector<Constraint *> DelayedBy;
 
     /// Whether the bindings of this type involve other type variables.
     bool InvolvesTypeVariables = false;
@@ -4765,6 +4764,17 @@ class ConstraintSystem {
     /// Determine whether the set of bindings is non-empty.
     explicit operator bool() const { return !Bindings.empty(); }
 
+    /// Determine whether attempting this type variable should be
+    /// delayed until the rest of the constraint system is considered
+    /// "fully bound" meaning constraints, which affect completeness
+    /// of the binding set, for this type variable such as - member
+    /// constraint, disjunction, function application etc. - are simplified.
+    ///
+    /// Note that in some situations i.e. when there are no more
+    /// disjunctions or type variables left to attempt, it's still
+    /// okay to attempt "delayed" type variable to make forward progress.
+    bool isDelayed() const;
+
     /// Whether the bindings represent (potentially) incomplete set,
     /// there is no way to say with absolute certainty if that's the
     /// case, but that could happen when certain constraints like
@@ -4807,7 +4817,7 @@ class ConstraintSystem {
 
       return std::make_tuple(b.isHole(),
                              !hasNoDefaultableBindings,
-                             b.FullyBound,
+                             b.isDelayed(),
                              b.isSubtypeOfExistentialType(),
                              b.InvolvesTypeVariables,
                              static_cast<unsigned char>(b.LiteralBinding),
@@ -4954,8 +4964,8 @@ class ConstraintSystem {
       out.indent(indent);
       if (isPotentiallyIncomplete())
         out << "potentially_incomplete ";
-      if (FullyBound)
-        out << "fully_bound ";
+      if (isDelayed())
+        out << "delayed ";
       if (isSubtypeOfExistentialType())
         out << "subtype_of_existential ";
       if (LiteralBinding != LiteralBindingKind::None)

lib/Sema/CSBindings.cpp

@@ -33,6 +33,31 @@ bool ConstraintSystem::PotentialBinding::isViableForJoin() const {
          !isDefaultableBinding();
 }
 
+bool ConstraintSystem::PotentialBindings::isDelayed() const {
+  if (!DelayedBy.empty())
+    return true;
+
+  if (isHole()) {
+    auto *locator = TypeVar->getImpl().getLocator();
+    assert(locator && "a hole without locator?");
+
+    // Delay resolution of the code completion expression until
+    // the very end to give it a chance to be bound to some
+    // contextual type even if it's a hole.
+    if (locator->directlyAt<CodeCompletionExpr>())
+      return true;
+
+    // Delay resolution of the `nil` literal to a hole until
+    // the very end to give it a change to be bound to some
+    // other type, just like code completion expression which
+    // relies solely on contextual information.
+    if (locator->directlyAt<NilLiteralExpr>())
+      return true;
+  }
+
+  return false;
+}
+
 bool ConstraintSystem::PotentialBindings::isPotentiallyIncomplete() const {
   // Generic parameters are always potentially incomplete.
   if (isGenericParameter())
@@ -555,19 +580,6 @@ void ConstraintSystem::PotentialBindings::finalize(
     // very last moment possible, just like generic parameters.
     auto *locator = TypeVar->getImpl().getLocator();
 
-    // Delay resolution of the code completion expression until
-    // the very end to give it a chance to be bound to some
-    // contextual type even if it's a hole.
-    if (locator->directlyAt<CodeCompletionExpr>())
-      FullyBound = true;
-
-    // Delay resolution of the `nil` literal to a hole until
-    // the very end to give it a change to be bound to some
-    // other type, just like code completion expression which
-    // relies solely on contextual information.
-    if (locator->directlyAt<NilLiteralExpr>())
-      FullyBound = true;
-
     // 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"
@@ -776,7 +788,7 @@ bool ConstraintSystem::PotentialBindings::isViable(
 
 bool ConstraintSystem::PotentialBindings::favoredOverDisjunction(
     Constraint *disjunction) const {
-  if (isHole() || FullyBound)
+  if (isHole() || isDelayed())
     return false;
 
   // If this bindings are for a closure and there are no holes,
@@ -871,8 +883,7 @@ ConstraintSystem::getPotentialBindingForRelationalConstraint(
     // of bindings for them until closure's body is opened.
     if (auto *typeVar = first->getAs<TypeVariableType>()) {
       if (typeVar->getImpl().isClosureType()) {
-        result.InvolvesTypeVariables = true;
-        result.FullyBound = true;
+        result.DelayedBy.push_back(constraint);
         return None;
       }
     }
@@ -918,7 +929,7 @@ ConstraintSystem::getPotentialBindingForRelationalConstraint(
   if (type->is<DependentMemberType>()) {
     if (!ConstraintSystem::typeVarOccursInType(typeVar, type,
                                                &result.InvolvesTypeVariables)) {
-      result.FullyBound = true;
+      result.DelayedBy.push_back(constraint);
     }
 
     return None;
@@ -1056,7 +1067,7 @@ bool ConstraintSystem::PotentialBindings::infer(
         // delaying bindings for as long as possible.
         if (isExpr<ForceValueExpr>(anchor) && !type->is<LValueType>()) {
           addPotentialBinding(binding->withType(LValueType::get(type)));
-          FullyBound = true;
+          DelayedBy.push_back(constraint);
         }
 
         // If this is a type variable representing closure result,
@@ -1076,9 +1087,6 @@ bool ConstraintSystem::PotentialBindings::infer(
     break;
   }
   case ConstraintKind::KeyPathApplication: {
-    if (FullyBound)
-      return false;
-
     // If this variable is in the application projected result type, mark the
     // result as `FullyBound` to ensure we delay binding until we've bound
     // other type variables in the KeyPathApplication constraint. This ensures
@@ -1087,8 +1095,9 @@ bool ConstraintSystem::PotentialBindings::infer(
     SmallPtrSet<TypeVariableType *, 4> typeVars;
     findInferableTypeVars(cs.simplifyType(constraint->getThirdType()),
                           typeVars);
-    if (typeVars.count(TypeVar))
-      FullyBound = true;
+    if (typeVars.count(TypeVar)) {
+      DelayedBy.push_back(constraint);
+    }
 
     break;
   }
@@ -1130,17 +1139,14 @@ bool ConstraintSystem::PotentialBindings::infer(
     break;
 
   case ConstraintKind::Disjunction:
-    // FIXME: Recurse into these constraints to see whether this
-    // type variable is fully bound by any of them.
-    InvolvesTypeVariables = true;
-
     // If there is additional context available via disjunction
     // associated with closure literal (e.g. coercion to some other
     // type) let's delay resolving the closure until the disjunction
     // is attempted.
     if (TypeVar->getImpl().isClosureType())
       return true;
 
+    DelayedBy.push_back(constraint);
     break;
 
   case ConstraintKind::ConformsTo:
@@ -1162,50 +1168,46 @@ bool ConstraintSystem::PotentialBindings::infer(
   case ConstraintKind::ApplicableFunction:
   case ConstraintKind::DynamicCallableApplicableFunction:
   case ConstraintKind::BindOverload: {
-    if (FullyBound && InvolvesTypeVariables)
-      return false;
-
-    // If this variable is in the left-hand side, it is fully bound.
-    SmallPtrSet<TypeVariableType *, 4> typeVars;
-    findInferableTypeVars(cs.simplifyType(constraint->getFirstType()),
-                          typeVars);
-    if (typeVars.count(TypeVar))
-      FullyBound = true;
-
-    if (InvolvesTypeVariables)
-      return false;
-
-    // If this and another type variable occur, this result involves
-    // type variables.
-    findInferableTypeVars(cs.simplifyType(constraint->getSecondType()),
-                          typeVars);
-    if (typeVars.size() > 1 && typeVars.count(TypeVar))
-      InvolvesTypeVariables = true;
+    // It's possible that type of member couldn't be determined,
+    // and if so it would be beneficial to bind member to a hole
+    // early to propagate that information down to arguments,
+    // result type of a call that references such a member.
+    if (cs.shouldAttemptFixes() && TypeVar->getImpl().canBindToHole()) {
+      if (ConstraintSystem::typeVarOccursInType(
+              TypeVar, cs.simplifyType(constraint->getSecondType())))
+        break;
+    }
 
+    DelayedBy.push_back(constraint);
     break;
   }
 
   case ConstraintKind::ValueMember:
   case ConstraintKind::UnresolvedValueMember:
-  case ConstraintKind::ValueWitness:
-    // If our type variable shows up in the base type, there's
-    // nothing to do.
-    // FIXME: Can we avoid simplification here?
-    if (ConstraintSystem::typeVarOccursInType(
-            TypeVar, cs.simplifyType(constraint->getFirstType()),
-            &InvolvesTypeVariables)) {
-      return false;
+  case ConstraintKind::ValueWitness: {
+    // If current type variable represents a member type of some reference,
+    // it would be bound once member is resolved either to a actual member
+    // type or to a hole if member couldn't be found.
+    auto memberTy = constraint->getSecondType()->castTo<TypeVariableType>();
+
+    if (memberTy->getImpl().hasRepresentativeOrFixed()) {
+      if (auto type = memberTy->getImpl().getFixedType(/*record=*/nullptr)) {
+        // It's possible that member has been bound to some other type variable
+        // instead of merged with it because it's wrapped in an l-value type.
+        if (type->getWithoutSpecifierType()->isEqual(TypeVar)) {
+          DelayedBy.push_back(constraint);
+          break;
+        }
+      } else {
+        memberTy = memberTy->getImpl().getRepresentative(/*record=*/nullptr);
+      }
     }
 
-    // If the type variable is in the list of member type
-    // variables, it is fully bound.
-    // FIXME: Can we avoid simplification here?
-    if (ConstraintSystem::typeVarOccursInType(
-            TypeVar, cs.simplifyType(constraint->getSecondType()),
-            &InvolvesTypeVariables)) {
-      FullyBound = true;
-    }
+    if (memberTy == TypeVar)
+      DelayedBy.push_back(constraint);
+
     break;
+  }
 
   case ConstraintKind::OneWayEqual:
   case ConstraintKind::OneWayBindParam: {