Skip to content

Rewrite rc::Rc using cell::Cell #12654

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

Merged
merged 1 commit into from
Mar 21, 2014
Merged
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
99 changes: 65 additions & 34 deletions src/libstd/rc.rs
Original file line number Diff line number Diff line change
@@ -24,18 +24,20 @@ pointers, and then storing the parent pointers as `Weak` pointers.
*/

use cast::transmute;
use cell::Cell;
use clone::Clone;
use cmp::{Eq, Ord};
use kinds::marker;
use ops::{Deref, Drop};
use option::{Option, Some, None};
use ptr;
use ptr::RawPtr;
use rt::global_heap::exchange_free;

struct RcBox<T> {
value: T,
strong: uint,
weak: uint
strong: Cell<uint>,
weak: Cell<uint>
}

/// Immutable reference counted pointer type
@@ -56,7 +58,11 @@ impl<T> Rc<T> {
// destructor never frees the allocation while the
// strong destructor is running, even if the weak
// pointer is stored inside the strong one.
ptr: transmute(~RcBox { value: value, strong: 1, weak: 1 }),
ptr: transmute(~RcBox {
value: value,
strong: Cell::new(1),
weak: Cell::new(1)
}),
nosend: marker::NoSend,
noshare: marker::NoShare
}
@@ -67,13 +73,11 @@ impl<T> Rc<T> {
impl<T> Rc<T> {
/// Downgrade the reference-counted pointer to a weak reference
pub fn downgrade(&self) -> Weak<T> {
unsafe {
(*self.ptr).weak += 1;
Weak {
ptr: self.ptr,
nosend: marker::NoSend,
noshare: marker::NoShare
}
self.inc_weak();
Weak {
ptr: self.ptr,
nosend: marker::NoSend,
noshare: marker::NoShare
}
}
}
@@ -82,24 +86,24 @@ impl<T> Deref<T> for Rc<T> {
/// Borrow the value contained in the reference-counted box
#[inline(always)]
fn deref<'a>(&'a self) -> &'a T {
unsafe { &(*self.ptr).value }
&self.inner().value
}
}

#[unsafe_destructor]
impl<T> Drop for Rc<T> {
fn drop(&mut self) {
unsafe {
if self.ptr != 0 as *mut RcBox<T> {
(*self.ptr).strong -= 1;
if (*self.ptr).strong == 0 {
if !self.ptr.is_null() {
self.dec_strong();
if self.strong() == 0 {
ptr::read(self.deref()); // destroy the contained object

// remove the implicit "strong weak" pointer now
// that we've destroyed the contents.
(*self.ptr).weak -= 1;
self.dec_weak();

if (*self.ptr).weak == 0 {
if self.weak() == 0 {
exchange_free(self.ptr as *u8)
}
}
@@ -111,10 +115,8 @@ impl<T> Drop for Rc<T> {
impl<T> Clone for Rc<T> {
#[inline]
fn clone(&self) -> Rc<T> {
unsafe {
(*self.ptr).strong += 1;
Rc { ptr: self.ptr, nosend: marker::NoSend, noshare: marker::NoShare }
}
self.inc_strong();
Rc { ptr: self.ptr, nosend: marker::NoSend, noshare: marker::NoShare }
}
}

@@ -151,13 +153,11 @@ pub struct Weak<T> {
impl<T> Weak<T> {
/// Upgrade a weak reference to a strong reference
pub fn upgrade(&self) -> Option<Rc<T>> {
unsafe {
if (*self.ptr).strong == 0 {
None
} else {
(*self.ptr).strong += 1;
Some(Rc { ptr: self.ptr, nosend: marker::NoSend, noshare: marker::NoShare })
}
if self.strong() == 0 {
None
} else {
self.inc_strong();
Some(Rc { ptr: self.ptr, nosend: marker::NoSend, noshare: marker::NoShare })
}
}
}
@@ -166,11 +166,11 @@ impl<T> Weak<T> {
impl<T> Drop for Weak<T> {
fn drop(&mut self) {
unsafe {
if self.ptr != 0 as *mut RcBox<T> {
(*self.ptr).weak -= 1;
if !self.ptr.is_null() {
self.dec_weak();
// the weak count starts at 1, and will only go to
// zero if all the strong pointers have disappeared.
if (*self.ptr).weak == 0 {
if self.weak() == 0 {
exchange_free(self.ptr as *u8)
}
}
@@ -181,13 +181,44 @@ impl<T> Drop for Weak<T> {
impl<T> Clone for Weak<T> {
#[inline]
fn clone(&self) -> Weak<T> {
unsafe {
(*self.ptr).weak += 1;
Weak { ptr: self.ptr, nosend: marker::NoSend, noshare: marker::NoShare }
}
self.inc_weak();
Weak { ptr: self.ptr, nosend: marker::NoSend, noshare: marker::NoShare }
}
}

#[allow(missing_doc)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add some docs? I'd like us to avoid merging patches that add undocumented features.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an internal implementation trait, I think, so I'm not sure that docs are needed since these methods aren't publicly available and their purpose is fairly clear.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough.

Side Note: I personally prefer to have everything documented even internal and pretty obvious things. I think it's good for the language and to make newcomers life easier.

trait RcBoxPtr<T> {
fn inner<'a>(&'a self) -> &'a RcBox<T>;

#[inline]
fn strong(&self) -> uint { self.inner().strong.get() }

#[inline]
fn inc_strong(&self) { self.inner().strong.set(self.strong() + 1); }

#[inline]
fn dec_strong(&self) { self.inner().strong.set(self.strong() - 1); }

#[inline]
fn weak(&self) -> uint { self.inner().weak.get() }

#[inline]
fn inc_weak(&self) { self.inner().weak.set(self.weak() + 1); }

#[inline]
fn dec_weak(&self) { self.inner().weak.set(self.weak() - 1); }
}

impl<T> RcBoxPtr<T> for Rc<T> {
#[inline(always)]
fn inner<'a>(&'a self) -> &'a RcBox<T> { unsafe { &(*self.ptr) } }
}

impl<T> RcBoxPtr<T> for Weak<T> {
#[inline(always)]
fn inner<'a>(&'a self) -> &'a RcBox<T> { unsafe { &(*self.ptr) } }
}

#[cfg(test)]
mod tests {
use prelude::*;
8 changes: 0 additions & 8 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
@@ -1156,14 +1156,6 @@ mod test {
use codemap::*;
use super::*;

fn is_freeze<T: Freeze>() {}

// Assert that the AST remains Freeze (#10693).
#[test]
fn ast_is_freeze() {
is_freeze::<Item>();
}

// are ASTs encodable?
#[test]
fn check_asts_encodable() {