Skip to content

Constraint simplification cleanups #21818

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 3 commits into from
Jan 13, 2019
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
1 change: 1 addition & 0 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1610,6 +1610,7 @@ namespace {
CS.getConstraintLocator(expr,
ConstraintLocator::ApplyArgument),
(TVO_CanBindToLValue |
TVO_CanBindToInOut |
TVO_PrefersSubtypeBinding));
CS.addConstraint(
ConstraintKind::FunctionInput, methodTy, argTy,
Expand Down
75 changes: 41 additions & 34 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1547,6 +1547,12 @@ ConstraintSystem::matchTypesBindTypeVar(
// If the left-hand type variable cannot bind to an lvalue,
// but we still have an lvalue, fail.
if (!typeVar->getImpl().canBindToLValue() && type->hasLValueType()) {
return getTypeMatchFailure(locator);
}

// If the left-hand type variable cannot bind to an inout,
// but we still have an inout, fail.
if (!typeVar->getImpl().canBindToInOut() && type->is<InOutType>()) {
return getTypeMatchFailure(locator);
}

Expand All @@ -1568,15 +1574,6 @@ ConstraintSystem::matchTypesBindTypeVar(
}
}

// Check whether the type variable must be bound to a materializable
// type.
if (typeVar->getImpl().mustBeMaterializable()) {
if (!type->isMaterializable())
return getTypeMatchFailure(locator);

setMustBeMaterializableRecursive(type);
}

// Okay. Bind below.

// A constraint that binds any pointer to a void pointer is
Expand All @@ -1588,6 +1585,16 @@ ConstraintSystem::matchTypesBindTypeVar(
return getTypeMatchSuccess();
}

if (!typeVar->getImpl().canBindToLValue()) {
// When binding a fixed type to a type variable that cannot contain
// lvalues, any type variables within the fixed type cannot contain
// lvalues either.
type.visit([&](Type t) {
if (auto *tvt = dyn_cast<TypeVariableType>(t.getPointer()))
typeVar->getImpl().setCannotBindToLValue(getSavedBindings());
});
}

assignFixedType(typeVar, type);

return getTypeMatchSuccess();
Expand Down Expand Up @@ -1723,7 +1730,8 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,

// If exactly one of the type variables can bind to an lvalue, we
// can't merge these two type variables.
if (rep1->getImpl().canBindToLValue()
if (kind == ConstraintKind::Equal &&
rep1->getImpl().canBindToLValue()
!= rep2->getImpl().canBindToLValue())
return formUnsolvedResult();

Expand Down Expand Up @@ -1751,46 +1759,44 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
case ConstraintKind::BindParam: {
if (typeVar2 && !typeVar1) {
// Simplify the left-hand type and perform the "occurs" check.
typeVar2 = getRepresentative(typeVar2);
auto rep2 = getRepresentative(typeVar2);
type1 = simplifyType(type1, flags);
if (!isBindable(typeVar2, type1))
return formUnsolvedResult();

if (auto *iot = type1->getAs<InOutType>()) {
assignFixedType(typeVar2, LValueType::get(iot->getObjectType()));
if (!rep2->getImpl().canBindToLValue())
return getTypeMatchFailure(locator);
assignFixedType(rep2, LValueType::get(iot->getObjectType()));
} else {
assignFixedType(typeVar2, type1);
assignFixedType(rep2, type1);
}
return getTypeMatchSuccess();
} else if (typeVar1 && !typeVar2) {
// Simplify the right-hand type and perform the "occurs" check.
typeVar1 = getRepresentative(typeVar1);
auto rep1 = getRepresentative(typeVar1);
type2 = simplifyType(type2, flags);
if (!isBindable(typeVar1, type2))
if (!isBindable(rep1, type2))
return formUnsolvedResult();

// If the right-hand side of the BindParam constraint
// is `lvalue` type, we'll have to make sure that
// left-hand side is bound to type variable which
// is wrapped in `inout` type to preserve inout/lvalue pairing.
if (auto *lvt = type2->getAs<LValueType>()) {
auto *tv = createTypeVariable(typeVar1->getImpl().getLocator());
assignFixedType(typeVar1, InOutType::get(tv));

typeVar1 = tv;
type2 = lvt->getObjectType();
if (!rep1->getImpl().canBindToInOut())
return getTypeMatchFailure(locator);
assignFixedType(rep1, InOutType::get(lvt->getObjectType()));
} else {
assignFixedType(rep1, type2);
}

// If we have a binding for the right-hand side
// (argument type used in the body) don't try
// to bind it to the left-hand side (parameter type)
// directly, because there could be an implicit
// conversion between them, and actual binding
// can only come from the left-hand side.
addUnsolvedConstraint(
Constraint::create(*this, ConstraintKind::Equal, typeVar1, type2,
getConstraintLocator(locator)));
return getTypeMatchSuccess();
} if (typeVar1 && typeVar2) {
auto rep1 = getRepresentative(typeVar1);
auto rep2 = getRepresentative(typeVar2);

if (!rep1->getImpl().canBindToInOut() ||
!rep2->getImpl().canBindToLValue()) {
// Merge the equivalence classes corresponding to these two variables.
mergeEquivalenceClasses(rep1, rep2);
return getTypeMatchSuccess();
}
}

return formUnsolvedResult();
Expand Down Expand Up @@ -2623,6 +2629,7 @@ ConstraintSystem::simplifyConstructionConstraint(
auto argType = createTypeVariable(
getConstraintLocator(locator, ConstraintLocator::ApplyArgument),
(TVO_CanBindToLValue |
TVO_CanBindToInOut |
TVO_PrefersSubtypeBinding));
addConstraint(ConstraintKind::FunctionInput, memberType, argType, locator);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/ConstraintGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ bool ConstraintGraph::contractEdges() {
auto type = binding.BindingType;
isNotContractable = type.findIf([&](Type nestedType) -> bool {
if (auto tv = nestedType->getAs<TypeVariableType>()) {
if (!tv->getImpl().mustBeMaterializable())
if (tv->getImpl().canBindToInOut())
return true;
}

Expand Down
17 changes: 0 additions & 17 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,23 +201,6 @@ void ConstraintSystem::assignFixedType(TypeVariableType *typeVar, Type type,
addTypeVariableConstraintsToWorkList(typeVar);
}

void ConstraintSystem::setMustBeMaterializableRecursive(Type type)
{
assert(type->isMaterializable() &&
"argument to setMustBeMaterializableRecursive may not be inherently "
"non-materializable");
type = getFixedTypeRecursive(type, /*wantRValue=*/false);
type = type->lookThroughAllOptionalTypes();

if (auto typeVar = type->getAs<TypeVariableType>()) {
typeVar->getImpl().setMustBeMaterializable(getSavedBindings());
} else if (auto *tupleTy = type->getAs<TupleType>()) {
for (auto elt : tupleTy->getElementTypes()) {
setMustBeMaterializableRecursive(elt);
}
}
}

void ConstraintSystem::addTypeVariableConstraintsToWorkList(
TypeVariableType *typeVar) {
// Gather the constraints affected by a change to this type variable.
Expand Down
24 changes: 9 additions & 15 deletions lib/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,6 @@ class TypeVariableType::Implementation {
return getRawOptions() & TVO_PrefersSubtypeBinding;
}

bool mustBeMaterializable() const {
return !(getRawOptions() & TVO_CanBindToInOut) &&
!(getRawOptions() & TVO_CanBindToLValue);
}

/// Retrieve the corresponding node in the constraint graph.
constraints::ConstraintGraphNode *getGraphNode() const { return GraphNode; }

Expand Down Expand Up @@ -343,10 +338,16 @@ class TypeVariableType::Implementation {
if (record)
otherRep->getImpl().recordBinding(*record);
otherRep->getImpl().ParentOrFixed = getTypeVariable();
if (!mustBeMaterializable() && otherRep->getImpl().mustBeMaterializable()) {

if (canBindToLValue() && !otherRep->getImpl().canBindToLValue()) {
if (record)
recordBinding(*record);
getTypeVariable()->Bits.TypeVariableType.Options &= ~TVO_CanBindToLValue;
}

if (canBindToInOut() && !otherRep->getImpl().canBindToInOut()) {
if (record)
recordBinding(*record);
getTypeVariable()->Bits.TypeVariableType.Options &= ~TVO_CanBindToInOut;
}
}
Expand Down Expand Up @@ -380,15 +381,13 @@ class TypeVariableType::Implementation {
rep->getImpl().ParentOrFixed = type.getPointer();
}

void setMustBeMaterializable(constraints::SavedTypeVariableBindings *record) {
void setCannotBindToLValue(constraints::SavedTypeVariableBindings *record) {
auto rep = getRepresentative(record);
if (!rep->getImpl().mustBeMaterializable()) {
if (rep->getImpl().canBindToLValue()) {
if (record)
rep->getImpl().recordBinding(*record);
rep->getImpl().getTypeVariable()->Bits.TypeVariableType.Options
&= ~TVO_CanBindToLValue;
rep->getImpl().getTypeVariable()->Bits.TypeVariableType.Options
&= ~TVO_CanBindToInOut;
}
}

Expand Down Expand Up @@ -2114,11 +2113,6 @@ class ConstraintSystem {
/// a complete solution from partial solutions.
void assignFixedType(TypeVariableType *typeVar, Type type,
bool updateState = true);

/// Set the TVO_MustBeMaterializable bit on all type variables
/// necessary to ensure that the type in question is materializable in a
/// viable solution.
void setMustBeMaterializableRecursive(Type type);

/// Determine if the type in question is an Array<T> and, if so, provide the
/// element type of the array.
Expand Down
13 changes: 10 additions & 3 deletions test/Constraints/generics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func generic_metatypes<T : SomeProtocol>(_ x: T)
}

// Inferring a variable's type from a call to a generic.
struct Pair<T, U> { } // expected-note 4 {{'T' declared as parameter to type 'Pair'}} expected-note {{'U' declared as parameter to type 'Pair'}}
struct Pair<T, U> { } // expected-note 3 {{'T' declared as parameter to type 'Pair'}} expected-note 2 {{'U' declared as parameter to type 'Pair'}}

func pair<T, U>(_ x: T, _ y: U) -> Pair<T, U> { }

Expand Down Expand Up @@ -364,7 +364,7 @@ func testFixItCasting(x: Any) {

func testFixItContextualKnowledge() {
// FIXME: These could propagate backwards.
let _: Int = Pair().first // expected-error {{generic parameter 'T' could not be inferred}} expected-note {{explicitly specify the generic arguments to fix this issue}} {{20-20=<Any, Any>}}
let _: Int = Pair().first // expected-error {{generic parameter 'U' could not be inferred}} expected-note {{explicitly specify the generic arguments to fix this issue}} {{20-20=<Any, Any>}}
let _: Int = Pair().second // expected-error {{generic parameter 'T' could not be inferred}} expected-note {{explicitly specify the generic arguments to fix this issue}} {{20-20=<Any, Any>}}
}

Expand Down Expand Up @@ -622,7 +622,7 @@ func rdar40537858() {
let _: E = .bar([s]) // expected-error {{enum case 'bar' requires that 'S' conform to 'P'}}
}

// SR-8934
// https://bugs.swift.org/browse/SR-8934
struct BottleLayout {
let count : Int
}
Expand All @@ -631,3 +631,10 @@ let layout = BottleLayout(count:1)
let ix = arr.firstIndex(of:layout) // expected-error {{argument type 'BottleLayout' does not conform to expected type 'Equatable'}}

let _: () -> UInt8 = { .init("a" as Unicode.Scalar) } // expected-error {{initializer 'init(_:)' requires that 'Unicode.Scalar' conform to 'BinaryInteger'}}

// https://bugs.swift.org/browse/SR-9068
func compare<C: Collection, Key: Hashable, Value: Equatable>(c: C)
-> Bool where C.Element == (key: Key, value: Value)
{
_ = Dictionary(uniqueKeysWithValues: Array(c))
}