Skip to content

[CS] Move constructor ranking rule into CSRanking #39543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions include/swift/Sema/Constraint.h
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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:
Expand Down
19 changes: 19 additions & 0 deletions include/swift/Sema/ConstraintLocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions include/swift/Sema/ConstraintLocatorPathElts.def
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
7 changes: 0 additions & 7 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 0 additions & 2 deletions lib/Sema/CSBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
40 changes: 16 additions & 24 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
67 changes: 64 additions & 3 deletions lib/Sema/CSRanking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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.
Expand All @@ -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) {
Expand Down
Loading