Auto merge of #8286 - eefriedman:no-move, r=nox

Remove unnecessary uses of #[no_move]

The patch makes RootCollection a bit safer by making the StackRootTLS hold
it in place.

RootedVec was doing an extremely delicate dance and just hoping nobody
messed it up; switch to a Box to be safe.

CodeGenRust seemed to be using no_move for no particularly good reason.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8286)
<!-- Reviewable:end -->

components/script/dom/bindings/codegen/CodegenRust.py

@@ -4927,7 +4927,6 @@ class CGDictionary(CGThing):
                        for m in self.memberInfo]
 
         return (string.Template(
-                "#[no_move]\n" +
                 "pub struct ${selfName} {\n" +
                 "${inheritance}" +
                 "\n".join(memberDecls) + "\n" +

components/script/dom/bindings/js.rs

@@ -454,7 +454,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>>,
 }

components/script/dom/bindings/mod.rs

@@ -152,9 +152,7 @@ pub mod xmlname;
 /// Generated JS-Rust bindings.
 #[allow(missing_docs, non_snake_case)]
 pub mod codegen {
-    // FIXME(#5853) we shouldn't need to
+    #[allow(unrooted_must_root)]
-    // allow moved_no_move here
-    #[allow(unrooted_must_root, moved_no_move)]
     pub mod Bindings {
         include!(concat!(env!("OUT_DIR"), "/Bindings/mod.rs"));
     }

components/script/dom/bindings/trace.rs

@@ -382,7 +382,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 =
@@ -395,7 +395,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) };
@@ -432,14 +432,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);
+        }
     }
 }
 
@@ -461,15 +465,13 @@ 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> {
+    pub unsafe fn new_with_destination_address(addr: *const libc::c_void) -> RootedVec<T> {
-        unsafe {
+        RootedTraceableSet::add::<RootedVec<T>>(&*(addr as *const _));
-            RootedTraceableSet::add::<RootedVec<T>>(&*(addr as *const _));
-        }
         RootedVec::<T> { v: vec!() }
     }
 }
@@ -477,13 +479,15 @@ impl<T: JSTraceable> RootedVec<T> {
 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);
+        }
     }
 }
 
@@ -503,9 +507,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 {
+        let mut vec = unsafe {
-            return_address() as *const libc::c_void
+            RootedVec::new_with_destination_address(return_address() as *const libc::c_void)
-        });
+        };
         vec.extend(iterable.into_iter().map(|item| JS::from_rooted(&item)));
         vec
     }

components/script/script_task.rs

@@ -90,6 +90,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;
@@ -367,18 +368,18 @@ impl TimerEventChan for MainThreadTimerEventChan {
     }
 }
 
-pub struct StackRootTLS;
+pub struct StackRootTLS<'a>(PhantomData<&'a u32>);
 
-impl StackRootTLS {
+impl<'a> StackRootTLS<'a> {
-    pub fn new(roots: &RootCollection) -> StackRootTLS {
+    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));
     }