From 19c0c7b02f5584187a163365bcc6a9b57ccf1248 Mon Sep 17 00:00:00 2001 From: Charlie Fan Date: Fri, 10 Feb 2017 15:16:28 +0800 Subject: [PATCH 1/2] Add more methods in Cell for non-Copy types Complement to #39264 --- src/libcore/cell.rs | 111 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 109 insertions(+), 2 deletions(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index ab44342ebf02f..20867efa9908c 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -186,6 +186,7 @@ use fmt::{self, Debug, Display}; use marker::Unsize; use mem; use ops::{Deref, DerefMut, CoerceUnsized}; +use ptr; /// A mutable memory location. /// @@ -280,6 +281,19 @@ unsafe impl Send for Cell where T: Send {} #[stable(feature = "rust1", since = "1.0.0")] impl !Sync for Cell {} +#[unstable(feature = "move_cell", issue = "39264")] +impl Clone for Cell { + #[inline] + default fn clone(&self) -> Cell { + let temp = unsafe { mem::uninitialized() }; + let inner = self.replace(temp); + // If `clone` panics, let it crash. + let another = inner.clone(); + self.set(inner); + Cell::new(another) + } +} + #[stable(feature = "rust1", since = "1.0.0")] impl Clone for Cell { #[inline] @@ -289,7 +303,7 @@ impl Clone for Cell { } #[stable(feature = "rust1", since = "1.0.0")] -impl Default for Cell { +impl Default for Cell { /// Creates a `Cell`, with the `Default` value for T. #[inline] fn default() -> Cell { @@ -297,6 +311,18 @@ impl Default for Cell { } } +#[unstable(feature = "move_cell", issue = "39264")] +impl PartialEq for Cell { + #[inline] + default fn eq(&self, other: &Cell) -> bool { + if ptr::eq(self, other) { + true + } else { + unsafe { Cell::take_and_restore(self, other, |l, r| l == r) } + } + } +} + #[stable(feature = "rust1", since = "1.0.0")] impl PartialEq for Cell { #[inline] @@ -305,9 +331,60 @@ impl PartialEq for Cell { } } +#[unstable(feature = "move_cell", issue = "39264")] +impl Eq for Cell {} + #[stable(feature = "cell_eq", since = "1.2.0")] impl Eq for Cell {} +#[unstable(feature = "move_cell", issue = "39264")] +impl PartialOrd for Cell { + #[inline] + default fn partial_cmp(&self, other: &Cell) -> Option { + if ptr::eq(self, other) { + Some(Ordering::Equal) + } else { + unsafe { Cell::take_and_restore(self, other, |l, r| l.partial_cmp(r)) } + } + } + + #[inline] + default fn lt(&self, other: &Cell) -> bool { + if ptr::eq(self, other) { + true + } else { + unsafe { Cell::take_and_restore(self, other, |l, r| l < r) } + } + } + + #[inline] + default fn le(&self, other: &Cell) -> bool { + if ptr::eq(self, other) { + true + } else { + unsafe { Cell::take_and_restore(self, other, |l, r| l <= r) } + } + } + + #[inline] + default fn gt(&self, other: &Cell) -> bool { + if ptr::eq(self, other) { + true + } else { + unsafe { Cell::take_and_restore(self, other, |l, r| l > r) } + } + } + + #[inline] + default fn ge(&self, other: &Cell) -> bool { + if ptr::eq(self, other) { + true + } else { + unsafe { Cell::take_and_restore(self, other, |l, r| l >= r) } + } + } +} + #[stable(feature = "cell_ord", since = "1.10.0")] impl PartialOrd for Cell { #[inline] @@ -336,6 +413,18 @@ impl PartialOrd for Cell { } } +#[unstable(feature = "move_cell", issue = "39264")] +impl Ord for Cell { + #[inline] + default fn cmp(&self, other: &Cell) -> Ordering { + if ptr::eq(self, other) { + Ordering::Equal + } else { + unsafe { Cell::take_and_restore(self, other, |l, r| l.cmp(r)) } + } + } +} + #[stable(feature = "cell_ord", since = "1.10.0")] impl Ord for Cell { #[inline] @@ -345,7 +434,7 @@ impl Ord for Cell { } #[stable(feature = "cell_from", since = "1.12.0")] -impl From for Cell { +impl From for Cell { fn from(t: T) -> Cell { Cell::new(t) } @@ -422,6 +511,24 @@ impl Cell { pub fn into_inner(self) -> T { unsafe { self.value.into_inner() } } + + /// Private function used for implementing other methods when `T` is not `Copy`. + /// + /// This is `unsafe` because `left` and `right` may point to the same object. + #[unstable(feature = "move_cell", issue = "39264")] + unsafe fn take_and_restore(left: &Self, right: &Self, f: F) -> Result + where F: FnOnce(&T, &T) -> Result + { + let temp_left = mem::uninitialized(); + let temp_right = mem::uninitialized(); + let lhs = left.replace(temp_left); + let rhs = right.replace(temp_right); + // If panic happens in `f`, just let it crash. + let result = f(&lhs, &rhs); + left.set(lhs); + right.set(rhs); + return result; + } } impl Cell { From 3775c4dc8f95aa7a5d7abf89e03aab8ba74df2d0 Mon Sep 17 00:00:00 2001 From: f001 Date: Fri, 10 Feb 2017 22:53:12 +0800 Subject: [PATCH 2/2] ensure panic safety --- src/libcore/cell.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index 20867efa9908c..98956a8da9802 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -285,11 +285,11 @@ impl !Sync for Cell {} impl Clone for Cell { #[inline] default fn clone(&self) -> Cell { - let temp = unsafe { mem::uninitialized() }; - let inner = self.replace(temp); - // If `clone` panics, let it crash. + // convert `*mut T` to `&T`, because we can ensure that + // there are no mutations or mutable aliases going on when casting to &T. + let inner : &T = unsafe { self.value.get().as_ref().unwrap() }; + // If `clone` panics, the `Cell` itself is in a valid state. let another = inner.clone(); - self.set(inner); Cell::new(another) } } @@ -519,14 +519,12 @@ impl Cell { unsafe fn take_and_restore(left: &Self, right: &Self, f: F) -> Result where F: FnOnce(&T, &T) -> Result { - let temp_left = mem::uninitialized(); - let temp_right = mem::uninitialized(); - let lhs = left.replace(temp_left); - let rhs = right.replace(temp_right); - // If panic happens in `f`, just let it crash. - let result = f(&lhs, &rhs); - left.set(lhs); - right.set(rhs); + // convert `*mut T` to `&T`, because we can ensure that + // there are no mutations or mutable aliases going on when casting to &T. + let lhs = left.value.get().as_ref().unwrap(); + let rhs = right.value.get().as_ref().unwrap(); + // If panic happens in `f`, both `Cell`s are in a valid state. + let result = f(lhs, rhs); return result; } }