Skip to content

Commit 70c60e5

Browse files
jckarterjckarter
committed
KeyPaths: Make references to let properties properly immutable.
1 parent 00b50ce commit 70c60e5

File tree

9 files changed

+125
-64
lines changed

9 files changed

+125
-64
lines changed

‎include/swift/ABI/KeyPath.h

+25-14
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ class KeyPathComponentHeader {
8282
offset;
8383
}
8484

85+
static constexpr uint32_t isLetBit(bool isLet) {
86+
return isLet ? 0 : _SwiftKeyPathComponentHeader_StoredMutableFlag;
87+
}
88+
8589
public:
8690
static constexpr bool offsetCanBeInline(unsigned offset) {
8791
return offset <= _SwiftKeyPathComponentHeader_MaximumOffsetPayload;
@@ -92,57 +96,64 @@ class KeyPathComponentHeader {
9296
unsigned offset) {
9397
return KeyPathComponentHeader(
9498
(_SwiftKeyPathComponentHeader_StructTag
95-
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
96-
| validateInlineOffset(offset));
99+
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
100+
| validateInlineOffset(offset)
101+
| isLetBit(isLet));
97102
}
98103

99104
constexpr static KeyPathComponentHeader
100105
forStructComponentWithOutOfLineOffset(bool isLet) {
101106
return KeyPathComponentHeader(
102107
(_SwiftKeyPathComponentHeader_StructTag
103-
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
104-
| _SwiftKeyPathComponentHeader_OutOfLineOffsetPayload);
108+
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
109+
| _SwiftKeyPathComponentHeader_OutOfLineOffsetPayload
110+
| isLetBit(isLet));
105111
}
106112

107113
constexpr static KeyPathComponentHeader
108114
forStructComponentWithUnresolvedFieldOffset(bool isLet) {
109115
return KeyPathComponentHeader(
110116
(_SwiftKeyPathComponentHeader_StructTag
111-
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
112-
| _SwiftKeyPathComponentHeader_UnresolvedFieldOffsetPayload);
117+
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
118+
| _SwiftKeyPathComponentHeader_UnresolvedFieldOffsetPayload
119+
| isLetBit(isLet));
113120
}
114121

115122
constexpr static KeyPathComponentHeader
116123
forClassComponentWithInlineOffset(bool isLet,
117124
unsigned offset) {
118125
return KeyPathComponentHeader(
119126
(_SwiftKeyPathComponentHeader_ClassTag
120-
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
121-
| validateInlineOffset(offset));
127+
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
128+
| validateInlineOffset(offset)
129+
| isLetBit(isLet));
122130
}
123131

124132
constexpr static KeyPathComponentHeader
125133
forClassComponentWithOutOfLineOffset(bool isLet) {
126134
return KeyPathComponentHeader(
127135
(_SwiftKeyPathComponentHeader_ClassTag
128-
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
129-
| _SwiftKeyPathComponentHeader_OutOfLineOffsetPayload);
136+
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
137+
| _SwiftKeyPathComponentHeader_OutOfLineOffsetPayload
138+
| isLetBit(isLet));
130139
}
131140

132141
constexpr static KeyPathComponentHeader
133142
forClassComponentWithUnresolvedFieldOffset(bool isLet) {
134143
return KeyPathComponentHeader(
135144
(_SwiftKeyPathComponentHeader_ClassTag
136-
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
137-
| _SwiftKeyPathComponentHeader_UnresolvedFieldOffsetPayload);
145+
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
146+
| _SwiftKeyPathComponentHeader_UnresolvedFieldOffsetPayload
147+
| isLetBit(isLet));
138148
}
139149

140150
constexpr static KeyPathComponentHeader
141151
forClassComponentWithUnresolvedIndirectOffset(bool isLet) {
142152
return KeyPathComponentHeader(
143153
(_SwiftKeyPathComponentHeader_ClassTag
144-
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
145-
| _SwiftKeyPathComponentHeader_UnresolvedIndirectOffsetPayload);
154+
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
155+
| _SwiftKeyPathComponentHeader_UnresolvedIndirectOffsetPayload
156+
| isLetBit(isLet));
146157
}
147158

148159
constexpr static KeyPathComponentHeader

‎stdlib/public/core/KeyPath.swift

+34-21
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,9 @@ internal struct RawKeyPathComponent {
758758
internal static var endOfReferencePrefixFlag: UInt32 {
759759
return _SwiftKeyPathComponentHeader_EndOfReferencePrefixFlag
760760
}
761+
internal static var storedMutableFlag: UInt32 {
762+
return _SwiftKeyPathComponentHeader_StoredMutableFlag
763+
}
761764
internal static var storedOffsetPayloadMask: UInt32 {
762765
return _SwiftKeyPathComponentHeader_StoredOffsetPayloadMask
763766
}
@@ -774,6 +777,11 @@ internal struct RawKeyPathComponent {
774777
return _SwiftKeyPathComponentHeader_MaximumOffsetPayload
775778
}
776779

780+
internal var isStoredMutable: Bool {
781+
_sanityCheck(kind == .struct || kind == .class)
782+
return _value & Header.storedMutableFlag != 0
783+
}
784+
777785
internal static var computedMutatingFlag: UInt32 {
778786
return _SwiftKeyPathComponentHeader_ComputedMutatingFlag
779787
}
@@ -2281,6 +2289,17 @@ internal func _getKeyPathClassAndInstanceSizeFromPattern(
22812289
return true
22822290
}())
22832291

2292+
func setStoredCapability(for header: RawKeyPathComponent.Header) {
2293+
// Mutable class properties can be the root of a reference mutation.
2294+
// Mutable struct properties pass through the existing capability.
2295+
if header.isStoredMutable {
2296+
if header.kind == .class { capability = .reference }
2297+
} else {
2298+
// Immutable properties can only be read.
2299+
capability = .readOnly
2300+
}
2301+
}
2302+
22842303
func setComputedCapability(for header: RawKeyPathComponent.Header) {
22852304
let settable = header.isComputedSettable
22862305
let mutating = header.isComputedMutating
@@ -2301,14 +2320,8 @@ internal func _getKeyPathClassAndInstanceSizeFromPattern(
23012320
}
23022321

23032322
switch header.kind {
2304-
case .class:
2305-
// The rest of the key path could be reference-writable.
2306-
capability = .reference
2307-
fallthrough
2308-
case .struct:
2309-
// No effect on the capability.
2310-
// TODO: we should dynamically prevent "let" properties from being
2311-
// reassigned.
2323+
case .class, .struct:
2324+
setStoredCapability(for: header)
23122325

23132326
// Check the final instantiated size of the offset.
23142327
if header.storedOffsetPayload == RawKeyPathComponent.Header.unresolvedFieldOffsetPayload
@@ -2368,13 +2381,9 @@ internal func _getKeyPathClassAndInstanceSizeFromPattern(
23682381
// Measure the instantiated size of the external component.
23692382
let newComponentSize: Int
23702383
switch descriptorHeader.kind {
2371-
case .class:
2372-
// A stored class property is reference-writable.
2373-
// TODO: we should dynamically prevent "let" properties from being
2374-
// reassigned (in both the struct and class cases).
2375-
capability = .reference
2376-
fallthrough
2377-
case .struct:
2384+
case .class, .struct:
2385+
setStoredCapability(for: descriptorHeader)
2386+
23782387
// Discard the local candidate.
23792388
_ = buffer.popRaw(size: localCandidateSize,
23802389
alignment: Int32.self)
@@ -2769,9 +2778,11 @@ internal func _instantiateKeyPathBuffer(
27692778
tryToResolveOffset(header: header,
27702779
getOutOfLineOffset: { patternBuffer.pop(UInt32.self) })
27712780
case .class:
2772-
// Crossing a class can end the reference prefix, and makes the following
2773-
// key path potentially reference-writable.
2774-
endOfReferencePrefixComponent = previousComponentAddr
2781+
// Accessing a mutable class property can end the reference prefix, and
2782+
// makes the following key path potentially reference-writable.
2783+
if header.isStoredMutable {
2784+
endOfReferencePrefixComponent = previousComponentAddr
2785+
}
27752786
// The offset may need to be resolved dynamically.
27762787
tryToResolveOffset(header: header,
27772788
getOutOfLineOffset: { patternBuffer.pop(UInt32.self) })
@@ -2821,9 +2832,11 @@ internal func _instantiateKeyPathBuffer(
28212832
// descriptor.
28222833
switch descriptorHeader.kind {
28232834
case .class:
2824-
// Crossing a class can end the reference prefix, and makes the following
2825-
// key path potentially reference-writable.
2826-
endOfReferencePrefixComponent = previousComponentAddr
2835+
// Accessing a mutable class property can end the reference prefix,
2836+
// and makes the following key path potentially reference-writable.
2837+
if descriptorHeader.isStoredMutable {
2838+
endOfReferencePrefixComponent = previousComponentAddr
2839+
}
28272840
fallthrough
28282841

28292842
case .struct:

‎test/IRGen/keypaths.sil

+23-23
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ sil_vtable C {}
4545
// -- instantiable in-line, size 4
4646
// CHECK-SAME: <i32 0x8000_0004>,
4747
// CHECK-64-SAME: [4 x i8] zeroinitializer,
48-
// -- offset of S.x
49-
// CHECK-SAME: <i32 0x0100_0000> }>
48+
// -- offset of S.x, mutable
49+
// CHECK-SAME: <i32 0x0180_0000> }>
5050

5151
// -- %b: S.y
5252
// CHECK: [[KP_B:@keypath.*]] = private global <{ {{.*}} }> <{
@@ -56,7 +56,7 @@ sil_vtable C {}
5656
// -- instantiable in-line, size 4
5757
// CHECK-SAME: <i32 0x8000_0004>,
5858
// CHECK-64-SAME: [4 x i8] zeroinitializer,
59-
// -- offset of S.y
59+
// -- offset of S.y, immutable
6060
// CHECK-32-SAME: <i32 0x0100_0004> }>
6161
// CHECK-64-SAME: <i32 0x0100_0008> }>
6262

@@ -68,9 +68,9 @@ sil_vtable C {}
6868
// -- instantiable in-line, size 4
6969
// CHECK-SAME: <i32 0x8000_0004>,
7070
// CHECK-64-SAME: [4 x i8] zeroinitializer,
71-
// -- offset of S.z
72-
// CHECK-32-SAME: <i32 0x0100_0010> }>
73-
// CHECK-64-SAME: <i32 0x0100_0018> }>
71+
// -- offset of S.z, mutable
72+
// CHECK-32-SAME: <i32 0x0180_0010> }>
73+
// CHECK-64-SAME: <i32 0x0180_0018> }>
7474

7575
// -- %d: C.x
7676
// CHECK: [[KP_D:@keypath.*]] = private global <{ {{.*}} }> <{
@@ -80,9 +80,9 @@ sil_vtable C {}
8080
// -- instantiable in-line, size 4
8181
// CHECK-SAME: <i32 0x8000_0004>,
8282
// CHECK-64-SAME: [4 x i8] zeroinitializer,
83-
// -- 0x0300_0000 (class) + offset of C.x
84-
// CHECK-32-SAME: <i32 0x0300_0008> }>
85-
// CHECK-64-SAME: <i32 0x0300_0010> }>
83+
// -- 0x0300_0000 (class) + mutable + offset of C.x
84+
// CHECK-32-SAME: <i32 0x0380_0008> }>
85+
// CHECK-64-SAME: <i32 0x0380_0010> }>
8686

8787
// -- %e: C.y
8888
// CHECK: [[KP_E:@keypath.*]] = private global <{ {{.*}} }> <{
@@ -92,7 +92,7 @@ sil_vtable C {}
9292
// -- instantiable in-line, size 4
9393
// CHECK-SAME: <i32 0x8000_0004>,
9494
// CHECK-64-SAME: [4 x i8] zeroinitializer,
95-
// -- 0x0300_0000 (class) + offset of C.y
95+
// -- 0x0300_0000 (class) + immutable + offset of C.y
9696
// CHECK-32-SAME: <i32 0x0300_000c> }>
9797
// CHECK-64-SAME: <i32 0x0300_0018> }>
9898

@@ -104,9 +104,9 @@ sil_vtable C {}
104104
// -- instantiable in-line, size 4
105105
// CHECK-SAME: <i32 0x8000_0004>,
106106
// CHECK-64-SAME: [4 x i8] zeroinitializer,
107-
// -- 0x0300_0000 (class) + offset of C.z
108-
// CHECK-32-SAME: <i32 0x0300_0018> }>
109-
// CHECK-64-SAME: <i32 0x0300_0028> }>
107+
// -- 0x0300_0000 (class) + mutable offset of C.z
108+
// CHECK-32-SAME: <i32 0x0380_0018> }>
109+
// CHECK-64-SAME: <i32 0x0380_0028> }>
110110

111111
// -- %g: S.z.x
112112
// CHECK: [[KP_G:@keypath.*]] = private global <{ {{.*}} }> <{
@@ -119,13 +119,13 @@ sil_vtable C {}
119119
// CHECK-64-SAME: <i32 0x8000_0014>,
120120
// CHECK-64-SAME: [4 x i8] zeroinitializer,
121121
// -- offset of S.z
122-
// CHECK-32-SAME: <i32 0x0100_0010>,
123-
// CHECK-64-SAME: <i32 0x0100_0018>,
122+
// CHECK-32-SAME: <i32 0x0180_0010>,
123+
// CHECK-64-SAME: <i32 0x0180_0018>,
124124
// CHECK-64-SAME: [4 x i8] zeroinitializer,
125125
// CHECK: %swift.type* (i8*)*
126126
// -- 0x0300_0000 (class) + offset of C.x
127-
// CHECK-32-SAME: <i32 0x0300_0008> }>
128-
// CHECK-64-SAME: <i32 0x0300_0010> }>
127+
// CHECK-32-SAME: <i32 0x0380_0008> }>
128+
// CHECK-64-SAME: <i32 0x0380_0010> }>
129129

130130
// -- %h: C.z.x
131131
// CHECK: [[KP_H:@keypath.*]] = private global <{ {{.*}} }> <{
@@ -138,12 +138,12 @@ sil_vtable C {}
138138
// CHECK-64-SAME: <i32 0x8000_0014>,
139139
// CHECK-64-SAME: [4 x i8] zeroinitializer,
140140
// -- 0x0300_0000 (class) + offset of C.z
141-
// CHECK-32-SAME: <i32 0x0300_0018>,
142-
// CHECK-64-SAME: <i32 0x0300_0028>,
141+
// CHECK-32-SAME: <i32 0x0380_0018>,
142+
// CHECK-64-SAME: <i32 0x0380_0028>,
143143
// CHECK-64-SAME: [4 x i8] zeroinitializer,
144144
// CHECK: %swift.type* (i8*)*
145145
// -- offset of S.x
146-
// CHECK-SAME: <i32 0x0100_0000> }>
146+
// CHECK-SAME: <i32 0x0180_0000> }>
147147

148148
// -- %k: computed
149149
// CHECK: [[KP_K:@keypath.*]] = private global <{ {{.*}} }> <{
@@ -204,8 +204,8 @@ sil_vtable C {}
204204
// -- size 8
205205
// CHECK-SAME: i32 8,
206206
// CHECK-64-SAME: [4 x i8] zeroinitializer,
207-
// -- struct with runtime-resolved offset
208-
// CHECK-SAME: <i32 0x017ffffe>,
207+
// -- struct with runtime-resolved offset, mutable
208+
// CHECK-SAME: <i32 0x01fffffe>,
209209
// CHECK-32-SAME: i32 16 }>
210210
// CHECK-64-SAME: i32 32 }>
211211

@@ -218,7 +218,7 @@ sil_vtable C {}
218218
// CHECK-SAME: i32 8,
219219
// CHECK-64-SAME: [4 x i8] zeroinitializer,
220220
// -- struct with runtime-resolved offset
221-
// CHECK-SAME: <i32 0x017ffffe>,
221+
// CHECK-SAME: <i32 0x01fffffe>,
222222
// CHECK-32-SAME: i32 20 }>
223223
// CHECK-64-SAME: i32 36 }>
224224

‎test/IRGen/keypaths_objc.sil

+3-3
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ sil_vtable C {}
1818
sil @x_get : $@convention(thin) (@in_guaranteed C) -> @out NSString
1919

2020
// CHECK: [[KEYPATH_A:@keypath.*]] = private global
21-
// -- 0x0200_0002: computed, get-only, indirect identifier
21+
// -- computed, get-only, indirect identifier
2222
// CHECK-SAME: <i32 0x0200_0002>,
2323
// CHECK-SAME: i8** @"\01L_selector(x)"
2424

2525
// CHECK: [[KEYPATH_B:@keypath.*]] = private global
26-
// -- 0x037ffffd: class stored property with indirect offset
27-
// CHECK-SAME: <i32 0x037ffffd>,
26+
// -- class mutable stored property with indirect offset
27+
// CHECK-SAME: <i32 0x03fffffd>,
2828
// CHECK-SAME: @"$S13keypaths_objc1CC6storedSivpWvd"
2929

3030
// CHECK-LABEL: define swiftcc void @objc_only_property()

‎test/IRGen/property_descriptor.sil

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,12 @@ public struct ExternalReabstractions<T> {
4242
// -- struct property, offset resolved from field offset vector in metadata
4343
// CHECK-64: @"$S19property_descriptor15ExternalGenericV2roxvpMV" =
4444
// CHECK-32: @"$S19property_descriptor15ExternalGenericV2roxvpMV" =
45-
// CHECK-SAME: <{ <i32 0x017f_fffe>,
45+
// CHECK-SAME: <{ <i32 0x01ff_fffe>,
4646
sil_property #ExternalGeneric.ro <T: Comparable> (
4747
stored_property #ExternalGeneric.ro : $T)
4848
// CHECK-64: @"$S19property_descriptor15ExternalGenericV2rwxvpMV" =
4949
// CHECK-32: @"$S19property_descriptor15ExternalGenericV2rwxvpMV" =
50-
// CHECK-SAME: <{ <i32 0x017f_fffe>,
50+
// CHECK-SAME: <{ <i32 0x01ff_fffe>,
5151
sil_property #ExternalGeneric.rw <T: Comparable> (
5252
stored_property #ExternalGeneric.rw : $T)
5353

‎test/stdlib/Inputs/KeyPathMultiModule_b.swift

+11
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ public struct A {
3535
get { return 0 }
3636
set { }
3737
}
38+
39+
public let immutable: Int = 1738
3840
}
3941

4042
public struct B<U> {
@@ -88,6 +90,10 @@ public func A_z_keypath() -> KeyPath<A, Int> {
8890
return \A.z
8991
}
9092

93+
public func A_immutable_keypath() -> KeyPath<A, Int> {
94+
return \A.immutable
95+
}
96+
9197
public func A_subscript_withGeneric_keypath<T: Hashable>(index: T)
9298
-> KeyPath<A, T> {
9399
return \A.[withGeneric: index]
@@ -197,6 +203,8 @@ open class ResilientRoot {
197203
open var storedA = "a"
198204
open var storedB = "b"
199205

206+
public let storedLet = "c"
207+
200208
open var virtual: String {
201209
get { return "foo" }
202210
set { }
@@ -237,6 +245,9 @@ public func ResilientRoot_storedA_keypath() -> KeyPath<ResilientRoot, String> {
237245
public func ResilientRoot_storedB_keypath() -> KeyPath<ResilientRoot, String> {
238246
return \ResilientRoot.storedB
239247
}
248+
public func ResilientRoot_storedLet_keypath() -> KeyPath<ResilientRoot, String> {
249+
return \ResilientRoot.storedLet
250+
}
240251
public func ResilientRoot_virtual_keypath() -> KeyPath<ResilientRoot, String> {
241252
return \ResilientRoot.virtual
242253
}

0 commit comments

Comments
 (0)