Skip to content

Commit

Permalink
Clean up some code related to #[no_move].
Browse files Browse the repository at this point in the history
The patch makes RootCollection a bit safer by making the StackRootTLS hold
it in place.

The use of no_move in CodeGenRust was leftover from when roots couldn't
be moved.
  • Loading branch information
eefriedman committed Nov 8, 2015
1 parent db1163b commit 1a50fce
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 24 deletions.
1 change: 0 additions & 1 deletion components/script/dom/bindings/codegen/CodegenRust.py
Expand Up @@ -4927,7 +4927,6 @@ def struct(self):
for m in self.memberInfo]

return (string.Template(
"#[no_move]\n" +
"pub struct ${selfName} {\n" +
"${inheritance}" +
"\n".join(memberDecls) + "\n" +
Expand Down
1 change: 0 additions & 1 deletion components/script/dom/bindings/js.rs
Expand Up @@ -453,7 +453,6 @@ impl<T: Reflectable> OptionalRootedReference<T> for Option<Option<Root<T>>> {
///
/// See also [*Exact Stack Rooting - Storing a GCPointer on the CStack*]
/// (https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Internals/GC/Exact_Stack_Rooting).
#[no_move]
pub struct RootCollection {
roots: UnsafeCell<Vec<*const Reflector>>,
}
Expand Down
4 changes: 1 addition & 3 deletions components/script/dom/bindings/mod.rs
Expand Up @@ -149,9 +149,7 @@ pub mod utils;
/// Generated JS-Rust bindings.
#[allow(missing_docs, non_snake_case)]
pub mod codegen {
// FIXME(#5853) we shouldn't need to
// allow moved_no_move here
#[allow(unrooted_must_root, moved_no_move)]
#[allow(unrooted_must_root)]
pub mod Bindings {
include!(concat!(env!("OUT_DIR"), "/Bindings/mod.rs"));
}
Expand Down
32 changes: 18 additions & 14 deletions components/script/dom/bindings/trace.rs
Expand Up @@ -403,7 +403,7 @@ impl RootedTraceableSet {
}
}

fn remove<T: JSTraceable>(traceable: &T) {
unsafe fn remove<T: JSTraceable>(traceable: &T) {
ROOTED_TRACEABLES.with(|ref traceables| {
let mut traceables = traceables.borrow_mut();
let idx =
Expand All @@ -416,7 +416,7 @@ impl RootedTraceableSet {
});
}

fn add<T: JSTraceable>(traceable: &T) {
unsafe fn add<T: JSTraceable>(traceable: &T) {
ROOTED_TRACEABLES.with(|ref traceables| {
fn trace<T: JSTraceable>(obj: *const libc::c_void, tracer: *mut JSTracer) {
let obj: &T = unsafe { &*(obj as *const T) };
Expand Down Expand Up @@ -453,14 +453,18 @@ pub struct RootedTraceable<'a, T: 'a + JSTraceable> {
impl<'a, T: JSTraceable> RootedTraceable<'a, T> {
/// Root a JSTraceable thing for the life of this RootedTraceable
pub fn new(traceable: &'a T) -> RootedTraceable<'a, T> {
RootedTraceableSet::add(traceable);
unsafe {
RootedTraceableSet::add(traceable);
}
RootedTraceable { ptr: traceable }
}
}

impl<'a, T: JSTraceable> Drop for RootedTraceable<'a, T> {
fn drop(&mut self) {
RootedTraceableSet::remove(self.ptr);
unsafe {
RootedTraceableSet::remove(self.ptr);
}
}
}

Expand All @@ -482,29 +486,29 @@ impl<T: JSTraceable> RootedVec<T> {
return_address() as *const libc::c_void
};

RootedVec::new_with_destination_address(addr)
unsafe { RootedVec::new_with_destination_address(addr) }
}

/// Create a vector of items of type T. This constructor is specific
/// for RootTraceableSet.
pub fn new_with_destination_address(addr: *const libc::c_void) -> RootedVec<T> {
unsafe {
RootedTraceableSet::add::<RootedVec<T>>(&*(addr as *const _));
}
pub unsafe fn new_with_destination_address(addr: *const libc::c_void) -> RootedVec<T> {
RootedTraceableSet::add::<RootedVec<T>>(&*(addr as *const _));
RootedVec::<T> { v: vec!() }
}
}

impl<T: JSTraceable + Reflectable> RootedVec<JS<T>> {
/// Obtain a safe slice of references that can't outlive that RootedVec.
pub fn r(&self) -> &[&T] {
unsafe { mem::transmute(&*self.v) }
unsafe { mem::transmute(&self.v[..]) }
}
}

impl<T: JSTraceable> Drop for RootedVec<T> {
fn drop(&mut self) {
RootedTraceableSet::remove(self);
unsafe {
RootedTraceableSet::remove(self);
}
}
}

Expand All @@ -524,9 +528,9 @@ impl<T: JSTraceable> DerefMut for RootedVec<T> {
impl<A: JSTraceable + Reflectable> FromIterator<Root<A>> for RootedVec<JS<A>> {
#[allow(moved_no_move)]
fn from_iter<T>(iterable: T) -> RootedVec<JS<A>> where T: IntoIterator<Item=Root<A>> {
let mut vec = RootedVec::new_with_destination_address(unsafe {
return_address() as *const libc::c_void
});
let mut vec = unsafe {
RootedVec::new_with_destination_address(return_address() as *const libc::c_void)
};
vec.extend(iterable.into_iter().map(|item| JS::from_rooted(&item)));
vec
}
Expand Down
11 changes: 6 additions & 5 deletions components/script/script_task.rs
Expand Up @@ -89,6 +89,7 @@ use std::borrow::ToOwned;
use std::cell::{Cell, RefCell};
use std::collections::HashSet;
use std::io::{Write, stdout};
use std::marker::PhantomData;
use std::mem as std_mem;
use std::option::Option;
use std::ptr;
Expand Down Expand Up @@ -334,18 +335,18 @@ impl TimerEventChan for MainThreadTimerEventChan {
}
}

pub struct StackRootTLS;
pub struct StackRootTLS<'a>(PhantomData<&'a u32>);

impl StackRootTLS {
pub fn new(roots: &RootCollection) -> StackRootTLS {
impl<'a> StackRootTLS<'a> {
pub fn new(roots: &'a RootCollection) -> StackRootTLS<'a> {
STACK_ROOTS.with(|ref r| {
r.set(Some(RootCollectionPtr(roots as *const _)))
});
StackRootTLS
StackRootTLS(PhantomData)
}
}

impl Drop for StackRootTLS {
impl<'a> Drop for StackRootTLS<'a> {
fn drop(&mut self) {
STACK_ROOTS.with(|ref r| r.set(None));
}
Expand Down

0 comments on commit 1a50fce

Please sign in to comment.