Make the rvalue-as-lvalue fix symmetric for bind constraints, which causes another set of assignment errors to be discovered in

the CS and slightly reduces the code in CSDiag.
This commit is contained in:
gregomni
2018-08-27 06:59:26 -07:00
parent 6e16e29aa1
commit 13d02bb85c
8 changed files with 37 additions and 52 deletions

View File

@@ -4420,12 +4420,6 @@ bool FailureDiagnosis::diagnoseSubscriptErrors(SubscriptExpr *SE,
if (calleeInfo.diagnoseSimpleErrors(SE)) if (calleeInfo.diagnoseSimpleErrors(SE))
return true; return true;
// If we haven't found a diagnostic yet, and we are in an assignment's
// destination, continue with diagnosing the assignment rather than giving
// a last resort diagnostic here.
if (inAssignmentDestination)
return false;
diagnose(SE->getLoc(), diag::cannot_subscript_with_index, baseType, diagnose(SE->getLoc(), diag::cannot_subscript_with_index, baseType,
indexType); indexType);
@@ -5826,10 +5820,16 @@ bool FailureDiagnosis::visitAssignExpr(AssignExpr *assignExpr) {
if (diagnoseSubscriptErrors(subscriptExpr, /* inAssignmentDestination = */ true)) if (diagnoseSubscriptErrors(subscriptExpr, /* inAssignmentDestination = */ true))
return true; return true;
} }
// Member ref assignment errors detected elsewhere, so not an assignment issue if found here.
AssignmentFailure failure(destExpr, CS, assignExpr->getLoc()); // The remaining exception involves mutable pointer conversions which aren't always caught elsewhere.
if (failure.diagnoseAsError()) PointerTypeKind ptk;
return true; if (!isa<MemberRefExpr>(destExpr) || CS.getType(destExpr)
->lookThroughAllOptionalTypes()
->getAnyPointerElementType(ptk)) {
AssignmentFailure failure(destExpr, CS, assignExpr->getLoc());
if (failure.diagnoseAsError())
return true;
}
} }
auto *srcExpr = assignExpr->getSrc(); auto *srcExpr = assignExpr->getSrc();

View File

@@ -644,16 +644,12 @@ bool RValueTreatedAsLValueFailure::diagnoseAsError() {
} }
} }
} }
if (auto resolvedOverload = getResolvedOverload(getLocator()))
if (resolvedOverload->Choice.getKind() == OverloadChoiceKind::DynamicMemberLookup)
subElementDiagID = diag::assignment_dynamic_property_has_immutable_base;
} else if (auto sub = dyn_cast<SubscriptExpr>(diagExpr)) { } else if (auto sub = dyn_cast<SubscriptExpr>(diagExpr)) {
subElementDiagID = diag::assignment_subscript_has_immutable_base; subElementDiagID = diag::assignment_subscript_has_immutable_base;
// If the destination is a subscript with a 'dynamicLookup:' label and if
// the tuple is implicit, then this was actually a @dynamicMemberLookup
// access. Emit a more specific diagnostic.
if (sub->getIndex()->isImplicit() &&
sub->getArgumentLabels().size() == 1 &&
sub->getArgumentLabels().front() == getTypeChecker().Context.Id_dynamicMember)
subElementDiagID = diag::assignment_dynamic_property_has_immutable_base;
} else { } else {
subElementDiagID = diag::assignment_lhs_is_immutable_variable; subElementDiagID = diag::assignment_lhs_is_immutable_variable;
} }
@@ -728,23 +724,6 @@ bool AssignmentFailure::diagnoseAsError() {
auto *DC = getDC(); auto *DC = getDC();
auto *destExpr = getParentExpr(); auto *destExpr = getParentExpr();
// Assignments to let-properties in delegating initializers will be caught
// elsewhere now, so if we see them here, it isn't an assignment problem.
if (auto *ctor = dyn_cast<ConstructorDecl>(DC)) {
DeclRefExpr *baseRef = nullptr;
if (auto *member = dyn_cast<UnresolvedDotExpr>(destExpr)) {
baseRef = dyn_cast<DeclRefExpr>(member->getBase());
} else if (auto *member = dyn_cast<MemberRefExpr>(destExpr)) {
if (auto *load = dyn_cast<LoadExpr>(member->getBase()))
baseRef = dyn_cast<DeclRefExpr>(load->getSubExpr());
}
if (baseRef && baseRef->getDecl() == ctor->getImplicitSelfDecl() &&
ctor->getDelegatingOrChainedInitKind(nullptr) ==
ConstructorDecl::BodyInitKind::Delegating) {
return false;
}
}
// Walk through the destination expression, resolving what the problem is. If // Walk through the destination expression, resolving what the problem is. If
// we find a node in the lvalue path that is problematic, this returns it. // we find a node in the lvalue path that is problematic, this returns it.
auto immInfo = resolveImmutableBase(destExpr); auto immInfo = resolveImmutableBase(destExpr);

View File

@@ -2792,7 +2792,7 @@ namespace {
return TupleType::get(destTupleTypes, CS.getASTContext()); return TupleType::get(destTupleTypes, CS.getASTContext());
} else { } else {
Type destTy = CS.createTypeVariable(CS.getConstraintLocator(expr)); Type destTy = CS.createTypeVariable(CS.getConstraintLocator(expr));
CS.addConstraint(ConstraintKind::Bind, CS.getType(expr), LValueType::get(destTy), CS.addConstraint(ConstraintKind::Bind, LValueType::get(destTy), CS.getType(expr),
CS.getConstraintLocator(expr)); CS.getConstraintLocator(expr));
return destTy; return destTy;
} }

View File

@@ -2369,7 +2369,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
// If we're converting an lvalue to an inout type, add the missing '&'. // If we're converting an lvalue to an inout type, add the missing '&'.
conversionsOrFixes.push_back( conversionsOrFixes.push_back(
AddAddressOf::create(*this, getConstraintLocator(locator))); AddAddressOf::create(*this, getConstraintLocator(locator)));
} else if (!isTypeVarOrMember1) { } else {
// If we have a concrete type that's an rvalue, "fix" it. // If we have a concrete type that's an rvalue, "fix" it.
conversionsOrFixes.push_back( conversionsOrFixes.push_back(
TreatRValueAsLValue::create(*this, getConstraintLocator(locator))); TreatRValueAsLValue::create(*this, getConstraintLocator(locator)));
@@ -2377,8 +2377,11 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
} }
} }
if (attemptFixes && type2->is<LValueType>() && !isTypeVarOrMember1) { if (attemptFixes && type2->is<LValueType>()) {
conversionsOrFixes.push_back( 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))); TreatRValueAsLValue::create(*this, getConstraintLocator(locator)));
} }
@@ -5008,7 +5011,11 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
} }
case FixKind::TreatRValueAsLValue: { case FixKind::TreatRValueAsLValue: {
auto result = matchTypes(LValueType::get(type1), type2, if (type2->is<LValueType>() || type2->is<InOutType>())
type1 = LValueType::get(type1);
else
type2 = LValueType::get(type2);
auto result = matchTypes(type1, type2,
matchKind, subflags, locator); matchKind, subflags, locator);
if (result == SolutionKind::Solved) if (result == SolutionKind::Solved)
if (recordFix(fix)) if (recordFix(fix))

View File

@@ -53,8 +53,7 @@ value(_) // expected-error{{'_' can only appear in a pattern or on the left side
// <rdar://problem/23798944> = vs. == in Swift if string character count statement causes segmentation fault // <rdar://problem/23798944> = vs. == in Swift if string character count statement causes segmentation fault
func f23798944() { func f23798944() {
let s = "" let s = ""
// FIXME: better would be: use of '=' in a boolean context, did you mean '=='? if s.count = 0 { // expected-error {{use of '=' in a boolean context, did you mean '=='?}}
if s.count = 0 { // expected-error {{cannot assign to property: 'count' is a get-only property}}
} }
} }

View File

@@ -180,7 +180,7 @@ obj.generic4!(5) // expected-error{{value of type 'Id' (aka 'AnyObject') has no
// Find properties via dynamic lookup. // Find properties via dynamic lookup.
var prop1Result : Int = obj.prop1! var prop1Result : Int = obj.prop1!
var prop2Result : String = obj.prop2! var prop2Result : String = obj.prop2!
obj.prop2 = "hello" // expected-error{{cannot assign to immutable expression of type 'String?'}} obj.prop2 = "hello" // expected-error{{cannot assign to property: 'obj' is immutable}}
var protoPropResult : Int = obj.protoProp! var protoPropResult : Int = obj.protoProp!
// Find subscripts via dynamic lookup // Find subscripts via dynamic lookup
@@ -290,9 +290,9 @@ let _: String = o[s]
let _: String = o[s]! let _: String = o[s]!
let _: String? = o[s] let _: String? = o[s]
// FIXME: These should all produce lvalues that we can write through // FIXME: These should all produce lvalues that we can write through
o.s = s // expected-error {{cannot assign to immutable expression of type 'String?'}} o.s = s // expected-error {{cannot assign to property: 'o' is immutable}}
o.s! = s // expected-error {{cannot assign through '!': 'o' is immutable}} o.s! = s // expected-error {{cannot assign through '!': 'o' is immutable}}
o[s] = s // expected-error {{cannot assign to immutable expression of type 'String?'}} o[s] = s // expected-error {{cannot assign through subscript: 'o' is immutable}}
o[s]! = s // expected-error {{cannot assign through '!': 'o' is immutable}} o[s]! = s // expected-error {{cannot assign through '!': 'o' is immutable}}
let _: String = o.t let _: String = o.t
@@ -309,7 +309,7 @@ let _: DynamicIUO = o[dyn_iuo]!
let _: DynamicIUO = o[dyn_iuo]!! let _: DynamicIUO = o[dyn_iuo]!!
let _: DynamicIUO? = o[dyn_iuo] let _: DynamicIUO? = o[dyn_iuo]
// FIXME: These should all produce lvalues that we can write through // FIXME: These should all produce lvalues that we can write through
o[dyn_iuo] = dyn_iuo // expected-error {{cannot assign to immutable expression of type 'DynamicIUO??'}} o[dyn_iuo] = dyn_iuo // expected-error {{cannot assign through subscript: 'o' is immutable}}
o[dyn_iuo]! = dyn_iuo // expected-error {{cannot assign through '!': 'o' is immutable}} o[dyn_iuo]! = dyn_iuo // expected-error {{cannot assign through '!': 'o' is immutable}}
o[dyn_iuo]!! = dyn_iuo // expected-error {{cannot assign through '!': 'o' is immutable}} o[dyn_iuo]!! = dyn_iuo // expected-error {{cannot assign through '!': 'o' is immutable}}

View File

@@ -67,7 +67,7 @@ func test(a: Gettable, b: Settable, c: MutGettable, d: NonMutSettable) {
func test_iuo(a : Gettable!, b : Settable!) { func test_iuo(a : Gettable!, b : Settable!) {
global = a.wyverns global = a.wyverns
a.flavor = global // expected-error {{cannot assign through dynamic lookup property: subscript is get-only}} a.flavor = global // expected-error {{cannot assign through dynamic lookup property: 'a' is a 'let' constant}}
global = b.flavor global = b.flavor
b.universal = global // expected-error {{cannot assign through dynamic lookup property: 'b' is a 'let' constant}} b.universal = global // expected-error {{cannot assign through dynamic lookup property: 'b' is a 'let' constant}}

View File

@@ -314,9 +314,9 @@ func testKeyPathSubscript(readonly: ZwithSubscript, writable: inout ZwithSubscri
sink = writable[keyPath: wkp] sink = writable[keyPath: wkp]
sink = readonly[keyPath: rkp] sink = readonly[keyPath: rkp]
sink = writable[keyPath: rkp] sink = writable[keyPath: rkp]
readonly[keyPath: kp] = sink // expected-error{{cannot assign through subscript: 'kp' is a read-only key path}} readonly[keyPath: kp] = sink // expected-error{{cannot assign through subscript: subscript is get-only}}
writable[keyPath: kp] = sink // expected-error{{cannot assign through subscript: 'kp' is a read-only key path}} writable[keyPath: kp] = sink // expected-error{{cannot assign through subscript: subscript is get-only}}
readonly[keyPath: wkp] = sink // expected-error{{cannot assign through subscript: 'readonly' is a 'let' constant}} readonly[keyPath: wkp] = sink // expected-error{{cannot assign through subscript: subscript is get-only}}
// FIXME: silently falls back to keypath application, which seems inconsistent // FIXME: silently falls back to keypath application, which seems inconsistent
writable[keyPath: wkp] = sink writable[keyPath: wkp] = sink
// FIXME: silently falls back to keypath application, which seems inconsistent // FIXME: silently falls back to keypath application, which seems inconsistent
@@ -331,8 +331,8 @@ func testKeyPathSubscript(readonly: ZwithSubscript, writable: inout ZwithSubscri
var anySink2 = writable[keyPath: pkp] var anySink2 = writable[keyPath: pkp]
expect(&anySink2, toHaveType: Exactly<Any>.self) expect(&anySink2, toHaveType: Exactly<Any>.self)
readonly[keyPath: pkp] = anySink1 // expected-error{{cannot assign through subscript: 'readonly' is a 'let' constant}} readonly[keyPath: pkp] = anySink1 // expected-error{{cannot assign through subscript: subscript is get-only}}
writable[keyPath: pkp] = anySink2 // expected-error{{cannot assign through subscript: 'writable' is immutable}} writable[keyPath: pkp] = anySink2 // expected-error{{cannot assign through subscript: subscript is get-only}}
let akp: AnyKeyPath = pkp let akp: AnyKeyPath = pkp