Skip to content

Commit aa9e9ed

Browse files
committed
[Stdlib] Improves sort and sorted to accept throwing clousre
This commit resolves https://bugs.swift.org/browse/SR-715
1 parent 3019898 commit aa9e9ed

File tree

7 files changed

+324
-42
lines changed

7 files changed

+324
-42
lines changed

stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift.gyb

+149
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ public struct PartitionExhaustiveTest {
2626
}
2727
}
2828

29+
enum SillyError : Error { case JazzHands }
30+
2931
public let partitionExhaustiveTests = [
3032
PartitionExhaustiveTest([]),
3133
PartitionExhaustiveTest([ 10 ]),
@@ -48,6 +50,20 @@ public let partitionExhaustiveTests = [
4850
PartitionExhaustiveTest([ 10, 20, 30, 40, 50, 60 ]),
4951
]
5052

53+
//Random collection of 30 elements
54+
public let largeElementSortTests = [
55+
[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
56+
16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30],
57+
[30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 18,
58+
17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1],
59+
[30, 29, 28, 27, 26, 25, 20, 19, 18, 5, 4, 3, 2, 1,
60+
15, 14, 13, 12, 11, 10, 9, 24, 23, 22, 21, 8, 17, 16, 7, 6],
61+
[30, 29, 25, 20, 19, 18, 5, 4, 3, 2, 1, 28, 27, 26,
62+
15, 14, 13, 12, 24, 23, 22, 21, 8, 17, 16, 7, 6, 11, 10, 9],
63+
[3, 2, 1, 20, 19, 18, 5, 4, 28, 27, 26, 11, 10, 9,
64+
15, 14, 13, 12, 24, 23, 22, 21, 8, 17, 16, 7, 6, 30, 29, 25],
65+
]
66+
5167
public func withInvalidOrderings(_ body: (@escaping (Int, Int) -> Bool) -> Void) {
5268
// Test some ordering predicates that don't create strict weak orderings
5369
body { (_,_) in true }
@@ -463,6 +479,61 @@ self.test("\(testNamePrefix)._withUnsafeMutableBufferPointerIfSupported()/semant
463479
// sort()
464480
//===----------------------------------------------------------------------===//
465481

482+
func checkSortedPredicateThrow(
483+
sequence: [Int],
484+
lessImpl: ((Int, Int) -> Bool),
485+
throwIndex: Int
486+
) {
487+
let extract = extractValue
488+
let throwElement = sequence[throwIndex]
489+
var thrown = false
490+
let elements: [OpaqueValue<Int>] =
491+
zip(sequence, 0..<sequence.count).map {
492+
OpaqueValue($0, identity: $1)
493+
}
494+
var result: [C.Iterator.Element] = []
495+
let c = makeWrappedCollection(elements)
496+
let closureLifetimeTracker = LifetimeTracked(0)
497+
do {
498+
result = try c.sorted {
499+
(lhs, rhs) throws -> Bool in
500+
_blackHole(closureLifetimeTracker)
501+
if throwElement == extractValue(rhs).value {
502+
thrown = true
503+
throw SillyError.JazzHands
504+
}
505+
return lessImpl(extractValue(lhs).value, extractValue(rhs).value)
506+
}
507+
} catch {}
508+
509+
// Check that the original collection is unchanged.
510+
expectEqualSequence(
511+
elements.map { $0.value },
512+
c.map { extract($0).value })
513+
514+
// If `sorted` throws then result will be empty else
515+
// returned result must be sorted.
516+
if thrown {
517+
expectEqual(0, result.count)
518+
} else {
519+
// Check that the elements are sorted.
520+
let extractedResult = result.map(extract)
521+
for i in extractedResult.indices {
522+
if i != extractedResult.index(before: extractedResult.endIndex) {
523+
let first = extractedResult[i].value
524+
let second = extractedResult[extractedResult.index(after: i)].value
525+
let result = lessImpl(second, first)
526+
if result == true {
527+
print("yep ** Result should be true \(result)")
528+
} else {
529+
print("yep ** Test passed reult is false")
530+
}
531+
expectFalse(result)
532+
}
533+
}
534+
}
535+
}
536+
466537
% for predicate in [False, True]:
467538

468539
self.test("\(testNamePrefix).sorted/DispatchesThrough_withUnsafeMutableBufferPointerIfSupported/${'Predicate' if predicate else 'WhereElementIsComparable'}") {
@@ -587,6 +658,30 @@ self.test("\(testNamePrefix).sorted/${'Predicate' if predicate else 'WhereElemen
587658
}
588659
}
589660

661+
self.test("\(testNamePrefix).sorted/ThrowingPredicate") {
662+
for test in partitionExhaustiveTests {
663+
forAllPermutations(test.sequence) { (sequence) in
664+
for i in 0..<sequence.count {
665+
checkSortedPredicateThrow(
666+
sequence: sequence,
667+
lessImpl: { $0 < $1 },
668+
throwIndex: i)
669+
}
670+
}
671+
}
672+
}
673+
674+
self.test("\(testNamePrefix).sorted/ThrowingPredicateWithLargeNumberElememts") {
675+
for sequence in largeElementSortTests {
676+
for i in 0..<sequence.count {
677+
checkSortedPredicateThrow(
678+
sequence: sequence,
679+
lessImpl: { $0 < $1 },
680+
throwIndex: i)
681+
}
682+
}
683+
}
684+
590685
% end
591686

592687
//===----------------------------------------------------------------------===//
@@ -921,6 +1016,36 @@ self.test("\(testNamePrefix).partition/DispatchesThrough_withUnsafeMutableBuffer
9211016
// sort()
9221017
//===----------------------------------------------------------------------===//
9231018

1019+
func checkSortPredicateThrow(
1020+
sequence: [Int],
1021+
lessImpl: ((Int, Int) -> Bool),
1022+
throwIndex: Int
1023+
) {
1024+
let extract = extractValue
1025+
let throwElement = sequence[throwIndex]
1026+
let elements: [OpaqueValue<Int>] =
1027+
zip(sequence, 0..<sequence.count).map {
1028+
OpaqueValue($0, identity: $1)
1029+
}
1030+
var c = makeWrappedCollection(elements)
1031+
let closureLifetimeTracker = LifetimeTracked(0)
1032+
do {
1033+
try c.sort {
1034+
(lhs, rhs) throws -> Bool in
1035+
_blackHole(closureLifetimeTracker)
1036+
if throwElement == extractValue(rhs).value {
1037+
throw SillyError.JazzHands
1038+
}
1039+
return lessImpl(extractValue(lhs).value, extractValue(rhs).value)
1040+
}
1041+
} catch {}
1042+
1043+
//Check no element should lost and added
1044+
expectEqualsUnordered(
1045+
sequence,
1046+
c.map { extract($0).value })
1047+
}
1048+
9241049
% for predicate in [False, True]:
9251050

9261051
func checkSortInPlace_${'Predicate' if predicate else 'WhereElementIsComparable'}(
@@ -1005,6 +1130,30 @@ self.test("\(testNamePrefix).sort/${'Predicate' if predicate else 'WhereElementI
10051130
}
10061131
}
10071132

1133+
self.test("\(testNamePrefix).sort/ThrowingPredicate") {
1134+
for test in partitionExhaustiveTests {
1135+
forAllPermutations(test.sequence) { (sequence) in
1136+
for i in 0..<sequence.count {
1137+
checkSortPredicateThrow(
1138+
sequence: sequence,
1139+
lessImpl: { $0 < $1 },
1140+
throwIndex: i)
1141+
}
1142+
}
1143+
}
1144+
}
1145+
1146+
self.test("\(testNamePrefix).sort/ThrowingPredicateWithLargeNumberElements") {
1147+
for sequence in largeElementSortTests {
1148+
for i in 0..<sequence.count {
1149+
checkSortPredicateThrow(
1150+
sequence: sequence,
1151+
lessImpl: { $0 < $1 },
1152+
throwIndex: i)
1153+
}
1154+
}
1155+
}
1156+
10081157
% end
10091158

10101159
//===----------------------------------------------------------------------===//

stdlib/public/core/CollectionAlgorithms.swift.gyb

+11-8
Original file line numberDiff line numberDiff line change
@@ -339,10 +339,10 @@ ${orderingExplanation}
339339
@_inlineable
340340
public func sorted(
341341
by areInIncreasingOrder:
342-
(${IElement}, ${IElement}) -> Bool
343-
) -> [Iterator.Element] {
342+
(${IElement}, ${IElement}) throws -> Bool
343+
) rethrows -> [Iterator.Element] {
344344
var result = ContiguousArray(self)
345-
result.sort(by: areInIncreasingOrder)
345+
try result.sort(by: areInIncreasingOrder)
346346
return Array(result)
347347
}
348348
}
@@ -398,6 +398,9 @@ extension MutableCollection where Self : RandomAccessCollection {
398398
/// Sorts the collection in place, using the given predicate as the
399399
/// comparison between elements.
400400
///
401+
/// This method can take throwing closure. If closure throws error while sorting,
402+
/// order of elements may change. No elements will be lost.
403+
///
401404
/// When you want to sort a collection of elements that doesn't conform to
402405
/// the `Comparable` protocol, pass a closure to this method that returns
403406
/// `true` when the first element passed should be ordered before the
@@ -452,19 +455,19 @@ ${orderingExplanation}
452455
@_inlineable
453456
public mutating func sort(
454457
by areInIncreasingOrder:
455-
(${IElement}, ${IElement}) -> Bool
456-
) {
458+
(${IElement}, ${IElement}) throws -> Bool
459+
) rethrows {
457460

458461
let didSortUnsafeBuffer: Void? =
459-
_withUnsafeMutableBufferPointerIfSupported {
462+
try _withUnsafeMutableBufferPointerIfSupported {
460463
(baseAddress, count) -> Void in
461464
var bufferPointer =
462465
UnsafeMutableBufferPointer(start: baseAddress, count: count)
463-
bufferPointer.sort(by: areInIncreasingOrder)
466+
try bufferPointer.sort(by: areInIncreasingOrder)
464467
return ()
465468
}
466469
if didSortUnsafeBuffer == nil {
467-
_introSort(
470+
try _introSort(
468471
&self,
469472
subRange: startIndex..<endIndex,
470473
by: areInIncreasingOrder)

0 commit comments

Comments
 (0)