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

Use MaybeUninit instead of ManuallyDrop to inhibit the validity invariant of the storage type #3

Merged
merged 4 commits into from
Aug 29, 2019
Merged
Show file tree
Hide file tree
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
18 changes: 11 additions & 7 deletions src/inline_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::fmt::{Debug, Error, Formatter};
use std::hash::{Hash, Hasher};
use std::iter::{FromIterator, FusedIterator};
use std::marker::PhantomData;
use std::mem::{self, ManuallyDrop};
use std::mem::{self, MaybeUninit};
use std::ops::{Deref, DerefMut};
use std::ptr;
use std::slice::{from_raw_parts, from_raw_parts_mut, Iter as SliceIter, IterMut as SliceIterMut};
Expand Down Expand Up @@ -70,7 +70,7 @@ use std::slice::{from_raw_parts, from_raw_parts_mut, Iter as SliceIter, IterMut
/// Both of these will have the same size, and we can swap the `Inline` case out
/// with the `Full` case once the `InlineArray` runs out of capacity.
pub struct InlineArray<A, T> {
data: ManuallyDrop<T>,
data: MaybeUninit<T>,
phantom: PhantomData<A>,
}

Expand Down Expand Up @@ -153,7 +153,14 @@ impl<A, T> InlineArray<A, T> {
#[must_use]
pub fn new() -> Self {
debug_assert!(Self::HOST_SIZE > Self::HEADER_SIZE);
unsafe { mem::zeroed() }
let mut self_ = Self {
data: MaybeUninit::uninit(),
phantom: PhantomData,
};
unsafe {
*self_.len_mut() = 0
}
self_
}

#[inline]
Expand Down Expand Up @@ -260,10 +267,7 @@ impl<A, T> InlineArray<A, T> {
#[inline]
fn drop_contents(&mut self) {
unsafe {
let data = self.data_mut();
for i in 0..self.len() {
ptr::drop_in_place(data.add(i));
}
ptr::drop_in_place::<[A]>(&mut **self)
}
}

Expand Down
32 changes: 16 additions & 16 deletions src/ring_buffer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::cmp::Ordering;
use std::fmt::{Debug, Error, Formatter};
use std::hash::{Hash, Hasher};
use std::iter::FromIterator;
use std::mem::ManuallyDrop;
use std::mem::MaybeUninit;
use std::ops::{Bound, Range, RangeBounds};
use std::ops::{Index, IndexMut};

Expand Down Expand Up @@ -58,7 +58,7 @@ where
{
origin: RawIndex<A, N>,
length: usize,
data: ManuallyDrop<N::SizedType>,
data: MaybeUninit<N::SizedType>,
}

impl<A, N: ChunkLength<A>> Drop for RingBuffer<A, N> {
Expand Down Expand Up @@ -190,24 +190,23 @@ where
#[inline]
#[must_use]
pub fn new() -> Self {
let mut buffer: Self;
unsafe {
buffer = std::mem::zeroed();
std::ptr::write(&mut buffer.origin, 0.into());
std::ptr::write(&mut buffer.length, 0);
Self {
origin: 0.into(),
length: 0,
data: MaybeUninit::uninit(),
}
buffer
}

/// Construct a ring buffer with a single item.
#[inline]
#[must_use]
pub fn unit(value: A) -> Self {
let mut buffer: Self;
let mut buffer = Self {
origin: 0.into(),
length: 1,
data: MaybeUninit::uninit(),
};
unsafe {
buffer = std::mem::zeroed();
std::ptr::write(&mut buffer.origin, 0.into());
std::ptr::write(&mut buffer.length, 1);
buffer.force_write(0.into(), value);
}
buffer
Expand All @@ -217,11 +216,12 @@ where
#[inline]
#[must_use]
pub fn pair(value1: A, value2: A) -> Self {
let mut buffer: Self;
let mut buffer = Self {
origin: 0.into(),
length: 2,
data: MaybeUninit::uninit(),
};
unsafe {
buffer = std::mem::zeroed();
std::ptr::write(&mut buffer.origin, 0.into());
std::ptr::write(&mut buffer.length, 2);
buffer.force_write(0.into(), value1);
buffer.force_write(1.into(), value2);
}
Expand Down
105 changes: 41 additions & 64 deletions src/sized_chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::fmt::{Debug, Error, Formatter};
use std::hash::{Hash, Hasher};
use std::io;
use std::iter::{FromIterator, FusedIterator};
use std::mem::{self, replace, ManuallyDrop};
use std::mem::{replace, MaybeUninit};
use std::ops::{Deref, DerefMut, Index, IndexMut};
use std::ptr;
use std::slice::{
Expand Down Expand Up @@ -98,18 +98,16 @@ where
{
left: usize,
right: usize,
data: ManuallyDrop<N::SizedType>,
data: MaybeUninit<N::SizedType>,
}

impl<A, N> Drop for Chunk<A, N>
where
N: ChunkLength<A>,
{
fn drop(&mut self) {
if mem::needs_drop::<A>() {
for i in self.left..self.right {
unsafe { Chunk::force_drop(i, self) }
}
unsafe {
ptr::drop_in_place(self.as_mut_slice())
}
}
}
Expand All @@ -124,7 +122,7 @@ where
out.left = self.left;
out.right = self.right;
for index in self.left..self.right {
unsafe { Chunk::force_write(index, self.values()[index].clone(), &mut out) }
unsafe { Chunk::force_write(index, (*self.ptr(index)).clone(), &mut out) }
}
out
}
Expand All @@ -138,34 +136,34 @@ where

/// Construct a new empty chunk.
pub fn new() -> Self {
let mut chunk: Self;
unsafe {
chunk = mem::zeroed();
ptr::write(&mut chunk.left, 0);
ptr::write(&mut chunk.right, 0);
Self {
left: 0,
right: 0,
data: MaybeUninit::uninit(),
}
chunk
}

/// Construct a new chunk with one item.
pub fn unit(value: A) -> Self {
let mut chunk: Self;
let mut chunk = Self {
left: 0,
right: 1,
data: MaybeUninit::uninit(),
};
unsafe {
chunk = mem::zeroed();
ptr::write(&mut chunk.left, 0);
ptr::write(&mut chunk.right, 1);
Chunk::force_write(0, value, &mut chunk);
}
chunk
}

/// Construct a new chunk with two items.
pub fn pair(left: A, right: A) -> Self {
let mut chunk: Self;
let mut chunk = Self {
left: 0,
right: 2,
data: MaybeUninit::uninit(),
};
unsafe {
chunk = mem::zeroed();
ptr::write(&mut chunk.left, 0);
ptr::write(&mut chunk.right, 2);
Chunk::force_write(0, left, &mut chunk);
Chunk::force_write(1, right, &mut chunk);
}
Expand Down Expand Up @@ -249,38 +247,32 @@ where
}

#[inline]
fn values(&self) -> &[A] {
unsafe { from_raw_parts(&self.data as *const _ as *const A, N::USIZE) }
unsafe fn ptr(&self, index: usize) -> *const A {
(&self.data as *const _ as *const A).add(index)
}

#[inline]
fn values_mut(&mut self) -> &mut [A] {
unsafe { from_raw_parts_mut(&mut self.data as *mut _ as *mut A, N::USIZE) }
unsafe fn mut_ptr(&mut self, index: usize) -> *mut A {
(&mut self.data as *mut _ as *mut A).add(index)
}

/// Copy the value at an index, discarding ownership of the copied value
#[inline]
unsafe fn force_read(index: usize, chunk: &mut Self) -> A {
ptr::read(&chunk.values()[index])
chunk.ptr(index).read()
}

/// Write a value at an index without trying to drop what's already there
#[inline]
unsafe fn force_write(index: usize, value: A, chunk: &mut Self) {
ptr::write(&mut chunk.values_mut()[index], value)
}

/// Drop the value at an index
#[inline]
unsafe fn force_drop(index: usize, chunk: &mut Self) {
ptr::drop_in_place(&mut chunk.values_mut()[index])
chunk.mut_ptr(index).write(value)
}

/// Copy a range within a chunk
#[inline]
unsafe fn force_copy(from: usize, to: usize, count: usize, chunk: &mut Self) {
if count > 0 {
ptr::copy(&chunk.values()[from], &mut chunk.values_mut()[to], count)
ptr::copy(chunk.ptr(from), chunk.mut_ptr(to), count)
}
}

Expand All @@ -294,7 +286,7 @@ where
other: &mut Self,
) {
if count > 0 {
ptr::copy_nonoverlapping(&chunk.values()[from], &mut other.values_mut()[to], count)
ptr::copy_nonoverlapping(chunk.ptr(from), other.mut_ptr(to), count)
}
}

Expand Down Expand Up @@ -376,12 +368,8 @@ where
/// Time: O(n) for the number of items dropped
pub fn drop_left(&mut self, index: usize) {
if index > 0 {
if index > self.len() {
panic!("Chunk::drop_left: index out of bounds");
}
let start = self.left;
for i in start..(start + index) {
unsafe { Chunk::force_drop(i, self) }
unsafe {
ptr::drop_in_place(&mut self[..index])
}
self.left += index;
}
Expand All @@ -393,17 +381,12 @@ where
///
/// Time: O(n) for the number of items dropped
pub fn drop_right(&mut self, index: usize) {
if index > self.len() {
panic!("Chunk::drop_right: index out of bounds");
}
if index == self.len() {
return;
}
let start = self.left + index;
for i in start..self.right {
unsafe { Chunk::force_drop(i, self) }
if index != self.len() {
unsafe {
ptr::drop_in_place(&mut self[index..])
}
self.right = self.left + index;
}
self.right = start;
}

/// Split a chunk into two, the original chunk containing
Expand Down Expand Up @@ -570,8 +553,8 @@ where
///
/// Time: O(n)
pub fn clear(&mut self) {
for i in self.left..self.right {
unsafe { Chunk::force_drop(i, self) }
unsafe {
ptr::drop_in_place(self.as_mut_slice())
}
self.left = 0;
self.right = 0;
Expand All @@ -581,7 +564,7 @@ where
pub fn as_slice(&self) -> &[A] {
unsafe {
from_raw_parts(
(&self.data as *const ManuallyDrop<N::SizedType> as *const A).add(self.left),
(&self.data as *const MaybeUninit<N::SizedType> as *const A).add(self.left),
self.len(),
)
}
Expand All @@ -591,7 +574,7 @@ where
pub fn as_mut_slice(&mut self) -> &mut [A] {
unsafe {
from_raw_parts_mut(
(&mut self.data as *mut ManuallyDrop<N::SizedType> as *mut A).add(self.left),
(&mut self.data as *mut MaybeUninit<N::SizedType> as *mut A).add(self.left),
self.len(),
)
}
Expand Down Expand Up @@ -725,15 +708,9 @@ impl<A, N, T> From<InlineArray<A, T>> for Chunk<A, N>
where
N: ChunkLength<A>,
{
#[inline]
fn from(mut array: InlineArray<A, T>) -> Self {
let mut out = Self::new();
out.left = 0;
out.right = array.len();
unsafe {
ptr::copy_nonoverlapping(array.data(), &mut out.values_mut()[0], out.right);
*array.len_mut() = 0;
}
out
Self::from(&mut array)
}
}

Expand All @@ -746,7 +723,7 @@ where
out.left = 0;
out.right = array.len();
unsafe {
ptr::copy_nonoverlapping(array.data(), &mut out.values_mut()[0], out.right);
ptr::copy_nonoverlapping(array.data(), out.mut_ptr(0), out.right);
*array.len_mut() = 0;
}
out
Expand Down
9 changes: 6 additions & 3 deletions src/sparse_chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

use std::collections::{BTreeMap, HashMap};
use std::fmt::{Debug, Error, Formatter};
use std::mem::{self, ManuallyDrop};
use std::mem::{self, MaybeUninit};
use std::ops::Index;
use std::ops::IndexMut;
use std::ptr;
Expand Down Expand Up @@ -58,7 +58,7 @@ use crate::types::{Bits, ChunkLength};
/// [Unsigned]: https://docs.rs/typenum/1.10.0/typenum/marker_traits/trait.Unsigned.html
pub struct SparseChunk<A, N: Bits + ChunkLength<A> = U64> {
map: Bitmap<N>,
data: ManuallyDrop<N::SizedType>,
data: MaybeUninit<N::SizedType>,
}

impl<A, N: Bits + ChunkLength<A>> Drop for SparseChunk<A, N> {
Expand Down Expand Up @@ -117,7 +117,10 @@ where

/// Construct a new empty chunk.
pub fn new() -> Self {
unsafe { mem::zeroed() }
Self {
map: Bitmap::default(),
data: MaybeUninit::uninit(),
}
}

/// Construct a new chunk with one item.
Expand Down
9 changes: 9 additions & 0 deletions src/tests/inline_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ use proptest_derive::Arbitrary;

use crate::inline_array::InlineArray;

#[test]
fn validity_invariant() {
assert!(Some(InlineArray::<usize, [Box<()>; 2]>::new()).is_some());

let mut chunk = InlineArray::<usize, [Box<()>; 2]>::new();
chunk.push(0);
assert!(Some(chunk).is_some());
}

type TestType = [usize; 16];

#[derive(Arbitrary, Debug)]
Expand Down
Loading