Skip to content

Commit 729253c

Browse files
tshortlitshortli
authored
Merge pull request #72430 from tshortli/key-path-accessor-availability-diagnostics-6.0
[6.0] Sema: Diagnose availability of storage accessors in key paths and writebacks
2 parents 555278e + bf48b62 commit 729253c

File tree

2 files changed

+49
-35
lines changed

2 files changed

+49
-35
lines changed

‎lib/Sema/TypeCheckAvailability.cpp

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3537,10 +3537,20 @@ class ExprAvailabilityWalker : public ASTWalker {
35373537

35383538
/// Walk a member reference expression, checking for availability.
35393539
void walkMemberRef(MemberRefExpr *E) {
3540-
// Walk the base in a getter context.
3541-
// FIXME: We may need to look at the setter too, if we're going to do
3542-
// writeback. The AST should have this information.
3543-
walkInContext(E, E->getBase(), MemberAccessContext::Getter);
3540+
// Walk the base. If the access context is currently `Setter`, then we must
3541+
// be diagnosing the destination of an assignment. When recursing, diagnose
3542+
// any remaining member refs as if they were in an InOutExpr, since there is
3543+
// a writeback occurring through them as a result of the assignment.
3544+
//
3545+
// someVar.x.y = 1
3546+
// │ ╰─ MemberAccessContext::Setter
3547+
// ╰─── MemberAccessContext::InOut
3548+
//
3549+
MemberAccessContext accessContext =
3550+
(AccessContext == MemberAccessContext::Setter)
3551+
? MemberAccessContext::InOut
3552+
: AccessContext;
3553+
walkInContext(E, E->getBase(), accessContext);
35443554

35453555
ConcreteDeclRef DR = E->getMember();
35463556
// Diagnose for the member declaration itself.
@@ -3557,6 +3567,7 @@ class ExprAvailabilityWalker : public ASTWalker {
35573567
/// availability.
35583568
void maybeDiagKeyPath(KeyPathExpr *KP) {
35593569
auto flags = DeclAvailabilityFlags();
3570+
auto declContext = Where.getDeclContext();
35603571
if (KP->isObjC())
35613572
flags = DeclAvailabilityFlag::ForObjCKeyPath;
35623573

@@ -3566,7 +3577,10 @@ class ExprAvailabilityWalker : public ASTWalker {
35663577
case KeyPathExpr::Component::Kind::Subscript: {
35673578
auto decl = component.getDeclRef();
35683579
auto loc = component.getLoc();
3569-
diagnoseDeclRefAvailability(decl, loc, nullptr, flags);
3580+
auto range = component.getSourceRange();
3581+
if (diagnoseDeclRefAvailability(decl, loc, nullptr, flags))
3582+
break;
3583+
maybeDiagStorageAccess(decl.getDecl(), range, declContext);
35703584
break;
35713585
}
35723586

‎test/Sema/availability_accessors.swift

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -34,21 +34,21 @@ struct BaseStruct {
3434

3535
var unavailableGetter: Value {
3636
@available(*, unavailable)
37-
get { fatalError() } // expected-note 24 {{getter for 'unavailableGetter' has been explicitly marked unavailable here}}
37+
get { fatalError() } // expected-note 28 {{getter for 'unavailableGetter' has been explicitly marked unavailable here}}
3838
set {}
3939
}
4040

4141
var unavailableSetter: Value {
4242
get { .defaultValue }
4343
@available(*, unavailable)
44-
set { fatalError() } // expected-note 12 {{setter for 'unavailableSetter' has been explicitly marked unavailable here}}
44+
set { fatalError() } // expected-note 21 {{setter for 'unavailableSetter' has been explicitly marked unavailable here}}
4545
}
4646

4747
var unavailableGetterAndSetter: Value {
4848
@available(*, unavailable)
49-
get { fatalError() } // expected-note 24 {{getter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
49+
get { fatalError() } // expected-note 28 {{getter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
5050
@available(*, unavailable)
51-
set { fatalError() } // expected-note 12 {{setter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
51+
set { fatalError() } // expected-note 21 {{setter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
5252
}
5353
}
5454

@@ -96,12 +96,12 @@ func testLValueAssignments() {
9696
x.unavailableGetter[0].b = 1 // expected-error {{getter for 'unavailableGetter' is unavailable}}
9797

9898
x.unavailableSetter = someValue // expected-error {{setter for 'unavailableSetter' is unavailable}}
99-
x.unavailableSetter.a = someValue.a // FIXME: missing diagnostic for setter
99+
x.unavailableSetter.a = someValue.a // expected-error {{setter for 'unavailableSetter' is unavailable}}
100100
x.unavailableSetter[0] = someValue.a // expected-error {{setter for 'unavailableSetter' is unavailable}}
101101
x.unavailableSetter[0].b = 1 // expected-error {{setter for 'unavailableSetter' is unavailable}}
102102

103103
x.unavailableGetterAndSetter = someValue // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
104-
x.unavailableGetterAndSetter.a = someValue.a // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} FIXME: missing diagnostic for setter
104+
x.unavailableGetterAndSetter.a = someValue.a // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
105105
x.unavailableGetterAndSetter[0] = someValue.a // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
106106
x.unavailableGetterAndSetter[0].b = 1 // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
107107
}
@@ -117,25 +117,25 @@ func testKeyPathLoads() {
117117
_ = a[keyPath: \.[takesIntInOut(&x.available.a.b)]]
118118
_ = a[keyPath: \.[takesIntInOut(&x.available[0].b)]]
119119

120-
_ = x[keyPath: \.unavailableGetter] // FIXME: missing diagnostic for getter
121-
_ = x[keyPath: \.unavailableGetter.a] // FIXME: missing diagnostic for getter
122-
_ = x[keyPath: \.unavailableGetter[0]] // FIXME: missing diagnostic for getter
123-
_ = x[keyPath: \.unavailableGetter[0].b] // FIXME: missing diagnostic for getter
120+
_ = x[keyPath: \.unavailableGetter] // expected-error {{getter for 'unavailableGetter' is unavailable}}
121+
_ = x[keyPath: \.unavailableGetter.a] // expected-error {{getter for 'unavailableGetter' is unavailable}}
122+
_ = x[keyPath: \.unavailableGetter[0]] // expected-error {{getter for 'unavailableGetter' is unavailable}}
123+
_ = x[keyPath: \.unavailableGetter[0].b] // expected-error {{getter for 'unavailableGetter' is unavailable}}
124124
_ = a[keyPath: \.[takesIntInOut(&x.unavailableGetter.a.b)]] // expected-error {{getter for 'unavailableGetter' is unavailable}}
125125
_ = a[keyPath: \.[takesIntInOut(&x.unavailableGetter[0].b)]] // expected-error {{getter for 'unavailableGetter' is unavailable}}
126126

127127
_ = x[keyPath: \.unavailableSetter]
128128
_ = x[keyPath: \.unavailableSetter.a]
129129
_ = x[keyPath: \.unavailableSetter[0]]
130130
_ = x[keyPath: \.unavailableSetter[0].b]
131-
_ = a[keyPath: \.[takesIntInOut(&x.unavailableSetter.a.b)]] // FIXME: missing diagnostic for setter
131+
_ = a[keyPath: \.[takesIntInOut(&x.unavailableSetter.a.b)]] // expected-error {{setter for 'unavailableSetter' is unavailable}}
132132
_ = a[keyPath: \.[takesIntInOut(&x.unavailableSetter[0].b)]] // expected-error {{setter for 'unavailableSetter' is unavailable}}
133133

134-
_ = x[keyPath: \.unavailableGetterAndSetter] // FIXME: missing diagnostic for getter
135-
_ = x[keyPath: \.unavailableGetterAndSetter.a] // FIXME: missing diagnostic for getter
136-
_ = x[keyPath: \.unavailableGetterAndSetter[0]] // FIXME: missing diagnostic for getter
137-
_ = x[keyPath: \.unavailableGetterAndSetter[0].b] // FIXME: missing diagnostic for getter
138-
_ = a[keyPath: \.[takesIntInOut(&x.unavailableGetterAndSetter.a.b)]] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} FIXME: missing diagnostic for setter
134+
_ = x[keyPath: \.unavailableGetterAndSetter] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
135+
_ = x[keyPath: \.unavailableGetterAndSetter.a] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
136+
_ = x[keyPath: \.unavailableGetterAndSetter[0]] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
137+
_ = x[keyPath: \.unavailableGetterAndSetter[0].b] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
138+
_ = a[keyPath: \.[takesIntInOut(&x.unavailableGetterAndSetter.a.b)]] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
139139
_ = a[keyPath: \.[takesIntInOut(&x.unavailableGetterAndSetter[0].b)]] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
140140
}
141141

@@ -158,18 +158,18 @@ func testKeyPathAssignments() {
158158
a[keyPath: \.[takesIntInOut(&x.unavailableGetter.a.b)]] = 0 // expected-error {{getter for 'unavailableGetter' is unavailable}}
159159
a[keyPath: \.[takesIntInOut(&x.unavailableGetter[0].b)]] = 0 // expected-error {{getter for 'unavailableGetter' is unavailable}}
160160

161-
x[keyPath: \.unavailableSetter] = someValue
162-
x[keyPath: \.unavailableSetter.a] = someValue.a // FIXME: missing diagnostic for setter
163-
x[keyPath: \.unavailableSetter[0]] = someValue.a // FIXME: missing diagnostic for setter
164-
x[keyPath: \.unavailableSetter[0].b] = 1 // FIXME: missing diagnostic for setter
165-
a[keyPath: \.[takesIntInOut(&x.unavailableSetter.a.b)]] = 0 // FIXME: missing diagnostic for setter
161+
x[keyPath: \.unavailableSetter] = someValue // expected-error {{setter for 'unavailableSetter' is unavailable}}
162+
x[keyPath: \.unavailableSetter.a] = someValue.a // expected-error {{setter for 'unavailableSetter' is unavailable}}
163+
x[keyPath: \.unavailableSetter[0]] = someValue.a // expected-error {{setter for 'unavailableSetter' is unavailable}}
164+
x[keyPath: \.unavailableSetter[0].b] = 1 // expected-error {{setter for 'unavailableSetter' is unavailable}}
165+
a[keyPath: \.[takesIntInOut(&x.unavailableSetter.a.b)]] = 0 // expected-error {{setter for 'unavailableSetter' is unavailable}}
166166
a[keyPath: \.[takesIntInOut(&x.unavailableSetter[0].b)]] = 0 // expected-error {{setter for 'unavailableSetter' is unavailable}}
167167

168-
x[keyPath: \.unavailableGetterAndSetter] = someValue
169-
x[keyPath: \.unavailableGetterAndSetter.a] = someValue.a // FIXME: missing diagnostics for getter and setter
170-
x[keyPath: \.unavailableGetterAndSetter[0]] = someValue.a // FIXME: missing diagnostics for getter and setter
171-
x[keyPath: \.unavailableGetterAndSetter[0].b] = 1 // FIXME: missing diagnostics for getter and setter
172-
a[keyPath: \.[takesIntInOut(&x.unavailableGetterAndSetter.a.b)]] = 0 // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} FIXME: missing diagnostic for setter
168+
x[keyPath: \.unavailableGetterAndSetter] = someValue // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
169+
x[keyPath: \.unavailableGetterAndSetter.a] = someValue.a // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
170+
x[keyPath: \.unavailableGetterAndSetter[0]] = someValue.a // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
171+
x[keyPath: \.unavailableGetterAndSetter[0].b] = 1 // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
172+
a[keyPath: \.[takesIntInOut(&x.unavailableGetterAndSetter.a.b)]] = 0 // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
173173
a[keyPath: \.[takesIntInOut(&x.unavailableGetterAndSetter[0].b)]] = 0 // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
174174
}
175175

@@ -192,13 +192,13 @@ func testMutatingMember() {
192192
x.unavailableSetter.setToZero() // expected-error {{setter for 'unavailableSetter' is unavailable}}
193193
x.unavailableSetter[0].setToZero() // expected-error {{setter for 'unavailableSetter' is unavailable}}
194194
_ = a[x.unavailableSetter.setToZero()] // expected-error {{setter for 'unavailableSetter' is unavailable}}
195-
_ = a[x.unavailableSetter.a.setToZero()] // FIXME: missing diagnostic for setter
195+
_ = a[x.unavailableSetter.a.setToZero()] // expected-error {{setter for 'unavailableSetter' is unavailable}}
196196
_ = a[x.unavailableSetter[0].setToZero()] // expected-error {{setter for 'unavailableSetter' is unavailable}}
197197

198198
x.unavailableGetterAndSetter.setToZero() // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
199199
x.unavailableGetterAndSetter[0].setToZero() // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
200200
_ = a[x.unavailableGetterAndSetter.setToZero()] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
201-
_ = a[x.unavailableGetterAndSetter.a.setToZero()] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} FIXME: should diagnose setter
201+
_ = a[x.unavailableGetterAndSetter.a.setToZero()] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
202202
_ = a[x.unavailableGetterAndSetter[0].setToZero()] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
203203
}
204204

@@ -218,12 +218,12 @@ func testPassAsInOutParameter() {
218218
takesInOut(&x.unavailableGetter[0].b) // expected-error {{getter for 'unavailableGetter' is unavailable}}
219219

220220
takesInOut(&x.unavailableSetter) // expected-error {{setter for 'unavailableSetter' is unavailable}}
221-
takesInOut(&x.unavailableSetter.a) //
221+
takesInOut(&x.unavailableSetter.a) // expected-error {{setter for 'unavailableSetter' is unavailable}}
222222
takesInOut(&x.unavailableSetter[0]) // expected-error {{setter for 'unavailableSetter' is unavailable}}
223223
takesInOut(&x.unavailableSetter[0].b) // expected-error {{setter for 'unavailableSetter' is unavailable}}
224224

225225
takesInOut(&x.unavailableGetterAndSetter) // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
226-
takesInOut(&x.unavailableGetterAndSetter.a) // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} FIXME: missing diagnostic for setter
226+
takesInOut(&x.unavailableGetterAndSetter.a) // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
227227
takesInOut(&x.unavailableGetterAndSetter[0]) // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
228228
takesInOut(&x.unavailableGetterAndSetter[0].b) // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
229229
}

0 commit comments

Comments
 (0)