Skip to content

[ConstraintSystem] Introduce LValueObject constraint to replace direct l-value assignments #76174

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
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
3 changes: 3 additions & 0 deletions include/swift/Sema/Constraint.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,8 @@ enum class ConstraintKind : char {
/// The first type is a tuple containing a single unlabeled element that is a
/// pack expansion. The second type is its pattern type.
MaterializePackExpansion,
/// The first type is a l-value type whose object type is the second type.
LValueObject,
};

/// Classification of the different kinds of constraints.
Expand Down Expand Up @@ -691,6 +693,7 @@ class Constraint final : public llvm::ilist_node<Constraint>,
case ConstraintKind::PackElementOf:
case ConstraintKind::SameShape:
case ConstraintKind::MaterializePackExpansion:
case ConstraintKind::LValueObject:
return ConstraintClassification::Relational;

case ConstraintKind::ValueMember:
Expand Down
6 changes: 6 additions & 0 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -5239,6 +5239,12 @@ class ConstraintSystem {
TypeMatchOptions flags,
ConstraintLocatorBuilder locator);

/// Extract the object type of the l-value type (type1) and bind it to
/// to type2.
SolutionKind simplifyLValueObjectConstraint(Type type1, Type type2,
TypeMatchOptions flags,
ConstraintLocatorBuilder locator);

public: // FIXME: Public for use by static functions.
/// Simplify a conversion constraint with a fix applied to it.
SolutionKind simplifyFixConstraint(ConstraintFix *fix, Type type1, Type type2,
Expand Down
58 changes: 33 additions & 25 deletions lib/Sema/CSBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1318,30 +1318,7 @@ bool BindingSet::favoredOverDisjunction(Constraint *disjunction) const {
// Such type variables might be connected to closure as well
// e.g. when result type is optional, so it makes sense to
// open closure before attempting such disjunction.
if (auto *typeVar = boundType->lookThroughAllOptionalTypes()
->getAs<TypeVariableType>()) {
auto fixedType = CS.getFixedType(typeVar);
// Handles "assignment to an overloaded member" pattern where
// type variable that represents the destination is bound to an
// l-value type during constraint generation. See \c genAssignDestType.
//
// Note that in all other circumstances we won't be here if the
// type variable that presents the closure is connected to a
// disjunction because that would mark closure as "delayed".
if (fixedType && fixedType->is<LValueType>()) {
auto lvalueObjTy = fixedType->castTo<LValueType>()->getObjectType();
// Prefer closure only if it's not connected to the type variable
// that represents l-value object type of the assignment destination.
// Eagerly attempting closure first could result in missing some of
// the contextual annotations i.e. `@Sendable`.
if (auto *lvalueObjVar = lvalueObjTy->getAs<TypeVariableType>())
return !AdjacentVars.count(lvalueObjVar);
}

return true;
}

return false;
return boundType->lookThroughAllOptionalTypes()->is<TypeVariableType>();
}

// If this is a collection literal type, it's preferrable to bind it
Expand Down Expand Up @@ -1577,6 +1554,18 @@ PotentialBindings::inferFromRelational(Constraint *constraint) {
}
}

// Situations like `v.<member> = { ... }` where member is overloaded.
// We need to wait until member is resolved otherwise there is a risk
// of losing some of the contextual attributes important for the closure
// such as @Sendable and global actor.
if (TypeVar->getImpl().isClosureType() &&
kind == AllowedBindingKind::Subtypes) {
if (type->isTypeVariableOrMember() &&
constraint->getLocator()->directlyAt<AssignExpr>()) {
DelayedBy.push_back(constraint);
}
}

if (auto *locator = TypeVar->getImpl().getLocator()) {
// Don't allow a protocol type to get propagated from the base to the result
// type of a chain, Result should always be a concrete type which conforms
Expand All @@ -1586,6 +1575,24 @@ PotentialBindings::inferFromRelational(Constraint *constraint) {
return std::nullopt;
}

if (constraint->getKind() == ConstraintKind::LValueObject) {
// Allow l-value type inference from its object type, but
// not the other way around, that would be handled by constraint
// simplification.
if (kind == AllowedBindingKind::Subtypes) {
if (type->isTypeVariableOrMember())
return std::nullopt;

type = LValueType::get(type);
} else {
// Right-hand side of the l-value object constraint can only
// be bound via constraint simplification when l-value type
// is inferred or contextually from other constraints.
DelayedBy.push_back(constraint);
return std::nullopt;
}
}

// If the source of the binding is 'OptionalObject' constraint
// and type variable is on the left-hand side, that means
// that it _has_ to be of optional type, since the right-hand
Expand Down Expand Up @@ -1757,7 +1764,8 @@ void PotentialBindings::infer(Constraint *constraint) {
case ConstraintKind::ArgumentConversion:
case ConstraintKind::OperatorArgumentConversion:
case ConstraintKind::OptionalObject:
case ConstraintKind::UnresolvedMemberChainBase: {
case ConstraintKind::UnresolvedMemberChainBase:
case ConstraintKind::LValueObject: {
auto binding = inferFromRelational(constraint);
if (!binding)
break;
Expand Down
17 changes: 2 additions & 15 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3510,23 +3510,10 @@ namespace {
} else {
auto *locator = CS.getConstraintLocator(expr);

auto isOrCanBeLValueType = [](Type type) {
if (auto *typeVar = type->getAs<TypeVariableType>()) {
return typeVar->getImpl().canBindToLValue();
}
return type->is<LValueType>();
};

auto exprType = CS.getType(expr);
if (!isOrCanBeLValueType(exprType)) {
// Pretend that destination is an l-value type.
exprType = LValueType::get(exprType);
(void)CS.recordFix(TreatRValueAsLValue::create(CS, locator));
}

auto *destTy = CS.createTypeVariable(locator, TVO_CanBindToNoEscape);
CS.addConstraint(ConstraintKind::Bind, LValueType::get(destTy),
exprType, locator);
CS.addConstraint(ConstraintKind::LValueObject, exprType, destTy,
locator);
return destTy;
}
}
Expand Down
86 changes: 86 additions & 0 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2313,6 +2313,7 @@ ConstraintSystem::matchTupleTypes(TupleType *tuple1, TupleType *tuple2,
case ConstraintKind::ExplicitGenericArguments:
case ConstraintKind::SameShape:
case ConstraintKind::MaterializePackExpansion:
case ConstraintKind::LValueObject:
llvm_unreachable("Bad constraint kind in matchTupleTypes()");
}

Expand Down Expand Up @@ -2673,6 +2674,7 @@ static bool matchFunctionRepresentations(FunctionType::ExtInfo einfo1,
case ConstraintKind::ExplicitGenericArguments:
case ConstraintKind::SameShape:
case ConstraintKind::MaterializePackExpansion:
case ConstraintKind::LValueObject:
return true;
}

Expand Down Expand Up @@ -3318,6 +3320,7 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
case ConstraintKind::ExplicitGenericArguments:
case ConstraintKind::SameShape:
case ConstraintKind::MaterializePackExpansion:
case ConstraintKind::LValueObject:
llvm_unreachable("Not a relational constraint");
}

Expand Down Expand Up @@ -7179,6 +7182,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
case ConstraintKind::ExplicitGenericArguments:
case ConstraintKind::SameShape:
case ConstraintKind::MaterializePackExpansion:
case ConstraintKind::LValueObject:
llvm_unreachable("Not a relational constraint");
}
}
Expand Down Expand Up @@ -14147,6 +14151,80 @@ ConstraintSystem::simplifyExplicitGenericArgumentsConstraint(
return SolutionKind::Solved;
}

ConstraintSystem::SolutionKind
ConstraintSystem::simplifyLValueObjectConstraint(
Type type1, Type type2, TypeMatchOptions flags,
ConstraintLocatorBuilder locator) {
auto lvalueTy = simplifyType(type1);

auto formUnsolved = [&]() {
// If we're supposed to generate constraints, do so.
if (flags.contains(TMF_GenerateConstraints)) {
auto *lvalueObject =
Constraint::create(*this, ConstraintKind::LValueObject,
type1, type2, getConstraintLocator(locator));

addUnsolvedConstraint(lvalueObject);
return SolutionKind::Solved;
}

return SolutionKind::Unsolved;
};

auto isOrCanBeLValueType = [](Type type) {
if (auto *typeVar = type->getAs<TypeVariableType>()) {
return typeVar->getImpl().canBindToLValue();
}
return type->is<LValueType>();
};

if (lvalueTy->isPlaceholder()) {
if (!shouldAttemptFixes())
return SolutionKind::Error;

recordAnyTypeVarAsPotentialHole(type2);
return SolutionKind::Solved;
}

if (!isOrCanBeLValueType(lvalueTy)) {
if (!shouldAttemptFixes())
return SolutionKind::Error;

auto assessImpact = [&]() -> unsigned {
// If this is a projection of a member reference
// let's check whether the member is unconditionally
// settable, if so than it's a problem with its base.
if (locator.directlyAt<UnresolvedDotExpr>()) {
auto *memberLoc = getConstraintLocator(locator.getAnchor(),
ConstraintLocator::Member);
if (auto selected = findSelectedOverloadFor(memberLoc)) {
if (auto *storage = dyn_cast_or_null<AbstractStorageDecl>(
selected->choice.getDeclOrNull())) {
return storage->isSettable(nullptr) ? 1 : 2;
}
}
}
return 2;
};

if (recordFix(
TreatRValueAsLValue::create(*this, getConstraintLocator(locator)),
assessImpact()))
return SolutionKind::Error;

lvalueTy = LValueType::get(lvalueTy);
}

if (lvalueTy->isTypeVariableOrMember())
return formUnsolved();

// TODO: This operation deserves its own locator just like OptionalObject.
addConstraint(ConstraintKind::Equal,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, does Bind not work here? I would expect that we shouldn't end up with nested lvalues or anything

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the Bind would work here the same way, I just prefer Equal in this case since the type could be bound from context as well which really means that it's an equality operation more than a Bind.

lvalueTy->castTo<LValueType>()->getObjectType(), type2,
getConstraintLocator(locator));
return SolutionKind::Solved;
}

static llvm::PointerIntPair<Type, 3, unsigned>
getBaseTypeForPointer(TypeBase *type) {
unsigned unwrapCount = 0;
Expand Down Expand Up @@ -15614,6 +15692,9 @@ ConstraintSystem::addConstraintImpl(ConstraintKind kind, Type first,
return simplifyMaterializePackExpansionConstraint(first, second, subflags,
locator);

case ConstraintKind::LValueObject:
return simplifyLValueObjectConstraint(first, second, subflags, locator);

case ConstraintKind::ValueMember:
case ConstraintKind::UnresolvedValueMember:
case ConstraintKind::ValueWitness:
Expand Down Expand Up @@ -16209,6 +16290,11 @@ ConstraintSystem::simplifyConstraint(const Constraint &constraint) {
return simplifyMaterializePackExpansionConstraint(
constraint.getFirstType(), constraint.getSecondType(),
/*flags*/ std::nullopt, constraint.getLocator());

case ConstraintKind::LValueObject:
return simplifyLValueObjectConstraint(
constraint.getFirstType(), constraint.getSecondType(),
/*flags*/ std::nullopt, constraint.getLocator());
}

llvm_unreachable("Unhandled ConstraintKind in switch.");
Expand Down
8 changes: 8 additions & 0 deletions lib/Sema/Constraint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ Constraint::Constraint(ConstraintKind Kind, Type First, Type Second,
case ConstraintKind::ExplicitGenericArguments:
case ConstraintKind::SameShape:
case ConstraintKind::MaterializePackExpansion:
case ConstraintKind::LValueObject:
assert(!First.isNull());
assert(!Second.isNull());
break;
Expand Down Expand Up @@ -176,6 +177,7 @@ Constraint::Constraint(ConstraintKind Kind, Type First, Type Second, Type Third,
case ConstraintKind::ExplicitGenericArguments:
case ConstraintKind::SameShape:
case ConstraintKind::MaterializePackExpansion:
case ConstraintKind::LValueObject:
llvm_unreachable("Wrong constructor");

case ConstraintKind::KeyPath:
Expand Down Expand Up @@ -326,6 +328,7 @@ Constraint *Constraint::clone(ConstraintSystem &cs) const {
case ConstraintKind::ExplicitGenericArguments:
case ConstraintKind::SameShape:
case ConstraintKind::MaterializePackExpansion:
case ConstraintKind::LValueObject:
return create(cs, getKind(), getFirstType(), getSecondType(), getLocator());

case ConstraintKind::ApplicableFunction:
Expand Down Expand Up @@ -590,6 +593,10 @@ void Constraint::print(llvm::raw_ostream &Out, SourceManager *sm,
Out << " materialize pack expansion ";
break;

case ConstraintKind::LValueObject:
Out << " l-value object type ";
break;

case ConstraintKind::Disjunction:
llvm_unreachable("disjunction handled above");
case ConstraintKind::Conjunction:
Expand Down Expand Up @@ -760,6 +767,7 @@ gatherReferencedTypeVars(Constraint *constraint,
case ConstraintKind::ExplicitGenericArguments:
case ConstraintKind::SameShape:
case ConstraintKind::MaterializePackExpansion:
case ConstraintKind::LValueObject:
constraint->getFirstType()->getTypeVariables(typeVars);
constraint->getSecondType()->getTypeVariables(typeVars);
break;
Expand Down
2 changes: 2 additions & 0 deletions test/Constraints/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,7 @@ func r23560128() {
var a : (Int,Int)?
a.0 = 42 // expected-error{{value of optional type '(Int, Int)?' must be unwrapped to refer to member '0' of wrapped base type '(Int, Int)'}}
// expected-note@-1{{chain the optional }}
// expected-note@-2 {{force-unwrap using '!' }}
}

// <rdar://problem/21890157> QoI: wrong error message when accessing properties on optional structs without unwrapping
Expand All @@ -656,6 +657,7 @@ struct ExampleStruct21890157 {
var example21890157: ExampleStruct21890157?
example21890157.property = "confusing" // expected-error {{value of optional type 'ExampleStruct21890157?' must be unwrapped to refer to member 'property' of wrapped base type 'ExampleStruct21890157'}}
// expected-note@-1{{chain the optional }}
// expected-note@-2 {{force-unwrap using '!' }}


struct UnaryOp {}
Expand Down
3 changes: 2 additions & 1 deletion test/ImportResolution/import-resolution-overload.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ scopedFunction = 42
// FIXME: Should be an error -- a type name and a function cannot overload.
var _ : Int = TypeNameWins(42)

TypeNameWins = 42 // expected-error {{cannot assign to immutable expression of type 'Int'}}
// FIXME: This should be an ambiguity where both candidates are mentioned as notes.
TypeNameWins = 42 // expected-error {{cannot assign to immutable expression of type '(Int) -> Int'}}
var _ : TypeNameWins // no-warning

// rdar://problem/21739333
Expand Down
2 changes: 1 addition & 1 deletion validation-test/Sema/SwiftUI/rdar84580119.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ extension EnvironmentValues {
var myHorizontalAlignment: AlignmentID? {
get { fatalError() }
set { self[\.MyHorizontalAlignmentEnvironmentKey.self] = newValue }
// expected-error@-1 {{generic parameter 'K' could not be inferred}}
// expected-error@-1 {{subscript 'subscript(_:)' requires that 'any AlignmentID' be a class type}}
// expected-error@-2 {{cannot infer key path type from context; consider explicitly specifying a root type}}
}
}
Expand Down