From e5fe222de3a61146ae34c4b85b6be3c91cbcf717 Mon Sep 17 00:00:00 2001 From: Ilya Lakhin Date: Mon, 27 Jul 2020 11:30:09 +0700 Subject: [PATCH 1/6] Shared variant of Dynamic type; All read/write access operations in Dynamic backed by Read/Write lock guards; new shared() script function --- Cargo.toml | 1 + src/any.rs | 287 ++++++++++++++++++++++++++++++++++-- src/engine.rs | 39 ++++- src/fn_call.rs | 36 +++-- src/fn_native.rs | 13 +- src/fn_register.rs | 46 +++--- src/module.rs | 20 +-- src/packages/array_basic.rs | 4 +- src/packages/string_more.rs | 4 +- src/scope.rs | 2 +- src/token.rs | 4 +- 11 files changed, 383 insertions(+), 73 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ec00a10c..85b57974 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,7 @@ no_index = [] # no arrays and indexing no_object = [] # no custom objects no_function = [] # no script-defined functions no_capture = [] # no automatic read/write binding of anonymous function's local variables to it's external context +no_shared = [] # no explicit shared variables in the script code no_module = [] # no modules internals = [] # expose internal data structures unicode-xid-ident = ["unicode-xid"] # allow Unicode Standard Annex #31 for identifiers. diff --git a/src/any.rs b/src/any.rs index 9d28c768..a7983030 100644 --- a/src/any.rs +++ b/src/any.rs @@ -1,6 +1,6 @@ //! Helper module which defines the `Any` trait to to allow dynamic value handling. -use crate::fn_native::{FnPtr, SendSync}; +use crate::fn_native::{FnPtr, SendSync, SharedMut}; use crate::parser::{ImmutableString, INT}; use crate::r#unsafe::{unsafe_cast_box, unsafe_try_cast}; @@ -18,8 +18,15 @@ use crate::stdlib::{ boxed::Box, fmt, string::String, + ops::{Deref, DerefMut} }; +#[cfg(not(feature = "sync"))] +use crate::stdlib::{rc::Rc, cell::{RefCell, Ref, RefMut}}; + +#[cfg(feature = "sync")] +use crate::stdlib::sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard}; + #[cfg(not(feature = "no_object"))] use crate::stdlib::collections::HashMap; @@ -144,6 +151,88 @@ pub enum Union { Map(Box), FnPtr(Box), Variant(Box>), + Shared(Box), +} + +/// Internal Shared Dynamic representation. +/// +/// Created with `Dynamic::into_shared()`. +#[derive(Clone)] +pub struct SharedCell { + value_type_id: TypeId, + value_type_name: &'static str, + container: SharedMut +} + +/// Dynamic's underlying `Variant` read guard that supports `Deref` for Variant's +/// reference reading. +/// +/// This data structure provides transparent interoperability between normal +/// `Dynamic` types and Shared Dynamic references. +#[derive(Debug)] +pub struct DynamicReadLock<'d, T: Variant + Clone>(DynamicReadLockInner<'d, T>); + +#[derive(Debug)] +enum DynamicReadLockInner<'d, T: Variant + Clone> { + Reference(&'d T), + #[cfg(not(feature = "sync"))] + Guard(Ref<'d, Dynamic>), + #[cfg(feature = "sync")] + Guard(RwLockReadGuard<'d, Dynamic>), +} + +impl<'d, T: Variant + Clone> Deref for DynamicReadLock<'d, T> { + type Target = T; + + #[inline(always)] + fn deref(&self) -> &Self::Target { + match &self.0 { + DynamicReadLockInner::Reference(reference) => reference.deref(), + // unwrapping is safe because all checks already done in it's constructor + DynamicReadLockInner::Guard(guard) => guard.downcast_ref().unwrap(), + } + } +} + +/// Dynamic's underlying `Variant` write guard that supports `Deref` and `DerefMut` +/// for Variant's reference reading/writing. +/// +/// This data structure provides transparent interoperability between normal +/// `Dynamic` types and Shared Dynamic references. +#[derive(Debug)] +pub struct DynamicWriteLock<'d, T: Variant + Clone>(DynamicWriteLockInner<'d, T>); + +#[derive(Debug)] +enum DynamicWriteLockInner<'d, T: Variant + Clone> { + Reference(&'d mut T), + #[cfg(not(feature = "sync"))] + Guard(RefMut<'d, Dynamic>), + #[cfg(feature = "sync")] + Guard(RwLockWriteGuard<'d, Dynamic>), +} + +impl<'d, T: Variant + Clone> Deref for DynamicWriteLock<'d, T> { + type Target = T; + + #[inline(always)] + fn deref(&self) -> &Self::Target { + match &self.0 { + DynamicWriteLockInner::Reference(reference) => reference.deref(), + // unwrapping is safe because all checks already done in it's constructor + DynamicWriteLockInner::Guard(guard) => guard.downcast_ref().unwrap(), + } + } +} + +impl<'d, T: Variant + Clone> DerefMut for DynamicWriteLock<'d, T> { + #[inline(always)] + fn deref_mut(&mut self) -> &mut Self::Target { + match &mut self.0 { + DynamicWriteLockInner::Reference(reference) => reference.deref_mut(), + // unwrapping is safe because all checks already done in it's constructor + DynamicWriteLockInner::Guard(guard) => guard.downcast_mut().unwrap(), + } + } } impl Dynamic { @@ -156,7 +245,19 @@ impl Dynamic { } } + /// Does this `Dynamic` hold a shared data type + /// instead of one of the support system primitive types? + pub fn is_shared(&self) -> bool { + match self.0 { + Union::Shared(_) => true, + _ => false, + } + } + /// Is the value held by this `Dynamic` a particular type? + /// + /// If the `Dynamic` is a Shared variant checking is performed on + /// top of it's internal value. pub fn is(&self) -> bool { self.type_id() == TypeId::of::() || match self.0 { @@ -181,6 +282,7 @@ impl Dynamic { Union::Map(_) => TypeId::of::(), Union::FnPtr(_) => TypeId::of::(), Union::Variant(value) => (***value).type_id(), + Union::Shared(cell) => (**cell).value_type_id, } } @@ -203,6 +305,7 @@ impl Dynamic { #[cfg(not(feature = "no_std"))] Union::Variant(value) if value.is::() => "timestamp", Union::Variant(value) => (***value).type_name(), + Union::Shared(cell) => (**cell).value_type_name, } } } @@ -258,6 +361,7 @@ impl fmt::Display for Dynamic { #[cfg(not(feature = "no_std"))] Union::Variant(value) if value.is::() => write!(f, ""), Union::Variant(value) => write!(f, "{}", (*value).type_name()), + Union::Shared(cell) => write!(f, "{}", (**cell).value_type_name), } } } @@ -284,6 +388,7 @@ impl fmt::Debug for Dynamic { #[cfg(not(feature = "no_std"))] Union::Variant(value) if value.is::() => write!(f, ""), Union::Variant(value) => write!(f, "{}", (*value).type_name()), + Union::Shared(cell) => write!(f, "{}", (**cell).value_type_name), } } } @@ -304,6 +409,7 @@ impl Clone for Dynamic { Union::Map(ref value) => Self(Union::Map(value.clone())), Union::FnPtr(ref value) => Self(Union::FnPtr(value.clone())), Union::Variant(ref value) => (***value).clone_into_dynamic(), + Union::Shared(ref cell) => Self(Union::Shared(Box::new((**cell).clone()))) } } } @@ -415,10 +521,53 @@ impl Dynamic { Self(Union::Variant(Box::new(boxed))) } + /// Turns Dynamic into Shared Dynamic variant backed by runtime + /// mutable reference-counter container(`Arc>` or + /// `Rc` depending on `sync` feature). + /// + /// Instances of Shared Dynamic are relatively cheap to clone. All clones of + /// Shared Dynamic references the same chunk of memory. + /// + /// The Dynamic is capable to work transparently with the it's inner + /// data, seamlessly casting between ordinary instances of Dynamic and + /// Shared Dynamic instances. + /// + /// If original value already a Shared variant returns identity. + pub fn into_shared(self) -> Self { + match self.0 { + Union::Shared(..) => self, + _ => { + let cell = SharedCell { + value_type_id: self.type_id(), + value_type_name: self.type_name(), + #[cfg(not(feature = "sync"))] + container: Rc::new(RefCell::new(self)), + #[cfg(feature = "sync")] + container: Arc::new(RwLock::new(self)), + }; + + Self(Union::Shared(Box::new(cell))) + }, + } + } + /// Get a copy of the `Dynamic` value as a specific type. /// Casting to a `Dynamic` just returns as is. /// - /// Returns an error with the name of the value's actual type when the cast fails. + /// Returns None if types mismatched. + /// + /// # Shared Dynamic + /// + /// When accessing Shared Dynamic in sync mode(`sync` feature enabled) + /// can block current thread while the underlined data is being written. + /// + /// When accessing Shared Dynamic in NON-sync mode can **panic** if the data + /// is currently borrowed for write. + /// + /// ## Safety + /// + /// Both situations normally shouldn't happen since all operations in Rhai + /// use pass-by-value data and the script executed in a single thread. /// /// # Example /// @@ -433,6 +582,24 @@ impl Dynamic { pub fn try_cast(self) -> Option { let type_id = TypeId::of::(); + if type_id == TypeId::of::() { + return unsafe_cast_box::<_, T>(Box::new(self)).ok().map(|v| *v); + } + + #[cfg(not(feature = "sync"))] + if let Union::Shared(cell) = self.0 { + let reference = cell.container.borrow(); + + return (*reference).clone().try_cast() + } + + #[cfg(feature = "sync")] + if let Union::Shared(cell) = self.0 { + let read_lock = cell.container.read().unwrap(); + + return (*read_lock).clone().try_cast() + } + if type_id == TypeId::of::() { return match self.0 { Union::Int(value) => unsafe_try_cast(value), @@ -496,9 +663,6 @@ impl Dynamic { _ => None, }; } - if type_id == TypeId::of::() { - return unsafe_cast_box::<_, T>(Box::new(self)).ok().map(|v| *v); - } match self.0 { Union::Variant(value) => (*value).as_box_any().downcast().map(|x| *x).ok(), @@ -511,7 +675,14 @@ impl Dynamic { /// /// # Panics /// - /// Panics if the cast fails (e.g. the type of the actual value is not the same as the specified type). + /// Panics if the cast fails (e.g. the type of the actual value is not the + /// same as the specified type), or if the data held by Shared Dynamic is + /// currently borrowed(when the `sync` feature disabled). + /// + /// # Notes + /// + /// If the `sync` feature enabled Shared Dynamic can block current thread + /// while the data being written. /// /// # Example /// @@ -531,7 +702,104 @@ impl Dynamic { /// Casting to `Dynamic` just returns a reference to it. /// Returns `None` if the cast fails. #[inline(always)] - pub fn downcast_ref(&self) -> Option<&T> { + pub fn read_lock(&self) -> Option> { + match self.0 { + Union::Shared(ref cell) => { + let type_id = TypeId::of::(); + + if type_id != TypeId::of::() && cell.value_type_id != type_id { + return None + } + + #[cfg(not(feature = "sync"))] + return Some(DynamicReadLock(DynamicReadLockInner::Guard( + cell.container.borrow() + ))); + + #[cfg(feature = "sync")] + return Some(DynamicReadLock(DynamicReadLockInner::Guard( + cell.container.read().unwrap() + ))); + }, + _ => { + self.downcast_ref().map(|reference| { + DynamicReadLock(DynamicReadLockInner::Reference(reference)) + }) + } + } + } + + /// Get a copy of a specific type to the `Dynamic`. + /// Casting to `Dynamic` just returns a reference to it. + /// Returns `None` if the cast fails. + #[inline(always)] + pub fn read(&self) -> Option { + match self.0 { + Union::Shared(ref cell) => { + let type_id = TypeId::of::(); + + if type_id != TypeId::of::() && cell.value_type_id != type_id { + return None + } + + #[cfg(not(feature = "sync"))] + return Some(cell + .container + .borrow() + .deref() + .downcast_ref::() + .unwrap() + .clone()); + + #[cfg(feature = "sync")] + return Some(cell + .container + .read() + .unwrap() + .deref() + .downcast_ref::() + .unwrap() + .clone()); + }, + _ => { + self.downcast_ref().cloned() + } + } + } + + /// Get a mutable reference of a specific type to the `Dynamic`. + /// Casting to `Dynamic` just returns a mutable reference to it. + /// Returns `None` if the cast fails. + #[inline(always)] + pub fn write_lock(&mut self) -> Option> { + match self.0 { + Union::Shared(ref cell) => { + let type_id = TypeId::of::(); + + if type_id != TypeId::of::() && cell.value_type_id != type_id { + return None + } + + #[cfg(not(feature = "sync"))] + return Some(DynamicWriteLock(DynamicWriteLockInner::Guard( + cell.container.borrow_mut() + ))); + + #[cfg(feature = "sync")] + return Some(DynamicWriteLock(DynamicWriteLockInner::Guard( + cell.container.write().unwrap() + ))); + }, + _ => { + self.downcast_mut().map(|reference| { + DynamicWriteLock(DynamicWriteLockInner::Reference(reference)) + }) + } + } + } + + #[inline(always)] + fn downcast_ref(&self) -> Option<&T> { let type_id = TypeId::of::(); if type_id == TypeId::of::() { @@ -607,11 +875,8 @@ impl Dynamic { } } - /// Get a mutable reference of a specific type to the `Dynamic`. - /// Casting to `Dynamic` just returns a mutable reference to it. - /// Returns `None` if the cast fails. #[inline(always)] - pub fn downcast_mut(&mut self) -> Option<&mut T> { + fn downcast_mut(&mut self) -> Option<&mut T> { let type_id = TypeId::of::(); if type_id == TypeId::of::() { diff --git a/src/engine.rs b/src/engine.rs index 7806382f..06ff8d7f 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -91,6 +91,7 @@ pub const KEYWORD_EVAL: &str = "eval"; pub const KEYWORD_FN_PTR: &str = "Fn"; pub const KEYWORD_FN_PTR_CALL: &str = "call"; pub const KEYWORD_FN_PTR_CURRY: &str = "curry"; +pub const KEYWORD_SHARED: &str = "shared"; pub const KEYWORD_THIS: &str = "this"; pub const FN_TO_STRING: &str = "to_string"; #[cfg(not(feature = "no_object"))] @@ -1068,6 +1069,18 @@ impl Engine { let val = target.as_mut(); + // if val.is_shared() { + // return self.get_indexed_mut( + // state, + // lib, + // &mut Target::Value(val.read::().unwrap()), + // idx, + // idx_pos, + // create, + // level, + // ); + // } + match val { #[cfg(not(feature = "no_index"))] Dynamic(Union::Array(arr)) => { @@ -1102,7 +1115,7 @@ impl Engine { map.entry(index).or_insert(Default::default()).into() } else { let index = _idx - .downcast_ref::() + .read_lock::() .ok_or_else(|| EvalAltResult::ErrorStringIndexExpr(idx_pos))?; map.get_mut(index.as_str()) @@ -1280,7 +1293,11 @@ impl Engine { )), // Normal assignment ScopeEntryType::Normal if op.is_empty() => { - *lhs_ptr = rhs_val; + if lhs_ptr.is_shared() { + *lhs_ptr.write_lock::().unwrap() = rhs_val; + } else { + *lhs_ptr = rhs_val; + } Ok(Default::default()) } // Op-assignment - in order of precedence: @@ -1298,8 +1315,13 @@ impl Engine { .get_fn(hash_fn, false) .or_else(|| self.packages.get_fn(hash_fn, false)) { - // Overriding exact implementation - func(self, lib, &mut [lhs_ptr, &mut rhs_val])?; + if lhs_ptr.is_shared() { + // Overriding exact implementation + func(self, lib, &mut [&mut lhs_ptr.write_lock::().unwrap(), &mut rhs_val])?; + } else { + // Overriding exact implementation + func(self, lib, &mut [lhs_ptr, &mut rhs_val])?; + } } else if run_builtin_op_assignment(op, lhs_ptr, &rhs_val)?.is_none() { // Not built in, map to `var = var op rhs` let op = &op[..op.len() - 1]; // extract operator without = @@ -1313,8 +1335,13 @@ impl Engine { level, ) .map_err(|err| err.new_position(*op_pos))?; - // Set value to LHS - *lhs_ptr = value; + if lhs_ptr.is_shared() { + // Set value to LHS + *lhs_ptr.write_lock::().unwrap() = value; + } else { + // Set value to LHS + *lhs_ptr = value; + } } Ok(Default::default()) } diff --git a/src/fn_call.rs b/src/fn_call.rs index 162410f3..6ca46046 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -5,7 +5,7 @@ use crate::calc_fn_hash; use crate::engine::{ search_imports, search_namespace, search_scope_only, Engine, Imports, State, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_FN_PTR_CALL, KEYWORD_FN_PTR_CURRY, KEYWORD_PRINT, - KEYWORD_TYPE_OF, + KEYWORD_TYPE_OF, KEYWORD_SHARED }; use crate::error::ParseErrorType; use crate::fn_native::{FnCallArgs, FnPtr}; @@ -16,6 +16,7 @@ use crate::result::EvalAltResult; use crate::scope::Scope; use crate::token::Position; use crate::utils::StaticVec; +use crate::stdlib::ops::Deref; #[cfg(not(feature = "no_function"))] use crate::{ @@ -540,7 +541,7 @@ impl Engine { let (result, updated) = if _fn_name == KEYWORD_FN_PTR_CALL && obj.is::() { // FnPtr call - let fn_ptr = obj.downcast_ref::().unwrap(); + let fn_ptr = obj.read_lock::().unwrap(); let mut curry = fn_ptr.curry().iter().cloned().collect::>(); // Redirect function name let fn_name = fn_ptr.fn_name(); @@ -578,7 +579,7 @@ impl Engine { ) } else if _fn_name == KEYWORD_FN_PTR_CURRY && obj.is::() { // Curry call - let fn_ptr = obj.downcast_ref::().unwrap(); + let fn_ptr = obj.read_lock::().unwrap(); Ok(( FnPtr::new_unchecked( fn_ptr.get_fn_name().clone(), @@ -599,9 +600,9 @@ impl Engine { // Check if it is a map method call in OOP style #[cfg(not(feature = "no_object"))] - if let Some(map) = obj.downcast_ref::() { + if let Some(map) = obj.read_lock::() { if let Some(val) = map.get(_fn_name) { - if let Some(f) = val.downcast_ref::() { + if let Some(f) = val.read_lock::() { // Remap the function name redirected = f.get_fn_name().clone(); _fn_name = &redirected; @@ -697,6 +698,15 @@ impl Engine { .into()); } + // Handle shared() + #[cfg(not(feature = "no_shared"))] + if name == KEYWORD_SHARED && args_expr.len() == 1 { + let expr = args_expr.get(0).unwrap(); + let value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + + return Ok(value.into_shared()); + } + // Handle eval() if name == KEYWORD_EVAL && args_expr.len() == 1 { let hash_fn = calc_fn_hash(empty(), name, 1, once(TypeId::of::())); @@ -984,8 +994,8 @@ pub fn run_builtin_binary_op( _ => (), } } else if args_type == TypeId::of::() { - let x = x.downcast_ref::().unwrap(); - let y = y.downcast_ref::().unwrap(); + let x = &*x.read_lock::().unwrap(); + let y = &*y.read_lock::().unwrap(); match op { "+" => return Ok(Some((x + y).into())), @@ -1058,7 +1068,7 @@ pub fn run_builtin_op_assignment( } if args_type == TypeId::of::() { - let x = x.downcast_mut::().unwrap(); + let mut x = x.write_lock::().unwrap(); let y = y.clone().cast::(); #[cfg(not(feature = "unchecked"))] @@ -1094,7 +1104,7 @@ pub fn run_builtin_op_assignment( _ => (), } } else if args_type == TypeId::of::() { - let x = x.downcast_mut::().unwrap(); + let mut x = x.write_lock::().unwrap(); let y = y.clone().cast::(); match op { @@ -1103,18 +1113,18 @@ pub fn run_builtin_op_assignment( _ => (), } } else if args_type == TypeId::of::() { - let x = x.downcast_mut::().unwrap(); - let y = y.downcast_ref::().unwrap(); + let mut x = x.write_lock::().unwrap(); + let y = y.read_lock::().unwrap(); match op { - "+=" => return Ok(Some(*x += y)), + "+=" => return Ok(Some(*x += y.deref())), _ => (), } } #[cfg(not(feature = "no_float"))] if args_type == TypeId::of::() { - let x = x.downcast_mut::().unwrap(); + let mut x = x.write_lock::().unwrap(); let y = y.clone().cast::(); match op { diff --git a/src/fn_native.rs b/src/fn_native.rs index 5751351c..e51bf71b 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -1,6 +1,6 @@ //! Module defining interfaces to native-Rust functions. -use crate::any::Dynamic; +use crate::any::{Dynamic, Variant}; use crate::calc_fn_hash; use crate::engine::Engine; use crate::module::Module; @@ -18,9 +18,9 @@ use crate::stdlib::{boxed::Box, convert::TryFrom, fmt, iter::empty, string::Stri use crate::stdlib::mem; #[cfg(not(feature = "sync"))] -use crate::stdlib::rc::Rc; +use crate::stdlib::{rc::Rc, cell::{RefCell, Ref, RefMut}}; #[cfg(feature = "sync")] -use crate::stdlib::sync::Arc; +use crate::stdlib::sync::{Arc, RwLock}; /// Trait that maps to `Send + Sync` only under the `sync` feature. #[cfg(feature = "sync")] @@ -34,11 +34,18 @@ pub trait SendSync {} #[cfg(not(feature = "sync"))] impl SendSync for T {} +/// Immutable reference counting container #[cfg(not(feature = "sync"))] pub type Shared = Rc; #[cfg(feature = "sync")] pub type Shared = Arc; +/// Mutable reference counting container(read-write lock) +#[cfg(not(feature = "sync"))] +pub type SharedMut = Rc>; +#[cfg(feature = "sync")] +pub type SharedMut = Arc>; + /// Consume a `Shared` resource and return a mutable reference to the wrapped value. /// If the resource is shared (i.e. has other outstanding references), a cloned copy is used. pub fn shared_make_mut(value: &mut Shared) -> &mut T { diff --git a/src/fn_register.rs b/src/fn_register.rs index c2eb5534..ddcddff3 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -2,7 +2,7 @@ #![allow(non_snake_case)] -use crate::any::{Dynamic, Variant}; +use crate::any::{Dynamic, Variant, DynamicWriteLock}; use crate::engine::Engine; use crate::fn_native::{CallableFunction, FnAny, FnCallArgs, SendSync}; use crate::module::Module; @@ -97,11 +97,11 @@ pub trait RegisterResultFn { pub struct Mut(T); //pub struct Ref(T); -/// Dereference into &mut. +/// Dereference into DynamicWriteLock #[inline(always)] -pub fn by_ref(data: &mut Dynamic) -> &mut T { - // Directly cast the &mut Dynamic into &mut T to access the underlying data. - data.downcast_mut::().unwrap() +pub fn by_ref(data: &mut Dynamic) -> DynamicWriteLock { + // Directly cast the &mut Dynamic into DynamicWriteLock to access the underlying data. + data.write_lock::().unwrap() } /// Dereference into value. @@ -124,24 +124,23 @@ pub fn by_value(data: &mut Dynamic) -> T { /// This macro creates a closure wrapping a registered function. macro_rules! make_func { - ($fn:ident : $map:expr ; $($par:ident => $convert:expr),*) => { + ($fn:ident : $map:expr ; $($par:ident => $let:stmt => $convert:expr => $arg:expr),*) => { // ^ function pointer // ^ result mapping function // ^ function parameter generic type name (A, B, C etc.) -// ^ dereferencing function +// ^ argument let statement(e.g. let mut A ...) +// ^ dereferencing function +// ^ argument reference expression(like A, *B, &mut C etc) Box::new(move |_: &Engine, _: &Module, args: &mut FnCallArgs| { // The arguments are assumed to be of the correct number and types! let mut _drain = args.iter_mut(); - $( - // Downcast every element, panic in case of a type mismatch (which shouldn't happen). - // Call the user-supplied function using ($convert) to access it either by value or by reference. - let $par = ($convert)(_drain.next().unwrap()); - )* + $($let)* + $($par = ($convert)(_drain.next().unwrap()); )* // Call the function with each parameter value - let r = $fn($($par),*); + let r = $fn($($arg),*); // Map the result $map(r) @@ -181,12 +180,13 @@ macro_rules! def_register { () => { def_register!(imp from_pure :); }; - (imp $abi:ident : $($par:ident => $mark:ty => $param:ty => $clone:expr),*) => { + (imp $abi:ident : $($par:ident => $arg:expr => $mark:ty => $param:ty => $let:stmt => $clone:expr),*) => { // ^ function ABI type // ^ function parameter generic type name (A, B, C etc.) - // ^ function parameter marker type (T, Ref or Mut) - // ^ function parameter actual type (T, &T or &mut T) - // ^ dereferencing function +// ^ call argument(like A, *B, &mut C etc) + // ^ function parameter marker type (T, Ref or Mut) + // ^ function parameter actual type (T, &T or &mut T) + // ^ argument let statement impl< $($par: Variant + Clone,)* FN: Fn($($param),*) -> RET + SendSync + 'static, @@ -196,7 +196,7 @@ macro_rules! def_register { fn register_fn(&mut self, name: &str, f: FN) -> &mut Self { self.global_module.set_fn(name, FnAccess::Public, &[$(map_type_id::<$par>()),*], - CallableFunction::$abi(make_func!(f : map_dynamic ; $($par => $clone),*)) + CallableFunction::$abi(make_func!(f : map_dynamic ; $($par => $let => $clone => $arg),*)) ); self } @@ -210,7 +210,7 @@ macro_rules! def_register { fn register_result_fn(&mut self, name: &str, f: FN) -> &mut Self { self.global_module.set_fn(name, FnAccess::Public, &[$(map_type_id::<$par>()),*], - CallableFunction::$abi(make_func!(f : map_result ; $($par => $clone),*)) + CallableFunction::$abi(make_func!(f : map_result ; $($par => $let => $clone => $arg),*)) ); self } @@ -219,11 +219,11 @@ macro_rules! def_register { //def_register!(imp_pop $($par => $mark => $param),*); }; ($p0:ident $(, $p:ident)*) => { - def_register!(imp from_pure : $p0 => $p0 => $p0 => by_value $(, $p => $p => $p => by_value)*); - def_register!(imp from_method : $p0 => Mut<$p0> => &mut $p0 => by_ref $(, $p => $p => $p => by_value)*); + def_register!(imp from_pure : $p0 => $p0 => $p0 => $p0 => let $p0 => by_value $(, $p => $p => $p => $p => let $p => by_value)*); + def_register!(imp from_method : $p0 => &mut $p0 => Mut<$p0> => &mut $p0 => let mut $p0 => by_ref $(, $p => $p => $p => $p => let $p => by_value)*); // ^ CallableFunction - // handle the first parameter ^ first parameter passed through - // ^ others passed by value (by_value) + // handle the first parameter ^ first parameter passed through + // ^ others passed by value (by_value) // Currently does not support first argument which is a reference, as there will be // conflicting implementations since &T: Any and T: Any cannot be distinguished diff --git a/src/module.rs b/src/module.rs index 8eacde7b..fa23abd6 100644 --- a/src/module.rs +++ b/src/module.rs @@ -442,7 +442,7 @@ impl Module { /// // Since it is a primary type, it can also be cheaply copied /// let double = args[1].clone().cast::(); /// // Get a mutable reference to the first argument. - /// let x = args[0].downcast_mut::().unwrap(); + /// let mut x = args[0].write_lock::().unwrap(); /// /// let orig = *x; /// @@ -534,7 +534,7 @@ impl Module { func: impl Fn(&mut A) -> FuncReturn + SendSync + 'static, ) -> u64 { let f = move |_: &Engine, _: &Module, args: &mut FnCallArgs| { - func(args[0].downcast_mut::().unwrap()).map(Dynamic::from) + func(&mut args[0].write_lock::().unwrap()).map(Dynamic::from) }; let arg_types = [TypeId::of::()]; self.set_fn(name, Public, &arg_types, Func::from_method(Box::new(f))) @@ -615,9 +615,9 @@ impl Module { ) -> u64 { let f = move |_: &Engine, _: &Module, args: &mut FnCallArgs| { let b = mem::take(args[1]).cast::(); - let a = args[0].downcast_mut::().unwrap(); + let mut a = args[0].write_lock::().unwrap(); - func(a, b).map(Dynamic::from) + func(&mut a, b).map(Dynamic::from) }; let arg_types = [TypeId::of::(), TypeId::of::()]; self.set_fn(name, Public, &arg_types, Func::from_method(Box::new(f))) @@ -739,9 +739,9 @@ impl Module { let f = move |_: &Engine, _: &Module, args: &mut FnCallArgs| { let b = mem::take(args[1]).cast::(); let c = mem::take(args[2]).cast::(); - let a = args[0].downcast_mut::().unwrap(); + let mut a = args[0].write_lock::().unwrap(); - func(a, b, c).map(Dynamic::from) + func(&mut a, b, c).map(Dynamic::from) }; let arg_types = [TypeId::of::(), TypeId::of::(), TypeId::of::()]; self.set_fn(name, Public, &arg_types, Func::from_method(Box::new(f))) @@ -773,9 +773,9 @@ impl Module { let f = move |_: &Engine, _: &Module, args: &mut FnCallArgs| { let b = mem::take(args[1]).cast::(); let c = mem::take(args[2]).cast::(); - let a = args[0].downcast_mut::().unwrap(); + let mut a = args[0].write_lock::().unwrap(); - func(a, b, c).map(Dynamic::from) + func(&mut a, b, c).map(Dynamic::from) }; let arg_types = [TypeId::of::(), TypeId::of::(), TypeId::of::()]; self.set_fn( @@ -896,9 +896,9 @@ impl Module { let b = mem::take(args[1]).cast::(); let c = mem::take(args[2]).cast::(); let d = mem::take(args[3]).cast::(); - let a = args[0].downcast_mut::().unwrap(); + let mut a = args[0].write_lock::().unwrap(); - func(a, b, c, d).map(Dynamic::from) + func(&mut a, b, c, d).map(Dynamic::from) }; let arg_types = [ TypeId::of::(), diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index c0bea7ee..5ca3b1c7 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -34,7 +34,7 @@ fn pad( _: &Module, args: &mut [&mut Dynamic], ) -> FuncReturn<()> { - let len = *args[1].downcast_ref::().unwrap(); + let len = *args[1].read_lock::().unwrap(); // Check if array will be over max size limit #[cfg(not(feature = "unchecked"))] @@ -52,7 +52,7 @@ fn pad( if len > 0 { let item = args[2].clone(); - let list = args[0].downcast_mut::().unwrap(); + let mut list = args[0].write_lock::().unwrap(); if len as usize > list.len() { list.resize(len as usize, item); diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index 6ddc2b0c..b50d6915 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -228,7 +228,7 @@ def_package!(crate:MoreStringPackage:"Additional string utilities, including str "pad", &[TypeId::of::(), TypeId::of::(), TypeId::of::()], |_engine: &Engine, _: &Module, args: &mut [&mut Dynamic]| { - let len = *args[1].downcast_ref::< INT>().unwrap(); + let len = *args[1].read_lock::< INT>().unwrap(); // Check if string will be over max size limit #[cfg(not(feature = "unchecked"))] @@ -243,7 +243,7 @@ def_package!(crate:MoreStringPackage:"Additional string utilities, including str if len > 0 { let ch = mem::take(args[2]).cast::(); - let s = args[0].downcast_mut::().unwrap(); + let mut s = args[0].write_lock::().unwrap(); let orig_len = s.chars().count(); diff --git a/src/scope.rs b/src/scope.rs index 5693554c..b095f782 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -333,7 +333,7 @@ impl<'a> Scope<'a> { .iter() .rev() .find(|Entry { name: key, .. }| name == key) - .and_then(|Entry { value, .. }| value.downcast_ref::().cloned()) + .and_then(|Entry { value, .. }| value.read::()) } /// Update the value of the named entry. diff --git a/src/token.rs b/src/token.rs index 7d1a07ba..bd496afb 100644 --- a/src/token.rs +++ b/src/token.rs @@ -2,7 +2,7 @@ use crate::engine::{ Engine, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_FN_PTR_CALL, KEYWORD_FN_PTR_CURRY, - KEYWORD_PRINT, KEYWORD_THIS, KEYWORD_TYPE_OF, + KEYWORD_SHARED, KEYWORD_PRINT, KEYWORD_THIS, KEYWORD_TYPE_OF, }; use crate::error::LexError; @@ -507,7 +507,7 @@ impl Token { | "async" | "await" | "yield" => Reserved(syntax.into()), KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_TYPE_OF | KEYWORD_EVAL | KEYWORD_FN_PTR - | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY | KEYWORD_THIS => Reserved(syntax.into()), + | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY | KEYWORD_SHARED | KEYWORD_THIS => Reserved(syntax.into()), _ => return None, }) From aa87a7f5efee45f712e20e4601e5954c26ec2e2f Mon Sep 17 00:00:00 2001 From: Ilya Lakhin Date: Fri, 31 Jul 2020 05:34:20 +0700 Subject: [PATCH 2/6] Fixes in Engine to properly interpret Shared Dynamic --- src/any.rs | 43 +++++++++++-- src/engine.rs | 105 +++++++++++++++++++------------ src/fn_call.rs | 1 - src/fn_native.rs | 2 +- src/fn_register.rs | 2 +- src/parser.rs | 28 +++++++-- src/token.rs | 11 +++- tests/closures.rs | 151 ++++++++++++++++++++++++++++++++++++++++++++- 8 files changed, 288 insertions(+), 55 deletions(-) diff --git a/src/any.rs b/src/any.rs index a7983030..7990ff10 100644 --- a/src/any.rs +++ b/src/any.rs @@ -18,7 +18,7 @@ use crate::stdlib::{ boxed::Box, fmt, string::String, - ops::{Deref, DerefMut} + ops::{DerefMut, Deref} }; #[cfg(not(feature = "sync"))] @@ -259,11 +259,13 @@ impl Dynamic { /// If the `Dynamic` is a Shared variant checking is performed on /// top of it's internal value. pub fn is(&self) -> bool { - self.type_id() == TypeId::of::() - || match self.0 { - Union::Str(_) => TypeId::of::() == TypeId::of::(), - _ => false, - } + let mut target_type_id = TypeId::of::(); + + if target_type_id == TypeId::of::() { + target_type_id = TypeId::of::(); + } + + self.type_id() == target_type_id } /// Get the TypeId of the value held by this `Dynamic`. @@ -606,6 +608,7 @@ impl Dynamic { _ => None, }; } + #[cfg(not(feature = "no_float"))] if type_id == TypeId::of::() { return match self.0 { @@ -613,30 +616,35 @@ impl Dynamic { _ => None, }; } + if type_id == TypeId::of::() { return match self.0 { Union::Bool(value) => unsafe_try_cast(value), _ => None, }; } + if type_id == TypeId::of::() { return match self.0 { Union::Str(value) => unsafe_try_cast(value), _ => None, }; } + if type_id == TypeId::of::() { return match self.0 { Union::Str(value) => unsafe_try_cast(value.into_owned()), _ => None, }; } + if type_id == TypeId::of::() { return match self.0 { Union::Char(value) => unsafe_try_cast(value), _ => None, }; } + #[cfg(not(feature = "no_index"))] if type_id == TypeId::of::() { return match self.0 { @@ -644,6 +652,7 @@ impl Dynamic { _ => None, }; } + #[cfg(not(feature = "no_object"))] if type_id == TypeId::of::() { return match self.0 { @@ -651,12 +660,14 @@ impl Dynamic { _ => None, }; } + if type_id == TypeId::of::() { return match self.0 { Union::FnPtr(value) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v), _ => None, }; } + if type_id == TypeId::of::<()>() { return match self.0 { Union::Unit(value) => unsafe_try_cast(value), @@ -951,6 +962,7 @@ impl Dynamic { pub fn as_int(&self) -> Result { match self.0 { Union::Int(n) => Ok(n), + Union::Shared(_) => self.read::().ok_or_else(|| self.type_name()), _ => Err(self.type_name()), } } @@ -961,6 +973,7 @@ impl Dynamic { pub fn as_float(&self) -> Result { match self.0 { Union::Float(n) => Ok(n), + Union::Shared(_) => self.read::().ok_or_else(|| self.type_name()), _ => Err(self.type_name()), } } @@ -970,6 +983,7 @@ impl Dynamic { pub fn as_bool(&self) -> Result { match self.0 { Union::Bool(b) => Ok(b), + Union::Shared(_) => self.read::().ok_or_else(|| self.type_name()), _ => Err(self.type_name()), } } @@ -979,12 +993,15 @@ impl Dynamic { pub fn as_char(&self) -> Result { match self.0 { Union::Char(n) => Ok(n), + Union::Shared(_) => self.read::().ok_or_else(|| self.type_name()), _ => Err(self.type_name()), } } /// Cast the `Dynamic` as a string and return the string slice. /// Returns the name of the actual type if the cast fails. + /// + /// Cast is failing if `self` is Shared Dynamic pub fn as_str(&self) -> Result<&str, &'static str> { match &self.0 { Union::Str(s) => Ok(s), @@ -1006,6 +1023,20 @@ impl Dynamic { match self.0 { Union::Str(s) => Ok(s), Union::FnPtr(f) => Ok(f.take_data().0), + Union::Shared(cell) => { + #[cfg(not(feature = "sync"))] + match &cell.container.borrow().deref().0 { + Union::Str(s) => Ok(s.clone()), + Union::FnPtr(f) => Ok(f.clone().take_data().0), + _ => Err(cell.value_type_name), + } + #[cfg(feature = "sync")] + match &cell.container.read().deref().0 { + Union::Str(s) => Ok(s.clone()), + Union::FnPtr(f) => Ok(f.clone().take_data().0), + _ => Err(cell.value_type_name), + } + } _ => Err(self.type_name()), } } diff --git a/src/engine.rs b/src/engine.rs index 06ff8d7f..8f727211 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1,6 +1,6 @@ //! Main module defining the script evaluation `Engine`. -use crate::any::{map_std_type_name, Dynamic, Union}; +use crate::any::{map_std_type_name, Dynamic, Union, DynamicWriteLock}; use crate::calc_fn_hash; use crate::fn_call::run_builtin_op_assignment; use crate::fn_native::{CallableFunction, Callback, FnPtr}; @@ -39,6 +39,7 @@ use crate::stdlib::{ iter::{empty, once}, string::{String, ToString}, vec::Vec, + ops::DerefMut, }; #[cfg(not(feature = "no_index"))] @@ -123,6 +124,9 @@ pub enum ChainType { pub enum Target<'a> { /// The target is a mutable reference to a `Dynamic` value somewhere. Ref(&'a mut Dynamic), + /// The target is a mutable reference to a Shared `Dynamic` value. + /// It holds the access guard and the original container both for cloning purposes + LockGuard((DynamicWriteLock<'a, Dynamic>, Dynamic)), /// The target is a temporary `Dynamic` value (i.e. the mutation can cause no side effects). Value(Dynamic), /// The target is a character inside a String. @@ -137,6 +141,7 @@ impl Target<'_> { pub fn is_ref(&self) -> bool { match self { Self::Ref(_) => true, + Self::LockGuard(_) => true, Self::Value(_) => false, #[cfg(not(feature = "no_index"))] Self::StringChar(_, _, _) => false, @@ -146,6 +151,7 @@ impl Target<'_> { pub fn is_value(&self) -> bool { match self { Self::Ref(_) => false, + Self::LockGuard(_) => false, Self::Value(_) => true, #[cfg(not(feature = "no_index"))] Self::StringChar(_, _, _) => false, @@ -156,6 +162,7 @@ impl Target<'_> { pub fn is(&self) -> bool { match self { Target::Ref(r) => r.is::(), + Target::LockGuard((r, _)) => r.is::(), Target::Value(r) => r.is::(), #[cfg(not(feature = "no_index"))] Target::StringChar(_, _, _) => TypeId::of::() == TypeId::of::(), @@ -165,6 +172,7 @@ impl Target<'_> { pub fn clone_into_dynamic(self) -> Dynamic { match self { Self::Ref(r) => r.clone(), // Referenced value is cloned + Self::LockGuard((_, orig)) => orig, // Return original container of the Shared Dynamic Self::Value(v) => v, // Owned value is simply taken #[cfg(not(feature = "no_index"))] Self::StringChar(_, _, ch) => ch, // Character is taken @@ -174,6 +182,7 @@ impl Target<'_> { pub fn as_mut(&mut self) -> &mut Dynamic { match self { Self::Ref(r) => *r, + Self::LockGuard((r, _)) => r.deref_mut(), Self::Value(ref mut r) => r, #[cfg(not(feature = "no_index"))] Self::StringChar(_, _, ref mut r) => r, @@ -184,13 +193,16 @@ impl Target<'_> { pub fn set_value(&mut self, new_val: Dynamic) -> Result<(), Box> { match self { Self::Ref(r) => **r = new_val, + Self::LockGuard((r, _)) => **r = new_val, Self::Value(_) => { return Err(Box::new(EvalAltResult::ErrorAssignmentToUnknownLHS( Position::none(), ))) } #[cfg(not(feature = "no_index"))] - Self::StringChar(Dynamic(Union::Str(ref mut s)), index, _) => { + Self::StringChar(string, index, _) if string.is::() => { + let mut s = string.write_lock::().unwrap(); + // Replace the character at the specified index position let new_ch = new_val .as_char() @@ -216,7 +228,13 @@ impl Target<'_> { #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] impl<'a> From<&'a mut Dynamic> for Target<'a> { fn from(value: &'a mut Dynamic) -> Self { - Self::Ref(value) + if value.is_shared() { + // clone is cheap since it holds Arc/Rw under the hood + let container = value.clone(); + Self::LockGuard((value.write_lock::().unwrap(), container)) + } else { + Self::Ref(value) + } } } #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] @@ -676,15 +694,46 @@ impl Engine { } // xxx[rhs] = new_val _ if _new_val.is_some() => { - let mut new_val = _new_val.unwrap(); let mut idx_val2 = idx_val.clone(); + // `next` is introduced to bypass double mutable borrowing of target + #[cfg(not(feature = "no_index"))] + let mut next: Option<(u8, Dynamic)>; + match self.get_indexed_mut(state, lib, target, idx_val, pos, true, level) { // Indexed value is an owned value - the only possibility is an indexer // Try to call an index setter #[cfg(not(feature = "no_index"))] Ok(obj_ptr) if obj_ptr.is_value() => { - let args = &mut [target.as_mut(), &mut idx_val2, &mut new_val]; + next = Some((1, _new_val.unwrap())); + } + // Indexed value is a reference - update directly + Ok(ref mut obj_ptr) => { + obj_ptr + .set_value(_new_val.unwrap()) + .map_err(|err| err.new_position(rhs.position()))?; + + #[cfg(not(feature = "no_index"))] + { + next = None; + } + } + Err(err) => match *err { + // No index getter - try to call an index setter + #[cfg(not(feature = "no_index"))] + EvalAltResult::ErrorIndexingType(_, _) => { + next = Some((2, _new_val.unwrap())); + } + // Error + err => return Err(Box::new(err)), + }, + }; + + #[cfg(not(feature = "no_index"))] + match &mut next { + // next step is custom index setter call + Some((1, _new_val)) => { + let args = &mut [target.as_mut(), &mut idx_val2, _new_val]; self.exec_fn_call( state, lib, FN_IDX_SET, true, 0, args, is_ref, true, false, @@ -698,27 +747,20 @@ impl Engine { _ => Err(err), })?; } - // Indexed value is a reference - update directly - Ok(ref mut obj_ptr) => { - obj_ptr - .set_value(new_val) - .map_err(|err| err.new_position(rhs.position()))?; - } - Err(err) => match *err { - // No index getter - try to call an index setter - #[cfg(not(feature = "no_index"))] - EvalAltResult::ErrorIndexingType(_, _) => { - let args = &mut [target.as_mut(), &mut idx_val2, &mut new_val]; - self.exec_fn_call( - state, lib, FN_IDX_SET, true, 0, args, is_ref, true, false, - None, level, - )?; - } - // Error - err => return Err(Box::new(err)), - }, + // next step is custom index setter call in case of error + Some((2, _new_val)) => { + let args = &mut [target.as_mut(), &mut idx_val2, _new_val]; + + self.exec_fn_call( + state, lib, FN_IDX_SET, true, 0, args, is_ref, true, false, + None, level, + )?; + } + None => (), + _ => unreachable!() } + Ok(Default::default()) } // xxx[rhs] @@ -835,11 +877,10 @@ impl Engine { .map_err(|err| err.new_position(*pos))?; let val = &mut val; - let target = &mut val.into(); let (result, may_be_changed) = self .eval_dot_index_chain_helper( - state, lib, this_ptr, target, expr, idx_values, next_chain, + state, lib, this_ptr, &mut val.into(), expr, idx_values, next_chain, level, _new_val, ) .map_err(|err| err.new_position(*pos))?; @@ -1069,18 +1110,6 @@ impl Engine { let val = target.as_mut(); - // if val.is_shared() { - // return self.get_indexed_mut( - // state, - // lib, - // &mut Target::Value(val.read::().unwrap()), - // idx, - // idx_pos, - // create, - // level, - // ); - // } - match val { #[cfg(not(feature = "no_index"))] Dynamic(Union::Array(arr)) => { diff --git a/src/fn_call.rs b/src/fn_call.rs index 6ca46046..19f3fd33 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -699,7 +699,6 @@ impl Engine { } // Handle shared() - #[cfg(not(feature = "no_shared"))] if name == KEYWORD_SHARED && args_expr.len() == 1 { let expr = args_expr.get(0).unwrap(); let value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; diff --git a/src/fn_native.rs b/src/fn_native.rs index e51bf71b..54f3da08 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -18,7 +18,7 @@ use crate::stdlib::{boxed::Box, convert::TryFrom, fmt, iter::empty, string::Stri use crate::stdlib::mem; #[cfg(not(feature = "sync"))] -use crate::stdlib::{rc::Rc, cell::{RefCell, Ref, RefMut}}; +use crate::stdlib::{rc::Rc, cell::RefCell}; #[cfg(feature = "sync")] use crate::stdlib::sync::{Arc, RwLock}; diff --git a/src/fn_register.rs b/src/fn_register.rs index ddcddff3..eef4e66b 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -114,7 +114,7 @@ pub fn by_value(data: &mut Dynamic) -> T { ref_T.clone() } else if TypeId::of::() == TypeId::of::() { // If T is String, data must be ImmutableString, so map directly to it - *unsafe_cast_box(Box::new(data.as_str().unwrap().to_string())).unwrap() + *unsafe_cast_box(Box::new(data.clone().take_string().unwrap())).unwrap() } else { // We consume the argument and then replace it with () - the argument is not supposed to be used again. // This way, we avoid having to clone the argument again, because it is already a clone when passed here. diff --git a/src/parser.rs b/src/parser.rs index 3068bad0..2128f38c 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -411,6 +411,11 @@ struct ParseState<'e> { /// Tracks a list of external variables (variables that are not explicitly declared in the scope). #[cfg(not(feature = "no_capture"))] externals: HashMap, + /// An indicator that prevents variables capturing into externals one time. + /// If set to true the next call of `access_var` will not capture the variable. + /// All consequent calls to `access_var` will not be affected + #[cfg(not(feature = "no_capture"))] + capture: bool, /// Encapsulates a local stack with variable names to simulate an actual runtime scope. modules: Vec, /// Maximum levels of expression nesting. @@ -436,6 +441,8 @@ impl<'e> ParseState<'e> { max_function_expr_depth, #[cfg(not(feature = "no_capture"))] externals: Default::default(), + #[cfg(not(feature = "no_capture"))] + capture: true, stack: Default::default(), modules: Default::default(), } @@ -458,8 +465,12 @@ impl<'e> ParseState<'e> { .and_then(|(i, _)| NonZeroUsize::new(i + 1)); #[cfg(not(feature = "no_capture"))] - if index.is_none() && !self.externals.contains_key(name) { - self.externals.insert(name.to_string(), pos); + if self.capture { + if index.is_none() && !self.externals.contains_key(name) { + self.externals.insert(name.to_string(), pos); + } + } else { + self.capture = true } index @@ -2171,9 +2182,18 @@ fn parse_binary_op( let (op_token, pos) = input.next().unwrap(); - let rhs = parse_unary(input, state, lib, settings)?; + let next = input.peek().unwrap(); + let next_precedence = next.0.precedence(custom); - let next_precedence = input.peek().unwrap().0.precedence(custom); + #[cfg(any(not(feature = "no_object"), not(feature = "no_capture")))] + if op_token == Token::Period { + if let (Token::Identifier(_), _) = next { + // prevents capturing of the object properties as vars: xxx. + state.capture = false; + } + } + + let rhs = parse_unary(input, state, lib, settings)?; // Bind to right if the next operator has higher precedence // If same precedence, then check if the operator binds right diff --git a/src/token.rs b/src/token.rs index bd496afb..71ec09a6 100644 --- a/src/token.rs +++ b/src/token.rs @@ -1430,13 +1430,20 @@ fn get_identifier( /// Is this keyword allowed as a function? #[inline(always)] pub fn is_keyword_function(name: &str) -> bool { - name == KEYWORD_PRINT + let mut result = name == KEYWORD_PRINT || name == KEYWORD_DEBUG || name == KEYWORD_TYPE_OF || name == KEYWORD_EVAL || name == KEYWORD_FN_PTR || name == KEYWORD_FN_PTR_CALL - || name == KEYWORD_FN_PTR_CURRY + || name == KEYWORD_FN_PTR_CURRY; + + #[cfg(not(feature = "no-shared"))] + { + result = result || name == KEYWORD_SHARED; + } + + result } pub fn is_valid_identifier(name: impl Iterator) -> bool { diff --git a/tests/closures.rs b/tests/closures.rs index 3aec4865..ae0c6535 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -1,6 +1,6 @@ #![cfg(not(feature = "no_function"))] -use rhai::{Dynamic, Engine, EvalAltResult, FnPtr, Module, INT}; -use std::any::TypeId; +use rhai::{Dynamic, Engine, EvalAltResult, FnPtr, Module, INT, Array}; +use std::any::{TypeId, Any}; #[test] fn test_fn_ptr_curry_call() -> Result<(), Box> { @@ -58,3 +58,150 @@ fn test_closures() -> Result<(), Box> { Ok(()) } + +#[test] +#[cfg(not(feature = "no_shared"))] +fn test_shared() -> Result<(), Box> { + let engine = Engine::new(); + + // assert_eq!( + // engine.eval::( + // r#" + // shared(42) + // "# + // )?, + // 42 + // ); + // + // assert_eq!( + // engine.eval::( + // r#" + // shared(true) + // "# + // )?, + // true + // ); + // + // #[cfg(not(feature = "no_float"))] + // assert_eq!( + // engine.eval::( + // r#" + // shared(4.2) + // "# + // )?, + // 4.2 + // ); + // + // assert_eq!( + // engine.eval::( + // r#" + // shared("test") + // "# + // )?, + // "test" + // ); + // + // #[cfg(not(feature = "no_index"))] + // { + // assert_eq!( + // engine.eval::( + // r#" + // let x = shared([1, 2, 3]); + // let y = shared([4, 5]); + // x + y + // "# + // )?.len(), + // 5 + // ); + // + // assert_eq!( + // engine.eval::( + // r" + // let x = shared([2, 9]); + // x.insert(-1, 1); + // x.insert(999, 3); + // + // let r = x.remove(2); + // + // let y = shared([4, 5]); + // x.append(y); + // + // x.len + r + // " + // )?, + // 14 + // ); + // + // assert_eq!( + // engine.eval::( + // r#" + // let x = shared([1, 2, 3]); + // + // if x[0] + x[2] == 4 { + // true + // } else { + // false + // } + // "# + // )?, + // true + // ); + // } + // + // #[cfg(not(feature = "no_object"))] + // assert_eq!( + // engine.eval::(r#" + // let y = shared(#{a: 1, b: 2, c: 3}); + // y.c = shared(5); + // y.c + // "#)?, + // 5 + // ); + // + // #[cfg(not(feature = "no_object"))] + // assert_eq!( + // engine.eval::(r#" + // let y = shared(#{a: 1, b: 2, c: shared(3)}); + // let c = y.c; + // c = 5;// "c" still holds Dynamic Shared + // y.c + // "#)?, + // 5 + // ); + // + // #[cfg(not(feature = "no_capture"))] + // assert_eq!( + // engine.eval::(r#" + // let x = shared(1); + // (|| x = x + 41).call(); + // x + // "#)?, + // 42 + // ); + + #[cfg(all(not(feature = "no_object"), not(feature = "no_capture")))] + assert_eq!( + engine.eval::(r#" + // let x = shared(#{a: 1, b: shared(2), c: 3}); + // let a = x.a; + // let b = x.b; + // a = 100; + // b = 20; + // + // let f = |a| { + // x.c = x.a + x.b;// + a; + // }; + // + // f.call(20); + // + // x.c + + let x = #{a: 1, b: 2}; + + x.a + x.b + "#)?, + 42 + ); + + Ok(()) +} From 060dd33046136d2f36b0380442116a433c47bf0d Mon Sep 17 00:00:00 2001 From: Ilya Lakhin Date: Fri, 31 Jul 2020 10:44:57 +0700 Subject: [PATCH 3/6] Shared Dynamic tests and fixes in Engine; Also fixed a bug in Parser variable capturing --- src/any.rs | 2 +- src/parser.rs | 7 +- tests/closures.rs | 359 +++++++++++++++++++++++++++++----------------- 3 files changed, 231 insertions(+), 137 deletions(-) diff --git a/src/any.rs b/src/any.rs index 7990ff10..db2ada1b 100644 --- a/src/any.rs +++ b/src/any.rs @@ -1031,7 +1031,7 @@ impl Dynamic { _ => Err(cell.value_type_name), } #[cfg(feature = "sync")] - match &cell.container.read().deref().0 { + match &cell.container.read().unwrap().deref().0 { Union::Str(s) => Ok(s.clone()), Union::FnPtr(f) => Ok(f.clone().take_data().0), _ => Err(cell.value_type_name), diff --git a/src/parser.rs b/src/parser.rs index 2128f38c..1d9f26dd 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2182,12 +2182,9 @@ fn parse_binary_op( let (op_token, pos) = input.next().unwrap(); - let next = input.peek().unwrap(); - let next_precedence = next.0.precedence(custom); - #[cfg(any(not(feature = "no_object"), not(feature = "no_capture")))] if op_token == Token::Period { - if let (Token::Identifier(_), _) = next { + if let (Token::Identifier(_), _) = input.peek().unwrap() { // prevents capturing of the object properties as vars: xxx. state.capture = false; } @@ -2195,6 +2192,8 @@ fn parse_binary_op( let rhs = parse_unary(input, state, lib, settings)?; + let next_precedence = input.peek().unwrap().0.precedence(custom); + // Bind to right if the next operator has higher precedence // If same precedence, then check if the operator binds right let rhs = if (precedence == next_precedence && bind_right) || precedence < next_precedence { diff --git a/tests/closures.rs b/tests/closures.rs index ae0c6535..25166ee1 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -1,5 +1,5 @@ #![cfg(not(feature = "no_function"))] -use rhai::{Dynamic, Engine, EvalAltResult, FnPtr, Module, INT, Array}; +use rhai::{Dynamic, Engine, EvalAltResult, RegisterFn, FnPtr, Module, INT, Array}; use std::any::{TypeId, Any}; #[test] @@ -62,146 +62,241 @@ fn test_closures() -> Result<(), Box> { #[test] #[cfg(not(feature = "no_shared"))] fn test_shared() -> Result<(), Box> { - let engine = Engine::new(); + let mut engine = Engine::new(); - // assert_eq!( - // engine.eval::( - // r#" - // shared(42) - // "# - // )?, - // 42 - // ); - // - // assert_eq!( - // engine.eval::( - // r#" - // shared(true) - // "# - // )?, - // true - // ); - // - // #[cfg(not(feature = "no_float"))] - // assert_eq!( - // engine.eval::( - // r#" - // shared(4.2) - // "# - // )?, - // 4.2 - // ); - // - // assert_eq!( - // engine.eval::( - // r#" - // shared("test") - // "# - // )?, - // "test" - // ); - // - // #[cfg(not(feature = "no_index"))] - // { - // assert_eq!( - // engine.eval::( - // r#" - // let x = shared([1, 2, 3]); - // let y = shared([4, 5]); - // x + y - // "# - // )?.len(), - // 5 - // ); - // - // assert_eq!( - // engine.eval::( - // r" - // let x = shared([2, 9]); - // x.insert(-1, 1); - // x.insert(999, 3); - // - // let r = x.remove(2); - // - // let y = shared([4, 5]); - // x.append(y); - // - // x.len + r - // " - // )?, - // 14 - // ); - // - // assert_eq!( - // engine.eval::( - // r#" - // let x = shared([1, 2, 3]); - // - // if x[0] + x[2] == 4 { - // true - // } else { - // false - // } - // "# - // )?, - // true - // ); - // } - // - // #[cfg(not(feature = "no_object"))] - // assert_eq!( - // engine.eval::(r#" - // let y = shared(#{a: 1, b: 2, c: 3}); - // y.c = shared(5); - // y.c - // "#)?, - // 5 - // ); - // - // #[cfg(not(feature = "no_object"))] - // assert_eq!( - // engine.eval::(r#" - // let y = shared(#{a: 1, b: 2, c: shared(3)}); - // let c = y.c; - // c = 5;// "c" still holds Dynamic Shared - // y.c - // "#)?, - // 5 - // ); - // - // #[cfg(not(feature = "no_capture"))] - // assert_eq!( - // engine.eval::(r#" - // let x = shared(1); - // (|| x = x + 41).call(); - // x - // "#)?, - // 42 - // ); + assert_eq!( + engine.eval::( + r#" + shared(42) + "# + )?, + 42 + ); - #[cfg(all(not(feature = "no_object"), not(feature = "no_capture")))] + assert_eq!( + engine.eval::( + r#" + shared(true) + "# + )?, + true + ); + + #[cfg(not(feature = "no_float"))] + assert_eq!( + engine.eval::( + r#" + shared(4.2) + "# + )?, + 4.2 + ); + + assert_eq!( + engine.eval::( + r#" + shared("test") + "# + )?, + "test" + ); + + assert_eq!( + engine.eval::( + r#" + shared('x') + "# + )?, + 'x' + ); + + assert_eq!( + engine.eval::( + r#" + let s = shared("test"); + let i = shared(0); + i = 2; + + s[i] = 'S'; + s + "# + )?, + "teSt" + ); + + #[cfg(not(feature = "no_index"))] + { + assert_eq!( + engine.eval::( + r#" + let x = shared([1, 2, 3]); + let y = shared([4, 5]); + x + y + "# + )?.len(), + 5 + ); + + assert_eq!( + engine.eval::( + r" + let x = shared([2, 9]); + x.insert(-1, 1); + x.insert(999, 3); + + let r = x.remove(2); + + let y = shared([4, 5]); + x.append(y); + + x.len + r + " + )?, + 14 + ); + + assert_eq!( + engine.eval::( + r#" + let x = shared([1, 2, 3]); + + if x[0] + x[2] == 4 { + true + } else { + false + } + "# + )?, + true + ); + + assert_eq!( + engine.eval::( + r#" + let x = shared([1, 2, 3]); + let y = shared(()); + + (|| { + for i in x { + y = i * 10; + } + }).call(); + + y + "# + )?, + 30 + ); + } + + #[cfg(not(feature = "no_object"))] assert_eq!( engine.eval::(r#" - // let x = shared(#{a: 1, b: shared(2), c: 3}); - // let a = x.a; - // let b = x.b; - // a = 100; - // b = 20; - // - // let f = |a| { - // x.c = x.a + x.b;// + a; - // }; - // - // f.call(20); - // - // x.c + let y = shared(#{a: 1, b: 2, c: 3}); + y.c = shared(5); + y.c + "#)?, + 5 + ); - let x = #{a: 1, b: 2}; + #[cfg(not(feature = "no_object"))] + assert_eq!( + engine.eval::(r#" + let y = shared(#{a: 1, b: 2, c: shared(3)}); + let c = y.c; + c = 5;// "c" holds Dynamic Shared + y.c + "#)?, + 5 + ); - x.a + x.b + #[cfg(not(feature = "no_capture"))] + assert_eq!( + engine.eval::(r#" + let x = shared(1); + (|| x = x + 41).call(); + x "#)?, 42 ); + #[cfg(all(not(feature = "no_object"), not(feature = "no_capture")))] + assert_eq!( + engine.eval::(r#" + let x = shared(#{a: 1, b: shared(2), c: 3}); + let a = x.a; + let b = x.b; + a = 100; // does not hold reference to x.a + b = 20; // does hold reference to x.b + + let f = |a| { + x.c = x.a + x.b + a; + }; + + f.call(21); + + x.c + "#)?, + 42 + ); + + // Register a binary function named `foo` + engine.register_fn("custom_addition", |x: INT, y: INT| x + y); + + assert_eq!( + engine.eval::(r#" + custom_addition(shared(20), shared(22)) + "#)?, + 42 + ); + + #[derive(Clone)] + struct TestStruct { + x: INT, + } + + impl TestStruct { + fn update(&mut self) { + self.x += 1000; + } + + fn merge(&mut self, other: Self) { + self.x += other.x; + } + + fn get_x(&mut self) -> INT { + self.x + } + + fn set_x(&mut self, new_x: INT) { + self.x = new_x; + } + + fn new() -> Self { + TestStruct { x: 1 } + } + } + + engine.register_type::(); + + engine.register_get_set("x", TestStruct::get_x, TestStruct::set_x); + engine.register_fn("update", TestStruct::update); + engine.register_fn("merge", TestStruct::merge); + engine.register_fn("new_ts", TestStruct::new); + + assert_eq!( + engine.eval::( + r" + let a = shared(new_ts()); + + a.x = 100; + a.update(); + // a.merge(a); + a.x + " + )?, + 1100 + ); + Ok(()) } From ca64668e583ec7b34377a1db5a11294c5c2570ff Mon Sep 17 00:00:00 2001 From: Ilya Lakhin Date: Fri, 31 Jul 2020 11:41:22 +0700 Subject: [PATCH 4/6] take() keyword; shared test with registered functions with callbacks --- Cargo.toml | 2 +- src/any.rs | 6 ++++-- src/engine.rs | 1 + src/fn_call.rs | 5 ++++- src/token.rs | 9 +++++---- tests/closures.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++--- 6 files changed, 60 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 85b57974..0cc0e20d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,7 @@ no_index = [] # no arrays and indexing no_object = [] # no custom objects no_function = [] # no script-defined functions no_capture = [] # no automatic read/write binding of anonymous function's local variables to it's external context -no_shared = [] # no explicit shared variables in the script code +no_shared = [] # no explicit shared() and take() functions in the script code no_module = [] # no modules internals = [] # expose internal data structures unicode-xid-ident = ["unicode-xid"] # allow Unicode Standard Annex #31 for identifiers. diff --git a/src/any.rs b/src/any.rs index db2ada1b..1a82d237 100644 --- a/src/any.rs +++ b/src/any.rs @@ -568,7 +568,7 @@ impl Dynamic { /// /// ## Safety /// - /// Both situations normally shouldn't happen since all operations in Rhai + /// Both situations normally shouldn't happen since most operations in Rhai /// use pass-by-value data and the script executed in a single thread. /// /// # Example @@ -741,7 +741,9 @@ impl Dynamic { } /// Get a copy of a specific type to the `Dynamic`. - /// Casting to `Dynamic` just returns a reference to it. + /// Casting to `Dynamic` returns a clone of the value in case of NON-shared + /// Dynamic. In case of Shared Dynamic returns a clone of the inner data of + /// Shared Dynamic. /// Returns `None` if the cast fails. #[inline(always)] pub fn read(&self) -> Option { diff --git a/src/engine.rs b/src/engine.rs index 8f727211..4cc3b266 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -93,6 +93,7 @@ pub const KEYWORD_FN_PTR: &str = "Fn"; pub const KEYWORD_FN_PTR_CALL: &str = "call"; pub const KEYWORD_FN_PTR_CURRY: &str = "curry"; pub const KEYWORD_SHARED: &str = "shared"; +pub const KEYWORD_TAKE: &str = "take"; pub const KEYWORD_THIS: &str = "this"; pub const FN_TO_STRING: &str = "to_string"; #[cfg(not(feature = "no_object"))] diff --git a/src/fn_call.rs b/src/fn_call.rs index 19f3fd33..6f82df42 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -5,7 +5,7 @@ use crate::calc_fn_hash; use crate::engine::{ search_imports, search_namespace, search_scope_only, Engine, Imports, State, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_FN_PTR_CALL, KEYWORD_FN_PTR_CURRY, KEYWORD_PRINT, - KEYWORD_TYPE_OF, KEYWORD_SHARED + KEYWORD_TYPE_OF, KEYWORD_SHARED, KEYWORD_TAKE }; use crate::error::ParseErrorType; use crate::fn_native::{FnCallArgs, FnPtr}; @@ -593,6 +593,9 @@ impl Engine { .into(), false, )) + } else if _fn_name == KEYWORD_TAKE { + // take call + return Ok((obj.read::().unwrap(), false)); } else { #[cfg(not(feature = "no_object"))] let redirected; diff --git a/src/token.rs b/src/token.rs index 71ec09a6..b0c77e38 100644 --- a/src/token.rs +++ b/src/token.rs @@ -2,7 +2,7 @@ use crate::engine::{ Engine, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_FN_PTR_CALL, KEYWORD_FN_PTR_CURRY, - KEYWORD_SHARED, KEYWORD_PRINT, KEYWORD_THIS, KEYWORD_TYPE_OF, + KEYWORD_SHARED, KEYWORD_TAKE, KEYWORD_PRINT, KEYWORD_THIS, KEYWORD_TYPE_OF, }; use crate::error::LexError; @@ -503,11 +503,12 @@ impl Token { "===" | "!==" | "->" | "<-" | "=>" | ":=" | "::<" | "(*" | "*)" | "#" | "public" | "new" | "use" | "module" | "package" | "var" | "static" | "with" | "do" | "each" | "then" | "goto" | "exit" | "switch" | "match" | "case" | "try" | "catch" - | "default" | "void" | "null" | "nil" | "spawn" | "go" | "shared" | "sync" + | "default" | "void" | "null" | "nil" | "spawn" | "go" | "sync" | "async" | "await" | "yield" => Reserved(syntax.into()), KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_TYPE_OF | KEYWORD_EVAL | KEYWORD_FN_PTR - | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY | KEYWORD_SHARED | KEYWORD_THIS => Reserved(syntax.into()), + | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY | KEYWORD_SHARED + | KEYWORD_TAKE |KEYWORD_THIS => Reserved(syntax.into()), _ => return None, }) @@ -1440,7 +1441,7 @@ pub fn is_keyword_function(name: &str) -> bool { #[cfg(not(feature = "no-shared"))] { - result = result || name == KEYWORD_SHARED; + result = result || name == KEYWORD_SHARED || name == KEYWORD_TAKE; } result diff --git a/tests/closures.rs b/tests/closures.rs index 25166ee1..6198bc26 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -116,8 +116,8 @@ fn test_shared() -> Result<(), Box> { let s = shared("test"); let i = shared(0); i = 2; - s[i] = 'S'; + s "# )?, @@ -283,6 +283,26 @@ fn test_shared() -> Result<(), Box> { engine.register_fn("update", TestStruct::update); engine.register_fn("merge", TestStruct::merge); engine.register_fn("new_ts", TestStruct::new); + engine. + register_raw_fn( + "mutate_with_cb", + &[ + TypeId::of::(), + TypeId::of::(), + TypeId::of::(), + ], + move |engine: &Engine, lib: &Module, args: &mut [&mut Dynamic]| { + let fp = std::mem::take(args[2]).cast::(); + let mut value = args[1].clone(); + { + let mut lock = value.write_lock::().unwrap(); + *lock = *lock + 1; + } + let this_ptr = args.get_mut(0).unwrap(); + + fp.call_dynamic(engine, lib, Some(this_ptr), [value]) + }, + ); assert_eq!( engine.eval::( @@ -291,11 +311,33 @@ fn test_shared() -> Result<(), Box> { a.x = 100; a.update(); - // a.merge(a); + a.merge(a.take()); // take is important to prevent a deadlock + a.x " )?, - 1100 + 2200 + ); + + assert_eq!( + engine.eval::( + r" + let a = shared(new_ts()); + let b = shared(100); + + a.mutate_with_cb(b, |param| { + this.x = param; + param = 50; + this.update(); + }); + + a.update(); + a.x += b; + + a.x + " + )?, + 2151 ); Ok(()) From 4f771d904ab5dc944abba237500c90bacd2208b4 Mon Sep 17 00:00:00 2001 From: Ilya Lakhin Date: Fri, 31 Jul 2020 12:08:14 +0700 Subject: [PATCH 5/6] Code cleanup --- src/fn_call.rs | 4 +- src/fn_native.rs | 2 +- src/fn_register.rs | 2 +- tests/closures.rs | 184 ++++++++++++++++++++++++--------------------- 4 files changed, 101 insertions(+), 91 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index 6f82df42..dd56b88d 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -5,7 +5,7 @@ use crate::calc_fn_hash; use crate::engine::{ search_imports, search_namespace, search_scope_only, Engine, Imports, State, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_FN_PTR_CALL, KEYWORD_FN_PTR_CURRY, KEYWORD_PRINT, - KEYWORD_TYPE_OF, KEYWORD_SHARED, KEYWORD_TAKE + KEYWORD_TYPE_OF, KEYWORD_SHARED, }; use crate::error::ParseErrorType; use crate::fn_native::{FnCallArgs, FnPtr}; @@ -31,7 +31,7 @@ use crate::parser::FLOAT; use crate::engine::{FN_IDX_GET, FN_IDX_SET}; #[cfg(not(feature = "no_object"))] -use crate::engine::{Map, Target, FN_GET, FN_SET}; +use crate::engine::{Map, Target, FN_GET, FN_SET, KEYWORD_TAKE}; use crate::stdlib::{ any::{type_name, TypeId}, diff --git a/src/fn_native.rs b/src/fn_native.rs index 54f3da08..de926a47 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -1,6 +1,6 @@ //! Module defining interfaces to native-Rust functions. -use crate::any::{Dynamic, Variant}; +use crate::any::Dynamic; use crate::calc_fn_hash; use crate::engine::Engine; use crate::module::Module; diff --git a/src/fn_register.rs b/src/fn_register.rs index eef4e66b..e01f44a1 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -15,7 +15,7 @@ use crate::stdlib::{ any::TypeId, boxed::Box, mem, - string::{String, ToString}, + string::String, }; /// Trait to register custom functions with the `Engine`. diff --git a/tests/closures.rs b/tests/closures.rs index 6198bc26..4f8c16dd 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -1,6 +1,12 @@ #![cfg(not(feature = "no_function"))] -use rhai::{Dynamic, Engine, EvalAltResult, RegisterFn, FnPtr, Module, INT, Array}; -use std::any::{TypeId, Any}; +use rhai::{Dynamic, Engine, EvalAltResult, FnPtr, Module, INT}; +use std::any::TypeId; + +#[cfg(not(feature = "no_shared"))] +use rhai::RegisterFn; + +#[cfg(not(feature = "no_index"))] +use rhai::Array; #[test] fn test_fn_ptr_curry_call() -> Result<(), Box> { @@ -35,7 +41,7 @@ fn test_fn_ptr_curry_call() -> Result<(), Box> { } #[test] -#[cfg(not(feature = "no_capture"))] +#[cfg(all(not(feature = "no_capture"), not(feature = "no_object")))] fn test_closures() -> Result<(), Box> { let engine = Engine::new(); @@ -110,9 +116,11 @@ fn test_shared() -> Result<(), Box> { 'x' ); - assert_eq!( - engine.eval::( - r#" + #[cfg(not(feature = "no_index"))] + { + assert_eq!( + engine.eval::( + r#" let s = shared("test"); let i = shared(0); i = 2; @@ -120,12 +128,10 @@ fn test_shared() -> Result<(), Box> { s "# - )?, - "teSt" - ); + )?, + "teSt" + ); - #[cfg(not(feature = "no_index"))] - { assert_eq!( engine.eval::( r#" @@ -137,6 +143,7 @@ fn test_shared() -> Result<(), Box> { 5 ); + #[cfg(not(feature = "no_object"))] assert_eq!( engine.eval::( r" @@ -170,6 +177,7 @@ fn test_shared() -> Result<(), Box> { true ); + #[cfg(not(feature = "no_object"))] assert_eq!( engine.eval::( r#" @@ -250,95 +258,97 @@ fn test_shared() -> Result<(), Box> { 42 ); - #[derive(Clone)] - struct TestStruct { - x: INT, - } - - impl TestStruct { - fn update(&mut self) { - self.x += 1000; + #[cfg(not(feature = "no_object"))] + { + #[derive(Clone)] + struct TestStruct { + x: INT, } - fn merge(&mut self, other: Self) { - self.x += other.x; + impl TestStruct { + fn update(&mut self) { + self.x += 1000; + } + + fn merge(&mut self, other: Self) { + self.x += other.x; + } + + fn get_x(&mut self) -> INT { + self.x + } + + fn set_x(&mut self, new_x: INT) { + self.x = new_x; + } + + fn new() -> Self { + TestStruct { x: 1 } + } } - fn get_x(&mut self) -> INT { - self.x - } + engine + .register_type::() + .register_get_set("x", TestStruct::get_x, TestStruct::set_x) + .register_fn("update", TestStruct::update) + .register_fn("merge", TestStruct::merge) + .register_fn("new_ts", TestStruct::new) + .register_raw_fn( + "mutate_with_cb", + &[ + TypeId::of::(), + TypeId::of::(), + TypeId::of::(), + ], + move |engine: &Engine, lib: &Module, args: &mut [&mut Dynamic]| { + let fp = std::mem::take(args[2]).cast::(); + let mut value = args[1].clone(); + { + let mut lock = value.write_lock::().unwrap(); + *lock = *lock + 1; + } + let this_ptr = args.get_mut(0).unwrap(); - fn set_x(&mut self, new_x: INT) { - self.x = new_x; - } + fp.call_dynamic(engine, lib, Some(this_ptr), [value]) + }, + ); - fn new() -> Self { - TestStruct { x: 1 } - } - } + assert_eq!( + engine.eval::( + r" + let a = shared(new_ts()); - engine.register_type::(); + a.x = 100; + a.update(); + a.merge(a.take()); // take is important to prevent a deadlock - engine.register_get_set("x", TestStruct::get_x, TestStruct::set_x); - engine.register_fn("update", TestStruct::update); - engine.register_fn("merge", TestStruct::merge); - engine.register_fn("new_ts", TestStruct::new); - engine. - register_raw_fn( - "mutate_with_cb", - &[ - TypeId::of::(), - TypeId::of::(), - TypeId::of::(), - ], - move |engine: &Engine, lib: &Module, args: &mut [&mut Dynamic]| { - let fp = std::mem::take(args[2]).cast::(); - let mut value = args[1].clone(); - { - let mut lock = value.write_lock::().unwrap(); - *lock = *lock + 1; - } - let this_ptr = args.get_mut(0).unwrap(); - - fp.call_dynamic(engine, lib, Some(this_ptr), [value]) - }, + a.x + " + )?, + 2200 ); - assert_eq!( - engine.eval::( - r" - let a = shared(new_ts()); + assert_eq!( + engine.eval::( + r" + let a = shared(new_ts()); + let b = shared(100); - a.x = 100; - a.update(); - a.merge(a.take()); // take is important to prevent a deadlock + a.mutate_with_cb(b, |param| { + this.x = param; + param = 50; + this.update(); + }); - a.x - " - )?, - 2200 - ); + a.update(); + a.x += b; - assert_eq!( - engine.eval::( - r" - let a = shared(new_ts()); - let b = shared(100); - - a.mutate_with_cb(b, |param| { - this.x = param; - param = 50; - this.update(); - }); - - a.update(); - a.x += b; - - a.x - " - )?, - 2151 - ); + a.x + " + )?, + 2151 + ); + } Ok(()) } From 5d1f5cc2b41286b03fae08ae2cf11c8fef3527c8 Mon Sep 17 00:00:00 2001 From: Ilya Lakhin Date: Fri, 31 Jul 2020 13:10:05 +0700 Subject: [PATCH 6/6] Dynamic::read renamed to Dynamic::clone_inner_data --- src/any.rs | 88 +++++++++++++++++++++++++------------------------- src/fn_call.rs | 2 +- src/scope.rs | 2 +- 3 files changed, 46 insertions(+), 46 deletions(-) diff --git a/src/any.rs b/src/any.rs index 1a82d237..cbb3e444 100644 --- a/src/any.rs +++ b/src/any.rs @@ -709,6 +709,46 @@ impl Dynamic { self.try_cast::().unwrap() } + /// Get a copy of a specific type to the `Dynamic`. + /// Casting to `Dynamic` returns a clone of the value in case of NON-shared + /// Dynamic. In case of Shared Dynamic returns a clone of the inner data of + /// Shared Dynamic. + /// Returns `None` if the cast fails. + #[inline(always)] + pub fn clone_inner_data(&self) -> Option { + match self.0 { + Union::Shared(ref cell) => { + let type_id = TypeId::of::(); + + if type_id != TypeId::of::() && cell.value_type_id != type_id { + return None + } + + #[cfg(not(feature = "sync"))] + return Some(cell + .container + .borrow() + .deref() + .downcast_ref::() + .unwrap() + .clone()); + + #[cfg(feature = "sync")] + return Some(cell + .container + .read() + .unwrap() + .deref() + .downcast_ref::() + .unwrap() + .clone()); + }, + _ => { + self.downcast_ref().cloned() + } + } + } + /// Get a reference of a specific type to the `Dynamic`. /// Casting to `Dynamic` just returns a reference to it. /// Returns `None` if the cast fails. @@ -740,46 +780,6 @@ impl Dynamic { } } - /// Get a copy of a specific type to the `Dynamic`. - /// Casting to `Dynamic` returns a clone of the value in case of NON-shared - /// Dynamic. In case of Shared Dynamic returns a clone of the inner data of - /// Shared Dynamic. - /// Returns `None` if the cast fails. - #[inline(always)] - pub fn read(&self) -> Option { - match self.0 { - Union::Shared(ref cell) => { - let type_id = TypeId::of::(); - - if type_id != TypeId::of::() && cell.value_type_id != type_id { - return None - } - - #[cfg(not(feature = "sync"))] - return Some(cell - .container - .borrow() - .deref() - .downcast_ref::() - .unwrap() - .clone()); - - #[cfg(feature = "sync")] - return Some(cell - .container - .read() - .unwrap() - .deref() - .downcast_ref::() - .unwrap() - .clone()); - }, - _ => { - self.downcast_ref().cloned() - } - } - } - /// Get a mutable reference of a specific type to the `Dynamic`. /// Casting to `Dynamic` just returns a mutable reference to it. /// Returns `None` if the cast fails. @@ -964,7 +964,7 @@ impl Dynamic { pub fn as_int(&self) -> Result { match self.0 { Union::Int(n) => Ok(n), - Union::Shared(_) => self.read::().ok_or_else(|| self.type_name()), + Union::Shared(_) => self.clone_inner_data::().ok_or_else(|| self.type_name()), _ => Err(self.type_name()), } } @@ -975,7 +975,7 @@ impl Dynamic { pub fn as_float(&self) -> Result { match self.0 { Union::Float(n) => Ok(n), - Union::Shared(_) => self.read::().ok_or_else(|| self.type_name()), + Union::Shared(_) => self.clone_inner_data::().ok_or_else(|| self.type_name()), _ => Err(self.type_name()), } } @@ -985,7 +985,7 @@ impl Dynamic { pub fn as_bool(&self) -> Result { match self.0 { Union::Bool(b) => Ok(b), - Union::Shared(_) => self.read::().ok_or_else(|| self.type_name()), + Union::Shared(_) => self.clone_inner_data::().ok_or_else(|| self.type_name()), _ => Err(self.type_name()), } } @@ -995,7 +995,7 @@ impl Dynamic { pub fn as_char(&self) -> Result { match self.0 { Union::Char(n) => Ok(n), - Union::Shared(_) => self.read::().ok_or_else(|| self.type_name()), + Union::Shared(_) => self.clone_inner_data::().ok_or_else(|| self.type_name()), _ => Err(self.type_name()), } } diff --git a/src/fn_call.rs b/src/fn_call.rs index e0dd1184..bed49230 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -658,7 +658,7 @@ impl Engine { )) } else if _fn_name == KEYWORD_TAKE { // take call - return Ok((obj.read::().unwrap(), false)); + return Ok((obj.clone_inner_data::().unwrap(), false)); } else { #[cfg(not(feature = "no_object"))] let redirected; diff --git a/src/scope.rs b/src/scope.rs index 2fb3df21..b1b9a3b3 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -340,7 +340,7 @@ impl<'a> Scope<'a> { /// ``` pub fn get_value(&self, name: &str) -> Option { self.get_entry(name) - .and_then(|Entry { value, .. }| value.read::()) + .and_then(|Entry { value, .. }| value.clone_inner_data::()) } /// Update the value of the named entry.