Skip to content

Commit 6cd49ef

Browse files
tshortlitshortli
authored
Merge pull request #72369 from tshortli/spurious-unavailable-setter-diagnostic
Sema: Do not diagnose set accessor availability for `InOutExpr`s inside of `LoadExpr`s
2 parents 538cdba + ca925ab commit 6cd49ef

File tree

4 files changed

+274
-4
lines changed

4 files changed

+274
-4
lines changed

‎lib/Parse/ParseIfConfig.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,6 @@ Result Parser::parseIfConfigRaw(
772772
bool isActive, IfConfigElementsRole role)>
773773
parseElements,
774774
llvm::function_ref<Result(SourceLoc endLoc, bool hadMissingEnd)> finish) {
775-
auto startLoc = Tok.getLoc();
776775
assert(Tok.is(tok::pound_if));
777776

778777
Parser::StructureMarkerRAII ParsingDecl(

‎lib/Sema/TypeCheckAvailability.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3496,6 +3496,20 @@ class ExprAvailabilityWalker : public ASTWalker {
34963496
return call;
34973497
}
34983498

3499+
/// Walks up to the first enclosing LoadExpr and returns it.
3500+
const LoadExpr *getEnclosingLoadExpr() const {
3501+
assert(!ExprStack.empty() && "must be called while visiting an expression");
3502+
ArrayRef<const Expr *> stack = ExprStack;
3503+
stack = stack.drop_back();
3504+
3505+
for (auto expr : llvm::reverse(stack)) {
3506+
if (auto loadExpr = dyn_cast<LoadExpr>(expr))
3507+
return loadExpr;
3508+
}
3509+
3510+
return nullptr;
3511+
}
3512+
34993513
/// Walk an assignment expression, checking for availability.
35003514
void walkAssignExpr(AssignExpr *E) {
35013515
// We take over recursive walking of assignment expressions in order to
@@ -3575,7 +3589,12 @@ class ExprAvailabilityWalker : public ASTWalker {
35753589

35763590
/// Walk an inout expression, checking for availability.
35773591
void walkInOutExpr(InOutExpr *E) {
3578-
walkInContext(E, E->getSubExpr(), MemberAccessContext::InOut);
3592+
// If there is a LoadExpr in the stack, then this InOutExpr is not actually
3593+
// indicative of any mutation so the access context should just be Getter.
3594+
auto accessContext = getEnclosingLoadExpr() ? MemberAccessContext::Getter
3595+
: MemberAccessContext::InOut;
3596+
3597+
walkInContext(E, E->getSubExpr(), accessContext);
35793598
}
35803599

35813600
bool shouldWalkIntoClosure(AbstractClosureExpr *closure) const {

‎stdlib/public/core/StringCreate.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ internal func _allASCII(_ input: UnsafeBufferPointer<UInt8>) -> Bool {
2121
//
2222
let count = input.count
2323
var ptr = UnsafeRawPointer(input.baseAddress._unsafelyUnwrappedUnchecked)
24-
var i = 0
2524

2625
let asciiMask64 = 0x8080_8080_8080_8080 as UInt64
2726
let asciiMask32 = UInt32(truncatingIfNeeded: asciiMask64)
@@ -63,7 +62,7 @@ internal func _allASCII(_ input: UnsafeBufferPointer<UInt8>) -> Bool {
6362
}
6463

6564
if ptr < end {
66-
let value = ptr.loadUnaligned(fromByteOffset: i, as: UInt8.self)
65+
let value = ptr.loadUnaligned(fromByteOffset: 0, as: UInt8.self)
6766
guard value & asciiMask8 == 0 else { return false }
6867
}
6968
_internalInvariant(ptr == end || ptr + 1 == end)

‎test/Sema/availability_accessors.swift

Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,253 @@
1+
// RUN: %target-typecheck-verify-swift -parse-as-library -module-name MyModule
2+
3+
struct Value {
4+
static let defaultValue = Value(a: Nested(b: 1))
5+
6+
struct Nested {
7+
var b: Int
8+
9+
@discardableResult
10+
mutating func setToZero() -> Int {
11+
let prev = b
12+
b = 0
13+
return prev
14+
}
15+
}
16+
17+
var a: Nested
18+
19+
subscript(_ i: Int) -> Nested {
20+
get { a }
21+
set { a = newValue }
22+
}
23+
24+
@discardableResult
25+
mutating func setToZero() -> Int {
26+
let prev = a.b
27+
a.setToZero()
28+
return prev
29+
}
30+
}
31+
32+
struct BaseStruct {
33+
var available: Value = .defaultValue
34+
35+
var unavailableGetter: Value {
36+
@available(*, unavailable)
37+
get { fatalError() } // expected-note 24 {{getter for 'unavailableGetter' has been explicitly marked unavailable here}}
38+
set {}
39+
}
40+
41+
var unavailableSetter: Value {
42+
get { .defaultValue }
43+
@available(*, unavailable)
44+
set { fatalError() } // expected-note 12 {{setter for 'unavailableSetter' has been explicitly marked unavailable here}}
45+
}
46+
47+
var unavailableGetterAndSetter: Value {
48+
@available(*, unavailable)
49+
get { fatalError() } // expected-note 24 {{getter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
50+
@available(*, unavailable)
51+
set { fatalError() } // expected-note 12 {{setter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
52+
}
53+
}
54+
55+
func takesIntInOut(_ i: inout Int) -> Int {
56+
return 0
57+
}
58+
59+
let someValue = Value.defaultValue
60+
61+
func testRValueLoads() {
62+
var x = BaseStruct() // expected-warning {{variable 'x' was never mutated; consider changing to 'let' constant}}
63+
64+
_ = x.available
65+
_ = x.available.a
66+
_ = x.available[0]
67+
_ = x.available[0].b
68+
69+
_ = x.unavailableGetter // expected-error {{getter for 'unavailableGetter' is unavailable}}
70+
_ = x.unavailableGetter.a // expected-error {{getter for 'unavailableGetter' is unavailable}}
71+
_ = x.unavailableGetter[0] // expected-error {{getter for 'unavailableGetter' is unavailable}}
72+
_ = x.unavailableGetter[0].b // expected-error {{getter for 'unavailableGetter' is unavailable}}
73+
74+
_ = x.unavailableSetter
75+
_ = x.unavailableSetter.a
76+
_ = x.unavailableSetter[0]
77+
_ = x.unavailableSetter[0].b
78+
79+
_ = x.unavailableGetterAndSetter // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
80+
_ = x.unavailableGetterAndSetter.a // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
81+
_ = x.unavailableGetterAndSetter[0] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
82+
_ = x.unavailableGetterAndSetter[0].b // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
83+
}
84+
85+
func testLValueAssignments() {
86+
var x = BaseStruct()
87+
88+
x.available = someValue
89+
x.available.a = someValue.a
90+
x.available[0] = someValue.a
91+
x.available[0].b = 1
92+
93+
x.unavailableGetter = someValue
94+
x.unavailableGetter.a = someValue.a // expected-error {{getter for 'unavailableGetter' is unavailable}}
95+
x.unavailableGetter[0] = someValue.a // expected-error {{getter for 'unavailableGetter' is unavailable}}
96+
x.unavailableGetter[0].b = 1 // expected-error {{getter for 'unavailableGetter' is unavailable}}
97+
98+
x.unavailableSetter = someValue // expected-error {{setter for 'unavailableSetter' is unavailable}}
99+
x.unavailableSetter.a = someValue.a // FIXME: missing diagnostic for setter
100+
x.unavailableSetter[0] = someValue.a // expected-error {{setter for 'unavailableSetter' is unavailable}}
101+
x.unavailableSetter[0].b = 1 // expected-error {{setter for 'unavailableSetter' is unavailable}}
102+
103+
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
105+
x.unavailableGetterAndSetter[0] = someValue.a // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
106+
x.unavailableGetterAndSetter[0].b = 1 // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
107+
}
108+
109+
func testKeyPathLoads() {
110+
let a = [0]
111+
var x = BaseStruct()
112+
113+
_ = x[keyPath: \.available]
114+
_ = x[keyPath: \.available.a]
115+
_ = x[keyPath: \.available[0]]
116+
_ = x[keyPath: \.available[0].b]
117+
_ = a[keyPath: \.[takesIntInOut(&x.available.a.b)]]
118+
_ = a[keyPath: \.[takesIntInOut(&x.available[0].b)]]
119+
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
124+
_ = a[keyPath: \.[takesIntInOut(&x.unavailableGetter.a.b)]] // expected-error {{getter for 'unavailableGetter' is unavailable}}
125+
_ = a[keyPath: \.[takesIntInOut(&x.unavailableGetter[0].b)]] // expected-error {{getter for 'unavailableGetter' is unavailable}}
126+
127+
_ = x[keyPath: \.unavailableSetter]
128+
_ = x[keyPath: \.unavailableSetter.a]
129+
_ = x[keyPath: \.unavailableSetter[0]]
130+
_ = x[keyPath: \.unavailableSetter[0].b]
131+
_ = a[keyPath: \.[takesIntInOut(&x.unavailableSetter.a.b)]] // FIXME: missing diagnostic for setter
132+
_ = a[keyPath: \.[takesIntInOut(&x.unavailableSetter[0].b)]] // expected-error {{setter for 'unavailableSetter' is unavailable}}
133+
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
139+
_ = a[keyPath: \.[takesIntInOut(&x.unavailableGetterAndSetter[0].b)]] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
140+
}
141+
142+
func testKeyPathAssignments() {
143+
var a = [0]
144+
var x = BaseStruct()
145+
146+
x[keyPath: \.available] = someValue
147+
x[keyPath: \.available.a] = someValue.a
148+
x[keyPath: \.available[0]] = someValue.a
149+
x[keyPath: \.available[0].b] = 1
150+
x[keyPath: \.available] = someValue
151+
a[keyPath: \.[takesIntInOut(&x.available.a.b)]] = 0
152+
a[keyPath: \.[takesIntInOut(&x.available[0].b)]] = 0
153+
154+
x[keyPath: \.unavailableGetter] = someValue
155+
x[keyPath: \.unavailableGetter.a] = someValue.a // FIXME: missing diagnostic for getter
156+
x[keyPath: \.unavailableGetter[0]] = someValue.a // FIXME: missing diagnostic for getter
157+
x[keyPath: \.unavailableGetter[0].b] = 1 // FIXME: missing diagnostic for getter
158+
a[keyPath: \.[takesIntInOut(&x.unavailableGetter.a.b)]] = 0 // expected-error {{getter for 'unavailableGetter' is unavailable}}
159+
a[keyPath: \.[takesIntInOut(&x.unavailableGetter[0].b)]] = 0 // expected-error {{getter for 'unavailableGetter' is unavailable}}
160+
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
166+
a[keyPath: \.[takesIntInOut(&x.unavailableSetter[0].b)]] = 0 // expected-error {{setter for 'unavailableSetter' is unavailable}}
167+
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
173+
a[keyPath: \.[takesIntInOut(&x.unavailableGetterAndSetter[0].b)]] = 0 // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
174+
}
175+
176+
func testMutatingMember() {
177+
var x = BaseStruct()
178+
let a = [0]
179+
180+
x.available.setToZero()
181+
x.available[0].setToZero()
182+
_ = a[x.available.setToZero()]
183+
_ = a[x.available.a.setToZero()]
184+
_ = a[x.available[0].setToZero()]
185+
186+
x.unavailableGetter.setToZero() // expected-error {{getter for 'unavailableGetter' is unavailable}}
187+
x.unavailableGetter[0].setToZero() // expected-error {{getter for 'unavailableGetter' is unavailable}}
188+
_ = a[x.unavailableGetter.setToZero()] // expected-error {{getter for 'unavailableGetter' is unavailable}}
189+
_ = a[x.unavailableGetter.a.setToZero()] // expected-error {{getter for 'unavailableGetter' is unavailable}}
190+
_ = a[x.unavailableGetter[0].setToZero()] // expected-error {{getter for 'unavailableGetter' is unavailable}}
191+
192+
x.unavailableSetter.setToZero() // expected-error {{setter for 'unavailableSetter' is unavailable}}
193+
x.unavailableSetter[0].setToZero() // expected-error {{setter for 'unavailableSetter' is unavailable}}
194+
_ = a[x.unavailableSetter.setToZero()] // expected-error {{setter for 'unavailableSetter' is unavailable}}
195+
_ = a[x.unavailableSetter.a.setToZero()] // FIXME: missing diagnostic for setter
196+
_ = a[x.unavailableSetter[0].setToZero()] // expected-error {{setter for 'unavailableSetter' is unavailable}}
197+
198+
x.unavailableGetterAndSetter.setToZero() // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
199+
x.unavailableGetterAndSetter[0].setToZero() // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
200+
_ = 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
202+
_ = a[x.unavailableGetterAndSetter[0].setToZero()] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
203+
}
204+
205+
func testPassAsInOutParameter() {
206+
func takesInOut<T>(_ t: inout T) {}
207+
208+
var x = BaseStruct()
209+
210+
takesInOut(&x.available)
211+
takesInOut(&x.available.a)
212+
takesInOut(&x.available[0])
213+
takesInOut(&x.available[0].b)
214+
215+
takesInOut(&x.unavailableGetter) // expected-error {{getter for 'unavailableGetter' is unavailable}}
216+
takesInOut(&x.unavailableGetter.a) // expected-error {{getter for 'unavailableGetter' is unavailable}}
217+
takesInOut(&x.unavailableGetter[0]) // expected-error {{getter for 'unavailableGetter' is unavailable}}
218+
takesInOut(&x.unavailableGetter[0].b) // expected-error {{getter for 'unavailableGetter' is unavailable}}
219+
220+
takesInOut(&x.unavailableSetter) // expected-error {{setter for 'unavailableSetter' is unavailable}}
221+
takesInOut(&x.unavailableSetter.a) //
222+
takesInOut(&x.unavailableSetter[0]) // expected-error {{setter for 'unavailableSetter' is unavailable}}
223+
takesInOut(&x.unavailableSetter[0].b) // expected-error {{setter for 'unavailableSetter' is unavailable}}
224+
225+
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
227+
takesInOut(&x.unavailableGetterAndSetter[0]) // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
228+
takesInOut(&x.unavailableGetterAndSetter[0].b) // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
229+
}
230+
231+
var global = BaseStruct()
232+
233+
struct TestPatternBindingInitExprs {
234+
var available = global.available
235+
var available_a = global.available.a
236+
var available_0 = global.available[0]
237+
var available_0_b = global.available[0].b
238+
239+
var unavailableGetter = global.unavailableGetter // expected-error {{getter for 'unavailableGetter' is unavailable}}
240+
var unavailableGetter_a = global.unavailableGetter.a // expected-error {{getter for 'unavailableGetter' is unavailable}}
241+
var unavailableGetter_0 = global.unavailableGetter[0] // expected-error {{getter for 'unavailableGetter' is unavailable}}
242+
var unavailableGetter_0_b = global.unavailableGetter[0].b // expected-error {{getter for 'unavailableGetter' is unavailable}}
243+
244+
var unavailableSetter = global.unavailableSetter
245+
var unavailableSetter_a = global.unavailableSetter.a
246+
var unavailableSetter_0 = global.unavailableSetter[0]
247+
var unavailableSetter_0_b = global.unavailableSetter[0].b
248+
249+
var unavailableGetterAndSetter = global.unavailableGetterAndSetter // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
250+
var unavailableGetterAndSetter_a = global.unavailableGetterAndSetter.a // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
251+
var unavailableGetterAndSetter_0 = global.unavailableGetterAndSetter[0] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
252+
var unavailableGetterAndSetter_0_b = global.unavailableGetterAndSetter[0].b // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
253+
}

0 commit comments

Comments
 (0)