Skip to content

Commit d5febdb

Browse files
slavapestovslavapestov
committed
Sema: Fix diagnoseSelfAssignment() with computed properties
This check did not handle MemberRefExprs with an InOutExpr base, giving it inconsistent behavior: - With a class, we would diagnose self-assignment of computed properties - With a struct, accesses to computed properties would build a MemberRefExpr with an InOutExpr base, so self-assignments were *not* diagnosed. Let's tweak the check to consistently diagnose self-assignments to stored properties only, instead of the emergent behavior as described above.
1 parent f436b60 commit d5febdb

File tree

2 files changed

+94
-18
lines changed

2 files changed

+94
-18
lines changed

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,31 +1479,31 @@ bool RValueTreatedAsLValueFailure::diagnoseAsNote() {
14791479
return true;
14801480
}
14811481

1482-
static Decl *findSimpleReferencedDecl(const Expr *E) {
1482+
static VarDecl *findSimpleReferencedVarDecl(const Expr *E) {
14831483
if (auto *LE = dyn_cast<LoadExpr>(E))
14841484
E = LE->getSubExpr();
14851485

14861486
if (auto *DRE = dyn_cast<DeclRefExpr>(E))
1487-
return DRE->getDecl();
1487+
return dyn_cast<VarDecl>(DRE->getDecl());
14881488

14891489
return nullptr;
14901490
}
14911491

1492-
static std::pair<Decl *, Decl *> findReferencedDecl(const Expr *E) {
1492+
static std::pair<VarDecl *, VarDecl *> findReferencedVarDecl(const Expr *E) {
14931493
E = E->getValueProvidingExpr();
14941494

14951495
if (auto *LE = dyn_cast<LoadExpr>(E))
1496-
return findReferencedDecl(LE->getSubExpr());
1496+
return findReferencedVarDecl(LE->getSubExpr());
14971497

14981498
if (auto *AE = dyn_cast<AssignExpr>(E))
1499-
return findReferencedDecl(AE->getDest());
1499+
return findReferencedVarDecl(AE->getDest());
15001500

1501-
if (auto *D = findSimpleReferencedDecl(E))
1501+
if (auto *D = findSimpleReferencedVarDecl(E))
15021502
return std::make_pair(nullptr, D);
15031503

15041504
if (auto *MRE = dyn_cast<MemberRefExpr>(E)) {
1505-
if (auto *BaseDecl = findSimpleReferencedDecl(MRE->getBase()))
1506-
return std::make_pair(BaseDecl, MRE->getMember().getDecl());
1505+
if (auto *BaseDecl = findSimpleReferencedVarDecl(MRE->getBase()))
1506+
return std::make_pair(BaseDecl, cast<VarDecl>(MRE->getMember().getDecl()));
15071507
}
15081508

15091509
return std::make_pair(nullptr, nullptr);
@@ -1517,10 +1517,12 @@ bool TypeChecker::diagnoseSelfAssignment(const Expr *expr) {
15171517
auto *dstExpr = assignExpr->getDest();
15181518
auto *srcExpr = assignExpr->getSrc();
15191519

1520-
auto dstDecl = findReferencedDecl(dstExpr);
1521-
auto srcDecl = findReferencedDecl(srcExpr);
1520+
auto dstDecl = findReferencedVarDecl(dstExpr);
1521+
auto srcDecl = findReferencedVarDecl(srcExpr);
15221522

1523-
if (dstDecl.second && dstDecl == srcDecl) {
1523+
if (dstDecl.second &&
1524+
dstDecl.second->hasStorage() &&
1525+
dstDecl == srcDecl) {
15241526
auto &DE = dstDecl.second->getASTContext().Diags;
15251527
DE.diagnose(expr->getLoc(), dstDecl.first ? diag::self_assignment_prop
15261528
: diag::self_assignment_var)

‎test/Sema/diag_self_assign.swift

Lines changed: 81 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,24 @@ class SA1 {
2121
}
2222
}
2323

24+
struct SA1a {
25+
var foo: Int = 0
26+
init(fooi: Int) {
27+
var foo = fooi
28+
foo = foo // expected-error {{assigning a variable to itself}}
29+
self.foo = self.foo // expected-error {{assigning a property to itself}}
30+
foo = self.foo // no-error
31+
self.foo = foo // no-error
32+
}
33+
mutating func f(fooi: Int) {
34+
var foo = fooi
35+
foo = foo // expected-error {{assigning a variable to itself}}
36+
self.foo = self.foo // expected-error {{assigning a property to itself}}
37+
foo = self.foo // no-error
38+
self.foo = foo // no-error
39+
}
40+
}
41+
2442
class SA2 {
2543
var foo: Int {
2644
get {
@@ -31,14 +49,37 @@ class SA2 {
3149
init(fooi: Int) {
3250
var foo = fooi
3351
foo = foo // expected-error {{assigning a variable to itself}}
34-
self.foo = self.foo // expected-error {{assigning a property to itself}}
52+
self.foo = self.foo // no-error
3553
foo = self.foo // no-error
3654
self.foo = foo // no-error
3755
}
3856
func f(fooi: Int) {
3957
var foo = fooi
4058
foo = foo // expected-error {{assigning a variable to itself}}
41-
self.foo = self.foo // expected-error {{assigning a property to itself}}
59+
self.foo = self.foo // no-error
60+
foo = self.foo // no-error
61+
self.foo = foo // no-error
62+
}
63+
}
64+
65+
struct SA2a {
66+
var foo: Int {
67+
get {
68+
return 0
69+
}
70+
set {}
71+
}
72+
init(fooi: Int) {
73+
var foo = fooi
74+
foo = foo // expected-error {{assigning a variable to itself}}
75+
self.foo = self.foo // no-error
76+
foo = self.foo // no-error
77+
self.foo = foo // no-error
78+
}
79+
mutating func f(fooi: Int) {
80+
var foo = fooi
81+
foo = foo // expected-error {{assigning a variable to itself}}
82+
self.foo = self.foo // no-error
4283
foo = self.foo // no-error
4384
self.foo = foo // no-error
4485
}
@@ -50,10 +91,24 @@ class SA3 {
5091
return foo // expected-warning {{attempting to access 'foo' within its own getter}} expected-note{{access 'self' explicitly to silence this warning}} {{14-14=self.}}
5192
}
5293
set {
53-
foo = foo // expected-error {{assigning a property to itself}} expected-warning {{attempting to modify 'foo' within its own setter}} expected-note{{access 'self' explicitly to silence this warning}} {{7-7=self.}} expected-warning{{setter argument 'newValue' was never used, but the property was accessed}} expected-note{{did you mean to use 'newValue' instead of accessing the property's current value?}}
54-
self.foo = self.foo // expected-error {{assigning a property to itself}}
55-
foo = self.foo // expected-error {{assigning a property to itself}} expected-warning {{attempting to modify 'foo' within its own setter}} expected-note{{access 'self' explicitly to silence this warning}} {{7-7=self.}}
56-
self.foo = foo // expected-error {{assigning a property to itself}}
94+
foo = foo // expected-warning {{attempting to modify 'foo' within its own setter}} expected-note{{access 'self' explicitly to silence this warning}} {{7-7=self.}} expected-warning{{setter argument 'newValue' was never used, but the property was accessed}} expected-note{{did you mean to use 'newValue' instead of accessing the property's current value?}}
95+
self.foo = self.foo // no-error
96+
foo = self.foo // expected-warning {{attempting to modify 'foo' within its own setter}} expected-note{{access 'self' explicitly to silence this warning}} {{7-7=self.}}
97+
self.foo = foo
98+
}
99+
}
100+
}
101+
102+
struct SA3a {
103+
var foo: Int {
104+
get {
105+
return foo // expected-warning {{attempting to access 'foo' within its own getter}} expected-note{{access 'self' explicitly to silence this warning}} {{14-14=self.}}
106+
}
107+
set {
108+
foo = foo // expected-warning {{attempting to modify 'foo' within its own setter}} expected-note{{access 'self' explicitly to silence this warning}} {{7-7=self.}} expected-warning{{setter argument 'newValue' was never used, but the property was accessed}} expected-note{{did you mean to use 'newValue' instead of accessing the property's current value?}}
109+
self.foo = self.foo // no-error
110+
foo = self.foo // expected-warning {{attempting to modify 'foo' within its own setter}} expected-note{{access 'self' explicitly to silence this warning}} {{7-7=self.}}
111+
self.foo = foo
57112
}
58113
}
59114
}
@@ -69,10 +124,29 @@ class SA4 {
69124
}
70125
}
71126

127+
struct SA4a {
128+
var foo: Int {
129+
get {
130+
return foo // expected-warning {{attempting to access 'foo' within its own getter}} expected-note{{access 'self' explicitly to silence this warning}} {{14-14=self.}}
131+
}
132+
set(value) {
133+
value = value // expected-error {{cannot assign to value: 'value' is a 'let' constant}}
134+
}
135+
}
136+
}
137+
72138
class SA5 {
73139
var foo: Int = 0
74140
}
75-
func SA5_test(a: SA4, b: SA4) {
141+
func SA5_test(a: SA5, b: SA5) {
142+
a.foo = a.foo // expected-error {{assigning a property to itself}}
143+
a.foo = b.foo
144+
}
145+
146+
struct SA5a {
147+
var foo: Int = 0
148+
}
149+
func SA5a_test(a: inout SA5, b: inout SA5) {
76150
a.foo = a.foo // expected-error {{assigning a property to itself}}
77151
a.foo = b.foo
78152
}

0 commit comments

Comments
 (0)