Skip to content

Commit c1d29b2

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 bc2aaf2 commit c1d29b2

File tree

2 files changed

+30
-15
lines changed

2 files changed

+30
-15
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 & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,14 @@ struct BaseStruct {
3535
var unavailableSetter: Value {
3636
get { .defaultValue }
3737
@available(*, unavailable)
38-
set { fatalError() } // expected-note 9 {{setter for 'unavailableSetter' has been explicitly marked unavailable here}}
38+
set { fatalError() } // expected-note 5 {{setter for 'unavailableSetter' has been explicitly marked unavailable here}}
3939
}
4040

4141
var unavailableGetterAndSetter: Value {
4242
@available(*, unavailable)
4343
get { fatalError() } // expected-note 17 {{getter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
4444
@available(*, unavailable)
45-
set { fatalError() } // expected-note 11 {{setter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
45+
set { fatalError() } // expected-note 7 {{setter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
4646
}
4747
}
4848

@@ -63,15 +63,13 @@ func testRValueLoads() {
6363

6464
_ = x.unavailableSetter
6565
_ = x.unavailableSetter.a
66-
// FIXME: setter should not be diagnosed when loading through a subscript
67-
_ = x.unavailableSetter[0] // expected-error {{setter for 'unavailableSetter' is unavailable}}
68-
_ = x.unavailableSetter[0].b // expected-error {{setter for 'unavailableSetter' is unavailable}}
66+
_ = x.unavailableSetter[0]
67+
_ = x.unavailableSetter[0].b
6968

7069
_ = x.unavailableGetterAndSetter // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
7170
_ = x.unavailableGetterAndSetter.a // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
72-
// FIXME: setter should not be diagnosed when loading through a subscript
73-
_ = x.unavailableGetterAndSetter[0] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
74-
_ = x.unavailableGetterAndSetter[0].b // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
71+
_ = x.unavailableGetterAndSetter[0] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
72+
_ = x.unavailableGetterAndSetter[0].b // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
7573
}
7674

7775
func testLValueAssignments() {
@@ -149,13 +147,11 @@ struct TestPatternBindingInitExprs {
149147

150148
var unavailableSetter = global.unavailableSetter
151149
var unavailableSetter_a = global.unavailableSetter.a
152-
// FIXME: setter should not be diagnosed when loading through a subscript
153-
var unavailableSetter_0 = global.unavailableSetter[0] // expected-error {{setter for 'unavailableSetter' is unavailable}}
154-
var unavailableSetter_0_b = global.unavailableSetter[0].b // expected-error {{setter for 'unavailableSetter' is unavailable}}
150+
var unavailableSetter_0 = global.unavailableSetter[0]
151+
var unavailableSetter_0_b = global.unavailableSetter[0].b
155152

156153
var unavailableGetterAndSetter = global.unavailableGetterAndSetter // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
157154
var unavailableGetterAndSetter_a = global.unavailableGetterAndSetter.a // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
158-
// FIXME: setter should not be diagnosed when loading through a subscript
159-
var unavailableGetterAndSetter_0 = global.unavailableGetterAndSetter[0] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
160-
var unavailableGetterAndSetter_0_b = global.unavailableGetterAndSetter[0].b // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
155+
var unavailableGetterAndSetter_0 = global.unavailableGetterAndSetter[0] // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
156+
var unavailableGetterAndSetter_0_b = global.unavailableGetterAndSetter[0].b // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}}
161157
}

0 commit comments

Comments
 (0)