Merge pull request swiftlang#39543 from hamishknight/constructing-a-tier-list

Author: hamishknight

include/swift/Sema/Constraint.h

@@ -171,12 +171,6 @@ enum class ConstraintKind : char {
   /// The key path type is chosen based on the selection of overloads for the
   /// member references along the path.
   KeyPath,
-  /// The first type is a function type, the second is the function's
-  /// input type.
-  FunctionInput,
-  /// The first type is a function type, the second is the function's
-  /// result type.
-  FunctionResult,
   /// The first type will be equal to the second type, but only when the
   /// second type has been fully determined (and mapped down to a concrete
   /// type). At that point, this constraint will be treated like an `Equal`
@@ -691,8 +685,6 @@ class Constraint final : public llvm::ilist_node<Constraint>,
     case ConstraintKind::KeyPath:
     case ConstraintKind::KeyPathApplication:
     case ConstraintKind::Defaultable:
-    case ConstraintKind::FunctionInput:
-    case ConstraintKind::FunctionResult:
       return ConstraintClassification::TypeProperty;
 
     case ConstraintKind::Disjunction:

include/swift/Sema/ConstraintLocator.h

@@ -960,6 +960,25 @@ class LocatorPathElt::KeyPathType final
   }
 };
 
+class LocatorPathElt::ConstructorMemberType final
+    : public StoredIntegerElement<1> {
+public:
+  ConstructorMemberType(bool isShortFormOrSelfDelegating = false)
+      : StoredIntegerElement(ConstraintLocator::ConstructorMemberType,
+                             isShortFormOrSelfDelegating) {}
+
+  /// Whether this constructor overload is for a short-form init call such as
+  /// 'X(...)', or a 'self.init(...)' call. Such calls have additional ranking
+  /// rules.
+  bool isShortFormOrSelfDelegatingConstructor() const {
+    return bool(getValue());
+  }
+
+  static bool classof(const LocatorPathElt *elt) {
+    return elt->getKind() == ConstraintLocator::ConstructorMemberType;
+  }
+};
+
 class LocatorPathElt::ClosureBodyElement final
     : public StoredPointerElement<void> {
 public:

include/swift/Sema/ConstraintLocatorPathElts.def

@@ -60,6 +60,9 @@ CUSTOM_LOCATOR_PATH_ELT(ClosureBody)
 /// The lookup for a constructor member.
 SIMPLE_LOCATOR_PATH_ELT(ConstructorMember)
 
+/// The constructor member type in the lookup of a constructor.
+CUSTOM_LOCATOR_PATH_ELT(ConstructorMemberType)
+
 /// The desired contextual type passed in to the constraint system.
 CUSTOM_LOCATOR_PATH_ELT(ContextualType)
 

include/swift/Sema/ConstraintSystem.h

@@ -4751,13 +4751,6 @@ class ConstraintSystem {
                                           TypeMatchOptions flags,
                                           ConstraintLocatorBuilder locator);
 
-  /// Attempt to simplify a function input or result constraint.
-  SolutionKind simplifyFunctionComponentConstraint(
-                                          ConstraintKind kind,
-                                          Type first, Type second,
-                                          TypeMatchOptions flags,
-                                          ConstraintLocatorBuilder locator);
-
   /// Attempt to simplify the BridgingConversion constraint.
   SolutionKind simplifyBridgingConstraint(Type type1,
                                          Type type2,

lib/Sema/CSBindings.cpp

@@ -1336,8 +1336,6 @@ void PotentialBindings::infer(Constraint *constraint) {
   case ConstraintKind::EscapableFunctionOf:
   case ConstraintKind::OpenedExistentialOf:
   case ConstraintKind::KeyPath:
-  case ConstraintKind::FunctionInput:
-  case ConstraintKind::FunctionResult:
   case ConstraintKind::ClosureBodyElement:
   case ConstraintKind::Conjunction:
     // Constraints from which we can't do anything.

lib/Sema/CSGen.cpp

@@ -1214,8 +1214,14 @@ namespace {
       auto *memberLoc =
           CS.getConstraintLocator(expr, ConstraintLocator::ConstructorMember);
 
+      auto *fnLoc =
+          CS.getConstraintLocator(expr, ConstraintLocator::ApplyFunction);
+
+      auto *memberTypeLoc = CS.getConstraintLocator(
+          fnLoc, LocatorPathElt::ConstructorMemberType());
+
       auto *memberType =
-          CS.createTypeVariable(memberLoc, TVO_CanBindToNoEscape);
+          CS.createTypeVariable(memberTypeLoc, TVO_CanBindToNoEscape);
 
       CS.addValueMemberConstraint(MetatypeType::get(witnessType, ctx),
                                   DeclNameRef(constrName), memberType, CurDC,
@@ -1228,10 +1234,9 @@ namespace {
           CS.getConstraintLocator(expr, ConstraintLocator::FunctionResult),
           TVO_CanBindToNoEscape);
 
-      CS.addConstraint(
-          ConstraintKind::ApplicableFunction,
-          FunctionType::get(params, resultType), memberType,
-          CS.getConstraintLocator(expr, ConstraintLocator::ApplyFunction));
+      CS.addConstraint(ConstraintKind::ApplicableFunction,
+                       FunctionType::get(params, resultType), memberType,
+                       fnLoc);
 
       if (constr->isFailable())
         return OptionalType::get(witnessType);
@@ -1500,25 +1505,12 @@ namespace {
         //   self.super = Super.init()
         baseTy = MetatypeType::get(baseTy, ctx);
 
-        auto methodTy = CS.createTypeVariable(
-            CS.getConstraintLocator(expr,
-                                    ConstraintLocator::ApplyFunction),
-            TVO_CanBindToNoEscape);
-
-        // FIXME: Once TVO_PrefersSubtypeBinding is replaced with something
-        // better, we won't need the second type variable at all.
-        {
-          auto argTy = CS.createTypeVariable(
-              CS.getConstraintLocator(expr,
-                                      ConstraintLocator::ApplyArgument),
-              (TVO_CanBindToLValue |
-               TVO_CanBindToInOut |
-               TVO_CanBindToNoEscape |
-               TVO_PrefersSubtypeBinding));
-          CS.addConstraint(
-              ConstraintKind::FunctionInput, methodTy, argTy,
-              CS.getConstraintLocator(expr));
-        }
+        auto memberTypeLoc = CS.getConstraintLocator(
+            expr, LocatorPathElt::ConstructorMemberType(
+                      /*shortFormOrSelfDelegating*/ true));
+
+        auto methodTy =
+            CS.createTypeVariable(memberTypeLoc, TVO_CanBindToNoEscape);
 
         CS.addValueMemberConstraint(
             baseTy, expr->getName(), methodTy, CurDC,

lib/Sema/CSRanking.cpp

@@ -766,6 +766,40 @@ static void addKeyPathDynamicMemberOverloads(
   }
 }
 
+/// Given the bound types of two constructor overloads, returns their parameter
+/// list types as tuples to compare for solution ranking, or \c None if they
+/// shouldn't be compared.
+static Optional<std::pair<Type, Type>>
+getConstructorParamsAsTuples(ASTContext &ctx, Type boundTy1, Type boundTy2) {
+  auto choiceTy1 =
+      boundTy1->lookThroughAllOptionalTypes()->getAs<FunctionType>();
+  auto choiceTy2 =
+      boundTy2->lookThroughAllOptionalTypes()->getAs<FunctionType>();
+
+  // If the type variables haven't been bound to functions yet, let's not try
+  // and rank them.
+  if (!choiceTy1 || !choiceTy2)
+    return None;
+
+  auto initParams1 = choiceTy1->getParams();
+  auto initParams2 = choiceTy2->getParams();
+  if (initParams1.size() != initParams2.size())
+    return None;
+
+  // Don't compare if there are variadic differences. This preserves the
+  // behavior of when we'd compare through matchTupleTypes with the parameter
+  // flags intact.
+  for (auto idx : indices(initParams1)) {
+    if (initParams1[idx].isVariadic() != initParams2[idx].isVariadic())
+      return None;
+  }
+  auto tuple1 = AnyFunctionType::composeTuple(ctx, initParams1,
+                                              /*wantParamFlags*/ false);
+  auto tuple2 = AnyFunctionType::composeTuple(ctx, initParams2,
+                                              /*wantParamFlags*/ false);
+  return std::make_pair(tuple1, tuple2);
+}
+
 SolutionCompareResult ConstraintSystem::compareSolutions(
     ConstraintSystem &cs, ArrayRef<Solution> solutions,
     const SolutionDiff &diff, unsigned idx1, unsigned idx2) {
@@ -1127,11 +1161,23 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
 
   for (const auto &binding1 : bindings1) {
     auto *typeVar = binding1.first;
+    auto *loc = typeVar->getImpl().getLocator();
+
+    // Check whether this is the overload type for a short-form init call
+    // 'X(...)' or 'self.init(...)' call.
+    auto isShortFormOrSelfDelegatingConstructorBinding = false;
+    if (auto initMemberTypeElt =
+            loc->getLastElementAs<LocatorPathElt::ConstructorMemberType>()) {
+      isShortFormOrSelfDelegatingConstructorBinding =
+          initMemberTypeElt->isShortFormOrSelfDelegatingConstructor();
+    }
 
     // If the type variable isn't one for which we should be looking at the
     // bindings, don't.
-    if (!typeVar->getImpl().prefersSubtypeBinding())
+    if (!typeVar->getImpl().prefersSubtypeBinding() &&
+        !isShortFormOrSelfDelegatingConstructorBinding) {
       continue;
+    }
 
     // If both solutions have a binding for this type variable
     // let's consider it.
@@ -1142,9 +1188,24 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
     auto concreteType1 = binding1.second;
     auto concreteType2 = binding2->second;
 
-    if (!concreteType1->isEqual(concreteType2)) {
-      typeDiff.insert({typeVar, {concreteType1, concreteType2}});
+    // For short-form and self-delegating init calls, we want to prefer
+    // parameter lists with subtypes over supertypes. To do this, compose tuples
+    // for the bound parameter lists, and compare them in the type diff. This
+    // logic preserves the behavior of when we used to bind the parameter list
+    // as a tuple to a TVO_PrefersSubtypeBinding type variable for such calls.
+    // FIXME: We should come up with a better way of doing this, though note we
+    // have some ranking and subtyping rules specific to tuples that we may need
+    // to preserve to avoid breaking source.
+    if (isShortFormOrSelfDelegatingConstructorBinding) {
+      auto diffs = getConstructorParamsAsTuples(cs.getASTContext(),
+                                                concreteType1, concreteType2);
+      if (!diffs)
+        continue;
+      std::tie(concreteType1, concreteType2) = *diffs;
     }
+
+    if (!concreteType1->isEqual(concreteType2))
+      typeDiff.insert({typeVar, {concreteType1, concreteType2}});
   }
 
   for (auto &binding : typeDiff) {