Skip to content

Commit 5a23239

Browse files
committed
Sema: Do not diagnose set accessor availability for InOutExprs inside of LoadExprs.
The recent deprecation of the set accessor for `CommandLine.arguments` exposed a bug in availability checking: ``` for arg in CommandLine.arguments[1...] { ╰─ warning: setter for 'arguments' is deprecated: ... ``` The parser generates an `InOutExpr` in the AST for the subscript access in the code above and the availabilty checker was unconditionally diagnosing set accessors for `InOutExprs`, resulting in spurious availability warnings for read-only accesses of properties. The fix is to check whether there is an enclosing `LoadExpr` in the AST and only diagnose the getter accessor in that case. Resolves rdar://124566405
1 parent bab6518 commit 5a23239

File tree

2 files changed

+30
-11
lines changed

2 files changed

+30
-11
lines changed

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 {

test/Sema/availability_accessors.swift

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ struct BaseStruct {
4141
var unavailableSetter: Value {
4242
get { .defaultValue }
4343
@available(*, unavailable)
44-
set { fatalError() } // expected-note 16 {{setter for 'unavailableSetter' has been explicitly marked unavailable here}}
44+
set { fatalError() } // expected-note 12 {{setter for 'unavailableSetter' has been explicitly marked unavailable here}}
4545
}
4646

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

@@ -73,13 +73,13 @@ func testRValueLoads() {
7373

7474
_ = x.unavailableSetter
7575
_ = x.unavailableSetter.a
76-
_ = x.unavailableSetter[0] // expected-error {{setter for 'unavailableSetter' is unavailable}} FIXME: setter should not be diagnosed
77-
_ = x.unavailableSetter[0].b // expected-error {{setter for 'unavailableSetter' is unavailable}} FIXME: setter should not be diagnosed
76+
_ = x.unavailableSetter[0]
77+
_ = x.unavailableSetter[0].b
7878

7979
_ = x.unavailableGetterAndSetter // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
8080
_ = x.unavailableGetterAndSetter.a // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
81-
_ = x.unavailableGetterAndSetter[0] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}} FIXME: setter should not be diagnosed
82-
_ = x.unavailableGetterAndSetter[0].b // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}} FIXME: setter should not be diagnosed
81+
_ = x.unavailableGetterAndSetter[0] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
82+
_ = x.unavailableGetterAndSetter[0].b // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
8383
}
8484

8585
func testLValueAssignments() {
@@ -243,11 +243,11 @@ struct TestPatternBindingInitExprs {
243243

244244
var unavailableSetter = global.unavailableSetter
245245
var unavailableSetter_a = global.unavailableSetter.a
246-
var unavailableSetter_0 = global.unavailableSetter[0] // expected-error {{setter for 'unavailableSetter' is unavailable}} FIXME: setter should not be diagnosed
247-
var unavailableSetter_0_b = global.unavailableSetter[0].b // expected-error {{setter for 'unavailableSetter' is unavailable}}
246+
var unavailableSetter_0 = global.unavailableSetter[0]
247+
var unavailableSetter_0_b = global.unavailableSetter[0].b
248248

249249
var unavailableGetterAndSetter = global.unavailableGetterAndSetter // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
250250
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}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}} FIXME: setter should not be diagnosed
252-
var unavailableGetterAndSetter_0_b = global.unavailableGetterAndSetter[0].b // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}} FIXME: setter should not be diagnosed
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}}
253253
}

0 commit comments

Comments
 (0)