Skip to content

Commit a354a29

Browse files
committed
Use PhantomData in InsertionHole.
1 parent 907e585 commit a354a29

File tree

3 files changed

+30
-57
lines changed

3 files changed

+30
-57
lines changed

src/insertion_sort.rs

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,33 @@
22
//!
33
//! [`core::slice::sort`]: https://doc.rust-lang.org/src/core/slice/sort.rs.html
44
5-
use core::{mem, ptr};
5+
use core::{marker::PhantomData, mem, ptr};
66
use ndarray::{s, ArrayViewMut1, IndexLonger};
77

88
// When dropped, copies from `src` into `dest`.
9-
struct InsertionHole<T> {
10-
src: *const T,
11-
dest: *mut T,
9+
#[must_use]
10+
pub(super) struct InsertionHole<'a, T> {
11+
pub(super) src: *const T,
12+
pub(super) dest: *mut T,
13+
/// `src` is often a local pointer here, make sure we have appropriate
14+
/// PhantomData so that dropck can protect us.
15+
marker: PhantomData<&'a mut T>,
1216
}
1317

14-
impl<T> Drop for InsertionHole<T> {
18+
impl<'a, T> InsertionHole<'a, T> {
19+
/// Construct from a source pointer and a destination
20+
/// Assumes dest lives longer than src, since there is no easy way to
21+
/// copy down lifetime information from another pointer
22+
pub(super) unsafe fn new(src: &'a T, dest: *mut T) -> Self {
23+
Self {
24+
src,
25+
dest,
26+
marker: PhantomData,
27+
}
28+
}
29+
}
30+
31+
impl<T> Drop for InsertionHole<'_, T> {
1532
fn drop(&mut self) {
1633
// SAFETY: This is a helper class. Please refer to its usage for correctness. Namely, one
1734
// must be sure that `src` and `dst` does not overlap as required by
@@ -58,10 +75,7 @@ where
5875
// If `is_less` panics at any point during the process, `hole` will get dropped and
5976
// fill the hole in `v` with `tmp`, thus ensuring that `v` still holds every object it
6077
// initially held exactly once.
61-
let mut hole = InsertionHole {
62-
src: &*tmp,
63-
dest: v.view_mut().uget(i - 1),
64-
};
78+
let mut hole = InsertionHole::new(&*tmp, v.view_mut().uget(i - 1));
6579
ptr::copy_nonoverlapping(hole.dest, v.view_mut().uget(i), 1);
6680

6781
// SAFETY: We know i is at least 1.
@@ -126,7 +140,7 @@ where
126140
// fill the hole in `v` with `tmp`, thus ensuring that `v` still holds every object it
127141
// initially held exactly once.
128142
let dest = v.view_mut().uget(1);
129-
let mut hole = InsertionHole { src: &*tmp, dest };
143+
let mut hole = InsertionHole::new(&*tmp, dest);
130144
ptr::copy_nonoverlapping(dest, v.view_mut().uget(0), 1);
131145

132146
for i in 2..v.len() {

src/par/insertion_sort.rs

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,26 +2,10 @@
22
//!
33
//! [`core::slice::sort`]: https://doc.rust-lang.org/src/core/slice/sort.rs.html
44
5+
use crate::insertion_sort::InsertionHole;
56
use core::{mem, ptr};
67
use ndarray::{s, ArrayViewMut1, IndexLonger};
78

8-
// When dropped, copies from `src` into `dest`.
9-
struct InsertionHole<T> {
10-
src: *const T,
11-
dest: *mut T,
12-
}
13-
14-
impl<T> Drop for InsertionHole<T> {
15-
fn drop(&mut self) {
16-
// SAFETY: This is a helper class. Please refer to its usage for correctness. Namely, one
17-
// must be sure that `src` and `dst` does not overlap as required by
18-
// `ptr::copy_nonoverlapping` and are both valid for writes.
19-
unsafe {
20-
ptr::copy_nonoverlapping(self.src, self.dest, 1);
21-
}
22-
}
23-
}
24-
259
/// Inserts `v[v.len() - 1]` into pre-sorted sequence `v[..v.len() - 1]` so that whole `v[..]`
2610
/// becomes sorted.
2711
unsafe fn insert_tail<T, F>(mut v: ArrayViewMut1<'_, T>, is_less: &F)
@@ -58,10 +42,7 @@ where
5842
// If `is_less` panics at any point during the process, `hole` will get dropped and
5943
// fill the hole in `v` with `tmp`, thus ensuring that `v` still holds every object it
6044
// initially held exactly once.
61-
let mut hole = InsertionHole {
62-
src: &*tmp,
63-
dest: v.view_mut().uget(i - 1),
64-
};
45+
let mut hole = InsertionHole::new(&*tmp, v.view_mut().uget(i - 1));
6546
ptr::copy_nonoverlapping(hole.dest, v.view_mut().uget(i), 1);
6647

6748
// SAFETY: We know i is at least 1.
@@ -126,7 +107,7 @@ where
126107
// fill the hole in `v` with `tmp`, thus ensuring that `v` still holds every object it
127108
// initially held exactly once.
128109
let dest = v.view_mut().uget(1);
129-
let mut hole = InsertionHole { src: &*tmp, dest };
110+
let mut hole = InsertionHole::new(&*tmp, dest);
130111
ptr::copy_nonoverlapping(dest, v.view_mut().uget(0), 1);
131112

132113
for i in 2..v.len() {

src/par/partition.rs

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
//! [`core::slice::sort`]: https://doc.rust-lang.org/src/core/slice/sort.rs.html
44
55
use crate::{
6+
insertion_sort::InsertionHole,
67
par::insertion_sort::insertion_sort_shift_left,
78
partition::{break_patterns, reverse},
89
};
@@ -361,10 +362,7 @@ where
361362
// operation panics, the pivot will be automatically written back into the slice.
362363
// SAFETY: The pointer here is valid because it is obtained from a reference to a slice.
363364
let tmp = ManuallyDrop::new(unsafe { ptr::read(pivot) });
364-
let _pivot_guard = CopyOnDrop {
365-
src: &*tmp,
366-
dest: pivot,
367-
};
365+
let _pivot_guard = unsafe { InsertionHole::new(&*tmp, pivot) };
368366
let pivot = &*tmp;
369367

370368
// Now partition the slice.
@@ -427,10 +425,7 @@ where
427425

428426
// SAFETY: `pivot` is a reference to the first element of `v`, so `ptr::read` is safe.
429427
let tmp = ManuallyDrop::new(unsafe { ptr::read(pivot) });
430-
let _pivot_guard = CopyOnDrop {
431-
src: &*tmp,
432-
dest: pivot,
433-
};
428+
let _pivot_guard = unsafe { InsertionHole::new(&*tmp, pivot) };
434429
let pivot = &*tmp;
435430

436431
// Find the first pair of out-of-order elements.
@@ -830,23 +825,6 @@ where
830825
}
831826
}
832827

833-
/// When dropped, copies from `src` into `dest`.
834-
pub struct CopyOnDrop<T> {
835-
pub src: *const T,
836-
pub dest: *mut T,
837-
}
838-
839-
impl<T> Drop for CopyOnDrop<T> {
840-
fn drop(&mut self) {
841-
// SAFETY: This is a helper class.
842-
// Please refer to its usage for correctness.
843-
// Namely, one must be sure that `src` and `dst` does not overlap as required by `ptr::copy_nonoverlapping`.
844-
unsafe {
845-
ptr::copy_nonoverlapping(self.src, self.dest, 1);
846-
}
847-
}
848-
}
849-
850828
#[cfg(test)]
851829
mod test {
852830
use super::{par_partition_at_indices, partition_at_index};

0 commit comments

Comments
 (0)