Backport changes from rlua 0.16 (master branch)

1 files changed, 115 insertions, 108 deletions

diff --git a/src/scope.rs b/src/scope.rs
index a724b29..5fd8332 100644
--- a/src/scope.rs
+++ b/src/scope.rs
@@ -1,67 +1,65 @@
 use std::any::Any;
 use std::cell::RefCell;
-use std::collections::HashMap;
 use std::marker::PhantomData;
 use std::mem;
 use std::os::raw::c_void;
 use std::rc::Rc;
-use std::string::String as StdString;
-
-use error::{Error, Result};
-use ffi;
-use function::Function;
-use lua::Lua;
-use types::Callback;
-use userdata::{AnyUserData, MetaMethod, UserData, UserDataMethods};
-use util::{
+use std::cell::Cell;
+
+use crate::error::{Error, Result};
+use crate::lua::Lua;
+use crate::ffi;
+use crate::function::Function;
+use crate::types::{Callback, LuaRef};
+use crate::userdata::{AnyUserData, MetaMethod, UserData, UserDataMethods};
+use crate::util::{
     assert_stack, init_userdata_metatable, protect_lua_closure, push_string, push_userdata,
     take_userdata, StackGuard,
 };
-use value::{FromLuaMulti, MultiValue, ToLuaMulti, Value};
+use crate::value::{FromLuaMulti, MultiValue, ToLuaMulti, Value};
 
-/// Constructed by the [`Lua::scope`] method, allows temporarily passing to Lua userdata that is
-/// !Send, and callbacks that are !Send and not 'static.
+/// Constructed by the [`Lua::scope`] method, allows temporarily creating Lua userdata and
+/// callbacks that are not required to be Send or 'static.
 ///
 /// See [`Lua::scope`] for more details.
 ///
 /// [`Lua::scope`]: struct.Lua.html#method.scope
-pub struct Scope<'scope> {
-    lua: &'scope Lua,
-    destructors: RefCell<Vec<Box<Fn() -> Box<Any> + 'scope>>>,
-    // 'scope lifetime must be invariant
-    _scope: PhantomData<&'scope mut &'scope ()>,
+pub struct Scope<'lua, 'scope> {
+    lua: &'lua Lua,
+    destructors: RefCell<Vec<(LuaRef<'lua>, fn(LuaRef<'lua>) -> Box<dyn Any>)>>,
+    _scope_invariant: PhantomData<Cell<&'scope ()>>,
 }
 
-impl<'scope> Scope<'scope> {
-    pub(crate) fn new(lua: &'scope Lua) -> Scope {
+impl<'lua, 'scope> Scope<'lua, 'scope> {
+    pub(crate) fn new(lua: &'lua Lua) -> Scope<'lua, 'scope> {
         Scope {
             lua,
             destructors: RefCell::new(Vec::new()),
-            _scope: PhantomData,
+            _scope_invariant: PhantomData,
         }
     }
 
     /// Wraps a Rust function or closure, creating a callable Lua function handle to it.
     ///
-    /// This is a version of [`Lua::create_function`] that creates a callback which expires on scope
-    /// drop.  See [`Lua::scope`] for more details.
+    /// This is a version of [`Lua::create_function`] that creates a callback which expires on
+    /// scope drop.  See [`Lua::scope`] for more details.
     ///
     /// [`Lua::create_function`]: struct.Lua.html#method.create_function
     /// [`Lua::scope`]: struct.Lua.html#method.scope
-    pub fn create_function<'lua, A, R, F>(&'lua self, func: F) -> Result<Function<'lua>>
+    pub fn create_function<'callback, A, R, F>(&'callback self, func: F) -> Result<Function<'lua>>
     where
-        A: FromLuaMulti<'lua>,
-        R: ToLuaMulti<'lua>,
-        F: 'scope + Fn(&'lua Lua, A) -> Result<R>,
+        A: FromLuaMulti<'callback>,
+        R: ToLuaMulti<'callback>,
+        F: 'scope + Fn(&'callback Lua, A) -> Result<R>,
     {
-        // Safe, because 'scope must outlive 'lua (due to Self containing 'scope), however the
+        // Safe, because 'scope must outlive 'callback (due to Self containing 'scope), however the
         // callback itself must be 'scope lifetime, so the function should not be able to capture
-        // anything of 'lua lifetime.  'scope can't be shortened due to being invariant, and the
-        // 'lua lifetime here can't be enlarged due to coming from a universal quantification in
-        // Lua::scope.
+        // anything of 'callback lifetime.  'scope can't be shortened due to being invariant, and
+        // the 'callback lifetime here can't be enlarged due to coming from a universal
+        // quantification in Lua::scope.
         //
         // I hope I got this explanation right, but in any case this is tested with compiletest_rs
-        // to make sure callbacks can't capture handles with lifetimes outside the scope, inside the
+        // to make sure callbacks can't capture handles with lifetime outside the scope, inside the
         // scope, and owned inside the callback itself.
         unsafe {
             self.create_callback(Box::new(move |lua, args| {
@@ -72,17 +70,20 @@ impl<'scope> Scope<'scope> {
 
     /// Wraps a Rust mutable closure, creating a callable Lua function handle to it.
     ///
-    /// This is a version of [`Lua::create_function_mut`] that creates a callback which expires on
-    /// scope drop.  See [`Lua::scope`] and [`Scope::create_function`] for more details.
+    /// This is a version of [`Lua::create_function_mut`] that creates a callback which expires
+    /// on scope drop.  See [`Lua::scope`] and [`Scope::create_function`] for more details.
     ///
     /// [`Lua::create_function_mut`]: struct.Lua.html#method.create_function_mut
     /// [`Lua::scope`]: struct.Lua.html#method.scope
     /// [`Scope::create_function`]: #method.create_function
-    pub fn create_function_mut<'lua, A, R, F>(&'lua self, func: F) -> Result<Function<'lua>>
+    pub fn create_function_mut<'callback, A, R, F>(
+        &'callback self,
+        func: F,
+    ) -> Result<Function<'lua>>
     where
-        A: FromLuaMulti<'lua>,
-        R: ToLuaMulti<'lua>,
-        F: 'scope + FnMut(&'lua Lua, A) -> Result<R>,
+        A: FromLuaMulti<'callback>,
+        R: ToLuaMulti<'callback>,
+        F: 'scope + FnMut(&'callback Lua, A) -> Result<R>,
     {
         let func = RefCell::new(func);
         self.create_function(move |lua, args| {
@@ -94,13 +95,13 @@ impl<'scope> Scope<'scope> {
 
     /// Create a Lua userdata object from a custom userdata type.
     ///
-    /// This is a version of [`Lua::create_userdata`] that creates a userdata which expires on scope
-    /// drop, and does not require that the userdata type be Send (but still requires that the
+    /// This is a version of [`Lua::create_userdata`] that creates a userdata which expires on
+    /// scope drop, and does not require that the userdata type be Send (but still requires that the
     /// UserData be 'static).  See [`Lua::scope`] for more details.
     ///
     /// [`Lua::create_userdata`]: struct.Lua.html#method.create_userdata
     /// [`Lua::scope`]: struct.Lua.html#method.scope
-    pub fn create_static_userdata<'lua, T>(&'lua self, data: T) -> Result<AnyUserData<'lua>>
+    pub fn create_static_userdata<T>(&self, data: T) -> Result<AnyUserData<'lua>>
     where
         T: 'static + UserData,
     {
@@ -108,13 +109,12 @@ impl<'scope> Scope<'scope> {
         // thread while the Scope is alive (or the returned AnyUserData handle even).
         unsafe {
             let u = self.lua.make_userdata(data)?;
-            let mut destructors = self.destructors.borrow_mut();
-            let u_destruct = u.0.clone();
-            destructors.push(Box::new(move || {
-                let state = u_destruct.lua.state;
-                let _sg = StackGuard::new(state);
-                assert_stack(state, 1);
-                u_destruct.lua.push_ref(&u_destruct);
+            self.destructors.borrow_mut().push((u.0.clone(), |u| {
+                let state = u.lua.state;
+                assert_stack(state, 2);
+                u.lua.push_ref(&u);
+                // We know the destructor has not run yet because we hold a reference to the
+                // userdata.
                 Box::new(take_userdata::<RefCell<T>>(state))
             }));
             Ok(u)
@@ -123,9 +123,9 @@ impl<'scope> Scope<'scope> {
 
     /// Create a Lua userdata object from a custom userdata type.
     ///
-    /// This is a version of [`Lua::create_userdata`] that creates a userdata which expires on scope
-    /// drop, and does not require that the userdata type be Send or 'static. See [`Lua::scope`] for
-    /// more details.
+    /// This is a version of [`Lua::create_userdata`] that creates a userdata which expires on
+    /// scope drop, and does not require that the userdata type be Send or 'static. See
+    /// [`Lua::scope`] for more details.
     ///
     /// Lifting the requirement that the UserData type be 'static comes with some important
     /// limitations, so if you only need to eliminate the Send requirement, it is probably better to
@@ -144,34 +144,34 @@ impl<'scope> Scope<'scope> {
     /// [`Lua::create_userdata`]: struct.Lua.html#method.create_userdata
     /// [`Lua::scope`]: struct.Lua.html#method.scope
     /// [`UserDataMethods`]: trait.UserDataMethods.html
-    pub fn create_nonstatic_userdata<'lua, T>(&'lua self, data: T) -> Result<AnyUserData<'lua>>
+    pub fn create_nonstatic_userdata<T>(&self, data: T) -> Result<AnyUserData<'lua>>
     where
         T: 'scope + UserData,
     {
         let data = Rc::new(RefCell::new(data));
 
         // 'callback outliving 'scope is a lie to make the types work out, required due to the
-        // inability to work with the "correct" universally quantified callback type.  This is safe
-        // though, because actual method callbacks are all 'static so they can't capture 'callback
-        // handles anyway.
+        // inability to work with the more correct callback type that is universally quantified over
+        // 'lua.  This is safe though, because `UserData::add_methods` does not get to pick the 'lua
+        // lifetime, so none of the static methods UserData types can add can possibly capture
+        // parameters.
         fn wrap_method<'scope, 'lua, 'callback: 'scope, T: 'scope>(
-            scope: &'lua Scope<'scope>,
+            scope: &Scope<'lua, 'scope>,
             data: Rc<RefCell<T>>,
             method: NonStaticMethod<'callback, T>,
         ) -> Result<Function<'lua>> {
             // On methods that actually receive the userdata, we fake a type check on the passed in
             // userdata, where we pretend there is a unique type per call to
-            // Scope::create_nonstatic_userdata.  You can grab a method from a userdata and call it
-            // on a mismatched userdata type, which when using normal 'static userdata will fail
+            // `Scope::create_nonstatic_userdata`.  You can grab a method from a userdata and call
+            // it on a mismatched userdata type, which when using normal 'static userdata will fail
             // with a type mismatch, but here without this check would proceed as though you had
             // called the method on the original value (since we otherwise completely ignore the
             // first argument).
             let check_data = data.clone();
-            let check_ud_type = move |lua: &Lua, value| {
+            let check_ud_type = move |lua: &'callback Lua, value| {
                 if let Some(value) = value {
                     if let Value::UserData(u) = value {
                         unsafe {
-                            let _sg = StackGuard::new(lua.state);
                             assert_stack(lua.state, 1);
                             lua.push_ref(&u.0);
                             ffi::lua_getuservalue(lua.state, -1);
@@ -248,7 +248,7 @@ impl<'scope> Scope<'scope> {
 
             for (k, m) in ud_methods.meta_methods {
                 push_string(lua.state, k.name())?;
-                lua.push_value(Value::Function(wrap_method(self, data.clone(), m)?));
+                lua.push_value(Value::Function(wrap_method(self, data.clone(), m)?))?;
 
                 protect_lua_closure(lua.state, 3, 1, |state| {
                     ffi::lua_rawset(state, -3);
@@ -263,7 +263,7 @@ impl<'scope> Scope<'scope> {
                 })?;
                 for (k, m) in ud_methods.methods {
                     push_string(lua.state, &k)?;
-                    lua.push_value(Value::Function(wrap_method(self, data.clone(), m)?));
+                    lua.push_value(Value::Function(wrap_method(self, data.clone(), m)?))?;
                     protect_lua_closure(lua.state, 3, 1, |state| {
                         ffi::lua_rawset(state, -3);
                     })?;
@@ -279,25 +279,26 @@ impl<'scope> Scope<'scope> {
         }
     }
 
-    // Unsafe, because the callback (since it is non-'static) can capture any value with 'callback
-    // scope, such as improperly holding onto an argument. So in order for this to be safe, the
-    // callback must NOT capture any arguments.
-    unsafe fn create_callback<'lua, 'callback>(
-        &'lua self,
+    // Unsafe, because the callback can improperly capture any value with 'callback scope, such as
+    // improperly capturing an argument. Since the 'callback lifetime is chosen by the user and the
+    // lifetime of the callback itself is 'scope (non-'static), the borrow checker will happily pick
+    // a 'callback that outlives 'scope to allow this.  In order for this to be safe, the callback
+    // must NOT capture any parameters.
+    unsafe fn create_callback<'callback>(
+        &self,
         f: Callback<'callback, 'scope>,
     ) -> Result<Function<'lua>> {
-        let f = mem::transmute::<Callback<'callback, 'scope>, Callback<'callback, 'static>>(f);
+        let f = mem::transmute::<Callback<'callback, 'scope>, Callback<'lua, 'static>>(f);
         let f = self.lua.create_callback(f)?;
 
         let mut destructors = self.destructors.borrow_mut();
-        let f_destruct = f.0.clone();
-        destructors.push(Box::new(move || {
-            let state = f_destruct.lua.state;
-            let _sg = StackGuard::new(state);
-            assert_stack(state, 2);
-            f_destruct.lua.push_ref(&f_destruct);
+        destructors.push((f.0.clone(), |f| {
+            let state = f.lua.state;
+            assert_stack(state, 3);
+            f.lua.push_ref(&f);
 
             ffi::lua_getupvalue(state, -1, 1);
+            // We know the destructor has not run yet because we hold a reference to the callback.
             let ud = take_userdata::<Callback>(state);
 
             ffi::lua_pushnil(state);
@@ -310,98 +311,104 @@ impl<'scope> Scope<'scope> {
     }
 }
 
-impl<'scope> Drop for Scope<'scope> {
+impl<'lua, 'scope> Drop for Scope<'lua, 'scope> {
     fn drop(&mut self) {
         // We separate the action of invalidating the userdata in Lua and actually dropping the
         // userdata type into two phases.  This is so that, in the event a userdata drop panics, we
         // can be sure that all of the userdata in Lua is actually invalidated.
 
+        // All destructors are non-panicking, so this is fine
         let to_drop = self
             .destructors
             .get_mut()
             .drain(..)
-            .map(|destructor| destructor())
+            .map(|(r, dest)| dest(r))
             .collect::<Vec<_>>();
+
         drop(to_drop);
     }
 }
 
 enum NonStaticMethod<'lua, T> {
-    Method(Box<Fn(&'lua Lua, &T, MultiValue<'lua>) -> Result<MultiValue<'lua>>>),
-    MethodMut(Box<FnMut(&'lua Lua, &mut T, MultiValue<'lua>) -> Result<MultiValue<'lua>>>),
-    Function(Box<Fn(&'lua Lua, MultiValue<'lua>) -> Result<MultiValue<'lua>>>),
-    FunctionMut(Box<FnMut(&'lua Lua, MultiValue<'lua>) -> Result<MultiValue<'lua>>>),
+    Method(Box<dyn Fn(&'lua Lua, &T, MultiValue<'lua>) -> Result<MultiValue<'lua>>>),
+    MethodMut(Box<dyn FnMut(&'lua Lua, &mut T, MultiValue<'lua>) -> Result<MultiValue<'lua>>>),
+    Function(Box<dyn Fn(&'lua Lua, MultiValue<'lua>) -> Result<MultiValue<'lua>>>),
+    FunctionMut(Box<dyn FnMut(&'lua Lua, MultiValue<'lua>) -> Result<MultiValue<'lua>>>),
 }
 
 struct NonStaticUserDataMethods<'lua, T: UserData> {
-    methods: HashMap<StdString, NonStaticMethod<'lua, T>>,
-    meta_methods: HashMap<MetaMethod, NonStaticMethod<'lua, T>>,
+    methods: Vec<(Vec<u8>, NonStaticMethod<'lua, T>)>,
+    meta_methods: Vec<(MetaMethod, NonStaticMethod<'lua, T>)>,
 }
 
 impl<'lua, T: UserData> Default for NonStaticUserDataMethods<'lua, T> {
     fn default() -> NonStaticUserDataMethods<'lua, T> {
         NonStaticUserDataMethods {
-            methods: HashMap::new(),
-            meta_methods: HashMap::new(),
+            methods: Vec::new(),
+            meta_methods: Vec::new(),
         }
     }
 }
 
 impl<'lua, T: UserData> UserDataMethods<'lua, T> for NonStaticUserDataMethods<'lua, T> {
-    fn add_method<A, R, M>(&mut self, name: &str, method: M)
+    fn add_method<S, A, R, M>(&mut self, name: &S, method: M)
     where
+        S: ?Sized + AsRef<[u8]>,
         A: FromLuaMulti<'lua>,
         R: ToLuaMulti<'lua>,
         M: 'static + Send + Fn(&'lua Lua, &T, A) -> Result<R>,
     {
-        self.methods.insert(
-            name.to_owned(),
+        self.methods.push((
+            name.as_ref().to_vec(),
             NonStaticMethod::Method(Box::new(move |lua, ud, args| {
                 method(lua, ud, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua)
             })),
-        );
+        ));
     }
 
-    fn add_method_mut<A, R, M>(&mut self, name: &str, mut method: M)
+    fn add_method_mut<S, A, R, M>(&mut self, name: &S, mut method: M)
     where
+        S: ?Sized + AsRef<[u8]>,
         A: FromLuaMulti<'lua>,
         R: ToLuaMulti<'lua>,
         M: 'static + Send + FnMut(&'lua Lua, &mut T, A) -> Result<R>,
     {
-        self.methods.insert(
-            name.to_owned(),
+        self.methods.push((
+            name.as_ref().to_vec(),
             NonStaticMethod::MethodMut(Box::new(move |lua, ud, args| {
                 method(lua, ud, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua)
             })),
-        );
+        ));
     }
 
-    fn add_function<A, R, F>(&mut self, name: &str, function: F)
+    fn add_function<S, A, R, F>(&mut self, name: &S, function: F)
     where
+        S: ?Sized + AsRef<[u8]>,
         A: FromLuaMulti<'lua>,
         R: ToLuaMulti<'lua>,
         F: 'static + Send + Fn(&'lua Lua, A) -> Result<R>,
     {
-        self.methods.insert(
-            name.to_owned(),
+        self.methods.push((
+            name.as_ref().to_vec(),
             NonStaticMethod::Function(Box::new(move |lua, args| {
                 function(lua, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua)
             })),
-        );
+        ));
     }
 
-    fn add_function_mut<A, R, F>(&mut self, name: &str, mut function: F)
+    fn add_function_mut<S, A, R, F>(&mut self, name: &S, mut function: F)
     where
+        S: ?Sized + AsRef<[u8]>,
         A: FromLuaMulti<'lua>,
         R: ToLuaMulti<'lua>,
         F: 'static + Send + FnMut(&'lua Lua, A) -> Result<R>,
     {
-        self.methods.insert(
-            name.to_owned(),
+        self.methods.push((
+            name.as_ref().to_vec(),
             NonStaticMethod::FunctionMut(Box::new(move |lua, args| {
                 function(lua, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua)
             })),
-        );
+        ));
     }
 
     fn add_meta_method<A, R, M>(&mut self, meta: MetaMethod, method: M)
@@ -410,12 +417,12 @@ impl<'lua, T: UserData> UserDataMethods<'lua, T> for NonStaticUserDataMethods<'l
         R: ToLuaMulti<'lua>,
         M: 'static + Send + Fn(&'lua Lua, &T, A) -> Result<R>,
     {
-        self.meta_methods.insert(
+        self.meta_methods.push((
             meta,
             NonStaticMethod::Method(Box::new(move |lua, ud, args| {
                 method(lua, ud, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua)
             })),
-        );
+        ));
     }
 
     fn add_meta_method_mut<A, R, M>(&mut self, meta: MetaMethod, mut method: M)
@@ -424,12 +431,12 @@ impl<'lua, T: UserData> UserDataMethods<'lua, T> for NonStaticUserDataMethods<'l
         R: ToLuaMulti<'lua>,
         M: 'static + Send + FnMut(&'lua Lua, &mut T, A) -> Result<R>,
     {
-        self.meta_methods.insert(
+        self.meta_methods.push((
             meta,
             NonStaticMethod::MethodMut(Box::new(move |lua, ud, args| {
                 method(lua, ud, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua)
             })),
-        );
+        ));
     }
 
     fn add_meta_function<A, R, F>(&mut self, meta: MetaMethod, function: F)
@@ -438,12 +445,12 @@ impl<'lua, T: UserData> UserDataMethods<'lua, T> for NonStaticUserDataMethods<'l
         R: ToLuaMulti<'lua>,
         F: 'static + Send + Fn(&'lua Lua, A) -> Result<R>,
     {
-        self.meta_methods.insert(
+        self.meta_methods.push((
             meta,
             NonStaticMethod::Function(Box::new(move |lua, args| {
                 function(lua, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua)
             })),
-        );
+        ));
     }
 
     fn add_meta_function_mut<A, R, F>(&mut self, meta: MetaMethod, mut function: F)
@@ -452,11 +459,11 @@ impl<'lua, T: UserData> UserDataMethods<'lua, T> for NonStaticUserDataMethods<'l
         R: ToLuaMulti<'lua>,
         F: 'static + Send + FnMut(&'lua Lua, A) -> Result<R>,
     {
-        self.meta_methods.insert(
+        self.meta_methods.push((
             meta,
             NonStaticMethod::FunctionMut(Box::new(move |lua, args| {
                 function(lua, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua)
             })),
-        );
+        ));
     }
 }