From f498a10b83d36829f6ac2aafd2a3eb9555f76107 Mon Sep 17 00:00:00 2001 From: maciejhirsz Date: Sun, 2 Dec 2018 13:14:31 +0100 Subject: [PATCH 1/2] Comments on, and hopefully an increase of `CopyCell` soundness --- src/cell.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/cell.rs b/src/cell.rs index 1875a5d..a68e57f 100644 --- a/src/cell.rs +++ b/src/cell.rs @@ -7,6 +7,7 @@ use std::marker::PhantomData; /// library, but always require that the internal type implements `Copy` /// and implements `Copy` itself. #[derive(PartialEq, Eq)] +#[repr(transparent)] pub struct CopyCell { /// Internal value value: T, @@ -34,15 +35,19 @@ impl CopyCell { impl CopyCell { #[inline] fn mut_ptr(&self) -> *mut T { - &self.value as *const T as *mut T + // We can just cast the pointer from `CopyCell` to `T` because of + // #[repr(transparent)] + // + // This behavior is copied over from the std implementation of + // the `UnsafeCell`, and it's the best we can do right now in terms + // of soundness till we get a stable `UnsafeCell` that implements `Copy`. + self as *const CopyCell as *const T as *mut T } /// Returns a copy of the contained value. #[inline] pub fn get(&self) -> T { - unsafe { - *self.mut_ptr() - } + self.value } /// Returns a mutable reference to the underlying data. From 86b7f93f92eea5ff813628589b2147ae4905d03d Mon Sep 17 00:00:00 2001 From: maciejhirsz Date: Sun, 2 Dec 2018 13:19:40 +0100 Subject: [PATCH 2/2] Remove `get_mut` altogether --- src/cell.rs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/cell.rs b/src/cell.rs index a68e57f..aac5539 100644 --- a/src/cell.rs +++ b/src/cell.rs @@ -33,17 +33,6 @@ impl CopyCell { } impl CopyCell { - #[inline] - fn mut_ptr(&self) -> *mut T { - // We can just cast the pointer from `CopyCell` to `T` because of - // #[repr(transparent)] - // - // This behavior is copied over from the std implementation of - // the `UnsafeCell`, and it's the best we can do right now in terms - // of soundness till we get a stable `UnsafeCell` that implements `Copy`. - self as *const CopyCell as *const T as *mut T - } - /// Returns a copy of the contained value. #[inline] pub fn get(&self) -> T { @@ -55,9 +44,11 @@ impl CopyCell { /// This call borrows `CopyCell` mutably, which gives us a compile time /// memory safety guarantee. #[inline] - pub fn get_mut(&mut self) -> &mut T { + pub fn get_mut<'a>(&'a mut self) -> &'a mut T { + // We can just cast the pointer from `CopyCell` to `T` because of + // #[repr(transparent)] unsafe { - &mut *self.mut_ptr() + &mut *(self as *mut CopyCell as *mut T) } } @@ -69,7 +60,14 @@ impl CopyCell { // Regular write produces abnormal behavior when running tests in // `--release` mode. Reordering writes when the compiler assumes // things are immutable is dangerous. - unsafe { write_volatile(self.mut_ptr(), value) }; + // + // We can just cast the pointer from `CopyCell` to `T` because of + // #[repr(transparent)] + // + // This behavior is copied over from the std implementation of + // the `UnsafeCell`, and it's the best we can do right now in terms + // of soundness till we get a stable `UnsafeCell` that implements `Copy`. + unsafe { write_volatile(self as *const CopyCell as *const T as *mut T, value) }; } }