Skip to content

Commit e9c4df0

Browse files
DougGregorDougGregor
authored
Merge pull request swiftlang#25142 from DougGregor/property-wrappers-private-backing-var
[SE-0258] Reduce access of backing storage variable to 'private'.
2 parents 46b5af5 + 59180db commit e9c4df0

File tree

4 files changed

+44
-57
lines changed

4 files changed

+44
-57
lines changed

‎lib/Sema/CodeSynthesis.cpp

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,15 +1693,9 @@ static VarDecl *synthesizePropertyWrapperStorageWrapperProperty(
16931693
addMemberToContextIfNeeded(pbd, dc, var);
16941694
pbd->setStatic(var->isStatic());
16951695

1696-
// Determine the access level for the property.
1697-
AccessLevel access =
1698-
std::min(AccessLevel::Internal, var->getFormalAccess());
1699-
property->overwriteAccess(access);
1700-
1701-
// Determine setter access.
1702-
AccessLevel setterAccess =
1703-
std::min(AccessLevel::Internal, var->getSetterFormalAccess());
1704-
property->overwriteSetterAccess(setterAccess);
1696+
// The property is always private.
1697+
property->overwriteAccess(AccessLevel::Private);
1698+
property->overwriteSetterAccess(AccessLevel::Private);
17051699

17061700
// Add the accessors we need.
17071701
bool hasSetter = wrapperVar->isSettable(nullptr) &&
@@ -1808,21 +1802,11 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
18081802
if (dc->getSelfClassDecl())
18091803
makeFinal(ctx, backingVar);
18101804

1811-
// When there is a `wrapperValue`, lower the access of the backing
1812-
// storage to 'private'.
1813-
AccessLevel defaultAccess =
1814-
wrapperInfo.wrapperValueVar ? AccessLevel::Private
1815-
: AccessLevel::Internal;
1816-
1817-
// Determine the access level for the backing property.
1818-
AccessLevel access =
1819-
std::min(defaultAccess, var->getFormalAccess());
1820-
backingVar->overwriteAccess(access);
1805+
// The backing storage is 'private'.
1806+
backingVar->overwriteAccess(AccessLevel::Private);
18211807

18221808
// Determine setter access.
1823-
AccessLevel setterAccess =
1824-
std::min(defaultAccess, var->getSetterFormalAccess());
1825-
backingVar->overwriteSetterAccess(setterAccess);
1809+
backingVar->overwriteSetterAccess(AccessLevel::Private);
18261810

18271811
Expr *originalInitialValue = nullptr;
18281812
if (Expr *init = parentPBD->getInit(patternNumber)) {

‎test/SILGen/property_wrappers.swift

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ func forceHasMemberwiseInit() {
4040
_ = HasMemberwiseInit<Int>()
4141
}
4242

43-
// CHECK: sil_global hidden @$s17property_wrappers9UseStaticV13$staticWibbleAA4LazyOySaySiGGvpZ : $Lazy<Array<Int>>
43+
// CHECK: sil_global private @$s17property_wrappers9UseStaticV13$staticWibble33_{{.*}}AA4LazyOySaySiGGvpZ : $Lazy<Array<Int>>
4444

4545
// HasMemberwiseInit.x.setter
46-
// CHECK-LABEL: sil hidden [transparent] [ossa] @$s17property_wrappers17HasMemberwiseInitV1xSbvs : $@convention(method) <T where T : DefaultInit> (Bool, @inout HasMemberwiseInit<T>) -> () {
46+
// CHECK-LABEL: sil hidden [ossa] @$s17property_wrappers17HasMemberwiseInitV1xSbvs : $@convention(method) <T where T : DefaultInit> (Bool, @inout HasMemberwiseInit<T>) -> () {
4747
// CHECK: bb0(%0 : $Bool, %1 : $*HasMemberwiseInit<T>):
4848
// CHECK: [[MODIFY_SELF:%.*]] = begin_access [modify] [unknown] %1 : $*HasMemberwiseInit<T>
4949
// CHECK: [[X_BACKING:%.*]] = struct_element_addr [[MODIFY_SELF]] : $*HasMemberwiseInit<T>, #HasMemberwiseInit.$x
@@ -52,7 +52,7 @@ func forceHasMemberwiseInit() {
5252
// CHECK: end_access [[MODIFY_SELF]] : $*HasMemberwiseInit<T>
5353

5454
// variable initialization expression of HasMemberwiseInit.$x
55-
// CHECK-LABEL: sil hidden [transparent] [ossa] @$s17property_wrappers17HasMemberwiseInitV2$xAA7WrapperVySbGvpfi : $@convention(thin) <T where T : DefaultInit> () -> Wrapper<Bool> {
55+
// CHECK-LABEL: sil hidden [transparent] [ossa] @$s17property_wrappers17HasMemberwiseInitV2$x33_{{.*}}AA7WrapperVySbGvpfi : $@convention(thin) <T where T : DefaultInit> () -> Wrapper<Bool> {
5656
// CHECK: integer_literal $Builtin.Int1, 0
5757
// CHECK-NOT: return
5858
// CHECK: function_ref @$sSb22_builtinBooleanLiteralSbBi1__tcfC : $@convention(method) (Builtin.Int1, @thin Bool.Type) -> Bool
@@ -61,13 +61,13 @@ func forceHasMemberwiseInit() {
6161
// CHECK: return {{%.*}} : $Wrapper<Bool>
6262

6363
// variable initialization expression of HasMemberwiseInit.$y
64-
// CHECK-LABEL: sil hidden [transparent] [ossa] @$s17property_wrappers17HasMemberwiseInitV2$yAA23WrapperWithInitialValueVyxGvpfi : $@convention(thin) <T where T : DefaultInit> () -> @out T {
64+
// CHECK-LABEL: sil hidden [transparent] [ossa] @$s17property_wrappers17HasMemberwiseInitV2$y33_{{.*}}23WrapperWithInitialValueVyxGvpfi : $@convention(thin) <T where T : DefaultInit> () -> @out
6565
// CHECK: bb0(%0 : $*T):
6666
// CHECK-NOT: return
6767
// CHECK: witness_method $T, #DefaultInit.init!allocator.1 : <Self where Self : DefaultInit> (Self.Type) -> () -> Self : $@convention(witness_method: DefaultInit) <τ_0_0 where τ_0_0 : DefaultInit> (@thick τ_0_0.Type) -> @out τ_0_0
6868

6969
// variable initialization expression of HasMemberwiseInit.$z
70-
// CHECK-LABEL: sil hidden [transparent] [ossa] @$s17property_wrappers17HasMemberwiseInitV2$zAA23WrapperWithInitialValueVySiGvpfi : $@convention(thin) <T where T : DefaultInit> () -> WrapperWithInitialValue<Int> {
70+
// CHECK-LABEL: sil hidden [transparent] [ossa] @$s17property_wrappers17HasMemberwiseInitV2$z33_{{.*}}23WrapperWithInitialValueVySiGvpfi : $@convention(thin) <T where T : DefaultInit> () -> WrapperWithInitialValue<Int> {
7171
// CHECK: bb0:
7272
// CHECK-NOT: return
7373
// CHECK: integer_literal $Builtin.IntLiteral, 17
@@ -89,17 +89,17 @@ func forceHasMemberwiseInit() {
8989

9090
// Initialization of x
9191
// CHECK-NOT: return
92-
// CHECK: function_ref @$s17property_wrappers17HasMemberwiseInitV2$xAA7WrapperVySbGvpfi : $@convention(thin) <τ_0_0 where τ_0_0 : DefaultInit> () -> Wrapper<Bool>
92+
// CHECK: function_ref @$s17property_wrappers17HasMemberwiseInitV2$x33_{{.*}}7WrapperVySbGvpfi : $@convention(thin) <τ_0_0 where τ_0_0 : DefaultInit> () -> Wrapper<Bool>
9393

9494
// Initialization of y
9595
// CHECK-NOT: return
96-
// CHECK: function_ref @$s17property_wrappers17HasMemberwiseInitV2$yAA23WrapperWithInitialValueVyxGvpfi : $@convention(thin) <τ_0_0 where τ_0_0 : DefaultInit> () -> @out τ_0_0
96+
// CHECK: function_ref @$s17property_wrappers17HasMemberwiseInitV2$y33_{{.*}}23WrapperWithInitialValueVyxGvpfi : $@convention(thin) <τ_0_0 where τ_0_0 : DefaultInit> () -> @out τ_0_0
9797
// CHECK-NOT: return
9898
// CHECK: function_ref @$s17property_wrappers23WrapperWithInitialValueV07initialF0ACyxGx_tcfC : $@convention(method) <τ_0_0> (@in τ_0_0, @thin WrapperWithInitialValue<τ_0_0>.Type) -> @out WrapperWithInitialValue<τ_0_0>
9999

100100
// Initialization of z
101101
// CHECK-NOT: return
102-
// CHECK: function_ref @$s17property_wrappers17HasMemberwiseInitV2$zAA23WrapperWithInitialValueVySiGvpfi : $@convention(thin) <τ_0_0 where τ_0_0 : DefaultInit> () -> WrapperWithInitialValue<Int>
102+
// CHECK: function_ref @$s17property_wrappers17HasMemberwiseInitV2$z33_{{.*}}23WrapperWithInitialValueVySiGvpfi : $@convention(thin) <τ_0_0 where τ_0_0 : DefaultInit> () -> WrapperWithInitialValue<Int>
103103

104104
// CHECK: return
105105

@@ -145,7 +145,7 @@ struct WrapperWithAccessors {
145145
var x: Int
146146

147147
// Synthesized setter
148-
// CHECK-LABEL: sil hidden [transparent] [ossa] @$s17property_wrappers20WrapperWithAccessorsV1xSivs : $@convention(method) (Int, @inout WrapperWithAccessors) -> ()
148+
// CHECK-LABEL: sil hidden [ossa] @$s17property_wrappers20WrapperWithAccessorsV1xSivs : $@convention(method) (Int, @inout WrapperWithAccessors) -> ()
149149
// CHECK-NOT: return
150150
// CHECK: struct_element_addr {{%.*}} : $*WrapperWithAccessors, #WrapperWithAccessors.$x
151151

@@ -191,10 +191,14 @@ struct WrapperWithStorageValue<T> {
191191

192192
struct UseWrapperWithStorageValue {
193193
// UseWrapperWithStorageValue.$x.getter
194-
// CHECK-LABEL: sil hidden [transparent] [ossa] @$s17property_wrappers26UseWrapperWithStorageValueV2$xAA0D0VySiGvg : $@convention(method) (UseWrapperWithStorageValue) -> Wrapper<Int>
194+
// CHECK-LABEL: sil private [ossa] @$s17property_wrappers26UseWrapperWithStorageValueV2$x33_{{.*}}SiGvg : $@convention(method) (UseWrapperWithStorageValue) -> Wrapper<Int>
195195
// CHECK-NOT: return
196196
// CHECK: function_ref @$s17property_wrappers23WrapperWithStorageValueV07wrapperF0AA0C0VyxGvg
197197
@WrapperWithStorageValue(value: 17) var x: Int
198+
199+
func foo() {
200+
_ = $x
201+
}
198202
}
199203

200204
@_propertyWrapper
@@ -229,7 +233,7 @@ struct UseLazy<T: DefaultInit> {
229233
@Lazy var wibble = [1, 2, 3]
230234

231235
// CHECK-LABEL: sil hidden [ossa] @$s17property_wrappers7UseLazyV3foo3bar6wibbleACyxGSi_xSaySiGtcfC : $@convention(method) <T where T : DefaultInit> (Int, @in T, @owned Array<Int>, @thin UseLazy<T>.Type) -> @out UseLazy<T>
232-
// CHECK: function_ref @$s17property_wrappers7UseLazyV4$fooAA0D0OySiGvpfiSiycfu_ : $@convention(thin) (@owned Int) -> Int
236+
// CHECK: function_ref @$s17property_wrappers7UseLazyV4$foo33_{{.*}}AA0D0OySiGvpfiSiycfu_ : $@convention(thin) (@owned Int) -> Int
233237
// CHECK: function_ref @$s17property_wrappers4LazyO12initialValueACyxGxyXA_tcfC : $@convention(method) <τ_0_0> (@owned @callee_guaranteed () -> @out τ_0_0, @thin Lazy<τ_0_0>.Type) -> @out Lazy<τ_0_0>
234238
}
235239

@@ -243,9 +247,9 @@ func triggerUseLazy() {
243247
}
244248

245249
struct UseStatic {
246-
// CHECK: sil hidden [transparent] [ossa] @$s17property_wrappers9UseStaticV12staticWibbleSaySiGvgZ
247-
// CHECK: sil hidden [global_init] [ossa] @$s17property_wrappers9UseStaticV13$staticWibbleAA4LazyOySaySiGGvau
248-
// CHECK: sil hidden [transparent] [ossa] @$s17property_wrappers9UseStaticV12staticWibbleSaySiGvsZ
250+
// CHECK: sil hidden [ossa] @$s17property_wrappers9UseStaticV12staticWibbleSaySiGvgZ
251+
// CHECK: sil private [global_init] [ossa] @$s17property_wrappers9UseStaticV13$staticWibble33_{{.*}}4LazyOySaySiGGvau
252+
// CHECK: sil hidden [ossa] @$s17property_wrappers9UseStaticV12staticWibbleSaySiGvsZ
249253
@Lazy static var staticWibble = [1, 2, 3]
250254
}
251255

@@ -257,16 +261,18 @@ class ClassUsingWrapper {
257261
@WrapperWithInitialValue var x = 0
258262
}
259263

260-
// CHECK-LABEL: sil hidden [ossa] @$s17property_wrappers21testClassUsingWrapper1cyAA0deF0C_tF : $@convention(thin) (@guaranteed ClassUsingWrapper) -> ()
261-
func testClassUsingWrapper(c: ClassUsingWrapper) {
262-
// CHECK: class_method [[GETTER:%.*]] : $ClassUsingWrapper, #ClassUsingWrapper.$x!getter.1
263-
c.$x.test()
264+
extension ClassUsingWrapper {
265+
// CHECK-LABEL: sil hidden [ossa] @$s17property_wrappers17ClassUsingWrapperC04testcdE01cyAC_tF : $@convention(method) (@guaranteed ClassUsingWrapper, @guaranteed ClassUsingWrapper) -> () {
266+
func testClassUsingWrapper(c: ClassUsingWrapper) {
267+
// CHECK: class_method [[GETTER:%.*]] : $ClassUsingWrapper, #ClassUsingWrapper.$x!getter.1
268+
self.$x.test()
269+
}
264270
}
265271

266272
// CHECK-LABEL: sil_vtable ClassUsingWrapper {
267273
// CHECK: #ClassUsingWrapper.x!getter.1: (ClassUsingWrapper) -> () -> Int : @$s17property_wrappers17ClassUsingWrapperC1xSivg // ClassUsingWrapper.x.getter
268274
// CHECK: #ClassUsingWrapper.x!setter.1: (ClassUsingWrapper) -> (Int) -> () : @$s17property_wrappers17ClassUsingWrapperC1xSivs // ClassUsingWrapper.x.setter
269275
// CHECK: #ClassUsingWrapper.x!modify.1: (ClassUsingWrapper) -> () -> () : @$s17property_wrappers17ClassUsingWrapperC1xSivM // ClassUsingWrapper.x.modify
270-
// CHECK: #ClassUsingWrapper.$x!getter.1: (ClassUsingWrapper) -> () -> WrapperWithInitialValue<Int> : @$s17property_wrappers17ClassUsingWrapperC2$xAA0E16WithInitialValueVySiGvg // ClassUsingWrapper.$x.getter
271-
// CHECK: #ClassUsingWrapper.$x!setter.1: (ClassUsingWrapper) -> (WrapperWithInitialValue<Int>) -> () : @$s17property_wrappers17ClassUsingWrapperC2$xAA0E16WithInitialValueVySiGvs // ClassUsingWrapper.$x.setter
272-
// CHECK: #ClassUsingWrapper.$x!modify.1: (ClassUsingWrapper) -> () -> () : @$s17property_wrappers17ClassUsingWrapperC2$xAA0E16WithInitialValueVySiGvM // ClassUsingWrapper.$x.modify
276+
// CHECK: #ClassUsingWrapper.$x!getter.1: (ClassUsingWrapper) -> () -> WrapperWithInitialValue<Int> : @$s17property_wrappers17ClassUsingWrapperC2$x33_{{.*}}16WithInitialValueVySiGvg // ClassUsingWrapper.$x.getter
277+
// CHECK: #ClassUsingWrapper.$x!setter.1: (ClassUsingWrapper) -> (WrapperWithInitialValue<Int>) -> () : @$s17property_wrappers17ClassUsingWrapperC2$x33_{{.*}}16WithInitialValueVySiGvs // ClassUsingWrapper.$x.setter
278+
// CHECK: #ClassUsingWrapper.$x!modify.1: (ClassUsingWrapper) -> () -> () : @$s17property_wrappers17ClassUsingWrapperC2$x33_{{.*}}16WithInitialValueVySiGvM // ClassUsingWrapper.$x.modify

‎test/Serialization/property_wrappers.swift

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,16 @@
22
// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/def_property_wrappers.swift
33
// RUN: %target-swift-frontend -typecheck -I%t -verify %s -verify-ignore-unknown
44

5-
// Same test, but with -enable-testing so we can see the backing properties
6-
// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/def_property_wrappers.swift -enable-testing
7-
// RUN: %target-swift-frontend -DTESTING -typecheck -I%t %s
8-
9-
#if TESTING
10-
@testable import def_property_wrappers
11-
#else
125
import def_property_wrappers
13-
#endif
146

157
func useWrappers(hd: HasWrappers) {
168
// Access the original properties
179
let _: Int = hd.x
1810

19-
let _: SomeWrapper<Int> = hd.$x // expected-error{{'$x' is inaccessible due to 'internal' protection level}}
11+
let _: SomeWrapper<Int> = hd.$x // expected-error{{'$x' is inaccessible due to 'private' protection level}}
2012

2113
var mutableHD = hd
2214
mutableHD.x = 17
2315

24-
mutableHD.$x = SomeWrapper(initialValue: 42) // expected-error{{'$x' is inaccessible due to 'internal' protection level}}
16+
mutableHD.$x = SomeWrapper(initialValue: 42) // expected-error{{'$x' is inaccessible due to 'private' protection level}}
2517
}

‎test/decl/var/property_wrappers.swift

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,8 @@ class Superclass {
171171

172172
class SubclassOfClassWithWrappers: ClassWithWrappers {
173173
override var x: Int {
174-
get { return $x.value }
175-
set { $x.value = newValue }
174+
get { return super.x }
175+
set { super.x = newValue }
176176
}
177177
}
178178

@@ -503,7 +503,7 @@ class Box<Value> {
503503

504504
struct UseBox {
505505
@Box
506-
var x = 17
506+
var x = 17 // expected-note{{'$x' declared here}}
507507
}
508508

509509
func testBox(ub: UseBox) {
@@ -514,6 +514,10 @@ func testBox(ub: UseBox) {
514514
mutableUB = ub
515515
}
516516

517+
func backingVarIsPrivate(ub: UseBox) {
518+
_ = ub.$x // expected-error{{'$x' is inaccessible due to 'private' protection level}}
519+
}
520+
517521
// ---------------------------------------------------------------------------
518522
// Memberwise initializers
519523
// ---------------------------------------------------------------------------
@@ -626,6 +630,7 @@ extension Wrapper {
626630

627631
struct TestStorageRef {
628632
@WrapperWithStorageRef var x: Int // expected-note{{'$$x' declared here}}
633+
// expected-note@-1{{'$x' declared here}}
629634

630635
init(x: Int) {
631636
self.$$x = WrapperWithStorageRef(value: x)
@@ -643,7 +648,7 @@ struct TestStorageRef {
643648
}
644649

645650
func testStorageRef(tsr: TestStorageRef) {
646-
let _: Wrapper = tsr.$x
651+
let _: Wrapper = tsr.$x // expected-error{{'$x' is inaccessible due to 'private' protection level}}
647652
_ = tsr.$$x // expected-error{{'$$x' is inaccessible due to 'private' protection level}}
648653
}
649654

0 commit comments

Comments
 (0)