Skip to content

Commit

Permalink
Move away from the ad-hoc NoDrop unions
Browse files Browse the repository at this point in the history
  • Loading branch information
nagisa committed Apr 9, 2017
1 parent f6e5661 commit 3871312
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 56 deletions.
1 change: 1 addition & 0 deletions src/libcollections/lib.rs
Expand Up @@ -44,6 +44,7 @@
#![feature(heap_api)]
#![feature(inclusive_range)]
#![feature(lang_items)]
#![feature(manually_drop)]
#![feature(nonzero)]
#![feature(pattern)]
#![feature(placement_in)]
Expand Down
12 changes: 3 additions & 9 deletions src/libcollections/slice.rs
Expand Up @@ -1558,7 +1558,7 @@ fn insert_head<T, F>(v: &mut [T], is_less: &mut F)
// performance than with the 2nd method.
//
// All methods were benchmarked, and the 3rd showed best results. So we chose that one.
let mut tmp = NoDrop { value: ptr::read(&v[0]) };
let mut tmp = mem::ManuallyDrop::new(ptr::read(&v[0]));

// Intermediate state of the insertion process is always tracked by `hole`, which
// serves two purposes:
Expand All @@ -1571,13 +1571,13 @@ fn insert_head<T, F>(v: &mut [T], is_less: &mut F)
// fill the hole in `v` with `tmp`, thus ensuring that `v` still holds every object it
// initially held exactly once.
let mut hole = InsertionHole {
src: &mut tmp.value,
src: &mut *tmp,
dest: &mut v[1],
};
ptr::copy_nonoverlapping(&v[1], &mut v[0], 1);

for i in 2..v.len() {
if !is_less(&v[i], &tmp.value) {
if !is_less(&v[i], &*tmp) {
break;
}
ptr::copy_nonoverlapping(&v[i], &mut v[i - 1], 1);
Expand All @@ -1587,12 +1587,6 @@ fn insert_head<T, F>(v: &mut [T], is_less: &mut F)
}
}

// Holds a value, but never drops it.
#[allow(unions_with_drop_fields)]
union NoDrop<T> {
value: T
}

// When dropped, copies from `src` into `dest`.
struct InsertionHole<T> {
src: *mut T,
Expand Down
30 changes: 12 additions & 18 deletions src/libcore/slice/sort.rs
Expand Up @@ -20,12 +20,6 @@ use cmp;
use mem;
use ptr;

/// Holds a value, but never drops it.
#[allow(unions_with_drop_fields)]
union NoDrop<T> {
value: T
}

/// When dropped, copies from `src` into `dest`.
struct CopyOnDrop<T> {
src: *mut T,
Expand All @@ -49,15 +43,15 @@ fn shift_head<T, F>(v: &mut [T], is_less: &mut F)
// Read the first element into a stack-allocated variable. If a following comparison
// operation panics, `hole` will get dropped and automatically write the element back
// into the slice.
let mut tmp = NoDrop { value: ptr::read(v.get_unchecked(0)) };
let mut tmp = mem::ManuallyDrop::new(ptr::read(v.get_unchecked(0)));
let mut hole = CopyOnDrop {
src: &mut tmp.value,
src: &mut *tmp,
dest: v.get_unchecked_mut(1),
};
ptr::copy_nonoverlapping(v.get_unchecked(1), v.get_unchecked_mut(0), 1);

for i in 2..len {
if !is_less(v.get_unchecked(i), &tmp.value) {
if !is_less(v.get_unchecked(i), &*tmp) {
break;
}

Expand All @@ -81,15 +75,15 @@ fn shift_tail<T, F>(v: &mut [T], is_less: &mut F)
// Read the last element into a stack-allocated variable. If a following comparison
// operation panics, `hole` will get dropped and automatically write the element back
// into the slice.
let mut tmp = NoDrop { value: ptr::read(v.get_unchecked(len - 1)) };
let mut tmp = mem::ManuallyDrop::new(ptr::read(v.get_unchecked(len - 1)));
let mut hole = CopyOnDrop {
src: &mut tmp.value,
src: &mut *tmp,
dest: v.get_unchecked_mut(len - 2),
};
ptr::copy_nonoverlapping(v.get_unchecked(len - 2), v.get_unchecked_mut(len - 1), 1);

for i in (0..len-2).rev() {
if !is_less(&tmp.value, v.get_unchecked(i)) {
if !is_less(&*tmp, v.get_unchecked(i)) {
break;
}

Expand Down Expand Up @@ -403,12 +397,12 @@ fn partition<T, F>(v: &mut [T], pivot: usize, is_less: &mut F) -> (usize, bool)

// 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.
let mut tmp = NoDrop { value: unsafe { ptr::read(pivot) } };
let mut tmp = mem::ManuallyDrop::new(unsafe { ptr::read(pivot) });
let _pivot_guard = CopyOnDrop {
src: unsafe { &mut tmp.value },
src: &mut *tmp,
dest: pivot,
};
let pivot = unsafe { &tmp.value };
let pivot = &*tmp;

// Find the first pair of out-of-order elements.
let mut l = 0;
Expand Down Expand Up @@ -452,12 +446,12 @@ fn partition_equal<T, F>(v: &mut [T], pivot: usize, is_less: &mut F) -> usize

// 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.
let mut tmp = NoDrop { value: unsafe { ptr::read(pivot) } };
let mut tmp = mem::ManuallyDrop::new(unsafe { ptr::read(pivot) });
let _pivot_guard = CopyOnDrop {
src: unsafe { &mut tmp.value },
src: &mut *tmp,
dest: pivot,
};
let pivot = unsafe { &tmp.value };
let pivot = &*tmp;

// Now partition the slice.
let mut l = 0;
Expand Down
37 changes: 8 additions & 29 deletions src/librustc_data_structures/array_vec.rs
Expand Up @@ -20,10 +20,11 @@ use std::fmt;
use std::mem;
use std::collections::range::RangeArgument;
use std::collections::Bound::{Excluded, Included, Unbounded};
use std::mem::ManuallyDrop;

pub unsafe trait Array {
type Element;
type PartialStorage: Default + Unsize<[ManuallyDrop<Self::Element>]>;
type PartialStorage: Unsize<[ManuallyDrop<Self::Element>]>;
const LEN: usize;
}

Expand Down Expand Up @@ -66,7 +67,7 @@ impl<A: Array> ArrayVec<A> {
pub fn new() -> Self {
ArrayVec {
count: 0,
values: Default::default(),
values: unsafe { ::std::mem::uninitialized() },
}
}

Expand All @@ -81,7 +82,7 @@ impl<A: Array> ArrayVec<A> {
/// Panics when the stack vector is full.
pub fn push(&mut self, el: A::Element) {
let arr = &mut self.values as &mut [ManuallyDrop<_>];
arr[self.count] = ManuallyDrop { value: el };
arr[self.count] = ManuallyDrop::new(el);
self.count += 1;
}

Expand All @@ -90,8 +91,8 @@ impl<A: Array> ArrayVec<A> {
let arr = &mut self.values as &mut [ManuallyDrop<_>];
self.count -= 1;
unsafe {
let value = ptr::read(&arr[self.count]);
Some(value.value)
let value = ptr::read(&*arr[self.count]);
Some(value)
}
} else {
None
Expand Down Expand Up @@ -210,7 +211,7 @@ impl<A: Array> Iterator for Iter<A> {
fn next(&mut self) -> Option<A::Element> {
let arr = &self.store as &[ManuallyDrop<_>];
unsafe {
self.indices.next().map(|i| ptr::read(&arr[i]).value)
self.indices.next().map(|i| ptr::read(&*arr[i]))
}
}

Expand All @@ -233,7 +234,7 @@ impl<'a, A: Array> Iterator for Drain<'a, A> {

#[inline]
fn next(&mut self) -> Option<A::Element> {
self.iter.next().map(|elt| unsafe { ptr::read(elt as *const ManuallyDrop<_>).value })
self.iter.next().map(|elt| unsafe { ptr::read(&**elt) })
}

fn size_hint(&self) -> (usize, Option<usize>) {
Expand Down Expand Up @@ -295,25 +296,3 @@ impl<'a, A: Array> IntoIterator for &'a mut ArrayVec<A> {
self.iter_mut()
}
}

// FIXME: This should use repr(transparent) from rust-lang/rfcs#1758.
#[allow(unions_with_drop_fields)]
pub union ManuallyDrop<T> {
value: T,
#[allow(dead_code)]
empty: (),
}

impl<T> ManuallyDrop<T> {
fn new() -> ManuallyDrop<T> {
ManuallyDrop {
empty: ()
}
}
}

impl<T> Default for ManuallyDrop<T> {
fn default() -> Self {
ManuallyDrop::new()
}
}
1 change: 1 addition & 0 deletions src/librustc_data_structures/lib.rs
Expand Up @@ -39,6 +39,7 @@
#![feature(conservative_impl_trait)]
#![feature(discriminant_value)]
#![feature(specialization)]
#![feature(manually_drop)]

#![cfg_attr(unix, feature(libc))]
#![cfg_attr(test, feature(test))]
Expand Down

0 comments on commit 3871312

Please sign in to comment.