Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ignore-tidy-undocumented-unsafe from core::slice::sort #88412

Merged
merged 1 commit into from
Sep 30, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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