Skip to content

Commit a2681f9

Browse files
authored
Rollup merge of #144314 - kornelski:pivot-safely, r=jhpratt
Hint that choose_pivot returns index in bounds Instead of using `unsafe` in multiple places, one `hint::assert_unchecked` allows use of safe code instead. Part of ##144326
2 parents f414e7a + 01fdafc commit a2681f9

File tree

4 files changed

+10
-10
lines changed

4 files changed

+10
-10
lines changed

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,7 @@ fn partition_at_index_loop<'a, T, F>(
101101
// slice. Partition the slice into elements equal to and elements greater than the pivot.
102102
// This case is usually hit when the slice contains many duplicate elements.
103103
if let Some(p) = ancestor_pivot {
104-
// SAFETY: choose_pivot promises to return a valid pivot position.
105-
let pivot = unsafe { v.get_unchecked(pivot_pos) };
104+
let pivot = &v[pivot_pos];
106105

107106
if !is_less(p, pivot) {
108107
let num_lt = partition(v, pivot_pos, &mut |a, b| !is_less(b, a));

library/core/src/slice/sort/shared/pivot.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! This module contains the logic for pivot selection.
22
3-
use crate::intrinsics;
3+
use crate::{hint, intrinsics};
44

55
// Recursively select a pseudomedian if above this threshold.
66
const PSEUDO_MEDIAN_REC_THRESHOLD: usize = 64;
@@ -9,6 +9,7 @@ const PSEUDO_MEDIAN_REC_THRESHOLD: usize = 64;
99
///
1010
/// This chooses a pivot by sampling an adaptive amount of points, approximating
1111
/// the quality of a median of sqrt(n) elements.
12+
#[inline]
1213
pub fn choose_pivot<T, F: FnMut(&T, &T) -> bool>(v: &[T], is_less: &mut F) -> usize {
1314
// We use unsafe code and raw pointers here because we're dealing with
1415
// heavy recursion. Passing safe slices around would involve a lot of
@@ -22,7 +23,7 @@ pub fn choose_pivot<T, F: FnMut(&T, &T) -> bool>(v: &[T], is_less: &mut F) -> us
2223
// SAFETY: a, b, c point to initialized regions of len_div_8 elements,
2324
// satisfying median3 and median3_rec's preconditions as v_base points
2425
// to an initialized region of n = len elements.
25-
unsafe {
26+
let index = unsafe {
2627
let v_base = v.as_ptr();
2728
let len_div_8 = len / 8;
2829

@@ -35,6 +36,11 @@ pub fn choose_pivot<T, F: FnMut(&T, &T) -> bool>(v: &[T], is_less: &mut F) -> us
3536
} else {
3637
median3_rec(a, b, c, len_div_8, is_less).offset_from_unsigned(v_base)
3738
}
39+
};
40+
// SAFETY: preconditions must have been met for offset_from_unsigned()
41+
unsafe {
42+
hint::assert_unchecked(index < v.len());
43+
index
3844
}
3945
}
4046

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,6 @@ pub fn quicksort<T, F: FnMut(&T, &T) -> bool>(
3737
limit -= 1;
3838

3939
let pivot_pos = choose_pivot(v, is_less);
40-
// SAFETY: choose_pivot promises to return a valid pivot index.
41-
unsafe {
42-
intrinsics::assume(pivot_pos < v.len());
43-
}
4440

4541
// SAFETY: We only access the temporary copy for Freeze types, otherwise
4642
// self-modifications via `is_less` would not be observed and this would

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ pub(crate) fn quicksort<'a, T, F>(
4848
// slice. Partition the slice into elements equal to and elements greater than the pivot.
4949
// This case is usually hit when the slice contains many duplicate elements.
5050
if let Some(p) = ancestor_pivot {
51-
// SAFETY: We assume choose_pivot yields an in-bounds position.
52-
if !is_less(p, unsafe { v.get_unchecked(pivot_pos) }) {
51+
if !is_less(p, &v[pivot_pos]) {
5352
let num_lt = partition(v, pivot_pos, &mut |a, b| !is_less(b, a));
5453

5554
// Continue sorting elements greater than the pivot. We know that `num_lt` contains

0 commit comments

Comments
 (0)