Move from using return_address to a macro-mediated rooting solution.

eddyb · eddyb · commit 71333b8f824b · 2016-06-27T20:01:27.000+03:00
diff --git a/src/conversions.rs b/src/conversions.rs
@@ -33,7 +33,7 @@ use jsapi::JSPROP_ENUMERATE;
 use jsapi::{JSContext, JSObject, JSString, HandleValue, MutableHandleValue};
 use jsapi::{JS_NewUCStringCopyN, JS_StringHasLatin1Chars, JS_WrapValue};
 use jsapi::{JS_GetLatin1StringCharsAndLength, JS_GetTwoByteStringCharsAndLength};
-use jsapi::{JS_NewArrayObject1, JS_DefineElement, RootedValue, RootedObject};
+use jsapi::{JS_NewArrayObject1, JS_DefineElement, RootedObject};
 use jsapi::{ForOfIterator, ForOfIterator_NonIterableBehavior};
 use jsval::{BooleanValue, Int32Value, NullValue, UInt32Value, UndefinedValue};
 use jsval::{JSVal, ObjectValue, ObjectOrNullValue, StringValue};
@@ -473,10 +473,10 @@ impl<T: FromJSValConvertible> FromJSValConvertible for Option<T> {
 // https://heycam.github.io/webidl/#es-sequence
 impl<T: ToJSValConvertible> ToJSValConvertible for Vec<T> {
     unsafe fn to_jsval(&self, cx: *mut JSContext, rval: MutableHandleValue) {
-        let js_array = RootedObject::new(cx, JS_NewArrayObject1(cx, self.len() as libc::size_t));
+        rooted!(in(cx) let js_array = JS_NewArrayObject1(cx, self.len() as libc::size_t));
         assert!(!js_array.handle().is_null());
 
-        let mut val = RootedValue::new(cx, UndefinedValue());
+        rooted!(in(cx) let mut val = UndefinedValue());
         for (index, obj) in self.iter().enumerate() {
             obj.to_jsval(cx, val.handle_mut());
 
@@ -488,6 +488,33 @@ impl<T: ToJSValConvertible> ToJSValConvertible for Vec<T> {
     }
 }
 
+/// Rooting guard for the iterator field of ForOfIterator.
+/// Behaves like RootedGuard (roots on creation, unroots on drop),
+/// but borrows and allows access to the whole ForOfIterator, so
+/// that methods on ForOfIterator can still be used through it.
+struct ForOfIteratorGuard<'a> {
+    root: &'a mut ForOfIterator
+}
+
+impl<'a> ForOfIteratorGuard<'a> {
+    fn new(cx: *mut JSContext, root: &'a mut ForOfIterator) -> Self {
+        unsafe {
+            root.iterator.add_to_root_stack(cx);
+        }
+        ForOfIteratorGuard {
+            root: root
+        }
+    }
+}
+
+impl<'a> Drop for ForOfIteratorGuard<'a> {
+    fn drop(&mut self) {
+        unsafe {
+            self.root.iterator.remove_from_root_stack();
+        }
+    }
+}
+
 impl<C: Clone, T: FromJSValConvertible<Config=C>> FromJSValConvertible for Vec<T> {
     type Config = C;
 
@@ -497,9 +524,11 @@ impl<C: Clone, T: FromJSValConvertible<Config=C>> FromJSValConvertible for Vec<T
                          -> Result<Vec<T>, ()> {
         let mut iterator = ForOfIterator {
             cx_: cx,
-            iterator: RootedObject::new(cx, ptr::null_mut()),
+            iterator: RootedObject::new_unrooted(ptr::null_mut()),
             index: ::std::u32::MAX, // NOT_ARRAY
         };
+        let mut iterator = ForOfIteratorGuard::new(cx, &mut iterator);
+        let iterator = &mut *iterator.root;
 
         if !iterator.init(value, ForOfIterator_NonIterableBehavior::ThrowOnNonIterable) {
             return Err(())
@@ -509,7 +538,7 @@ impl<C: Clone, T: FromJSValConvertible<Config=C>> FromJSValConvertible for Vec<T
 
         loop {
             let mut done = false;
-            let mut val = RootedValue::new(cx, UndefinedValue());
+            rooted!(in(cx) let mut val = UndefinedValue());
             if !iterator.next(val.handle_mut(), &mut done) {
                 return Err(())
             }
diff --git a/src/lib.rs b/src/lib.rs
@@ -5,7 +5,6 @@
 #![crate_name = "js"]
 #![crate_type = "rlib"]
 
-#![feature(core_intrinsics)]
 #![feature(filling_drop)]
 #![feature(link_args)]
 #![feature(unsafe_no_drop_flag)]
@@ -78,12 +77,15 @@ pub mod jsapi {
     include!("jsapi_linux_32_debug.rs");
 }
 
+#[macro_use]
+pub mod rust;
+
 mod consts;
 pub mod conversions;
 pub mod error;
 pub mod glue;
 pub mod jsval;
-pub mod rust;
+
 
 pub use consts::*;
 
diff --git a/src/rust.rs b/src/rust.rs
@@ -13,7 +13,6 @@ use std::slice;
 use std::mem;
 use std::u32;
 use std::default::Default;
-use std::intrinsics::return_address;
 use std::ops::{Deref, DerefMut};
 use std::cell::UnsafeCell;
 use std::marker::PhantomData;
@@ -269,39 +268,101 @@ impl RootKind for Value {
     fn rootKind() -> jsapi::RootKind { jsapi::RootKind::Value }
 }
 
-impl<T: RootKind + Copy> Rooted<T> {
-    pub fn new_with_addr(cx: *mut JSContext, initial: T, addr: *const u8) -> Rooted<T> {
-        let ctxfriend: &mut ContextFriendFields = unsafe {
-            mem::transmute(cx)
-        };
-
-        let kind = T::rootKind() as usize;
-        let root = Rooted::<T> {
+impl<T> Rooted<T> {
+    pub fn new_unrooted(initial: T) -> Rooted<T> {
+        Rooted {
             _base: RootedBase { _phantom0: PhantomData },
-            stack: &mut ctxfriend.roots.stackRoots_[kind] as *mut _ as *mut _,
-            prev: ctxfriend.roots.stackRoots_[kind] as *mut _,
+            stack: ptr::null_mut(),
+            prev: ptr::null_mut(),
             ptr: initial,
-        };
+        }
+    }
 
-        ctxfriend.roots.stackRoots_[kind] = unsafe { mem::transmute(addr) };
-        root
+    pub unsafe fn add_to_root_stack(&mut self, cx: *mut JSContext) where T: RootKind {
+        let ctxfriend: &mut ContextFriendFields = mem::transmute(cx);
+
+        let kind = T::rootKind() as usize;
+        self.stack = &mut ctxfriend.roots.stackRoots_[kind] as *mut _ as *mut _;
+        self.prev = ctxfriend.roots.stackRoots_[kind] as *mut _;
+
+        ctxfriend.roots.stackRoots_[kind] = self as *mut _ as usize as _;
     }
 
-    pub fn new(cx: *mut JSContext, initial: T) -> Rooted<T> {
-        Rooted::new_with_addr(cx, initial, unsafe { return_address() })
+    pub unsafe fn remove_from_root_stack(&mut self) {
+        assert!(*self.stack == mem::transmute(&*self));
+        *self.stack = self.prev;
     }
+}
 
-    pub fn handle(&self) -> Handle<T> {
+/// Rust API for keeping a Rooted value in the context's root stack.
+/// Example usage: `rooted!(in(cx) let x = UndefinedValue());`.
+/// `RootedGuard::new` also works, but the macro is preferred.
+pub struct RootedGuard<'a, T: 'a> {
+    root: &'a mut Rooted<T>
+}
+
+impl<'a, T> RootedGuard<'a, T> {
+    pub fn new(cx: *mut JSContext, root: &'a mut Rooted<T>) -> Self where T: RootKind {
+        unsafe {
+            root.add_to_root_stack(cx);
+        }
+        RootedGuard {
+            root: root
+        }
+    }
+
+    pub fn handle(&self) -> Handle<T> where T: Copy {
         unsafe {
-            Handle::from_marked_location(&self.ptr)
+            Handle::from_marked_location(&self.root.ptr)
         }
     }
 
-    pub fn handle_mut(&mut self) -> MutableHandle<T> {
+    pub fn handle_mut(&mut self) -> MutableHandle<T> where T: Copy {
         unsafe {
-            MutableHandle::from_marked_location(&mut self.ptr)
+            MutableHandle::from_marked_location(&mut self.root.ptr)
         }
     }
+
+    pub fn get(&self) -> T where T: Copy {
+        self.root.ptr
+    }
+
+    pub fn set(&mut self, v: T) {
+        self.root.ptr = v;
+    }
+}
+
+impl<'a, T> Deref for RootedGuard<'a, T> {
+    type Target = T;
+    fn deref(&self) -> &T {
+        &self.root.ptr
+    }
+}
+
+impl<'a, T> DerefMut for RootedGuard<'a, T> {
+    fn deref_mut(&mut self) -> &mut T {
+        &mut self.root.ptr
+    }
+}
+
+impl<'a, T> Drop for RootedGuard<'a, T> {
+    fn drop(&mut self) {
+        unsafe {
+            self.root.remove_from_root_stack();
+        }
+    }
+}
+
+#[macro_export]
+macro_rules! rooted {
+    (in($cx:expr) let $name:ident = $init:expr) => {
+        let mut __root = $crate::jsapi::Rooted::new_unrooted($init);
+        let $name = $crate::rust::RootedGuard::new($cx, &mut __root);
+    };
+    (in($cx:expr) let mut $name:ident = $init:expr) => {
+        let mut __root = $crate::jsapi::Rooted::new_unrooted($init);
+        let mut $name = $crate::rust::RootedGuard::new($cx, &mut __root);
+    }
 }
 
 impl<T: Copy> Handle<T> {
@@ -390,18 +451,6 @@ impl<T: Copy> MutableHandle<T> {
     }
 }
 
-impl<T> Drop for Rooted<T> {
-    fn drop(&mut self) {
-        unsafe {
-            if self.stack as usize == mem::POST_DROP_USIZE {
-                return;
-            }
-            assert!(*self.stack == mem::transmute(&*self));
-            *self.stack = self.prev;
-        }
-    }
-}
-
 impl Default for jsid {
     fn default() -> jsid { JSID_VOID }
 }
diff --git a/tests/callback.rs b/tests/callback.rs
@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#[macro_use]
 extern crate js;
 extern crate libc;
 
@@ -14,8 +15,6 @@ use js::jsapi::JS_EncodeStringToUTF8;
 use js::jsapi::JS_NewGlobalObject;
 use js::jsapi::JS_ReportError;
 use js::jsapi::OnNewGlobalHookOption;
-use js::jsapi::Rooted;
-use js::jsapi::RootedValue;
 use js::jsapi::Value;
 use js::jsval::UndefinedValue;
 use js::rust::{Runtime, SIMPLE_GLOBAL_CLASS};
@@ -33,14 +32,14 @@ fn callback() {
 
     unsafe {
         let global = JS_NewGlobalObject(context, &SIMPLE_GLOBAL_CLASS, ptr::null_mut(), h_option, &c_option);
-        let global_root = Rooted::new(context, global);
+        rooted!(in(context) let global_root = global);
         let global = global_root.handle();
         let _ac = JSAutoCompartment::new(context, global.get());
         let function = JS_DefineFunction(context, global, b"puts\0".as_ptr() as *const libc::c_char,
                                          Some(puts), 1, 0);
         assert!(!function.is_null());
         let javascript = "puts('Test Iñtërnâtiônàlizætiøn ┬─┬ノ( º _ ºノ) ');";
-        let mut rval = RootedValue::new(runtime.cx(), UndefinedValue());
+        rooted!(in(context) let mut rval = UndefinedValue());
         let _ = runtime.evaluate_script(global, javascript, "test.js", 0, rval.handle_mut());
     }
 }
@@ -55,7 +54,7 @@ unsafe extern "C" fn puts(context: *mut JSContext, argc: u32, vp: *mut Value) ->
 
     let arg = args.get(0);
     let js = js::rust::ToString(context, arg);
-    let message_root = Rooted::new(context, js);
+    rooted!(in(context) let message_root = js);
     let message = JS_EncodeStringToUTF8(context, message_root.handle());
     let message = CStr::from_ptr(message);
     println!("{}", str::from_utf8(message.to_bytes()).unwrap());
diff --git a/tests/evaluate.rs b/tests/evaluate.rs
@@ -2,13 +2,12 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#[macro_use]
 extern crate js;
 
 use js::jsapi::CompartmentOptions;
 use js::jsapi::JS_NewGlobalObject;
 use js::jsapi::OnNewGlobalHookOption;
-use js::jsapi::RootedObject;
-use js::jsapi::RootedValue;
 use js::jsval::UndefinedValue;
 use js::rust::{Runtime, SIMPLE_GLOBAL_CLASS};
 
@@ -21,12 +20,12 @@ fn evaluate() {
 
     unsafe {
 
-        let global = RootedObject::new(cx,
+        rooted!(in(cx) let global =
             JS_NewGlobalObject(cx, &SIMPLE_GLOBAL_CLASS, ptr::null_mut(),
                                OnNewGlobalHookOption::FireOnNewGlobalHook,
                                &CompartmentOptions::default())
         );
-        let mut rval = RootedValue::new(cx, UndefinedValue());
+        rooted!(in(cx) let mut rval = UndefinedValue());
         assert!(rt.evaluate_script(global.handle(), "1 + 1",
                                    "test", 1, rval.handle_mut()).is_ok());
     }
diff --git a/tests/stack_limit.rs b/tests/stack_limit.rs
@@ -2,13 +2,12 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#[macro_use]
 extern crate js;
 
 use js::jsapi::CompartmentOptions;
 use js::jsapi::JS_NewGlobalObject;
 use js::jsapi::OnNewGlobalHookOption;
-use js::jsapi::Rooted;
-use js::jsapi::RootedValue;
 use js::jsval::UndefinedValue;
 use js::rust::{Runtime, SIMPLE_GLOBAL_CLASS};
 
@@ -24,9 +23,9 @@ fn stack_limit() {
         let c_option = CompartmentOptions::default();
         let global = JS_NewGlobalObject(cx, &SIMPLE_GLOBAL_CLASS,
                                         ptr::null_mut(), h_option, &c_option);
-        let global_root = Rooted::new(cx, global);
+        rooted!(in(cx) let global_root = global);
         let global = global_root.handle();
-        let mut rval = RootedValue::new(cx, UndefinedValue());
+        rooted!(in(cx) let mut rval = UndefinedValue());
         assert!(rt.evaluate_script(global, "function f() { f.apply() } f()",
                                    "test", 1, rval.handle_mut()).is_err());
     }
diff --git a/tests/vec_conversion.rs b/tests/vec_conversion.rs
@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#[macro_use]
 extern crate js;
 
 use js::conversions::ConversionBehavior;
@@ -12,8 +13,6 @@ use js::jsapi::JSAutoCompartment;
 use js::jsapi::JS_InitStandardClasses;
 use js::jsapi::JS_NewGlobalObject;
 use js::jsapi::OnNewGlobalHookOption;
-use js::jsapi::Rooted;
-use js::jsapi::RootedValue;
 use js::jsval::UndefinedValue;
 use js::rust::{Runtime, SIMPLE_GLOBAL_CLASS};
 
@@ -30,13 +29,13 @@ fn vec_conversion() {
     unsafe {
         let global = JS_NewGlobalObject(cx, &SIMPLE_GLOBAL_CLASS,
                                         ptr::null_mut(), h_option, &c_option);
-        let global_root = Rooted::new(cx, global);
+        rooted!(in(cx) let global_root = global);
         let global = global_root.handle();
 
         let _ac = JSAutoCompartment::new(cx, global.get());
         assert!(JS_InitStandardClasses(cx, global));
 
-        let mut rval = RootedValue::new(cx, UndefinedValue());
+        rooted!(in(cx) let mut rval = UndefinedValue());
 
         let orig_vec: Vec<f32> = vec![1.0, 2.9, 3.0];
         orig_vec.to_jsval(cx, rval.handle_mut());
