Skip to content

Commit ba968d4

Browse files
authored
[stdlib] Fix removeLast(_:) performance for non-random-access collections (#32599)
This replaces the `count` comparison precondition with a limited index offset, which converts the method from O(n) to O(k).
1 parent 8e886ab commit ba968d4

File tree

2 files changed

+30
-10
lines changed

2 files changed

+30
-10
lines changed

stdlib/public/core/BidirectionalCollection.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -342,9 +342,12 @@ extension BidirectionalCollection where SubSequence == Self {
342342
public mutating func removeLast(_ k: Int) {
343343
if k == 0 { return }
344344
_precondition(k >= 0, "Number of elements to remove should be non-negative")
345-
_precondition(count >= k,
346-
"Can't remove more items from a collection than it contains")
347-
self = self[startIndex..<index(endIndex, offsetBy: -k)]
345+
guard let end = index(endIndex, offsetBy: -k, limitedBy: startIndex)
346+
else {
347+
_preconditionFailure(
348+
"Can't remove more items from a collection than it contains")
349+
}
350+
self = self[startIndex..<end]
348351
}
349352
}
350353

stdlib/public/core/RangeReplaceableCollection.swift

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -285,12 +285,17 @@ public protocol RangeReplaceableCollection: Collection
285285
/// Customization point for `removeLast()`. Implement this function if you
286286
/// want to replace the default implementation.
287287
///
288+
/// The collection must not be empty.
289+
///
288290
/// - Returns: A non-nil value if the operation was performed.
289291
mutating func _customRemoveLast() -> Element?
290292

291293
/// Customization point for `removeLast(_:)`. Implement this function if you
292294
/// want to replace the default implementation.
293295
///
296+
/// - Parameter n: The number of elements to remove from the collection.
297+
/// `n` must be greater than or equal to zero and must not exceed the
298+
/// number of elements in the collection.
294299
/// - Returns: `true` if the operation was performed.
295300
mutating func _customRemoveLast(_ n: Int) -> Bool
296301

@@ -802,7 +807,12 @@ extension RangeReplaceableCollection
802807

803808
@inlinable
804809
public mutating func _customRemoveLast(_ n: Int) -> Bool {
805-
self = self[startIndex..<index(endIndex, offsetBy: -n)]
810+
guard let end = index(endIndex, offsetBy: -n, limitedBy: startIndex)
811+
else {
812+
_preconditionFailure(
813+
"Can't remove more items from a collection than it contains")
814+
}
815+
self = self[startIndex..<end]
806816
return true
807817
}
808818
}
@@ -866,13 +876,17 @@ extension RangeReplaceableCollection where Self: BidirectionalCollection {
866876
public mutating func removeLast(_ k: Int) {
867877
if k == 0 { return }
868878
_precondition(k >= 0, "Number of elements to remove should be non-negative")
869-
_precondition(count >= k,
870-
"Can't remove more items from a collection than it contains")
871879
if _customRemoveLast(k) {
872880
return
873881
}
874882
let end = endIndex
875-
removeSubrange(index(end, offsetBy: -k)..<end)
883+
guard let start = index(end, offsetBy: -k, limitedBy: startIndex)
884+
else {
885+
_preconditionFailure(
886+
"Can't remove more items from a collection than it contains")
887+
}
888+
889+
removeSubrange(start..<end)
876890
}
877891
}
878892

@@ -936,13 +950,16 @@ where Self: BidirectionalCollection, SubSequence == Self {
936950
public mutating func removeLast(_ k: Int) {
937951
if k == 0 { return }
938952
_precondition(k >= 0, "Number of elements to remove should be non-negative")
939-
_precondition(count >= k,
940-
"Can't remove more items from a collection than it contains")
941953
if _customRemoveLast(k) {
942954
return
943955
}
944956
let end = endIndex
945-
removeSubrange(index(end, offsetBy: -k)..<end)
957+
guard let start = index(end, offsetBy: -k, limitedBy: startIndex)
958+
else {
959+
_preconditionFailure(
960+
"Can't remove more items from a collection than it contains")
961+
}
962+
removeSubrange(start..<end)
946963
}
947964
}
948965

0 commit comments

Comments
 (0)