Skip to content

Commit

Permalink
Merge pull request #26852 from xedin/refactor-treat-r-as-l-value
Browse files Browse the repository at this point in the history 
[ConstraintSystem] Be more principled about recording r-value -> l-value fix
  • Loading branch information
@xedin
xedin authored Aug 28, 2019
2 parents 77d77c0 + 1b397ed commit 7b8fb88
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 51 deletions.
23 changes: 19 additions & 4 deletions lib/Sema/CSGen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2757,10 +2757,25 @@ namespace {
}
return TupleType::get(destTupleTypes, CS.getASTContext());
} else {
Type destTy = CS.createTypeVariable(CS.getConstraintLocator(expr),
TVO_CanBindToNoEscape);
CS.addConstraint(ConstraintKind::Bind, LValueType::get(destTy), CS.getType(expr),
CS.getConstraintLocator(expr));
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);
return destTy;
}
}
Expand Down
123 changes: 81 additions & 42 deletions lib/Sema/CSSimplify.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2301,7 +2301,8 @@ repairViaBridgingCast(ConstraintSystem &cs, Type fromType, Type toType,
/// \return true if at least some of the failures has been repaired
/// successfully, which allows type matcher to continue.
bool ConstraintSystem::repairFailures(
Type lhs, Type rhs, SmallVectorImpl<RestrictionOrFix> &conversionsOrFixes,
Type lhs, Type rhs, ConstraintKind matchKind,
SmallVectorImpl<RestrictionOrFix> &conversionsOrFixes,
ConstraintLocatorBuilder locator) {
SmallVector<LocatorPathElt, 4> path;
auto *anchor = locator.getLocatorParts(path);
Expand Down Expand Up @@ -2385,10 +2386,43 @@ bool ConstraintSystem::repairFailures(
return true;
};

auto repairByTreatingRValueAsLValue = [&](Type lhs, Type rhs) -> bool {
if (!lhs->is<LValueType>() &&
(rhs->is<LValueType>() || rhs->is<InOutType>())) {
// Conversion from l-value to inout in an operator argument
// position (which doesn't require explicit `&`) decays into
// a `Bind` of involved object types, same goes for explicit
// `&` conversion from l-value to inout type.
auto kind = (isa<InOutExpr>(anchor) ||
(rhs->is<InOutType>() &&
matchKind == ConstraintKind::OperatorArgumentConversion))
? ConstraintKind::Bind
: matchKind;

auto result = matchTypes(lhs, rhs->getWithoutSpecifierType(), kind,
TMF_ApplyingFix, locator);

if (result.isSuccess()) {
conversionsOrFixes.push_back(
TreatRValueAsLValue::create(*this, getConstraintLocator(locator)));
return true;
}
}

return false;
};

if (path.empty()) {
if (!anchor)
return false;

// This could be:
// - `InOutExpr` used with r-value e.g. `foo(&x)` where `x` is a `let`.
// - `ForceValueExpr` e.g. `foo.bar! = 42` where `bar` or `foo` are
// immutable or a subscript e.g. `foo["bar"]! = 42`.
if (repairByTreatingRValueAsLValue(lhs, rhs))
return true;

// If method reference forms a value type of the key path,
// there is going to be a constraint to match result of the
// member lookup to the generic parameter `V` of *KeyPath<R, V>
Expand Down Expand Up @@ -2463,6 +2497,18 @@ bool ConstraintSystem::repairFailures(
if (repairViaBridgingCast(*this, lhs, rhs, conversionsOrFixes, locator))
break;

// Argument is a r-value but parameter expects an l-value e.g.
//
// func foo(_ x: inout Int) {}
// let x: Int = 42
// foo(x) // `x` can't be converted to `inout Int`.
//
// This has to happen before checking for optionality mismatch
// because otherwise `Int? arg conv inout Int` is going to get
// fixed as 2 fixes - "force unwrap" + r-value -> l-value mismatch.
if (repairByTreatingRValueAsLValue(lhs, rhs))
break;

if (lhs->getOptionalObjectType() && !rhs->getOptionalObjectType()) {
conversionsOrFixes.push_back(
ForceOptional::create(*this, lhs, lhs->getOptionalObjectType(), loc));
Expand All @@ -2487,6 +2533,7 @@ bool ConstraintSystem::repairFailures(
},
rhs)) {
conversionsOrFixes.push_back(fix);
break;
}

// If argument in l-value type and parameter is `inout` or a pointer,
Expand All @@ -2498,25 +2545,28 @@ bool ConstraintSystem::repairFailures(
ConstraintKind::ArgumentConversion,
TypeMatchFlags::TMF_ApplyingFix, locator);

if (result.isSuccess())
if (result.isSuccess()) {
conversionsOrFixes.push_back(AddAddressOf::create(
*this, lhs, rhs, getConstraintLocator(locator)));
break;
}
}

// If the argument is inout and the parameter is not inout or a pointer,
// suggest removing the &.
if (lhs->is<InOutType>() && !rhs->is<InOutType>()) {
auto objectType = rhs->lookThroughAllOptionalTypes();
if (objectType->getAnyPointerElementType())
break;

auto result = matchTypes(lhs->getInOutObjectType(), rhs,
ConstraintKind::ArgumentConversion,
TypeMatchFlags::TMF_ApplyingFix, locator);

if (result.isSuccess())
conversionsOrFixes.push_back(RemoveAddressOf::create(*this, lhs,
rhs, getConstraintLocator(locator)));
if (!objectType->getAnyPointerElementType()) {
auto result = matchTypes(lhs->getInOutObjectType(), rhs,
ConstraintKind::ArgumentConversion,
TypeMatchFlags::TMF_ApplyingFix, locator);

if (result.isSuccess()) {
conversionsOrFixes.push_back(RemoveAddressOf::create(
*this, lhs, rhs, getConstraintLocator(locator)));
break;
}
}
}

break;
Expand Down Expand Up @@ -2682,7 +2732,15 @@ bool ConstraintSystem::repairFailures(
break;
}

case ConstraintLocator::FunctionResult: {
case ConstraintLocator::Member:
case ConstraintLocator::FunctionResult:
case ConstraintLocator::DynamicLookupResult: {
// Most likely this is an attempt to use get-only subscript as mutating,
// or assign a value of a result of function/member ref e.g. `foo() = 42`
// or `foo.bar = 42`, or `foo.bar()! = 42`.
if (repairByTreatingRValueAsLValue(rhs, lhs))
break;

// `apply argument` -> `arg/param compare` ->
// `@autoclosure result` -> `function result`
if (path.size() > 3) {
Expand Down Expand Up @@ -2737,6 +2795,13 @@ bool ConstraintSystem::repairFailures(
break;
}

case ConstraintLocator::SubscriptMember: {
if (repairByTreatingRValueAsLValue(lhs, rhs))
break;

break;
}

default:
break;
}
Expand Down Expand Up @@ -3527,25 +3592,11 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
getConstraintLocator(locator)));
}
}

if (!type1->is<LValueType>() && type2->is<InOutType>()) {
// If we have a concrete type that's an rvalue, "fix" it.
conversionsOrFixes.push_back(
TreatRValueAsLValue::create(*this, getConstraintLocator(locator)));
}
}

if (attemptFixes && type2->is<LValueType>()) {
conversionsOrFixes.push_back(
TreatRValueAsLValue::create(*this, getConstraintLocator(locator)));
} else if (attemptFixes && kind == ConstraintKind::Bind && type1->is<LValueType>()) {
conversionsOrFixes.push_back(
TreatRValueAsLValue::create(*this, getConstraintLocator(locator)));
}

// Attempt to repair any failures identifiable at this point.
if (attemptFixes) {
if (repairFailures(type1, type2, conversionsOrFixes, locator)) {
if (repairFailures(type1, type2, kind, conversionsOrFixes, locator)) {
if (conversionsOrFixes.empty())
return getTypeMatchSuccess();
}
Expand Down Expand Up @@ -5082,6 +5133,7 @@ fixMemberRef(ConstraintSystem &cs, Type baseTy,
// overload here, that would help if such subscript has
// not been provided.
case MemberLookupResult::UR_WritableKeyPathOnReadOnlyMember:
return TreatRValueAsLValue::create(cs, cs.getConstraintLocator(locator));
case MemberLookupResult::UR_ReferenceWritableKeyPathOnMutatingMember:
break;
case MemberLookupResult::UR_KeyPathWithAnyObjectRootType:
Expand Down Expand Up @@ -7343,20 +7395,6 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
return result;
}

case FixKind::TreatRValueAsLValue: {
if (type2->is<LValueType>() || type2->is<InOutType>())
type1 = LValueType::get(type1);
else
type2 = LValueType::get(type2);
auto result = matchTypes(type1, type2,
matchKind, subflags, locator);
if (result == SolutionKind::Solved)
if (recordFix(fix))
return SolutionKind::Error;

return result;
}

case FixKind::AutoClosureForwarding: {
if (recordFix(fix))
return SolutionKind::Error;
Expand Down Expand Up @@ -7406,6 +7444,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
case FixKind::RemoveReturn:
case FixKind::AddConformance:
case FixKind::RemoveAddressOf:
case FixKind::TreatRValueAsLValue:
case FixKind::SkipSameTypeRequirement:
case FixKind::SkipSuperclassRequirement:
case FixKind::AddMissingArguments:
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/ConstraintSystem.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2778,7 +2778,7 @@ class ConstraintSystem {
/// Attempt to repair typing failures and record fixes if needed.
/// \return true if at least some of the failures has been repaired
/// successfully, which allows type matcher to continue.
bool repairFailures(Type lhs, Type rhs,
bool repairFailures(Type lhs, Type rhs, ConstraintKind matchKind,
SmallVectorImpl<RestrictionOrFix> &conversionsOrFixes,
ConstraintLocatorBuilder locator);

Expand Down
8 changes: 4 additions & 4 deletions test/stmt/statements.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ func funcdecl5(_ a: Int, y: Int) {
x = y
(x) = y

1 = x // expected-error {{cannot assign to a literal value}}
(1) = x // expected-error {{cannot assign to a literal value}}
"string" = "other" // expected-error {{cannot assign to a literal value}}
1 = x // expected-error {{cannot assign to value: literals are not mutable}}
(1) = x // expected-error {{cannot assign to value: literals are not mutable}}
"string" = "other" // expected-error {{cannot assign to value: literals are not mutable}}
[1, 1, 1, 1] = [1, 1] // expected-error {{cannot assign to immutable expression of type '[Int]}}
1.0 = x // expected-error {{cannot assign to a literal value}}
nil = 1 // expected-error {{cannot assign to a literal value}}
nil = 1 // expected-error {{cannot assign to value: literals are not mutable}}

(x:1).x = 1 // expected-error {{cannot assign to immutable expression of type 'Int'}}
var tup : (x:Int, y:Int)
Expand Down

0 comments on commit 7b8fb88

Please sign in to comment.