Skip to content

Commit 96c0c13

Browse files
AnthonyLatsisxedin
authored andcommitted
[Diagnostics] SR-7445 Improve diagnostics for assignment failures (#16179)
* [Diagnostics] SR-7445 Improve diagnostics for assignment failures * modified messages for assignments to function calls, modified messages for assignments to methods. removed comment for resolved radar. * removed extra line and braces * added tests for assignment_lhs_is_apply_expression eliminated redundant literal check which is always invoked before call reverted 'cannot assign to value' for literal assignments in subexpressions * Complemented assigning to literal tests & reverted to 'cannot asign to value' for methods (was 'cannot assign to member') * removed extra tabs * eliminated one more accidental spacing * Update CSDiag.cpp * added highlighting, fixed & rechecked tests * added highlighting for complex expressions involving assigning to literals Resolves: [SR-7445](https://bugs.swift.org/browse/SR-7445)
1 parent 8ad1d76 commit 96c0c13

File tree

7 files changed

+55
-22
lines changed

7 files changed

+55
-22
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2925,6 +2925,8 @@ WARNING(expression_unused_literal,none,
29252925

29262926
ERROR(assignment_lhs_not_lvalue,none,
29272927
"cannot assign to immutable expression of type %0", (Type))
2928+
ERROR(assignment_lhs_is_apply_expression,none,
2929+
"expression is not assignable: %0", (StringRef))
29282930
ERROR(assignment_lhs_is_immutable_variable,none,
29292931
"cannot assign to value: %0", (StringRef))
29302932
ERROR(assignment_lhs_is_immutable_property,none,

lib/Sema/CSDiag.cpp

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ static void diagnoseSubElementFailure(Expr *destExpr,
657657
Diag<StringRef> diagID,
658658
Diag<Type> unknownDiagID) {
659659
auto &TC = CS.getTypeChecker();
660-
660+
661661
// Walk through the destination expression, resolving what the problem is. If
662662
// we find a node in the lvalue path that is problematic, this returns it.
663663
auto immInfo = resolveImmutableBase(destExpr, CS);
@@ -706,18 +706,23 @@ static void diagnoseSubElementFailure(Expr *destExpr,
706706
.highlight(immInfo.first->getSourceRange());
707707
return;
708708
}
709-
709+
710710
// If we're trying to set an unapplied method, say that.
711-
if (auto *VD = dyn_cast_or_null<ValueDecl>(immInfo.second)) {
711+
if (auto *VD = immInfo.second) {
712712
std::string message = "'";
713713
message += VD->getBaseName().getIdentifier().str();
714714
message += "'";
715-
716-
if (auto *AFD = dyn_cast<AbstractFunctionDecl>(VD))
717-
message += AFD->getImplicitSelfDecl() ? " is a method" : " is a function";
718-
else
715+
716+
if (auto *AFD = dyn_cast<AbstractFunctionDecl>(VD)) {
717+
if (AFD->getImplicitSelfDecl()) {
718+
message += " is a method";
719+
diagID = diag::assignment_lhs_is_immutable_variable;
720+
} else {
721+
message += " is a function";
722+
}
723+
} else
719724
message += " is not settable";
720-
725+
721726
TC.diagnose(loc, diagID, message)
722727
.highlight(immInfo.first->getSourceRange());
723728
return;
@@ -730,11 +735,13 @@ static void diagnoseSubElementFailure(Expr *destExpr,
730735
auto argsTuple =
731736
dyn_cast<TupleExpr>(AE->getArg()->getSemanticsProvidingExpr());
732737
if (isa<CallExpr>(AE) && AE->isImplicit() && argsTuple &&
733-
argsTuple->getNumElements() == 1 &&
734-
isa<LiteralExpr>(argsTuple->getElement(0)->
735-
getSemanticsProvidingExpr())) {
736-
TC.diagnose(loc, diagID, "literals are not mutable");
737-
return;
738+
argsTuple->getNumElements() == 1) {
739+
if (auto LE = dyn_cast<LiteralExpr>(argsTuple->getElement(0)->
740+
getSemanticsProvidingExpr())) {
741+
TC.diagnose(loc, diagID, "literals are not mutable")
742+
.highlight(LE->getSourceRange());
743+
return;
744+
}
738745
}
739746

740747
std::string name = "call";
@@ -755,7 +762,7 @@ static void diagnoseSubElementFailure(Expr *destExpr,
755762
.highlight(AE->getSourceRange());
756763
return;
757764
}
758-
765+
759766
if (auto *ICE = dyn_cast<ImplicitConversionExpr>(immInfo.first))
760767
if (isa<LoadExpr>(ICE->getSubExpr())) {
761768
TC.diagnose(loc, diagID,
@@ -3155,15 +3162,17 @@ void ConstraintSystem::diagnoseAssignmentFailure(Expr *dest, Type destTy,
31553162
}
31563163

31573164
Diag<StringRef> diagID;
3158-
if (isa<DeclRefExpr>(dest))
3165+
if (isa<ApplyExpr>(dest))
3166+
diagID = diag::assignment_lhs_is_apply_expression;
3167+
else if (isa<DeclRefExpr>(dest))
31593168
diagID = diag::assignment_lhs_is_immutable_variable;
31603169
else if (isa<ForceValueExpr>(dest))
31613170
diagID = diag::assignment_bang_has_immutable_subcomponent;
31623171
else if (isa<UnresolvedDotExpr>(dest) || isa<MemberRefExpr>(dest))
31633172
diagID = diag::assignment_lhs_is_immutable_property;
31643173
else if (auto sub = dyn_cast<SubscriptExpr>(dest)) {
31653174
diagID = diag::assignment_subscript_has_immutable_base;
3166-
3175+
31673176
// If the destination is a subscript with a 'dynamicLookup:' label and if
31683177
// the tuple is implicit, then this was actually a @dynamicMemberLookup
31693178
// access. Emit a more specific diagnostic.

test/ClangImporter/objc_parse.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func properties(_ b: B) {
121121
// An informal property cannot be made formal in a subclass. The
122122
// formal property is simply ignored.
123123
b.informalMadeFormal()
124-
b.informalMadeFormal = i // expected-error{{cannot assign to property: 'informalMadeFormal' is a method}}
124+
b.informalMadeFormal = i // expected-error{{cannot assign to value: 'informalMadeFormal' is a method}}
125125
b.setInformalMadeFormal(5)
126126

127127
b.overriddenProp = 17

test/Sema/immutability.swift

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,25 @@ func QoI() {
430430
get_only = 92 // expected-error {{cannot assign to value: 'get_only' is a get-only property}}
431431
}
432432

433+
func func1() -> Int { return 7 }
433434

435+
func func2() {
436+
func func3() {}
437+
438+
func3() = 0 // expected-error {{expression is not assignable: 'func3' returns immutable value}}
439+
}
440+
441+
func assignmentsToFuncs() {
442+
443+
LetClassMembers(arg: 7).f() = 5 // expected-error {{expression is not assignable: function call returns immutable value}}
444+
LetStructMembers(arg: 7).f() = 5 // expected-error {{expression is not assignable: function call returns immutable value}}
445+
446+
func1() = 9 // expected-error {{expression is not assignable: 'func1' returns immutable value}}
447+
func2() = "rrr" // expected-error {{expression is not assignable: 'func2' returns immutable value}}
448+
449+
var x = 0
450+
(x, func1() = 0) = (4, 5) // expected-error {{expression is not assignable: 'func1' returns immutable value}}
451+
}
434452

435453
// <rdar://problem/17051675> Structure initializers in an extension cannot assign to constant properties
436454
struct rdar17051675_S {
@@ -600,10 +618,9 @@ func testConditional(b : Bool) {
600618

601619

602620

603-
// <rdar://problem/27384685> QoI: Poor diagnostic when assigning a value to a method
604621
func f(a : FooClass, b : LetStructMembers) {
605-
a.baz = 1 // expected-error {{cannot assign to property: 'baz' is a method}}
606-
b.f = 42 // expected-error {{cannot assign to property: 'f' is a method}}
622+
a.baz = 1 // expected-error {{cannot assign to value: 'baz' is a method}}
623+
b.f = 42 // expected-error {{cannot assign to value: 'f' is a method}}
607624
}
608625

609626
// SR-2354: Reject subscript declarations with mutable parameters.

test/decl/func/operator.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,6 @@ class C6 {
352352
static func == (lhs: C6, rhs: C6) -> Bool { return false }
353353

354354
func test1(x: C6) {
355-
if x == x && x = x { } // expected-error{{cannot assign to value: '&&' returns immutable value}}
355+
if x == x && x = x { } // expected-error{{expression is not assignable: '&&' returns immutable value}}
356356
}
357357
}

test/expr/expressions.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -885,7 +885,7 @@ var y = 1
885885
let _ = (x, x + 1).0
886886
let _ = (x, 3).1
887887
(x,y) = (2,3)
888-
(x,4) = (1,2) // expected-error {{cannot assign to value: literals are not mutable}}
888+
(x,4) = (1,2) // expected-error {{expression is not assignable: literals are not mutable}}
889889
(x,y).1 = 7 // expected-error {{cannot assign to immutable expression of type 'Int'}}
890890
x = (x,(3,y)).1.1
891891

test/stmt/statements.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ func funcdecl5(_ a: Int, y: Int) {
4141

4242
1 = x // expected-error {{cannot assign to a literal value}}
4343
(1) = x // expected-error {{cannot assign to a literal value}}
44+
"string" = "other" // expected-error {{cannot assign to a literal value}}
45+
[1, 1, 1, 1] = [1, 1] // expected-error {{cannot assign to immutable expression of type '[Int]}}
46+
1.0 = x // expected-error {{cannot assign to a literal value}}
47+
nil = 1 // expected-error {{cannot assign to a literal value}}
48+
4449
(x:1).x = 1 // expected-error {{cannot assign to immutable expression of type 'Int'}}
4550
var tup : (x:Int, y:Int)
4651
tup.x = 1

0 commit comments

Comments
 (0)