From 2052942d9dc67f48998b140810b428e151ec44f7 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 5 Mar 2021 13:34:58 +0800 Subject: [PATCH 1/8] Add Dynamic::as_unit. --- CHANGELOG.md | 1 + src/dynamic.rs | 44 ++++++++++++++++++++++++++------------------ src/optimize.rs | 17 ++++++++--------- 3 files changed, 35 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97aea371..e594d41e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ Enhancements * `range` function now supports negative step and decreasing streams (i.e. to < from). * More information is provided to the error variable captured by the `catch` statement in an _object map_. * Previously, `private` functions in an `AST` cannot be called with `call_fn` etc. This is inconvenient when trying to call a function inside a script which also serves as a loadable module exporting part (but not all) of the functions. Now, all functions (`private` or not) can be called in an `AST`. The `private` keyword is relegated to preventing a function from being exported. +* `Dynamic::as_unit` just for completeness sake. Version 0.19.13 diff --git a/src/dynamic.rs b/src/dynamic.rs index b0168ccb..47fe3e2d 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -134,17 +134,6 @@ pub enum AccessMode { ReadOnly, } -impl AccessMode { - /// Is the access type `ReadOnly`? - #[inline(always)] - pub fn is_read_only(self) -> bool { - match self { - Self::ReadWrite => false, - Self::ReadOnly => true, - } - } -} - /// Dynamic type containing any value. pub struct Dynamic(pub(crate) Union); @@ -748,17 +737,25 @@ impl Dynamic { pub fn is_read_only(&self) -> bool { match self.0 { #[cfg(not(feature = "no_closure"))] - Union::Shared(_, access) if access.is_read_only() => true, + Union::Shared(_, AccessMode::ReadOnly) => true, #[cfg(not(feature = "no_closure"))] - #[cfg(not(feature = "sync"))] - Union::Shared(ref cell, _) => cell.borrow().access_mode().is_read_only(), + Union::Shared(ref cell, _) => { + #[cfg(not(feature = "sync"))] + let access = cell.borrow().access_mode(); + #[cfg(feature = "sync")] + let access = cell.read().unwrap().access_mode(); - #[cfg(not(feature = "no_closure"))] - #[cfg(feature = "sync")] - Union::Shared(ref cell, _) => cell.read().unwrap().access_mode().is_read_only(), + match access { + AccessMode::ReadWrite => false, + AccessMode::ReadOnly => true, + } + } - _ => self.access_mode().is_read_only(), + _ => match self.access_mode() { + AccessMode::ReadWrite => false, + AccessMode::ReadOnly => true, + }, } } /// Can this [`Dynamic`] be hashed? @@ -1439,6 +1436,17 @@ impl Dynamic { _ => None, } } + /// Cast the [`Dynamic`] as a unit `()` and return it. + /// Returns the name of the actual type if the cast fails. + #[inline(always)] + pub fn as_unit(&self) -> Result<(), &'static str> { + match self.0 { + Union::Unit(value, _) => Ok(value), + #[cfg(not(feature = "no_closure"))] + Union::Shared(_, _) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), + _ => Err(self.type_name()), + } + } /// Cast the [`Dynamic`] as the system integer type [`INT`] and return it. /// Returns the name of the actual type if the cast fails. #[inline(always)] diff --git a/src/optimize.rs b/src/optimize.rs index b484e643..cff6580b 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -113,17 +113,16 @@ impl<'a> State<'a> { return None; } - for (n, access, expr) in self.variables.iter().rev() { + self.variables.iter().rev().find_map(|(n, access, expr)| { if n == name { - return if access.is_read_only() { - Some(expr) - } else { - None - }; + match access { + AccessMode::ReadWrite => None, + AccessMode::ReadOnly => Some(expr), + } + } else { + None } - } - - None + }) } } From 8f0830af1c3aab3156fca031afa51fbfc6bed445 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 5 Mar 2021 14:18:36 +0800 Subject: [PATCH 2/8] Expose Variant under internals. --- src/dynamic.rs | 92 ++++++++++++++++++++++++++++++-------------------- src/lib.rs | 4 +++ 2 files changed, 59 insertions(+), 37 deletions(-) diff --git a/src/dynamic.rs b/src/dynamic.rs index 47fe3e2d..b4d569f0 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -43,7 +43,10 @@ mod private { impl Sealed for T {} } -/// Trait to represent any type. +/// _(INTERNALS)_ Trait to represent any type. +/// Exported under the `internals` feature only. +/// +/// This trait is sealed and cannot be implemented. /// /// Currently, [`Variant`] is not [`Send`] nor [`Sync`], so it can practically be any type. /// Turn on the `sync` feature to restrict it to only types that implement [`Send`] `+` [`Sync`]. @@ -68,7 +71,10 @@ pub trait Variant: Any + private::Sealed { fn clone_into_dynamic(&self) -> Dynamic; } -/// Trait to represent any type. +/// _(INTERNALS)_ Trait to represent any type. +/// Exported under the `internals` feature only. +/// +/// This trait is sealed and cannot be implemented. #[cfg(feature = "sync")] pub trait Variant: Any + Send + Sync + private::Sealed { /// Convert this [`Variant`] trait object to [`&dyn Any`][Any]. @@ -326,11 +332,14 @@ impl Dynamic { Union::Variant(value, _) => (***value).type_id(), #[cfg(not(feature = "no_closure"))] - #[cfg(not(feature = "sync"))] - Union::Shared(cell, _) => (*cell.borrow()).type_id(), - #[cfg(not(feature = "no_closure"))] - #[cfg(feature = "sync")] - Union::Shared(cell, _) => (*cell.read().unwrap()).type_id(), + Union::Shared(cell, _) => { + #[cfg(not(feature = "sync"))] + let value = cell.borrow(); + #[cfg(feature = "sync")] + let value = cell.read().unwrap(); + + (*value).type_id() + } } } /// Get the name of the type of the value held by this [`Dynamic`]. @@ -397,11 +406,14 @@ impl Hash for Dynamic { } #[cfg(not(feature = "no_closure"))] - #[cfg(not(feature = "sync"))] - Union::Shared(cell, _) => (*cell.borrow()).hash(state), - #[cfg(not(feature = "no_closure"))] - #[cfg(feature = "sync")] - Union::Shared(cell, _) => (*cell.read().unwrap()).hash(state), + Union::Shared(cell, _) => { + #[cfg(not(feature = "sync"))] + let value = cell.borrow(); + #[cfg(feature = "sync")] + let value = cell.read().unwrap(); + + (*value).hash(state) + } _ => unimplemented!("{} cannot be hashed", self.type_name()), } @@ -742,11 +754,11 @@ impl Dynamic { #[cfg(not(feature = "no_closure"))] Union::Shared(ref cell, _) => { #[cfg(not(feature = "sync"))] - let access = cell.borrow().access_mode(); + let value = cell.borrow(); #[cfg(feature = "sync")] - let access = cell.read().unwrap().access_mode(); + let value = cell.read().unwrap(); - match access { + match value.access_mode() { AccessMode::ReadWrite => false, AccessMode::ReadOnly => true, } @@ -775,11 +787,14 @@ impl Dynamic { Union::Map(_, _) => true, #[cfg(not(feature = "no_closure"))] - #[cfg(not(feature = "sync"))] - Union::Shared(cell, _) => cell.borrow().is_hashable(), - #[cfg(not(feature = "no_closure"))] - #[cfg(feature = "sync")] - Union::Shared(cell, _) => cell.read().unwrap().is_hashable(), + Union::Shared(cell, _) => { + #[cfg(not(feature = "sync"))] + let value = cell.borrow(); + #[cfg(feature = "sync")] + let value = cell.read().unwrap(); + + value.is_hashable() + } _ => false, } @@ -1126,10 +1141,11 @@ impl Dynamic { #[cfg(not(feature = "no_closure"))] Union::Shared(cell, _) => { #[cfg(not(feature = "sync"))] - return cell.borrow().clone(); - + let value = cell.borrow(); #[cfg(feature = "sync")] - return cell.read().unwrap().clone(); + let value = cell.read().unwrap(); + + value.clone() } _ => self.clone(), } @@ -1147,9 +1163,11 @@ impl Dynamic { Union::Shared(cell, _) => crate::fn_native::shared_try_take(cell).map_or_else( |cell| { #[cfg(not(feature = "sync"))] - return cell.borrow().clone(); + let value = cell.borrow(); #[cfg(feature = "sync")] - return cell.read().unwrap().clone(); + let value = cell.read().unwrap(); + + value.clone() }, |value| { #[cfg(not(feature = "sync"))] @@ -1197,16 +1215,16 @@ impl Dynamic { #[cfg(not(feature = "no_closure"))] Union::Shared(ref cell, _) => { #[cfg(not(feature = "sync"))] - let data = cell.borrow(); + let value = cell.borrow(); #[cfg(feature = "sync")] - let data = cell.read().unwrap(); + let value = cell.read().unwrap(); - let type_id = (*data).type_id(); + let type_id = (*value).type_id(); if type_id != TypeId::of::() && TypeId::of::() != TypeId::of::() { None } else { - Some(DynamicReadLock(DynamicReadLockInner::Guard(data))) + Some(DynamicReadLock(DynamicReadLockInner::Guard(value))) } } _ => self @@ -1229,16 +1247,16 @@ impl Dynamic { #[cfg(not(feature = "no_closure"))] Union::Shared(ref cell, _) => { #[cfg(not(feature = "sync"))] - let data = cell.borrow_mut(); + let value = cell.borrow_mut(); #[cfg(feature = "sync")] - let data = cell.write().unwrap(); + let value = cell.write().unwrap(); - let type_id = (*data).type_id(); + let type_id = (*value).type_id(); if type_id != TypeId::of::() && TypeId::of::() != TypeId::of::() { None } else { - Some(DynamicWriteLock(DynamicWriteLockInner::Guard(data))) + Some(DynamicWriteLock(DynamicWriteLockInner::Guard(value))) } } _ => self @@ -1537,13 +1555,13 @@ impl Dynamic { #[cfg(not(feature = "no_closure"))] Union::Shared(cell, _) => { #[cfg(not(feature = "sync"))] - let data = cell.borrow(); + let value = cell.borrow(); #[cfg(feature = "sync")] - let data = cell.read().unwrap(); + let value = cell.read().unwrap(); - match &data.0 { + match &value.0 { Union::Str(s, _) => Ok(s.clone()), - _ => Err((*data).type_name()), + _ => Err((*value).type_name()), } } _ => Err(self.type_name()), diff --git a/src/lib.rs b/src/lib.rs index 94652bed..26b7b421 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -180,6 +180,10 @@ pub mod serde; #[cfg(not(feature = "no_optimize"))] pub use optimize::OptimizationLevel; +#[cfg(feature = "internals")] +#[deprecated = "this type is volatile and may change"] +pub use dynamic::Variant; + // Expose internal data structures. #[cfg(feature = "internals")] #[deprecated = "this type is volatile and may change"] From 4e5039d4fea5aebd356a2df2b31f0f9d32d4dcdf Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 5 Mar 2021 20:06:49 +0800 Subject: [PATCH 3/8] Fix bug in built-in string operators. --- src/fn_builtin.rs | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/fn_builtin.rs b/src/fn_builtin.rs index 7d4f9c04..3fdf9a42 100644 --- a/src/fn_builtin.rs +++ b/src/fn_builtin.rs @@ -118,6 +118,13 @@ pub fn get_builtin_binary_op_fn( $func(x, y) }) }; + ($base:ty => $op:tt) => { + return Some(|_, args| { + let x = &*args[0].read_lock::<$base>().unwrap(); + let y = &*args[1].read_lock::<$base>().unwrap(); + Ok((x $op y).into()) + }) + }; } macro_rules! impl_float { @@ -327,26 +334,14 @@ pub fn get_builtin_binary_op_fn( if type1 == TypeId::of::() { match op { - "+" => { - return Some(|_, args| { - let x = &*args[0].read_lock::().unwrap(); - let y = &*args[1].read_lock::().unwrap(); - Ok((x + y).into()) - }) - } - "-" => { - return Some(|_, args| { - let x = &*args[0].read_lock::().unwrap(); - let y = &*args[1].read_lock::().unwrap(); - Ok((x - y).into()) - }) - } - "==" => impl_op!(&str => as_str == as_str), - "!=" => impl_op!(&str => as_str != as_str), - ">" => impl_op!(&str => as_str > as_str), - ">=" => impl_op!(&str => as_str >= as_str), - "<" => impl_op!(&str => as_str < as_str), - "<=" => impl_op!(&str => as_str <= as_str), + "+" => impl_op!(ImmutableString => +), + "-" => impl_op!(ImmutableString => -), + "==" => impl_op!(ImmutableString => ==), + "!=" => impl_op!(ImmutableString => !=), + ">" => impl_op!(ImmutableString => >), + ">=" => impl_op!(ImmutableString => >=), + "<" => impl_op!(ImmutableString => <), + "<=" => impl_op!(ImmutableString => <=), _ => return None, } } From 65ef32af190fd71a490eeaa8b3f3c71ed6132d61 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 5 Mar 2021 20:07:35 +0800 Subject: [PATCH 4/8] Calculate whether contains global functions during indexing. --- src/engine.rs | 2 +- src/fn_call.rs | 83 +++++++++++++++++++++-------------------------- src/module/mod.rs | 60 +++++++++++++++++++--------------- tests/modules.rs | 10 +++--- 4 files changed, 78 insertions(+), 77 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 7a57d972..8c710c69 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1893,7 +1893,7 @@ impl Engine { if mods .scan_raw() .skip(_mods_len) - .any(|(_, m)| m.has_namespace(crate::FnNamespace::Global, true)) + .any(|(_, m)| m.contains_indexed_global_functions()) { if _restore_fn_resolution_cache { // When new module is imported with global functions and there is already diff --git a/src/fn_call.rs b/src/fn_call.rs index 18d11447..6a3db275 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -188,42 +188,6 @@ impl Engine { allow_dynamic: bool, is_op_assignment: bool, ) -> &'s Option<(CallableFunction, Option)> { - fn find_function( - engine: &Engine, - hash: NonZeroU64, - mods: &Imports, - lib: &[&Module], - ) -> Option<(CallableFunction, Option)> { - lib.iter() - .find_map(|m| { - m.get_fn(hash, false) - .map(|f| (f.clone(), m.id_raw().cloned())) - }) - .or_else(|| { - engine - .global_namespace - .get_fn(hash, false) - .cloned() - .map(|f| (f, None)) - }) - .or_else(|| { - engine.global_modules.iter().find_map(|m| { - m.get_fn(hash, false) - .map(|f| (f.clone(), m.id_raw().cloned())) - }) - }) - .or_else(|| { - mods.get_fn(hash) - .map(|(f, source)| (f.clone(), source.cloned())) - }) - .or_else(|| { - engine.global_sub_modules.values().find_map(|m| { - m.get_qualified_fn(hash) - .map(|f| (f.clone(), m.id_raw().cloned())) - }) - }) - } - &*state .fn_resolution_cache_mut() .entry(hash) @@ -237,7 +201,36 @@ impl Engine { let mut bitmask = 1usize; // Bitmask of which parameter to replace with `Dynamic` loop { - match find_function(self, hash, mods, lib) { + let func = lib + .iter() + .find_map(|m| { + m.get_fn(hash, false) + .map(|f| (f.clone(), m.id_raw().cloned())) + }) + .or_else(|| { + self.global_namespace + .get_fn(hash, false) + .cloned() + .map(|f| (f, None)) + }) + .or_else(|| { + self.global_modules.iter().find_map(|m| { + m.get_fn(hash, false) + .map(|f| (f.clone(), m.id_raw().cloned())) + }) + }) + .or_else(|| { + mods.get_fn(hash) + .map(|(f, source)| (f.clone(), source.cloned())) + }) + .or_else(|| { + self.global_sub_modules.values().find_map(|m| { + m.get_qualified_fn(hash) + .map(|f| (f.clone(), m.id_raw().cloned())) + }) + }); + + match func { // Specific version found Some(f) => return Some(f), @@ -520,18 +513,16 @@ impl Engine { ); // Merge in encapsulated environment, if any - let mut lib_merged: StaticVec<_>; - let mut unified = false; + let lib_merged; - let unified_lib = if let Some(ref env_lib) = fn_def.lib { - unified = true; + let (unified_lib, unified) = if let Some(ref env_lib) = fn_def.lib { state.push_fn_resolution_cache(); - lib_merged = Default::default(); - lib_merged.push(env_lib.as_ref()); - lib_merged.extend(lib.iter().cloned()); - lib_merged.as_ref() + lib_merged = once(env_lib.as_ref()) + .chain(lib.iter().cloned()) + .collect::>(); + (lib_merged.as_ref(), true) } else { - lib + (lib, false) }; #[cfg(not(feature = "no_module"))] diff --git a/src/module/mod.rs b/src/module/mod.rs index 0c3a0041..6bcdf3d8 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -127,6 +127,8 @@ pub struct Module { all_type_iterators: HashMap, /// Is the [`Module`] indexed? indexed: bool, + /// Does the [`Module`] contain indexed functions that have been exposed to the global namespace? + contains_indexed_global_functions: bool, } impl Default for Module { @@ -141,6 +143,7 @@ impl Default for Module { type_iterators: Default::default(), all_type_iterators: Default::default(), indexed: false, + contains_indexed_global_functions: false, } } } @@ -434,6 +437,7 @@ impl Module { ) -> &mut Self { self.variables.insert(name.into(), Dynamic::from(value)); self.indexed = false; + self.contains_indexed_global_functions = false; self } @@ -477,6 +481,7 @@ impl Module { }, ); self.indexed = false; + self.contains_indexed_global_functions = false; hash_script } @@ -521,6 +526,7 @@ impl Module { self.all_variables.clear(); self.all_type_iterators.clear(); self.indexed = false; + self.contains_indexed_global_functions = false; &mut self.modules } @@ -581,6 +587,7 @@ impl Module { ) -> &mut Self { self.modules.insert(name.into(), sub_module.into()); self.indexed = false; + self.contains_indexed_global_functions = false; self } @@ -649,6 +656,7 @@ impl Module { f.namespace = namespace; } self.indexed = false; + self.contains_indexed_global_functions = false; self } @@ -704,6 +712,7 @@ impl Module { ); self.indexed = false; + self.contains_indexed_global_functions = false; hash_fn } @@ -1497,6 +1506,7 @@ impl Module { self.all_variables.clear(); self.all_type_iterators.clear(); self.indexed = false; + self.contains_indexed_global_functions = false; self } @@ -1515,6 +1525,7 @@ impl Module { self.all_variables.clear(); self.all_type_iterators.clear(); self.indexed = false; + self.contains_indexed_global_functions = false; self } @@ -1542,6 +1553,7 @@ impl Module { self.all_variables.clear(); self.all_type_iterators.clear(); self.indexed = false; + self.contains_indexed_global_functions = false; self } @@ -1602,6 +1614,7 @@ impl Module { self.all_variables.clear(); self.all_type_iterators.clear(); self.indexed = false; + self.contains_indexed_global_functions = false; self } @@ -1634,6 +1647,7 @@ impl Module { self.all_variables.clear(); self.all_type_iterators.clear(); self.indexed = false; + self.contains_indexed_global_functions = false; self } @@ -1820,29 +1834,14 @@ impl Module { Ok(module) } - /// Are there functions (or type iterators) marked for the specified namespace? - pub fn has_namespace(&self, namespace: FnNamespace, recursive: bool) -> bool { - // Type iterators are default global - if !self.type_iterators.is_empty() { - return true; - } - // Any function marked global? - if self.functions.values().any(|f| f.namespace == namespace) { - return true; - } - - // Scan sub-modules - if recursive { - if self - .modules - .values() - .any(|m| m.has_namespace(namespace, recursive)) - { - return true; - } - } - - false + /// Does the [`Module`] contain indexed functions that have been exposed to the global namespace? + /// + /// # Panics + /// + /// Panics if the [`Module`] is not yet indexed via [`build_index`][Module::build_index]. + #[inline(always)] + pub fn contains_indexed_global_functions(&self) -> bool { + self.contains_indexed_global_functions } /// Scan through all the sub-modules in the [`Module`] and build a hash index of all @@ -1857,11 +1856,15 @@ impl Module { variables: &mut HashMap, functions: &mut HashMap, type_iterators: &mut HashMap, - ) { + ) -> bool { + let mut contains_indexed_global_functions = false; + module.modules.iter().for_each(|(name, m)| { // Index all the sub-modules first. qualifiers.push(name); - index_module(m, qualifiers, variables, functions, type_iterators); + if index_module(m, qualifiers, variables, functions, type_iterators) { + contains_indexed_global_functions = true; + } qualifiers.pop(); }); @@ -1875,6 +1878,7 @@ impl Module { // Index type iterators module.type_iterators.iter().for_each(|(&type_id, func)| { type_iterators.insert(type_id, func.clone()); + contains_indexed_global_functions = true; }); // Index all Rust functions @@ -1895,6 +1899,7 @@ impl Module { FnNamespace::Global => { // Flatten all functions with global namespace functions.insert(hash, func.clone()); + contains_indexed_global_functions = true; } FnNamespace::Internal => (), } @@ -1927,6 +1932,8 @@ impl Module { } }, ); + + contains_indexed_global_functions } if !self.indexed { @@ -1937,7 +1944,7 @@ impl Module { qualifiers.push("root"); - index_module( + self.contains_indexed_global_functions = index_module( self, &mut qualifiers, &mut variables, @@ -1968,6 +1975,7 @@ impl Module { pub fn set_iter(&mut self, typ: TypeId, func: IteratorFn) -> &mut Self { self.type_iterators.insert(typ, func); self.indexed = false; + self.contains_indexed_global_functions = false; self } diff --git a/tests/modules.rs b/tests/modules.rs index c42111b6..44f277e6 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -24,10 +24,12 @@ fn test_module_sub_module() -> Result<(), Box> { sub_module2.set_var("answer", 41 as INT); let hash_inc = sub_module2.set_fn_1_mut("inc", FnNamespace::Internal, |x: &mut INT| Ok(*x + 1)); - assert!(!sub_module2.has_namespace(FnNamespace::Global, true)); + sub_module2.build_index(); + assert!(!sub_module2.contains_indexed_global_functions()); sub_module2.set_fn_1_mut("super_inc", FnNamespace::Global, |x: &mut INT| Ok(*x + 1)); - assert!(sub_module2.has_namespace(FnNamespace::Global, true)); + sub_module2.build_index(); + assert!(sub_module2.contains_indexed_global_functions()); #[cfg(not(feature = "no_object"))] sub_module2.set_getter_fn("doubled", |x: &mut INT| Ok(*x * 2)); @@ -35,9 +37,9 @@ fn test_module_sub_module() -> Result<(), Box> { sub_module.set_sub_module("universe", sub_module2); module.set_sub_module("life", sub_module); module.set_var("MYSTIC_NUMBER", Dynamic::from(42 as INT)); + module.build_index(); - assert!(module.has_namespace(FnNamespace::Global, true)); - assert!(!module.has_namespace(FnNamespace::Global, false)); + assert!(module.contains_indexed_global_functions()); assert!(module.contains_sub_module("life")); let m = module.get_sub_module("life").unwrap(); From ca1ce6b6b838e28a446305403dea98177fe60dab Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 5 Mar 2021 22:58:20 +0800 Subject: [PATCH 5/8] Streamline macros. --- src/fn_builtin.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/fn_builtin.rs b/src/fn_builtin.rs index 3fdf9a42..8a1d96d8 100644 --- a/src/fn_builtin.rs +++ b/src/fn_builtin.rs @@ -70,6 +70,13 @@ pub fn get_builtin_binary_op_fn( let types_pair = (type1, type2); macro_rules! impl_op { + ($xx:ident $op:tt $yy:ident) => { + return Some(|_, args| { + let x = &*args[0].read_lock::<$xx>().unwrap(); + let y = &*args[1].read_lock::<$yy>().unwrap(); + Ok((x $op y).into()) + }) + }; ($func:ident ( $op:tt )) => { return Some(|_, args| { let (x, y) = $func(args); @@ -118,13 +125,6 @@ pub fn get_builtin_binary_op_fn( $func(x, y) }) }; - ($base:ty => $op:tt) => { - return Some(|_, args| { - let x = &*args[0].read_lock::<$base>().unwrap(); - let y = &*args[1].read_lock::<$base>().unwrap(); - Ok((x $op y).into()) - }) - }; } macro_rules! impl_float { @@ -334,14 +334,14 @@ pub fn get_builtin_binary_op_fn( if type1 == TypeId::of::() { match op { - "+" => impl_op!(ImmutableString => +), - "-" => impl_op!(ImmutableString => -), - "==" => impl_op!(ImmutableString => ==), - "!=" => impl_op!(ImmutableString => !=), - ">" => impl_op!(ImmutableString => >), - ">=" => impl_op!(ImmutableString => >=), - "<" => impl_op!(ImmutableString => <), - "<=" => impl_op!(ImmutableString => <=), + "+" => impl_op!(ImmutableString + ImmutableString), + "-" => impl_op!(ImmutableString - ImmutableString), + "==" => impl_op!(ImmutableString == ImmutableString), + "!=" => impl_op!(ImmutableString != ImmutableString), + ">" => impl_op!(ImmutableString > ImmutableString), + ">=" => impl_op!(ImmutableString >= ImmutableString), + "<" => impl_op!(ImmutableString < ImmutableString), + "<=" => impl_op!(ImmutableString <= ImmutableString), _ => return None, } } From a2512197300120598d30a61ce48ae31fd9f472ea Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 5 Mar 2021 23:00:27 +0800 Subject: [PATCH 6/8] Remove public Dynamic::as_str. --- CHANGELOG.md | 1 + src/dynamic.rs | 11 +++++++---- src/fn_call.rs | 24 ++++++++++++------------ src/result.rs | 2 +- tests/closures.rs | 2 +- 5 files changed, 22 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e594d41e..b77d3dea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Bug fixes Breaking changes ---------------- +* `Dynamic::as_str` is removed because it does not properly handle shared values. * Zero step in the `range` function now raises an error instead of creating an infinite stream. * Error variable captured by `catch` is now an _object map_ containing error fields. * `EvalAltResult::clear_position` is renamed `EvalAltResult::take_position` and returns the position taken. diff --git a/src/dynamic.rs b/src/dynamic.rs index b4d569f0..020ed7e8 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -1526,15 +1526,18 @@ impl Dynamic { _ => Err(self.type_name()), } } - /// Cast the [`Dynamic`] as a [`String`] and return the string slice. + /// Cast the [`Dynamic`] as an [`ImmutableString`] and return it. /// Returns the name of the actual type if the cast fails. /// - /// Fails if `self` is _shared_. + /// # Panics + /// + /// Panics if the value is shared. #[inline(always)] - pub fn as_str(&self) -> Result<&str, &'static str> { + pub(crate) fn as_str(&self) -> Result<&str, &'static str> { match &self.0 { Union::Str(s, _) => Ok(s), - Union::FnPtr(f, _) => Ok(f.fn_name()), + #[cfg(not(feature = "no_closure"))] + Union::Shared(_, _) => panic!("as_str() cannot be called on shared values"), _ => Err(self.type_name()), } } diff --git a/src/fn_call.rs b/src/fn_call.rs index 6a3db275..2250b5df 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -353,17 +353,17 @@ impl Engine { // See if the function match print/debug (which requires special processing) return Ok(match fn_name { KEYWORD_PRINT => { - let text = result.as_str().map_err(|typ| { + let text = result.take_immutable_string().map_err(|typ| { EvalAltResult::ErrorMismatchOutputType( self.map_type_name(type_name::()).into(), typ.into(), pos, ) })?; - ((self.print)(text).into(), false) + ((self.print)(&text).into(), false) } KEYWORD_DEBUG => { - let text = result.as_str().map_err(|typ| { + let text = result.take_immutable_string().map_err(|typ| { EvalAltResult::ErrorMismatchOutputType( self.map_type_name(type_name::()).into(), typ.into(), @@ -371,7 +371,7 @@ impl Engine { ) })?; let source = state.source.as_ref().map(|s| s.as_str()); - ((self.debug)(text, source, pos).into(), false) + ((self.debug)(&text, source, pos).into(), false) } _ => (result, func.is_method()), }); @@ -677,7 +677,7 @@ impl Engine { crate::engine::KEYWORD_IS_DEF_FN if args.len() == 2 && args[0].is::() && args[1].is::() => { - let fn_name = args[0].as_str().unwrap(); + let fn_name = mem::take(args[0]).take_immutable_string().unwrap(); let num_params = args[1].as_int().unwrap(); return Ok(( @@ -685,7 +685,7 @@ impl Engine { Dynamic::FALSE } else { let hash_script = - calc_script_fn_hash(empty(), fn_name, num_params as usize); + calc_script_fn_hash(empty(), &fn_name, num_params as usize); self.has_override(Some(mods), state, lib, None, hash_script) .into() }, @@ -1131,7 +1131,7 @@ impl Engine { crate::engine::KEYWORD_IS_DEF_FN if args_expr.len() == 2 => { let fn_name = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; - let fn_name = fn_name.as_str().map_err(|err| { + let fn_name = fn_name.take_immutable_string().map_err(|err| { self.make_type_mismatch_err::(err, args_expr[0].position()) })?; let num_params = @@ -1143,7 +1143,7 @@ impl Engine { return Ok(if num_params < 0 { Dynamic::FALSE } else { - let hash_script = calc_script_fn_hash(empty(), fn_name, num_params as usize); + let hash_script = calc_script_fn_hash(empty(), &fn_name, num_params as usize); self.has_override(Some(mods), state, lib, None, hash_script) .into() }); @@ -1153,10 +1153,10 @@ impl Engine { KEYWORD_IS_DEF_VAR if args_expr.len() == 1 => { let var_name = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; - let var_name = var_name.as_str().map_err(|err| { + let var_name = var_name.take_immutable_string().map_err(|err| { self.make_type_mismatch_err::(err, args_expr[0].position()) })?; - return Ok(scope.contains(var_name).into()); + return Ok(scope.contains(&var_name).into()); } // Handle eval() @@ -1168,7 +1168,7 @@ impl Engine { let prev_len = scope.len(); let script = self.eval_expr(scope, mods, state, lib, this_ptr, script_expr, level)?; - let script = script.as_str().map_err(|typ| { + let script = script.take_immutable_string().map_err(|typ| { self.make_type_mismatch_err::(typ, script_pos) })?; let result = self.eval_script_expr_in_place( @@ -1176,7 +1176,7 @@ impl Engine { mods, state, lib, - script, + &script, script_pos, level + 1, ); diff --git a/src/result.rs b/src/result.rs index 1376dbcb..49743d1d 100644 --- a/src/result.rs +++ b/src/result.rs @@ -190,7 +190,7 @@ impl fmt::Display for EvalAltResult { | Self::ErrorTerminated(_, _) => f.write_str(desc)?, Self::ErrorRuntime(d, _) if d.is::() => { - let s = d.as_str().unwrap(); + let s = &*d.read_lock::().unwrap(); write!(f, "{}: {}", desc, if s.is_empty() { desc } else { s })? } Self::ErrorRuntime(d, _) if d.is::<()>() => f.write_str(desc)?, diff --git a/tests/closures.rs b/tests/closures.rs index 017200d6..d64cccd7 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -300,7 +300,7 @@ fn test_closures_external() -> Result<(), Box> { // Closure 'f' captures: the engine, the AST, and the curried function pointer let f = move |x: INT| fn_ptr.call_dynamic(context, None, [x.into()]); - assert_eq!(f(42)?.as_str(), Ok("hello42")); + assert_eq!(f(42)?.take_string(), Ok("hello42".to_string())); Ok(()) } From 426f841aa28b5ea5f51a0523d97ff1e1829b6d4f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 5 Mar 2021 23:41:20 +0800 Subject: [PATCH 7/8] Fix serde build. --- src/serde/de.rs | 4 +-- tests/serde.rs | 85 +++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 70 insertions(+), 19 deletions(-) diff --git a/src/serde/de.rs b/src/serde/de.rs index 80ba7555..2d9adafa 100644 --- a/src/serde/de.rs +++ b/src/serde/de.rs @@ -440,8 +440,8 @@ impl<'de> Deserializer<'de> for &mut DynamicDeserializer<'de> { _variants: &'static [&'static str], visitor: V, ) -> Result> { - if let Ok(s) = self.value.as_str() { - visitor.visit_enum(s.into_deserializer()) + if let Some(s) = self.value.read_lock::() { + visitor.visit_enum(s.as_str().into_deserializer()) } else { #[cfg(not(feature = "no_object"))] if let Some(map) = self.value.downcast_ref::() { diff --git a/tests/serde.rs b/tests/serde.rs index 56b1edac..d2026506 100644 --- a/tests/serde.rs +++ b/tests/serde.rs @@ -2,10 +2,9 @@ use rhai::{ serde::{from_dynamic, to_dynamic}, - Dynamic, Engine, EvalAltResult, ImmutableString, INT, + Dynamic, Engine, EvalAltResult, INT, }; use serde::{Deserialize, Serialize}; -use std::str::FromStr; #[cfg(not(feature = "no_index"))] use rhai::Array; @@ -95,13 +94,13 @@ fn test_serde_ser_struct() -> Result<(), Box> { let mut map = d.cast::(); let obj = map.remove("obj").unwrap().cast::(); - let seq = map.remove("seq").unwrap().cast::(); + let mut seq = map.remove("seq").unwrap().cast::(); assert_eq!(Ok(123), obj["a"].as_int()); assert!(obj["b"].as_bool().unwrap()); assert_eq!(Ok(42), map["int"].as_int()); assert_eq!(seq.len(), 3); - assert_eq!(Ok("kitty"), seq[1].as_str()); + assert_eq!("kitty", seq.remove(1).take_string().unwrap()); Ok(()) } @@ -115,10 +114,10 @@ fn test_serde_ser_unit_enum() -> Result<(), Box> { } let d = to_dynamic(MyEnum::VariantFoo)?; - assert_eq!(Ok("VariantFoo"), d.as_str()); + assert_eq!("VariantFoo", d.take_string().unwrap()); let d = to_dynamic(MyEnum::VariantBar)?; - assert_eq!(Ok("VariantBar"), d.as_str()); + assert_eq!("VariantBar", d.take_string().unwrap()); Ok(()) } @@ -141,7 +140,13 @@ fn test_serde_ser_externally_tagged_enum() -> Result<(), Box> { } { - assert_eq!(Ok("VariantUnit"), to_dynamic(MyEnum::VariantUnit)?.as_str()); + assert_eq!( + "VariantUnit", + to_dynamic(MyEnum::VariantUnit)? + .take_immutable_string() + .unwrap() + .as_str() + ); } #[cfg(not(feature = "no_index"))] @@ -193,13 +198,24 @@ fn test_serde_ser_internally_tagged_enum() -> Result<(), Box> { let mut map = to_dynamic(MyEnum::VariantEmptyStruct {})?.cast::(); assert_eq!( - Ok("VariantEmptyStruct"), - map.remove("tag").unwrap().as_str() + "VariantEmptyStruct", + map.remove("tag") + .unwrap() + .take_immutable_string() + .unwrap() + .as_str() ); assert!(map.is_empty()); let mut map = to_dynamic(MyEnum::VariantStruct { a: 123 })?.cast::(); - assert_eq!(Ok("VariantStruct"), map.remove("tag").unwrap().as_str()); + assert_eq!( + "VariantStruct", + map.remove("tag") + .unwrap() + .take_immutable_string() + .unwrap() + .as_str() + ); assert_eq!(Ok(123), map.remove("a").unwrap().as_int()); assert!(map.is_empty()); @@ -225,20 +241,41 @@ fn test_serde_ser_adjacently_tagged_enum() -> Result<(), Box> { } let mut map = to_dynamic(MyEnum::VariantUnit)?.cast::(); - assert_eq!(Ok("VariantUnit"), map.remove("tag").unwrap().as_str()); + assert_eq!( + "VariantUnit", + map.remove("tag") + .unwrap() + .take_immutable_string() + .unwrap() + .as_str() + ); assert!(map.is_empty()); #[cfg(not(feature = "no_index"))] { let mut map = to_dynamic(MyEnum::VariantUnitTuple())?.cast::(); - assert_eq!(Ok("VariantUnitTuple"), map.remove("tag").unwrap().as_str()); + assert_eq!( + "VariantUnitTuple", + map.remove("tag") + .unwrap() + .take_immutable_string() + .unwrap() + .as_str() + ); let content = map.remove("content").unwrap().cast::(); assert!(map.is_empty()); assert!(content.is_empty()); } let mut map = to_dynamic(MyEnum::VariantNewtype(123))?.cast::(); - assert_eq!(Ok("VariantNewtype"), map.remove("tag").unwrap().as_str()); + assert_eq!( + "VariantNewtype", + map.remove("tag") + .unwrap() + .take_immutable_string() + .unwrap() + .as_str() + ); let content = map.remove("content").unwrap(); assert!(map.is_empty()); assert_eq!(Ok(123), content.as_int()); @@ -246,7 +283,14 @@ fn test_serde_ser_adjacently_tagged_enum() -> Result<(), Box> { #[cfg(not(feature = "no_index"))] { let mut map = to_dynamic(MyEnum::VariantTuple(123, 456))?.cast::(); - assert_eq!(Ok("VariantTuple"), map.remove("tag").unwrap().as_str()); + assert_eq!( + "VariantTuple", + map.remove("tag") + .unwrap() + .take_immutable_string() + .unwrap() + .as_str() + ); let content = map.remove("content").unwrap().cast::(); assert!(map.is_empty()); assert_eq!(2, content.len()); @@ -256,15 +300,22 @@ fn test_serde_ser_adjacently_tagged_enum() -> Result<(), Box> { let mut map = to_dynamic(MyEnum::VariantEmptyStruct {})?.cast::(); assert_eq!( - Ok("VariantEmptyStruct"), - map.remove("tag").unwrap().as_str() + "VariantEmptyStruct", + map.remove("tag") + .unwrap() + .take_immutable_string() + .unwrap() + .as_str() ); let map_inner = map.remove("content").unwrap().cast::(); assert!(map.is_empty()); assert!(map_inner.is_empty()); let mut map = to_dynamic(MyEnum::VariantStruct { a: 123 })?.cast::(); - assert_eq!(Ok("VariantStruct"), map.remove("tag").unwrap().as_str()); + assert_eq!( + "VariantStruct", + map.remove("tag").unwrap().take_string().unwrap() + ); let mut map_inner = map.remove("content").unwrap().cast::(); assert!(map.is_empty()); assert_eq!(Ok(123), map_inner.remove("a").unwrap().as_int()); From f92e6f3983052d8074e6fa943e26f255a72df84c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 5 Mar 2021 23:56:00 +0800 Subject: [PATCH 8/8] Fix metadata build. --- tests/serde.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/serde.rs b/tests/serde.rs index d2026506..8ee666c4 100644 --- a/tests/serde.rs +++ b/tests/serde.rs @@ -2,7 +2,7 @@ use rhai::{ serde::{from_dynamic, to_dynamic}, - Dynamic, Engine, EvalAltResult, INT, + Dynamic, Engine, EvalAltResult, ImmutableString, INT, }; use serde::{Deserialize, Serialize};