Skip to content

Commit

Permalink
Rollup merge of rust-lang#88412 - mdsn:slice-sort-safety, r=dtolnay
Browse files Browse the repository at this point in the history
Remove ignore-tidy-undocumented-unsafe from core::slice::sort

Write down the missing safety arguments to be able to remove `ignore-tidy-undocumented-unsafe` from `core::slice::sort`.

Helps with rust-lang#66219

`@rustbot` label C-cleanup T-libs
  • Loading branch information
GuillaumeGomez authored Sep 29, 2021
2 parents 8dfbfe8 + 18fb97e commit 024664d
Showing 1 changed file with 23 additions and 2 deletions.
25 changes: 23 additions & 2 deletions library/core/src/slice/sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
//! Unstable sorting is compatible with libcore because it doesn't allocate memory, unlike our
//! stable sorting implementation.

// ignore-tidy-undocumented-unsafe

use crate::cmp;
use crate::mem::{self, MaybeUninit};
use crate::ptr;
Expand Down Expand Up @@ -291,6 +289,9 @@ where
} else if start_r < end_r {
block_l = rem;
} else {
// There were the same number of elements to switch on both blocks during the last
// iteration, so there are no remaining elements on either block. Cover the remaining
// items with roughly equally-sized blocks.
block_l = rem / 2;
block_r = rem - block_l;
}
Expand Down Expand Up @@ -437,6 +438,17 @@ where
// Move its remaining out-of-order elements to the far right.
debug_assert_eq!(width(l, r), block_l);
while start_l < end_l {
// remaining-elements-safety
// SAFETY: while the loop condition holds there are still elements in `offsets_l`, so it
// is safe to point `end_l` to the previous element.
//
// The `ptr::swap` is safe if both its arguments are valid for reads and writes:
// - Per the debug assert above, the distance between `l` and `r` is `block_l`
// elements, so there can be at most `block_l` remaining offsets between `start_l`
// and `end_l`. This means `r` will be moved at most `block_l` steps back, which
// makes the `r.offset` calls valid (at that point `l == r`).
// - `offsets_l` contains valid offsets into `v` collected during the partitioning of
// the last block, so the `l.offset` calls are valid.
unsafe {
end_l = end_l.offset(-1);
ptr::swap(l.offset(*end_l as isize), r.offset(-1));
Expand All @@ -449,6 +461,7 @@ where
// Move its remaining out-of-order elements to the far left.
debug_assert_eq!(width(l, r), block_r);
while start_r < end_r {
// SAFETY: See the reasoning in [remaining-elements-safety].
unsafe {
end_r = end_r.offset(-1);
ptr::swap(l, r.offset(-(*end_r as isize) - 1));
Expand Down Expand Up @@ -481,6 +494,8 @@ where

// Read the pivot into a stack-allocated variable for efficiency. If a following comparison
// operation panics, the pivot will be automatically written back into the slice.

// SAFETY: `pivot` is a reference to the first element of `v`, so `ptr::read` is safe.
let mut tmp = mem::ManuallyDrop::new(unsafe { ptr::read(pivot) });
let _pivot_guard = CopyOnDrop { src: &mut *tmp, dest: pivot };
let pivot = &*tmp;
Expand Down Expand Up @@ -646,6 +661,12 @@ where

if len >= 8 {
// Swaps indices so that `v[a] <= v[b]`.
// SAFETY: `len >= 8` so there are at least two elements in the neighborhoods of
// `a`, `b` and `c`. This means the three calls to `sort_adjacent` result in
// corresponding calls to `sort3` with valid 3-item neighborhoods around each
// pointer, which in turn means the calls to `sort2` are done with valid
// references. Thus the `v.get_unchecked` calls are safe, as is the `ptr::swap`
// call.
let mut sort2 = |a: &mut usize, b: &mut usize| unsafe {
if is_less(v.get_unchecked(*b), v.get_unchecked(*a)) {
ptr::swap(a, b);
Expand Down

0 comments on commit 024664d

Please sign in to comment.