Skip to content

Commit ac2b421

Browse files
committed
Safer sort partition
1 parent d2acb42 commit ac2b421

File tree

2 files changed

+8
-21
lines changed

2 files changed

+8
-21
lines changed

library/core/src/slice/sort/select.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,9 @@ fn partition_at_index_loop<'a, T, F>(
126126

127127
// Split the slice into `left`, `pivot`, and `right`.
128128
let (left, right) = v.split_at_mut(mid);
129-
let (pivot, right) = right.split_at_mut(1);
130-
let pivot = &pivot[0];
131129

132130
if mid < index {
131+
let (pivot, right) = right.split_first_mut().unwrap();
133132
v = right;
134133
index = index - mid - 1;
135134
ancestor_pivot = Some(pivot);

library/core/src/slice/sort/unstable/quicksort.rs

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,6 @@ pub(crate) fn quicksort<'a, T, F>(
6161

6262
// Partition the slice.
6363
let num_lt = partition(v, pivot_pos, is_less);
64-
// SAFETY: partition ensures that `num_lt` will be in-bounds.
65-
unsafe { intrinsics::assume(num_lt < v.len()) };
6664

6765
// Split the slice into `left`, `pivot`, and `right`.
6866
let (left, right) = v.split_at_mut(num_lt);
@@ -84,6 +82,8 @@ pub(crate) fn quicksort<'a, T, F>(
8482
/// on the left side of `v` followed by the other elements, notionally considered greater or
8583
/// equal to `pivot`.
8684
///
85+
/// Aborts if the `pivot` or the returned value would be out of bounds of `v`.
86+
///
8787
/// Returns the number of elements that are compared true for `is_less(elem, pivot)`.
8888
///
8989
/// If `is_less` does not implement a total order the resulting order and return value are
@@ -97,27 +97,18 @@ where
9797
let len = v.len();
9898

9999
// Allows for panic-free code-gen by proving this property to the compiler.
100-
if len == 0 {
101-
return 0;
102-
}
103-
104-
if pivot >= len {
100+
if len == 0 || pivot >= len {
105101
intrinsics::abort();
106102
}
107103

108-
// SAFETY: We checked that `pivot` is in-bounds.
109-
unsafe {
110-
// Place the pivot at the beginning of slice.
111-
v.swap_unchecked(0, pivot);
112-
}
113-
let (pivot, v_without_pivot) = v.split_at_mut(1);
104+
v.swap(0, pivot);
105+
let (pivot, v_without_pivot) = v.split_first_mut().unwrap_or_else(|| intrinsics::abort());
114106

115107
// Assuming that Rust generates noalias LLVM IR we can be sure that a partition function
116108
// signature of the form `(v: &mut [T], pivot: &T)` guarantees that pivot and v can't alias.
117109
// Having this guarantee is crucial for optimizations. It's possible to copy the pivot value
118110
// into a stack value, but this creates issues for types with interior mutability mandating
119111
// a drop guard.
120-
let pivot = &mut pivot[0];
121112

122113
// This construct is used to limit the LLVM IR generated, which saves large amounts of
123114
// compile-time by only instantiating the code that is needed. Idea by Frank Steffahn.
@@ -127,11 +118,8 @@ where
127118
intrinsics::abort();
128119
}
129120

130-
// SAFETY: We checked that `num_lt` is in-bounds.
131-
unsafe {
132-
// Place the pivot between the two partitions.
133-
v.swap_unchecked(0, num_lt);
134-
}
121+
// Place the pivot between the two partitions.
122+
v.swap(0, num_lt);
135123

136124
num_lt
137125
}

0 commit comments

Comments
 (0)