Skip to content

Commit 23778cb

Browse files
committed
[WeakLookupTable] Fix precondition for maximum capacity
Also, added comments for justifying every unsafe '&+' and '&*' operation.
1 parent 8413472 commit 23778cb

File tree

1 file changed

+16
-2
lines changed

1 file changed

+16
-2
lines changed

Sources/SwiftSyntax/WeakLookupTable.swift

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,23 +76,28 @@ class WeakLookupTable<Element: Identifiable & AnyObject> {
7676
/// '_minimalBucketCount(for: capacity)'
7777
private static func _bucketCount(for capacity: Int,
7878
from current: Int = 2) -> Int {
79-
// Make sure it's representable.
80-
precondition(capacity <= (Int.max >> 1) + 1)
8179
// Bucket count must always be power of 2.
8280
precondition((current & (current - 1)) == 0)
8381
// Minimum is 2 to guarantee at least 1 hole.
8482
precondition(current >= 2)
8583

8684
let minimalBucketCount = _minimalBucketCount(for: capacity)
85+
86+
// Make sure it's representable. If 'minimalBucketCount' here is over
87+
// 0x4000_..., the bucket count must be 0x8000_... thus overflows.
88+
precondition(minimalBucketCount <= (Int.max >> 1) + 1)
89+
8790
var bucketCount = current
8891
while bucketCount < minimalBucketCount {
92+
// '&*=' for performance. Guaranteed by above 'precondition()'.
8993
bucketCount &*= 2
9094
}
9195
return bucketCount
9296
}
9397

9498
private var _bucketMask: Int {
9599
@inline(__always) get {
100+
// '&-' for performance. We know 'bucketCount >= 2'.
96101
return bucketCount &- 1
97102
}
98103
}
@@ -116,6 +121,7 @@ class WeakLookupTable<Element: Identifiable & AnyObject> {
116121
if obj.id == id {
117122
return (bucket, true)
118123
}
124+
// '&+' for performance. 'bucketCount' is 0x4000_... or below.
119125
bucket = (bucket &+ 1) & _bucketMask
120126
}
121127
}
@@ -157,6 +163,7 @@ class WeakLookupTable<Element: Identifiable & AnyObject> {
157163
private func _countOccupiedBuckets() -> Int {
158164
var count = 0
159165
for i in 0 ..< bucketCount where buckets[i].value != nil {
166+
// '&+=' for performance. 'bucketCount' is 0x4000_... or below.
160167
count &+= 1
161168
}
162169
return count
@@ -165,13 +172,18 @@ class WeakLookupTable<Element: Identifiable & AnyObject> {
165172
/// Reserves enough space to store a single new object. Returns true if
166173
/// resizing happened.
167174
private func _ensurePlusOneCapacity() -> Bool {
175+
// '&+' for performance. 'estimatedCount' is always less than 'bucketCount'
176+
// which is 0x4000_... or below.
168177
if bucketCount >= WeakLookupTable<Element>
169178
._minimalBucketCount(for: estimatedCount &+ 1) {
170179
return false
171180
}
172181

173182
// Slow path.
174183
estimatedCount = _countOccupiedBuckets()
184+
// '&+' for performance. We know 'estimatedCount' derived by
185+
// '_countOccupiedBuckets()' is equal to or less than previous
186+
// 'estimatedCount'.
175187
return reserveCapacity(estimatedCount &+ 1)
176188
}
177189

@@ -187,6 +199,7 @@ class WeakLookupTable<Element: Identifiable & AnyObject> {
187199
pos = _findHole(obj.id).pos
188200
}
189201
buckets[pos].value = obj
202+
// '&+=' for performance. '_ensurePlusOneCapacity()' ensures it's safe.
190203
estimatedCount &+= 1
191204
return true
192205
}
@@ -205,6 +218,7 @@ class WeakLookupTable<Element: Identifiable & AnyObject> {
205218
if let obj = buckets[bucket].value, obj.id == id {
206219
return obj
207220
}
221+
// '&+' for performance. 'bucketCount' is 0x4000_... or below.
208222
bucket = (bucket &+ 1) & _bucketMask
209223
} while bucket != idealBucket
210224

0 commit comments

Comments
 (0)