-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Extend Cell to work with non-Copy types #39287
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,10 +15,18 @@ | |
//! references. We say that `Cell<T>` and `RefCell<T>` provide 'interior mutability', in contrast | ||
//! with typical Rust types that exhibit 'inherited mutability'. | ||
//! | ||
//! Cell types come in two flavors: `Cell<T>` and `RefCell<T>`. `Cell<T>` provides `get` and `set` | ||
//! methods that change the interior value with a single method call. `Cell<T>` though is only | ||
//! compatible with types that implement `Copy`. For other types, one must use the `RefCell<T>` | ||
//! type, acquiring a write lock before mutating. | ||
//! Cell types come in two flavors: `Cell<T>` and `RefCell<T>`. `Cell<T>` implements interior | ||
//! mutability by moving values in and out of the `Cell<T>`. To use references instead of values, | ||
//! one must use the `RefCell<T>` type, acquiring a write lock before mutating. `Cell<T>` provides | ||
//! methods to retrieve and change the current interior value: | ||
//! | ||
//! - For types that implement `Copy`, the `get` method retrieves the current interior value. | ||
//! - For types that implement `Default`, the `take` method replaces the current interior value | ||
//! with `Default::default()` and returns the replaced value. | ||
//! - For all types, the `replace` method replaces the current interior value and returns the | ||
//! replaced value and the `into_inner` method consumes the `Cell<T>` and returns the interior | ||
//! value. Additionally, the `set` method replaces the interior value, dropping the replaced | ||
//! value. | ||
//! | ||
//! `RefCell<T>` uses Rust's lifetimes to implement 'dynamic borrowing', a process whereby one can | ||
//! claim temporary, exclusive, mutable access to the inner value. Borrows for `RefCell<T>`s are | ||
|
@@ -176,9 +184,10 @@ | |
use cmp::Ordering; | ||
use fmt::{self, Debug, Display}; | ||
use marker::Unsize; | ||
use mem; | ||
use ops::{Deref, DerefMut, CoerceUnsized}; | ||
|
||
/// A mutable memory location that admits only `Copy` data. | ||
/// A mutable memory location. | ||
/// | ||
/// See the [module-level documentation](index.html) for more. | ||
#[stable(feature = "rust1", since = "1.0.0")] | ||
|
@@ -187,23 +196,6 @@ pub struct Cell<T> { | |
} | ||
|
||
impl<T:Copy> Cell<T> { | ||
/// Creates a new `Cell` containing the given value. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// use std::cell::Cell; | ||
/// | ||
/// let c = Cell::new(5); | ||
/// ``` | ||
#[stable(feature = "rust1", since = "1.0.0")] | ||
#[inline] | ||
pub const fn new(value: T) -> Cell<T> { | ||
Cell { | ||
value: UnsafeCell::new(value), | ||
} | ||
} | ||
|
||
/// Returns a copy of the contained value. | ||
/// | ||
/// # Examples | ||
|
@@ -221,25 +213,6 @@ impl<T:Copy> Cell<T> { | |
unsafe{ *self.value.get() } | ||
} | ||
|
||
/// Sets the contained value. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// use std::cell::Cell; | ||
/// | ||
/// let c = Cell::new(5); | ||
/// | ||
/// c.set(10); | ||
/// ``` | ||
#[inline] | ||
#[stable(feature = "rust1", since = "1.0.0")] | ||
pub fn set(&self, value: T) { | ||
unsafe { | ||
*self.value.get() = value; | ||
} | ||
} | ||
|
||
/// Returns a reference to the underlying `UnsafeCell`. | ||
/// | ||
/// # Examples | ||
|
@@ -378,6 +351,100 @@ impl<T: Copy> From<T> for Cell<T> { | |
} | ||
} | ||
|
||
impl<T> Cell<T> { | ||
/// Creates a new `Cell` containing the given value. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// use std::cell::Cell; | ||
/// | ||
/// let c = Cell::new(5); | ||
/// ``` | ||
#[stable(feature = "rust1", since = "1.0.0")] | ||
#[inline] | ||
pub const fn new(value: T) -> Cell<T> { | ||
Cell { | ||
value: UnsafeCell::new(value), | ||
} | ||
} | ||
|
||
/// Sets the contained value. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// use std::cell::Cell; | ||
/// | ||
/// let c = Cell::new(5); | ||
/// | ||
/// c.set(10); | ||
/// ``` | ||
#[inline] | ||
#[stable(feature = "rust1", since = "1.0.0")] | ||
pub fn set(&self, val: T) { | ||
let old = self.replace(val); | ||
drop(old); | ||
Comment on lines
+386
to
+387
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wesleywiser do we actually need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a comment on a very old PR. :D But no, that drop is not necessary, it probably was just there for explicitness. |
||
} | ||
|
||
/// Replaces the contained value. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// #![feature(move_cell)] | ||
/// use std::cell::Cell; | ||
/// | ||
/// let c = Cell::new(5); | ||
/// let old = c.replace(10); | ||
/// | ||
/// assert_eq!(5, old); | ||
/// ``` | ||
#[unstable(feature = "move_cell", issue = "39264")] | ||
pub fn replace(&self, val: T) -> T { | ||
mem::replace(unsafe { &mut *self.value.get() }, val) | ||
} | ||
|
||
/// Unwraps the value. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// #![feature(move_cell)] | ||
/// use std::cell::Cell; | ||
/// | ||
/// let c = Cell::new(5); | ||
/// let five = c.into_inner(); | ||
/// | ||
/// assert_eq!(five, 5); | ||
/// ``` | ||
#[unstable(feature = "move_cell", issue = "39264")] | ||
pub fn into_inner(self) -> T { | ||
unsafe { self.value.into_inner() } | ||
} | ||
} | ||
|
||
impl<T: Default> Cell<T> { | ||
/// Takes the value of the cell, leaving `Default::default()` in its place. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// #![feature(move_cell)] | ||
/// use std::cell::Cell; | ||
/// | ||
/// let c = Cell::new(5); | ||
/// let five = c.take(); | ||
/// | ||
/// assert_eq!(five, 5); | ||
/// assert_eq!(c.into_inner(), 0); | ||
/// ``` | ||
#[unstable(feature = "move_cell", issue = "39264")] | ||
pub fn take(&self) -> T { | ||
self.replace(Default::default()) | ||
} | ||
} | ||
|
||
#[unstable(feature = "coerce_unsized", issue = "27732")] | ||
impl<T: CoerceUnsized<U>, U> CoerceUnsized<Cell<U>> for Cell<T> {} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think doing stable inside unstable makes the item stable again. At least that was the problem of bug #38860
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm not sure what the right thing to do here is. There needs to be some way to construct a non-
Copy
Cell
but perhaps it would be better to add a new function?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would advise moving the
unstable
attribute downward and applying it to each of the new methods. That generally makes it easier to track stabilization later, anyway.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do that. Do you think I should create a new function instead of moving
new()
here or is this ok?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
new
should be modified, given that we're changingset
as well. But cc @rust-lang/libs (since this makes the change roughly insta-stable).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I considered this as an implication of accepting the RFC, so immediately generalizing the behavior of
new
andset
seems ok to me.