From 36420f0b99b60e9744d09eb58a7d8425d4c5f2f8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 23 Feb 2021 19:08:05 +0800 Subject: [PATCH 01/23] Implement print/debug functions via Dynamic parameters. --- src/dynamic.rs | 88 ++++++++++++++++- src/fn_call.rs | 14 ++- src/packages/string_basic.rs | 184 ++++++++++------------------------- tests/print.rs | 29 +++++- 4 files changed, 173 insertions(+), 142 deletions(-) diff --git a/src/dynamic.rs b/src/dynamic.rs index 8e9e9efb..de5a4bb8 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -475,7 +475,45 @@ impl fmt::Display for Dynamic { #[cfg(not(feature = "no_std"))] Union::TimeStamp(_, _) => f.write_str(""), - Union::Variant(value, _) => f.write_str((*value).type_name()), + Union::Variant(value, _) => { + let _type_id = (***value).type_id(); + + #[cfg(not(feature = "only_i32"))] + #[cfg(not(feature = "only_i64"))] + if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } + + #[cfg(not(feature = "no_float"))] + if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } + + #[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))] + if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } + + f.write_str((***value).type_name()) + } #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] @@ -516,7 +554,53 @@ impl fmt::Debug for Dynamic { #[cfg(not(feature = "no_std"))] Union::TimeStamp(_, _) => write!(f, ""), - Union::Variant(value, _) => write!(f, "{}", (*value).type_name()), + Union::Variant(value, _) => { + let _type_id = (***value).type_id(); + + #[cfg(not(feature = "only_i32"))] + #[cfg(not(feature = "only_i64"))] + if _type_id == TypeId::of::() { + return write!(f, "{:?}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{:?}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{:?}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{:?}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{:?}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{:?}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{:?}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{:?}", (**value).as_any().downcast_ref::().unwrap()); + } + + #[cfg(not(feature = "no_float"))] + if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } else if _type_id == TypeId::of::() { + return write!(f, "{}", (**value).as_any().downcast_ref::().unwrap()); + } + + #[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))] + if _type_id == TypeId::of::() { + return write!( + f, + "{:?}", + (**value).as_any().downcast_ref::().unwrap() + ); + } else if _type_id == TypeId::of::() { + return write!( + f, + "{:?}", + (**value).as_any().downcast_ref::().unwrap() + ); + } + + write!(f, "{}", (*value).type_name()) + } #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] diff --git a/src/fn_call.rs b/src/fn_call.rs index a63ba83a..8e283a8d 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -186,9 +186,8 @@ impl Engine { .entry(hash_fn) .or_insert_with(|| { let num_args = args.len(); - let max_bitmask = 1usize << num_args; let mut hash = hash_fn; - let mut bitmask = 1usize; + let mut bitmask = 1usize; // Bitmask of which parameter to replace with `Dynamic` loop { //lib.get_fn(hash, pub_only).or_else(|| @@ -210,8 +209,8 @@ impl Engine { // Specific version found Some(f) => return Some(f), - // No more permutations with `Dynamic` wildcards - _ if bitmask >= max_bitmask => return None, + // No parameters + _ if num_args == 0 => return None, // Try all permutations with `Dynamic` wildcards _ => { @@ -219,12 +218,19 @@ impl Engine { let arg_types = args.iter().enumerate().map(|(i, a)| { let mask = 1usize << (num_args - i - 1); if bitmask & mask != 0 { + // Replace with `Dynamic` TypeId::of::() } else { a.type_id() } }); hash = calc_native_fn_hash(empty(), fn_name, arg_types).unwrap(); + + // Stop when all permutations are exhausted + if hash == hash_fn { + return None; + } + bitmask += 1; } } diff --git a/src/packages/string_basic.rs b/src/packages/string_basic.rs index 4024cf2f..faecf29e 100644 --- a/src/packages/string_basic.rs +++ b/src/packages/string_basic.rs @@ -1,13 +1,8 @@ #![allow(non_snake_case)] -use crate::engine::{KEYWORD_DEBUG, KEYWORD_PRINT}; use crate::plugin::*; -use crate::stdlib::{ - fmt::{Debug, Display}, - format, - string::ToString, -}; -use crate::{def_package, FnPtr, ImmutableString, INT}; +use crate::stdlib::{format, string::ToString}; +use crate::{def_package, FnPtr, ImmutableString}; #[cfg(not(feature = "no_index"))] use crate::Array; @@ -15,139 +10,12 @@ use crate::Array; #[cfg(not(feature = "no_object"))] use crate::Map; -#[cfg(feature = "decimal")] -use rust_decimal::Decimal; - -const FUNC_TO_STRING: &'static str = "to_string"; const FUNC_TO_DEBUG: &'static str = "to_debug"; -type Unit = (); - -macro_rules! gen_functions { - ($root:ident => $fn_name:ident ( $($arg_type:ident),+ )) => { - pub mod $root { $(pub mod $arg_type { - use super::super::*; - - #[export_fn(pure)] - pub fn to_string_func(x: &mut $arg_type) -> ImmutableString { - super::super::$fn_name(x) - } - })* } - } -} - -macro_rules! reg_print_functions { - ($mod_name:ident += $root:ident ; $($arg_type:ident),+) => { $( - set_exported_fn!($mod_name, FUNC_TO_STRING, $root::$arg_type::to_string_func); - set_exported_fn!($mod_name, KEYWORD_PRINT, $root::$arg_type::to_string_func); - )* } -} - -macro_rules! reg_debug_functions { - ($mod_name:ident += $root:ident ; $($arg_type:ident),+) => { $( - set_exported_fn!($mod_name, FUNC_TO_DEBUG, $root::$arg_type::to_string_func); - set_exported_fn!($mod_name, KEYWORD_DEBUG, $root::$arg_type::to_string_func); - )* } -} - def_package!(crate:BasicStringPackage:"Basic string utilities, including printing.", lib, { combine_with_exported_module!(lib, "print_debug", print_debug_functions); - - reg_print_functions!(lib += print_basic; INT, bool, char, FnPtr); - reg_debug_functions!(lib += debug_basic; INT, bool, Unit, char, ImmutableString); - - #[cfg(not(feature = "only_i32"))] - #[cfg(not(feature = "only_i64"))] - { - reg_print_functions!(lib += print_numbers; i8, u8, i16, u16, i32, u32, i64, u64); - reg_debug_functions!(lib += debug_numbers; i8, u8, i16, u16, i32, u32, i64, u64); - - #[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))] - { - reg_print_functions!(lib += print_num_128; i128, u128); - reg_debug_functions!(lib += debug_num_128; i128, u128); - } - } - - #[cfg(not(feature = "no_float"))] - { - reg_print_functions!(lib += print_float_64; f64); - reg_debug_functions!(lib += print_float_64; f64); - reg_print_functions!(lib += print_float_32; f32); - reg_debug_functions!(lib += print_float_32; f32); - } - - #[cfg(feature = "decimal")] - { - reg_print_functions!(lib += print_decimal; Decimal); - reg_debug_functions!(lib += debug_decimal; Decimal); - } }); -fn to_string(x: &mut T) -> ImmutableString { - x.to_string().into() -} -fn to_debug(x: &mut T) -> ImmutableString { - format!("{:?}", x).into() -} -#[cfg(not(feature = "no_float"))] -fn print_f64(x: &mut f64) -> ImmutableString { - #[cfg(feature = "no_std")] - use num_traits::Float; - - let abs = x.abs(); - if abs > 10000000000000.0 || abs < 0.0000000000001 { - format!("{:e}", x).into() - } else { - x.to_string().into() - } -} -#[cfg(not(feature = "no_float"))] -fn print_f32(x: &mut f32) -> ImmutableString { - #[cfg(feature = "no_std")] - use num_traits::Float; - - let abs = x.abs(); - if abs > 10000000000000.0 || abs < 0.0000000000001 { - format!("{:e}", x).into() - } else { - x.to_string().into() - } -} - -gen_functions!(print_basic => to_string(INT, bool, char, FnPtr)); -gen_functions!(debug_basic => to_debug(INT, bool, Unit, char, ImmutableString)); - -#[cfg(not(feature = "only_i32"))] -#[cfg(not(feature = "only_i64"))] -gen_functions!(print_numbers => to_string(i8, u8, i16, u16, i32, u32, i64, u64)); - -#[cfg(not(feature = "only_i32"))] -#[cfg(not(feature = "only_i64"))] -gen_functions!(debug_numbers => to_debug(i8, u8, i16, u16, i32, u32, i64, u64)); - -#[cfg(not(feature = "only_i32"))] -#[cfg(not(feature = "only_i64"))] -#[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))] -gen_functions!(print_num_128 => to_string(i128, u128)); - -#[cfg(not(feature = "only_i32"))] -#[cfg(not(feature = "only_i64"))] -#[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))] -gen_functions!(debug_num_128 => to_debug(i128, u128)); - -#[cfg(not(feature = "no_float"))] -gen_functions!(print_float_64 => print_f64(f64)); - -#[cfg(not(feature = "no_float"))] -gen_functions!(print_float_32 => print_f32(f32)); - -#[cfg(feature = "decimal")] -gen_functions!(print_decimal => to_string(Decimal)); - -#[cfg(feature = "decimal")] -gen_functions!(debug_decimal => to_debug(Decimal)); - // Register print and debug #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] @@ -162,6 +30,16 @@ fn print_with_func(fn_name: &str, ctx: &NativeCallContext, value: &mut Dynamic) #[export_module] mod print_debug_functions { + use crate::ImmutableString; + + #[rhai_fn(name = "print", name = "to_string", pure)] + pub fn print_generic(item: &mut Dynamic) -> ImmutableString { + item.to_string().into() + } + #[rhai_fn(name = "debug", name = "to_debug", pure)] + pub fn debug_generic(item: &mut Dynamic) -> ImmutableString { + format!("{:?}", item).into() + } #[rhai_fn(name = "print", name = "debug")] pub fn print_empty_string() -> ImmutableString { "".to_string().into() @@ -176,7 +54,43 @@ mod print_debug_functions { } #[rhai_fn(name = "debug", pure)] pub fn debug_fn_ptr(f: &mut FnPtr) -> ImmutableString { - to_string(f) + f.to_string().into() + } + + #[cfg(not(feature = "no_float"))] + pub mod float_functions { + #[rhai_fn(name = "print", name = "to_string")] + pub fn print_f64(number: f64) -> ImmutableString { + #[cfg(feature = "no_std")] + use num_traits::Float; + + let abs = number.abs(); + if abs > 10000000000000.0 || abs < 0.0000000000001 { + format!("{:e}", number).into() + } else { + number.to_string().into() + } + } + #[rhai_fn(name = "print", name = "to_string")] + pub fn print_f32(number: f32) -> ImmutableString { + #[cfg(feature = "no_std")] + use num_traits::Float; + + let abs = number.abs(); + if abs > 10000000000000.0 || abs < 0.0000000000001 { + format!("{:e}", number).into() + } else { + number.to_string().into() + } + } + #[rhai_fn(name = "debug", name = "to_debug")] + pub fn debug_f64(number: f64) -> ImmutableString { + number.to_string().into() + } + #[rhai_fn(name = "debug", name = "to_debug")] + pub fn debug_f32(number: f32) -> ImmutableString { + number.to_string().into() + } } #[cfg(not(feature = "no_index"))] diff --git a/tests/print.rs b/tests/print.rs index 4adda4e2..c9d7fd28 100644 --- a/tests/print.rs +++ b/tests/print.rs @@ -1,6 +1,33 @@ -use rhai::{Engine, EvalAltResult, RegisterFn, INT}; +use rhai::{Engine, EvalAltResult, RegisterFn, Scope, INT}; use std::sync::{Arc, RwLock}; +#[cfg(not(feature = "only_i32"))] +#[cfg(not(feature = "only_i64"))] +#[test] +fn test_to_string() -> Result<(), Box> { + let engine = Engine::new(); + + let mut scope = Scope::new(); + scope.push("x", 42_u8); + scope.push("y", 42_i32); + scope.push("z", 42_i16); + + assert_eq!( + engine.eval_with_scope::(&mut scope, "x.to_string()")?, + "42" + ); + assert_eq!( + engine.eval_with_scope::(&mut scope, "y.to_string()")?, + "42" + ); + assert_eq!( + engine.eval_with_scope::(&mut scope, "z.to_string()")?, + "42" + ); + + Ok(()) +} + #[test] fn test_print_debug() -> Result<(), Box> { let logbook = Arc::new(RwLock::new(Vec::::new())); From 93d970235e45084508de01e374f26fa7ec3c084f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 23 Feb 2021 19:31:43 +0800 Subject: [PATCH 02/23] Fix no_object. --- tests/print.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/print.rs b/tests/print.rs index c9d7fd28..d2679073 100644 --- a/tests/print.rs +++ b/tests/print.rs @@ -13,15 +13,15 @@ fn test_to_string() -> Result<(), Box> { scope.push("z", 42_i16); assert_eq!( - engine.eval_with_scope::(&mut scope, "x.to_string()")?, + engine.eval_with_scope::(&mut scope, "to_string(x)")?, "42" ); assert_eq!( - engine.eval_with_scope::(&mut scope, "y.to_string()")?, + engine.eval_with_scope::(&mut scope, "to_string(x)")?, "42" ); assert_eq!( - engine.eval_with_scope::(&mut scope, "z.to_string()")?, + engine.eval_with_scope::(&mut scope, "to_string(x)")?, "42" ); From 123e9d69014230e4b6dca02f7296dccee874e308 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 23 Feb 2021 20:03:28 +0800 Subject: [PATCH 03/23] Short-circuits op-assignment for indexing and dotting. --- src/engine.rs | 125 +++++++++++++++++++++++++++++++++++-------------- src/fn_call.rs | 10 ++-- 2 files changed, 96 insertions(+), 39 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 39c67d5c..91741fb5 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1088,7 +1088,7 @@ impl Engine { idx_values: &mut StaticVec, chain_type: ChainType, level: usize, - new_val: Option<(Dynamic, Position)>, + new_val: Option<((Dynamic, Position), (&str, Position))>, ) -> Result<(Dynamic, bool), Box> { if chain_type == ChainType::NonChaining { unreachable!("should not be ChainType::NonChaining"); @@ -1128,7 +1128,7 @@ impl Engine { ) .map_err(|err| err.fill_position(*x_pos)) } - // xxx[rhs] = new_val + // xxx[rhs] op= new_val _ if new_val.is_some() => { let idx_val = idx_val.as_index_value(); let mut idx_val2 = idx_val.clone(); @@ -1139,8 +1139,46 @@ impl Engine { ) { // Indexed value is a reference - update directly Ok(ref mut obj_ptr) => { - let (new_val, new_val_pos) = new_val.unwrap(); - obj_ptr.set_value(new_val, new_val_pos)?; + let ((mut new_val, new_val_pos), (op, op_pos)) = new_val.unwrap(); + + if op.is_empty() { + obj_ptr.set_value(new_val, new_val_pos)?; + } else { + let mut lock_guard; + let lhs_ptr_inner; + + if cfg!(not(feature = "no_closure")) && obj_ptr.is_shared() { + lock_guard = + obj_ptr.as_mut().write_lock::().unwrap(); + lhs_ptr_inner = lock_guard.deref_mut(); + } else { + lhs_ptr_inner = obj_ptr.as_mut(); + } + + let args = &mut [lhs_ptr_inner, &mut new_val]; + + match self.exec_fn_call( + mods, state, lib, op, None, args, true, false, false, + op_pos, None, None, level, + ) { + Ok(_) => (), + Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) => + { + // Expand to `var = var op rhs` + let op = &op[..op.len() - 1]; // extract operator without = + + // Run function + let (value, _) = self.exec_fn_call( + mods, state, lib, op, None, args, true, false, + false, op_pos, None, None, level, + )?; + + *args[0] = value.flatten(); + } + err => return err, + } + } + None } Err(err) => match *err { @@ -1155,11 +1193,13 @@ impl Engine { #[cfg(not(feature = "no_index"))] if let Some(mut new_val) = _call_setter { let val_type_name = target_val.type_name(); - let args = &mut [target_val, &mut idx_val2, &mut new_val.0]; + let ((_, val_pos), _) = new_val; + + let args = &mut [target_val, &mut idx_val2, &mut (new_val.0).0]; self.exec_fn_call( mods, state, lib, FN_IDX_SET, None, args, is_ref, true, false, - new_val.1, None, None, level, + val_pos, None, None, level, ) .map_err(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) @@ -1213,7 +1253,7 @@ impl Engine { Expr::FnCall(_, _) => { unreachable!("function call in dot chain should not be namespace-qualified") } - // {xxx:map}.id = ??? + // {xxx:map}.id op= ??? Expr::Property(x) if target_val.is::() && new_val.is_some() => { let Ident { name, pos } = &x.2; let index = name.clone().into(); @@ -1221,10 +1261,46 @@ impl Engine { mods, state, lib, target_val, index, *pos, true, is_ref, false, level, )?; - let (new_val, new_val_pos) = new_val.unwrap(); - val.set_value(new_val, new_val_pos)?; + let ((mut new_val, new_val_pos), (op, op_pos)) = new_val.unwrap(); - Ok((Default::default(), true)) + if op.is_empty() { + val.set_value(new_val, new_val_pos)?; + } else { + let mut lock_guard; + let lhs_ptr_inner; + + if cfg!(not(feature = "no_closure")) && val.is_shared() { + lock_guard = val.as_mut().write_lock::().unwrap(); + lhs_ptr_inner = lock_guard.deref_mut(); + } else { + lhs_ptr_inner = val.as_mut(); + } + + let args = &mut [lhs_ptr_inner, &mut new_val]; + + match self.exec_fn_call( + mods, state, lib, op, None, args, true, false, false, op_pos, None, + None, level, + ) { + Ok(_) => (), + Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) => + { + // Expand to `var = var op rhs` + let op = &op[..op.len() - 1]; // extract operator without = + + // Run function + let (value, _) = self.exec_fn_call( + mods, state, lib, op, None, args, true, false, false, + op_pos, None, None, level, + )?; + + *args[0] = value.flatten(); + } + err => return err, + } + } + + Ok((Dynamic::UNIT, true)) } // {xxx:map}.id Expr::Property(x) if target_val.is::() => { @@ -1240,7 +1316,7 @@ impl Engine { Expr::Property(x) if new_val.is_some() => { let (_, setter, Ident { pos, .. }) = x.as_ref(); let mut new_val = new_val; - let mut args = [target_val, &mut new_val.as_mut().unwrap().0]; + let mut args = [target_val, &mut (new_val.as_mut().unwrap().0).0]; self.exec_fn_call( mods, state, lib, setter, None, &mut args, is_ref, true, false, *pos, None, None, level, @@ -1401,7 +1477,7 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, expr: &Expr, level: usize, - new_val: Option<(Dynamic, Position)>, + new_val: Option<((Dynamic, Position), (&str, Position))>, ) -> Result> { let (crate::ast::BinaryExpr { lhs, rhs }, chain_type, op_pos) = match expr { Expr::Index(x, pos) => (x.as_ref(), ChainType::Index, *pos), @@ -2035,6 +2111,7 @@ impl Engine { } err => return err.map(|(v, _)| v), } + Ok(Dynamic::UNIT) } } @@ -2042,28 +2119,8 @@ impl Engine { // lhs op= rhs Stmt::Assignment(x, op_pos) => { let (lhs_expr, op, rhs_expr) = x.as_ref(); - let mut rhs_val = - self.eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)?; - - let _new_val = if op.is_empty() { - // Normal assignment - Some((rhs_val, rhs_expr.position())) - } else { - // Op-assignment - always map to `lhs = lhs op rhs` - let op = &op[..op.len() - 1]; // extract operator without = - let args = &mut [ - &mut self.eval_expr(scope, mods, state, lib, this_ptr, lhs_expr, level)?, - &mut rhs_val, - ]; - - Some( - self.exec_fn_call( - mods, state, lib, op, None, args, false, false, false, *op_pos, None, - None, level, - ) - .map(|(v, _)| (v, rhs_expr.position()))?, - ) - }; + let rhs_val = self.eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)?; + let _new_val = Some(((rhs_val, rhs_expr.position()), (op.as_ref(), *op_pos))); // Must be either `var[index] op= val` or `var.prop op= val` match lhs_expr { diff --git a/src/fn_call.rs b/src/fn_call.rs index 8e283a8d..943be50e 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -287,6 +287,11 @@ impl Engine { // See if it is built in. if args.len() == 2 { + match run_builtin_binary_op(fn_name, args[0], args[1])? { + Some(v) => return Ok((v, false)), + None => (), + } + if is_ref { let (first, second) = args.split_first_mut().unwrap(); @@ -295,11 +300,6 @@ impl Engine { None => (), } } - - match run_builtin_binary_op(fn_name, args[0], args[1])? { - Some(v) => return Ok((v, false)), - None => (), - } } // Return default value (if any) From 6f876e85cce83e6f55dd35a1c533e50eb26731a1 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 23 Feb 2021 20:32:50 +0800 Subject: [PATCH 04/23] Always call native for op-assignment operators. --- src/engine.rs | 59 ++++++++++++++++++++++++++++++----------------- src/fn_call.rs | 30 +++++++++++++----------- src/module/mod.rs | 2 -- src/parser.rs | 6 ----- 4 files changed, 54 insertions(+), 43 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 91741fb5..56d26829 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1156,21 +1156,33 @@ impl Engine { } let args = &mut [lhs_ptr_inner, &mut new_val]; + let hash_fn = calc_native_fn_hash( + empty(), + op, + args.iter().map(|a| a.type_id()), + ) + .unwrap(); - match self.exec_fn_call( - mods, state, lib, op, None, args, true, false, false, - op_pos, None, None, level, + match self.call_native_fn( + mods, state, lib, op, hash_fn, args, true, false, op_pos, + None, ) { Ok(_) => (), Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) => { // Expand to `var = var op rhs` let op = &op[..op.len() - 1]; // extract operator without = + let hash_fn = calc_native_fn_hash( + empty(), + op, + args.iter().map(|a| a.type_id()), + ) + .unwrap(); // Run function - let (value, _) = self.exec_fn_call( - mods, state, lib, op, None, args, true, false, - false, op_pos, None, None, level, + let (value, _) = self.call_native_fn( + mods, state, lib, op, hash_fn, args, true, false, + op_pos, None, )?; *args[0] = value.flatten(); @@ -1277,21 +1289,29 @@ impl Engine { } let args = &mut [lhs_ptr_inner, &mut new_val]; + let hash_fn = + calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())) + .unwrap(); - match self.exec_fn_call( - mods, state, lib, op, None, args, true, false, false, op_pos, None, - None, level, + match self.call_native_fn( + mods, state, lib, op, hash_fn, args, true, false, op_pos, None, ) { Ok(_) => (), Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) => { // Expand to `var = var op rhs` let op = &op[..op.len() - 1]; // extract operator without = + let hash_fn = calc_native_fn_hash( + empty(), + op, + args.iter().map(|a| a.type_id()), + ) + .unwrap(); // Run function - let (value, _) = self.exec_fn_call( - mods, state, lib, op, None, args, true, false, false, - op_pos, None, None, level, + let (value, _) = self.call_native_fn( + mods, state, lib, op, hash_fn, args, true, false, op_pos, + None, )?; *args[0] = value.flatten(); @@ -1757,12 +1777,9 @@ impl Engine { for value in rhs_value.iter_mut() { let args = &mut [&mut lhs_value.clone(), value]; - - // Qualifiers (none) + function name + number of arguments + argument `TypeId`'s. let hash_fn = calc_native_fn_hash(empty(), OP_EQUALS, args.iter().map(|a| a.type_id())) .unwrap(); - let pos = rhs.position(); if self @@ -2090,10 +2107,11 @@ impl Engine { } let args = &mut [lhs_ptr_inner, &mut rhs_val]; + let hash_fn = + calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())).unwrap(); - match self.exec_fn_call( - mods, state, lib, op, None, args, true, false, false, *op_pos, None, None, - level, + match self.call_native_fn( + mods, state, lib, op, hash_fn, args, true, false, *op_pos, None, ) { Ok(_) => (), Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op.as_ref())) => @@ -2102,9 +2120,8 @@ impl Engine { let op = &op[..op.len() - 1]; // extract operator without = // Run function - let (value, _) = self.exec_fn_call( - mods, state, lib, op, None, args, false, false, false, *op_pos, - None, None, level, + let (value, _) = self.call_native_fn( + mods, state, lib, op, hash_fn, args, false, false, *op_pos, None, )?; *args[0] = value.flatten(); diff --git a/src/fn_call.rs b/src/fn_call.rs index 943be50e..d8e7723c 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -214,17 +214,20 @@ impl Engine { // Try all permutations with `Dynamic` wildcards _ => { - // Qualifiers (none) + function name + number of arguments + argument `TypeId`'s. - let arg_types = args.iter().enumerate().map(|(i, a)| { - let mask = 1usize << (num_args - i - 1); - if bitmask & mask != 0 { - // Replace with `Dynamic` - TypeId::of::() - } else { - a.type_id() - } - }); - hash = calc_native_fn_hash(empty(), fn_name, arg_types).unwrap(); + hash = calc_native_fn_hash( + empty(), + fn_name, + args.iter().enumerate().map(|(i, a)| { + let mask = 1usize << (num_args - i - 1); + if bitmask & mask != 0 { + // Replace with `Dynamic` + TypeId::of::() + } else { + a.type_id() + } + }), + ) + .unwrap(); // Stop when all permutations are exhausted if hash == hash_fn { @@ -613,9 +616,8 @@ impl Engine { ensure_no_data_race(fn_name, args, is_ref)?; } - // Qualifiers (none) + function name + number of arguments + argument `TypeId`'s. - let arg_types = args.iter().map(|a| a.type_id()); - let hash_fn = calc_native_fn_hash(empty(), fn_name, arg_types).unwrap(); + let hash_fn = + calc_native_fn_hash(empty(), fn_name, args.iter().map(|a| a.type_id())).unwrap(); match fn_name { // type_of diff --git a/src/module/mod.rs b/src/module/mod.rs index 2c3f1e51..01a7ed25 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1855,7 +1855,6 @@ impl Module { // Index all variables module.variables.iter().for_each(|(var_name, value)| { - // Qualifiers + variable name let hash_var = crate::calc_script_fn_hash(qualifiers.iter().map(|&v| v), var_name, 0).unwrap(); variables.insert(hash_var, value.clone()); @@ -1888,7 +1887,6 @@ impl Module { functions.insert(hash, func.clone()); } - // Qualifiers + function name + number of arguments. let hash_qualified_script = crate::calc_script_fn_hash(qualifiers.iter().cloned(), name, *params) .unwrap(); diff --git a/src/parser.rs b/src/parser.rs index bd8b2556..e0d1073c 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -347,7 +347,6 @@ fn parse_fn_call( let qualifiers = modules.iter().map(|m| m.name.as_str()); calc_script_fn_hash(qualifiers, &id, 0) } else { - // Qualifiers (none) + function name + no parameters. calc_script_fn_hash(empty(), &id, 0) }; @@ -399,7 +398,6 @@ fn parse_fn_call( let qualifiers = modules.iter().map(|m| m.name.as_str()); calc_script_fn_hash(qualifiers, &id, args.len()) } else { - // Qualifiers (none) + function name + number of arguments. calc_script_fn_hash(empty(), &id, args.len()) }; @@ -1016,7 +1014,6 @@ fn parse_primary( }); lib.insert( - // Qualifiers (none) + function name + number of arguments. calc_script_fn_hash(empty(), &func.name, func.params.len()).unwrap(), func, ); @@ -1246,7 +1243,6 @@ fn parse_primary( } .map(|x| match x.as_mut() { (_, Some((ref mut hash, ref mut namespace)), Ident { name, .. }) => { - // Qualifiers + variable name *hash = calc_script_fn_hash(namespace.iter().map(|v| v.name.as_str()), name, 0).unwrap(); @@ -2621,8 +2617,6 @@ fn parse_stmt( }; let func = parse_fn(input, &mut new_state, lib, access, settings, _comments)?; - - // Qualifiers (none) + function name + number of arguments. let hash = calc_script_fn_hash(empty(), &func.name, func.params.len()).unwrap(); if lib.contains_key(&hash) { From 71680e3c772a60c649dfcb96e8d2b65e922fa967 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 24 Feb 2021 11:04:54 +0800 Subject: [PATCH 05/23] Extract op assignment into function. --- src/engine.rs | 224 ++++++++++++++++++-------------------------------- 1 file changed, 79 insertions(+), 145 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 56d26829..ed8b9c38 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1138,59 +1138,11 @@ impl Engine { mods, state, lib, target_val, idx_val, pos, true, is_ref, false, level, ) { // Indexed value is a reference - update directly - Ok(ref mut obj_ptr) => { - let ((mut new_val, new_val_pos), (op, op_pos)) = new_val.unwrap(); - - if op.is_empty() { - obj_ptr.set_value(new_val, new_val_pos)?; - } else { - let mut lock_guard; - let lhs_ptr_inner; - - if cfg!(not(feature = "no_closure")) && obj_ptr.is_shared() { - lock_guard = - obj_ptr.as_mut().write_lock::().unwrap(); - lhs_ptr_inner = lock_guard.deref_mut(); - } else { - lhs_ptr_inner = obj_ptr.as_mut(); - } - - let args = &mut [lhs_ptr_inner, &mut new_val]; - let hash_fn = calc_native_fn_hash( - empty(), - op, - args.iter().map(|a| a.type_id()), - ) - .unwrap(); - - match self.call_native_fn( - mods, state, lib, op, hash_fn, args, true, false, op_pos, - None, - ) { - Ok(_) => (), - Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) => - { - // Expand to `var = var op rhs` - let op = &op[..op.len() - 1]; // extract operator without = - let hash_fn = calc_native_fn_hash( - empty(), - op, - args.iter().map(|a| a.type_id()), - ) - .unwrap(); - - // Run function - let (value, _) = self.call_native_fn( - mods, state, lib, op, hash_fn, args, true, false, - op_pos, None, - )?; - - *args[0] = value.flatten(); - } - err => return err, - } - } - + Ok(obj_ptr) => { + let ((new_val, new_pos), (op, op_pos)) = new_val.unwrap(); + self.eval_op_assignment( + mods, state, lib, op, op_pos, obj_ptr, new_val, new_pos, + )?; None } Err(err) => match *err { @@ -1269,57 +1221,13 @@ impl Engine { Expr::Property(x) if target_val.is::() && new_val.is_some() => { let Ident { name, pos } = &x.2; let index = name.clone().into(); - let mut val = self.get_indexed_mut( + let val = self.get_indexed_mut( mods, state, lib, target_val, index, *pos, true, is_ref, false, level, )?; - - let ((mut new_val, new_val_pos), (op, op_pos)) = new_val.unwrap(); - - if op.is_empty() { - val.set_value(new_val, new_val_pos)?; - } else { - let mut lock_guard; - let lhs_ptr_inner; - - if cfg!(not(feature = "no_closure")) && val.is_shared() { - lock_guard = val.as_mut().write_lock::().unwrap(); - lhs_ptr_inner = lock_guard.deref_mut(); - } else { - lhs_ptr_inner = val.as_mut(); - } - - let args = &mut [lhs_ptr_inner, &mut new_val]; - let hash_fn = - calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())) - .unwrap(); - - match self.call_native_fn( - mods, state, lib, op, hash_fn, args, true, false, op_pos, None, - ) { - Ok(_) => (), - Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) => - { - // Expand to `var = var op rhs` - let op = &op[..op.len() - 1]; // extract operator without = - let hash_fn = calc_native_fn_hash( - empty(), - op, - args.iter().map(|a| a.type_id()), - ) - .unwrap(); - - // Run function - let (value, _) = self.call_native_fn( - mods, state, lib, op, hash_fn, args, true, false, op_pos, - None, - )?; - - *args[0] = value.flatten(); - } - err => return err, - } - } - + let ((new_val, new_pos), (op, op_pos)) = new_val.unwrap(); + self.eval_op_assignment( + mods, state, lib, op, op_pos, val, new_val, new_pos, + )?; Ok((Dynamic::UNIT, true)) } // {xxx:map}.id @@ -2037,6 +1945,63 @@ impl Engine { result } + pub(crate) fn eval_op_assignment( + &self, + mods: &mut Imports, + state: &mut State, + lib: &[&Module], + op: &str, + op_pos: Position, + mut target: Target, + mut new_value: Dynamic, + new_value_pos: Position, + ) -> Result<(), Box> { + if target.as_ref().is_read_only() { + unreachable!("LHS should not be read-only"); + } + + if op.is_empty() { + // Normal assignment + target.set_value(new_value, new_value_pos)?; + Ok(()) + } else { + let mut lock_guard; + let lhs_ptr_inner; + + if cfg!(not(feature = "no_closure")) && target.is_shared() { + lock_guard = target.as_mut().write_lock::().unwrap(); + lhs_ptr_inner = lock_guard.deref_mut(); + } else { + lhs_ptr_inner = target.as_mut(); + } + + let args = &mut [lhs_ptr_inner, &mut new_value]; + let hash_fn = + calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())).unwrap(); + + match self.call_native_fn( + mods, state, lib, op, hash_fn, args, true, false, op_pos, None, + ) { + Ok(_) => (), + Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) => + { + // Expand to `var = var op rhs` + let op = &op[..op.len() - 1]; // extract operator without = + + // Run function + let (value, _) = self.call_native_fn( + mods, state, lib, op, hash_fn, args, false, false, op_pos, None, + )?; + + *args[0] = value.flatten(); + } + err => return err.map(|_| ()), + } + + Ok(()) + } + } + /// Evaluate a statement. /// /// # Safety @@ -2065,10 +2030,10 @@ impl Engine { // var op= rhs Stmt::Assignment(x, op_pos) if x.0.get_variable_access(false).is_some() => { let (lhs_expr, op, rhs_expr) = x.as_ref(); - let mut rhs_val = self + let rhs_val = self .eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)? .flatten(); - let (mut lhs_ptr, pos) = + let (lhs_ptr, pos) = self.search_namespace(scope, mods, state, lib, this_ptr, lhs_expr)?; if !lhs_ptr.is_ref() { @@ -2087,48 +2052,17 @@ impl Engine { lhs_expr.get_variable_access(false).unwrap().to_string(), pos, ))) - } else if op.is_empty() { - // Normal assignment - if cfg!(not(feature = "no_closure")) && lhs_ptr.is_shared() { - *lhs_ptr.as_mut().write_lock::().unwrap() = rhs_val; - } else { - *lhs_ptr.as_mut() = rhs_val; - } - Ok(Dynamic::UNIT) } else { - let mut lock_guard; - let lhs_ptr_inner; - - if cfg!(not(feature = "no_closure")) && lhs_ptr.is_shared() { - lock_guard = lhs_ptr.as_mut().write_lock::().unwrap(); - lhs_ptr_inner = lock_guard.deref_mut(); - } else { - lhs_ptr_inner = lhs_ptr.as_mut(); - } - - let args = &mut [lhs_ptr_inner, &mut rhs_val]; - let hash_fn = - calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())).unwrap(); - - match self.call_native_fn( - mods, state, lib, op, hash_fn, args, true, false, *op_pos, None, - ) { - Ok(_) => (), - Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op.as_ref())) => - { - // Expand to `var = var op rhs` - let op = &op[..op.len() - 1]; // extract operator without = - - // Run function - let (value, _) = self.call_native_fn( - mods, state, lib, op, hash_fn, args, false, false, *op_pos, None, - )?; - - *args[0] = value.flatten(); - } - err => return err.map(|(v, _)| v), - } - + self.eval_op_assignment( + mods, + state, + lib, + op, + *op_pos, + lhs_ptr, + rhs_val, + rhs_expr.position(), + )?; Ok(Dynamic::UNIT) } } From 8b67a9a9bca2ae5f3314f0daffe1dc6e617bb075 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 24 Feb 2021 11:05:16 +0800 Subject: [PATCH 06/23] Do not test for op-assignment when not ending with '='. --- src/fn_call.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index d8e7723c..fa066981 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -295,7 +295,7 @@ impl Engine { None => (), } - if is_ref { + if is_ref && fn_name.ends_with('=') { let (first, second) = args.split_first_mut().unwrap(); match run_builtin_op_assignment(fn_name, first, second[0])? { From c739e54e5bc9a281887bc1d8332be23779ac59e0 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 24 Feb 2021 11:05:39 +0800 Subject: [PATCH 07/23] Refine strings package. --- src/packages/string_basic.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/packages/string_basic.rs b/src/packages/string_basic.rs index faecf29e..0a716158 100644 --- a/src/packages/string_basic.rs +++ b/src/packages/string_basic.rs @@ -45,14 +45,10 @@ mod print_debug_functions { "".to_string().into() } #[rhai_fn(name = "print", name = "to_string")] - pub fn print_unit(_x: ()) -> ImmutableString { - "".to_string().into() - } - #[rhai_fn(name = "print", name = "to_string")] pub fn print_string(s: ImmutableString) -> ImmutableString { s } - #[rhai_fn(name = "debug", pure)] + #[rhai_fn(name = "debug", name = "to_debug", pure)] pub fn debug_fn_ptr(f: &mut FnPtr) -> ImmutableString { f.to_string().into() } @@ -100,15 +96,14 @@ mod print_debug_functions { #[rhai_fn( name = "print", name = "to_string", - name = "to_debug", name = "debug", + name = "to_debug", pure )] pub fn format_array(ctx: NativeCallContext, array: &mut Array) -> ImmutableString { - let mut result = crate::stdlib::string::String::with_capacity(16); - result.push_str("["); - let len = array.len(); + let mut result = crate::stdlib::string::String::with_capacity(len * 5 + 2); + result.push_str("["); array.iter_mut().enumerate().for_each(|(i, x)| { result.push_str(&print_with_func(FUNC_TO_DEBUG, &ctx, x)); @@ -128,15 +123,14 @@ mod print_debug_functions { #[rhai_fn( name = "print", name = "to_string", - name = "to_debug", name = "debug", + name = "to_debug", pure )] pub fn format_map(ctx: NativeCallContext, map: &mut Map) -> ImmutableString { - let mut result = crate::stdlib::string::String::with_capacity(16); - result.push_str("#{"); - let len = map.len(); + let mut result = crate::stdlib::string::String::with_capacity(len * 5 + 3); + result.push_str("#{"); map.iter_mut().enumerate().for_each(|(i, (k, v))| { result.push_str(&format!( From c501b34191745f42ef89c02a0532de8b839b7135 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 24 Feb 2021 11:28:21 +0800 Subject: [PATCH 08/23] Implement strings concat via Dynamic wildcards. --- src/packages/string_more.rs | 89 +++++-------------------------------- 1 file changed, 10 insertions(+), 79 deletions(-) diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index 11357d88..7925c0ea 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -4,50 +4,9 @@ use crate::plugin::*; use crate::stdlib::{ any::TypeId, boxed::Box, format, mem, string::String, string::ToString, vec::Vec, }; -use crate::{def_package, Dynamic, FnPtr, ImmutableString, StaticVec, INT}; - -macro_rules! gen_concat_functions { - ($root:ident => $($arg_type:ident),+ ) => { - pub mod $root { $( pub mod $arg_type { - use super::super::*; - - #[export_module] - pub mod functions { - #[rhai_fn(name = "+")] - pub fn append_func(string: &str, arg: $arg_type) -> String { - format!("{}{}", string, arg) - } - - #[rhai_fn(name = "+", pure)] - pub fn prepend_func(arg: &mut $arg_type, string: &str) -> String { - format!("{}{}", arg, string) - } - } - } )* } - } -} - -macro_rules! reg_functions { - ($mod_name:ident += $root:ident ; $($arg_type:ident),+) => { $( - combine_with_exported_module!($mod_name, "strings_concat", $root::$arg_type::functions); - )* } -} +use crate::{def_package, Dynamic, ImmutableString, StaticVec, INT}; def_package!(crate:MoreStringPackage:"Additional string utilities, including string building.", lib, { - reg_functions!(lib += basic; INT, bool, FnPtr); - - #[cfg(not(feature = "only_i32"))] - #[cfg(not(feature = "only_i64"))] - { - reg_functions!(lib += numbers; i8, u8, i16, u16, i32, i64, u32, u64); - - #[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))] - reg_functions!(lib += num_128; i128, u128); - } - - #[cfg(not(feature = "no_float"))] - reg_functions!(lib += float; f32, f64); - combine_with_exported_module!(lib, "string", string_functions); // Register string iterator @@ -57,24 +16,19 @@ def_package!(crate:MoreStringPackage:"Additional string utilities, including str ); }); -gen_concat_functions!(basic => INT, bool, char, FnPtr); - -#[cfg(not(feature = "only_i32"))] -#[cfg(not(feature = "only_i64"))] -gen_concat_functions!(numbers => i8, u8, i16, u16, i32, i64, u32, u64); - -#[cfg(not(feature = "only_i32"))] -#[cfg(not(feature = "only_i64"))] -#[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))] -gen_concat_functions!(num_128 => i128, u128); - -#[cfg(not(feature = "no_float"))] -gen_concat_functions!(float => f32, f64); - #[export_module] mod string_functions { use crate::ImmutableString; + #[rhai_fn(name = "+")] + pub fn add_append(string: &str, item: Dynamic) -> ImmutableString { + format!("{}{}", string, item).into() + } + #[rhai_fn(name = "+", pure)] + pub fn add_prepend(item: &mut Dynamic, string: &str) -> ImmutableString { + format!("{}{}", item, string).into() + } + #[rhai_fn(name = "+")] pub fn add_append_unit(string: ImmutableString, _x: ()) -> ImmutableString { string @@ -88,7 +42,6 @@ mod string_functions { pub fn len(string: &str) -> INT { string.chars().count() as INT } - pub fn clear(string: &mut ImmutableString) { string.make_mut().clear(); } @@ -363,14 +316,6 @@ mod string_functions { use crate::stdlib::vec; use crate::{Array, ImmutableString}; - #[rhai_fn(name = "+")] - pub fn append(string: &str, array: Array) -> String { - format!("{}{:?}", string, array) - } - #[rhai_fn(name = "+", pure)] - pub fn prepend(array: &mut Array, string: &str) -> String { - format!("{:?}{}", array, string) - } #[rhai_fn(name = "split")] pub fn chars(string: &str) -> Array { string.chars().map(Into::::into).collect() @@ -439,18 +384,4 @@ mod string_functions { .collect() } } - - #[cfg(not(feature = "no_object"))] - pub mod maps { - use crate::Map; - - #[rhai_fn(name = "+")] - pub fn append(string: &str, map: Map) -> String { - format!("{}#{:?}", string, map) - } - #[rhai_fn(name = "+", pure)] - pub fn prepend(map: &mut Map, string: &str) -> String { - format!("#{:?}{}", map, string) - } - } } From 0d933d865a36fb864b2c197a98d7b522db18c51f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 24 Feb 2021 13:53:11 +0800 Subject: [PATCH 09/23] Do not test for built-in's when operands are not built-in. --- src/fn_call.rs | 20 ++++++++++++++------ src/token.rs | 8 ++++---- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index fa066981..d960bc44 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -289,18 +289,26 @@ impl Engine { } // See if it is built in. - if args.len() == 2 { + if args.len() == 2 && !args[0].is_variant() && !args[1].is_variant() { match run_builtin_binary_op(fn_name, args[0], args[1])? { Some(v) => return Ok((v, false)), None => (), } - if is_ref && fn_name.ends_with('=') { - let (first, second) = args.split_first_mut().unwrap(); + // Op-assignment? + if is_ref { + match fn_name { + _ if fn_name.len() <= 1 => (), + "==" | "!=" | ">=" | "<=" => (), + _ if fn_name.ends_with('=') => { + let (first, second) = args.split_first_mut().unwrap(); - match run_builtin_op_assignment(fn_name, first, second[0])? { - Some(_) => return Ok((Dynamic::UNIT, false)), - None => (), + match run_builtin_op_assignment(fn_name, first, second[0])? { + Some(_) => return Ok((Dynamic::UNIT, false)), + None => (), + } + } + _ => (), } } } diff --git a/src/token.rs b/src/token.rs index fcbcadf6..7221027c 100644 --- a/src/token.rs +++ b/src/token.rs @@ -1659,25 +1659,25 @@ pub fn is_valid_identifier(name: impl Iterator) -> bool { #[cfg(feature = "unicode-xid-ident")] #[inline(always)] -fn is_id_first_alphabetic(x: char) -> bool { +pub fn is_id_first_alphabetic(x: char) -> bool { unicode_xid::UnicodeXID::is_xid_start(x) } #[cfg(feature = "unicode-xid-ident")] #[inline(always)] -fn is_id_continue(x: char) -> bool { +pub fn is_id_continue(x: char) -> bool { unicode_xid::UnicodeXID::is_xid_continue(x) } #[cfg(not(feature = "unicode-xid-ident"))] #[inline(always)] -fn is_id_first_alphabetic(x: char) -> bool { +pub fn is_id_first_alphabetic(x: char) -> bool { x.is_ascii_alphabetic() } #[cfg(not(feature = "unicode-xid-ident"))] #[inline(always)] -fn is_id_continue(x: char) -> bool { +pub fn is_id_continue(x: char) -> bool { x.is_ascii_alphanumeric() || x == '_' } From 4ac05aee8b5de4b1f37632f9a0c05bfb38c2dbe3 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 24 Feb 2021 15:45:29 +0800 Subject: [PATCH 10/23] Disallow implicit comparisons between different numeric types. --- src/ast.rs | 2 - src/engine.rs | 48 ++++++-------------- src/fn_call.rs | 37 +++++++-------- src/fn_native.rs | 4 +- src/optimize.rs | 4 +- src/packages/array_basic.rs | 4 +- src/packages/logic.rs | 87 ++++++++++++++++++++++++++---------- src/packages/map_basic.rs | 4 +- src/packages/string_basic.rs | 2 +- src/parser.rs | 22 ++------- tests/ops.rs | 25 ++++++++++- 11 files changed, 125 insertions(+), 114 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 3ebf3373..b43e1c8c 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1112,8 +1112,6 @@ pub struct FnCallExpr { pub hash_script: Option, /// Does this function call capture the parent scope? pub capture: bool, - /// Default value when the function is not found, mostly used to provide a default for comparison functions. - pub def_value: Option, /// Namespace of the function, if any. Boxed because it occurs rarely. pub namespace: Option, /// Function name. diff --git a/src/engine.rs b/src/engine.rs index ed8b9c38..1dec6e27 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1163,7 +1163,7 @@ impl Engine { self.exec_fn_call( mods, state, lib, FN_IDX_SET, None, args, is_ref, true, false, - val_pos, None, None, level, + val_pos, None, level, ) .map_err(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) @@ -1199,14 +1199,11 @@ impl Engine { let FnCallExpr { name, hash_script: hash, - def_value, .. } = x.as_ref(); - let def_value = def_value.as_ref(); let args = idx_val.as_fn_call_args(); self.make_method_call( - mods, state, lib, name, *hash, target, args, def_value, false, *pos, - level, + mods, state, lib, name, *hash, target, args, false, *pos, level, ) } // xxx.fn_name(...) = ??? @@ -1247,7 +1244,7 @@ impl Engine { let mut args = [target_val, &mut (new_val.as_mut().unwrap().0).0]; self.exec_fn_call( mods, state, lib, setter, None, &mut args, is_ref, true, false, *pos, - None, None, level, + None, level, ) .map(|(v, _)| (v, true)) } @@ -1257,7 +1254,7 @@ impl Engine { let mut args = [target_val]; self.exec_fn_call( mods, state, lib, getter, None, &mut args, is_ref, true, false, *pos, - None, None, level, + None, level, ) .map(|(v, _)| (v, false)) } @@ -1277,14 +1274,11 @@ impl Engine { let FnCallExpr { name, hash_script: hash, - def_value, .. } = x.as_ref(); - let def_value = def_value.as_ref(); let args = idx_val.as_fn_call_args(); let (val, _) = self.make_method_call( - mods, state, lib, name, *hash, target, args, def_value, false, - *pos, level, + mods, state, lib, name, *hash, target, args, false, *pos, level, )?; val.into() } @@ -1312,7 +1306,7 @@ impl Engine { let args = &mut arg_values[..1]; let (mut val, updated) = self.exec_fn_call( mods, state, lib, getter, None, args, is_ref, true, false, - *pos, None, None, level, + *pos, None, level, )?; let val = &mut val; @@ -1338,7 +1332,7 @@ impl Engine { arg_values[1] = val; self.exec_fn_call( mods, state, lib, setter, None, arg_values, is_ref, true, - false, *pos, None, None, level, + false, *pos, None, level, ) .or_else( |err| match *err { @@ -1359,14 +1353,11 @@ impl Engine { let FnCallExpr { name, hash_script: hash, - def_value, .. } = f.as_ref(); - let def_value = def_value.as_ref(); let args = idx_val.as_fn_call_args(); let (mut val, _) = self.make_method_call( - mods, state, lib, name, *hash, target, args, def_value, false, - *pos, level, + mods, state, lib, name, *hash, target, args, false, *pos, level, )?; let val = &mut val; let target = &mut val.into(); @@ -1637,7 +1628,7 @@ impl Engine { let args = &mut [target, &mut idx]; self.exec_fn_call( _mods, state, _lib, FN_IDX_GET, None, args, _is_ref, true, false, idx_pos, - None, None, _level, + None, _level, ) .map(|(v, _)| v.into()) .map_err(|err| match *err { @@ -1680,9 +1671,6 @@ impl Engine { #[cfg(not(feature = "no_index"))] Dynamic(Union::Array(mut rhs_value, _)) => { // Call the `==` operator to compare each value - let def_value = Some(false.into()); - let def_value = def_value.as_ref(); - for value in rhs_value.iter_mut() { let args = &mut [&mut lhs_value.clone(), value]; let hash_fn = @@ -1693,7 +1681,6 @@ impl Engine { if self .call_native_fn( mods, state, lib, OP_EQUALS, hash_fn, args, false, false, pos, - def_value, )? .0 .as_bool() @@ -1799,13 +1786,11 @@ impl Engine { capture: cap_scope, hash_script: hash, args, - def_value, .. } = x.as_ref(); - let def_value = def_value.as_ref(); self.make_function_call( - scope, mods, state, lib, this_ptr, name, args, def_value, *hash, false, *pos, - *cap_scope, level, + scope, mods, state, lib, this_ptr, name, args, *hash, false, *pos, *cap_scope, + level, ) } @@ -1816,15 +1801,12 @@ impl Engine { namespace, hash_script, args, - def_value, .. } = x.as_ref(); let namespace = namespace.as_ref(); let hash = hash_script.unwrap(); - let def_value = def_value.as_ref(); self.make_qualified_function_call( - scope, mods, state, lib, this_ptr, namespace, name, args, def_value, hash, - *pos, level, + scope, mods, state, lib, this_ptr, namespace, name, args, hash, *pos, level, ) } @@ -1979,9 +1961,7 @@ impl Engine { let hash_fn = calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())).unwrap(); - match self.call_native_fn( - mods, state, lib, op, hash_fn, args, true, false, op_pos, None, - ) { + match self.call_native_fn(mods, state, lib, op, hash_fn, args, true, false, op_pos) { Ok(_) => (), Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) => { @@ -1990,7 +1970,7 @@ impl Engine { // Run function let (value, _) = self.call_native_fn( - mods, state, lib, op, hash_fn, args, false, false, op_pos, None, + mods, state, lib, op, hash_fn, args, false, false, op_pos, )?; *args[0] = value.flatten(); diff --git a/src/fn_call.rs b/src/fn_call.rs index d960bc44..edb27f13 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -174,7 +174,6 @@ impl Engine { is_ref: bool, pub_only: bool, pos: Position, - def_val: Option<&Dynamic>, ) -> Result<(Dynamic, bool), Box> { self.inc_operations(state, pos)?; @@ -313,11 +312,6 @@ impl Engine { } } - // Return default value (if any) - if let Some(val) = def_val { - return Ok((val.clone(), false)); - } - // Getter function not found? #[cfg(not(feature = "no_object"))] if let Some(prop) = extract_prop_from_getter(fn_name) { @@ -616,7 +610,6 @@ impl Engine { pub_only: bool, pos: Position, _capture_scope: Option, - def_val: Option<&Dynamic>, _level: usize, ) -> Result<(Dynamic, bool), Box> { // Check for data race. @@ -772,14 +765,14 @@ impl Engine { } else { // Native function call self.call_native_fn( - mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, pos, def_val, + mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, pos, ) } } // Native function call _ => self.call_native_fn( - mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, pos, def_val, + mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, pos, ), } } @@ -861,7 +854,6 @@ impl Engine { hash_script: Option, target: &mut crate::engine::Target, mut call_args: StaticVec, - def_val: Option<&Dynamic>, pub_only: bool, pos: Position, level: usize, @@ -890,8 +882,7 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - mods, state, lib, fn_name, hash, args, false, false, pub_only, pos, None, def_val, - level, + mods, state, lib, fn_name, hash, args, false, false, pub_only, pos, None, level, ) } else if fn_name == KEYWORD_FN_PTR_CALL && call_args.len() > 0 @@ -914,8 +905,7 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - mods, state, lib, fn_name, hash, args, is_ref, true, pub_only, pos, None, def_val, - level, + mods, state, lib, fn_name, hash, args, is_ref, true, pub_only, pos, None, level, ) } else if fn_name == KEYWORD_FN_PTR_CURRY && obj.is::() { // Curry call @@ -980,8 +970,7 @@ impl Engine { let args = arg_values.as_mut(); self.exec_fn_call( - mods, state, lib, fn_name, hash, args, is_ref, true, pub_only, pos, None, def_val, - level, + mods, state, lib, fn_name, hash, args, is_ref, true, pub_only, pos, None, level, ) }?; @@ -1003,7 +992,6 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, fn_name: &str, args_expr: impl AsRef<[Expr]>, - def_val: Option<&Dynamic>, mut hash_script: Option, pub_only: bool, pos: Position, @@ -1228,7 +1216,6 @@ impl Engine { pub_only, pos, capture, - def_val, level, ) .map(|(v, _)| v) @@ -1245,7 +1232,6 @@ impl Engine { namespace: Option<&NamespaceRef>, fn_name: &str, args_expr: impl AsRef<[Expr]>, - def_val: Option<&Dynamic>, hash_script: NonZeroU64, pos: Position, level: usize, @@ -1367,7 +1353,6 @@ impl Engine { args.as_mut(), ), Some(f) => unreachable!("unknown function type: {:?}", f), - None if def_val.is_some() => Ok(def_val.unwrap().clone()), None => EvalAltResult::ErrorFunctionNotFound( format!( "{}{} ({})", @@ -1500,7 +1485,11 @@ pub fn run_builtin_binary_op( } } - return Ok(None); + return Ok(match op { + "!=" => Some(Dynamic::TRUE), + "==" | ">" | ">=" | "<" | "<=" => Some(Dynamic::FALSE), + _ => None, + }); } if first_type == TypeId::of::() { @@ -1603,7 +1592,11 @@ pub fn run_builtin_binary_op( } } - Ok(None) + Ok(match op { + "!=" => Some(Dynamic::TRUE), + "==" | ">" | ">=" | "<" | "<=" => Some(Dynamic::FALSE), + _ => None, + }) } /// Build in common operator assignment implementations to avoid the cost of calling a registered function. diff --git a/src/fn_native.rs b/src/fn_native.rs index 59df6849..7f458ad2 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -186,7 +186,6 @@ impl<'e, 'n, 's, 'a, 'm> NativeCallContext<'e, 'n, 's, 'a, 'm> { is_method: bool, public_only: bool, args: &mut [&mut Dynamic], - def_value: Option<&Dynamic>, ) -> Result> { self.engine() .exec_fn_call( @@ -201,7 +200,6 @@ impl<'e, 'n, 's, 'a, 'm> NativeCallContext<'e, 'n, 's, 'a, 'm> { public_only, Position::NONE, None, - def_value, 0, ) .map(|(r, _)| r) @@ -339,7 +337,7 @@ impl FnPtr { args.insert(0, obj); } - ctx.call_fn_dynamic_raw(self.fn_name(), is_method, true, args.as_mut(), None) + ctx.call_fn_dynamic_raw(self.fn_name(), is_method, true, args.as_mut()) } } diff --git a/src/optimize.rs b/src/optimize.rs index 4fa5d960..e6fb62d2 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -151,7 +151,6 @@ fn call_fn_with_constant_arguments( false, true, Position::NONE, - None, ) .ok() .map(|(v, _)| v) @@ -717,8 +716,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { // Handle `type_of()` Some(arg_for_type_of.to_string().into()) } else { - // Otherwise use the default value, if any - x.def_value.clone() + None } }) .and_then(|result| map_dynamic_to_expr(result, *pos)) diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index d4a5394c..0eca9695 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -647,11 +647,9 @@ mod array_functions { return Ok(true.into()); } - let def_value = Some(false.into()); - for (a1, a2) in array.iter_mut().zip(array2.iter_mut()) { let equals = ctx - .call_fn_dynamic_raw(OP_EQUALS, true, false, &mut [a1, a2], def_value.as_ref()) + .call_fn_dynamic_raw(OP_EQUALS, true, false, &mut [a1, a2]) .map(|v| v.as_bool().unwrap_or(false))?; if !equals { diff --git a/src/packages/logic.rs b/src/packages/logic.rs index 19e7dd2d..9bd3c8ff 100644 --- a/src/packages/logic.rs +++ b/src/packages/logic.rs @@ -1,6 +1,7 @@ #![allow(non_snake_case)] use crate::def_package; +use crate::fn_call::run_builtin_binary_op; use crate::plugin::*; #[cfg(feature = "decimal")] @@ -17,30 +18,12 @@ macro_rules! gen_cmp_functions { #[export_module] pub mod functions { - #[rhai_fn(name = "<")] - pub fn lt(x: $arg_type, y: $arg_type) -> bool { - x < y - } - #[rhai_fn(name = "<=")] - pub fn lte(x: $arg_type, y: $arg_type) -> bool { - x <= y - } - #[rhai_fn(name = ">")] - pub fn gt(x: $arg_type, y: $arg_type) -> bool { - x > y - } - #[rhai_fn(name = ">=")] - pub fn gte(x: $arg_type, y: $arg_type) -> bool { - x >= y - } - #[rhai_fn(name = "==")] - pub fn eq(x: $arg_type, y: $arg_type) -> bool { - x == y - } - #[rhai_fn(name = "!=")] - pub fn ne(x: $arg_type, y: $arg_type) -> bool { - x != y - } + #[rhai_fn(name = "<")] pub fn lt(x: $arg_type, y: $arg_type) -> bool { x < y } + #[rhai_fn(name = "<=")] pub fn lte(x: $arg_type, y: $arg_type) -> bool { x <= y } + #[rhai_fn(name = ">")] pub fn gt(x: $arg_type, y: $arg_type) -> bool { x > y } + #[rhai_fn(name = ">=")] pub fn gte(x: $arg_type, y: $arg_type) -> bool { x >= y } + #[rhai_fn(name = "==")] pub fn eq(x: $arg_type, y: $arg_type) -> bool { x == y } + #[rhai_fn(name = "!=")] pub fn ne(x: $arg_type, y: $arg_type) -> bool { x != y } } })* } }; @@ -57,6 +40,8 @@ macro_rules! reg_functions { } def_package!(crate:LogicPackage:"Logical operators.", lib, { + combine_with_exported_module!(lib, "logic", logic_functions); + #[cfg(not(feature = "only_i32"))] #[cfg(not(feature = "only_i64"))] { @@ -112,6 +97,60 @@ gen_cmp_functions!(float => f64); #[cfg(feature = "decimal")] gen_cmp_functions!(decimal => Decimal); +#[export_module] +mod logic_functions { + fn is_numeric(type_id: TypeId) -> bool { + let result = type_id == TypeId::of::() + || type_id == TypeId::of::() + || type_id == TypeId::of::() + || type_id == TypeId::of::() + || type_id == TypeId::of::() + || type_id == TypeId::of::() + || type_id == TypeId::of::() + || type_id == TypeId::of::(); + + #[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))] + let result = result || type_id == TypeId::of::() || type_id == TypeId::of::(); + + #[cfg(not(feature = "no_float"))] + let result = result || type_id == TypeId::of::() || type_id == TypeId::of::(); + + #[cfg(feature = "decimal")] + let result = result || type_id_x == TypeId::of::(); + + result + } + + #[rhai_fn( + name = "==", + name = "!=", + name = ">", + name = ">=", + name = "<", + name = "<=", + return_raw + )] + pub fn cmp( + ctx: NativeCallContext, + x: Dynamic, + y: Dynamic, + ) -> Result> { + let type_x = x.type_id(); + let type_y = y.type_id(); + + if type_x != type_y && is_numeric(type_x) && is_numeric(type_y) { + // Disallow comparisons between different number types + } else if let Some(x) = run_builtin_binary_op(ctx.fn_name(), &x, &y)? { + return Ok(x); + } + + Err(Box::new(EvalAltResult::ErrorFunctionNotFound( + format!("{} ({}, {})", ctx.fn_name(), x.type_name(), y.type_name()), + Position::NONE, + ))) + } +} + #[cfg(not(feature = "no_float"))] #[export_module] mod f32_functions { diff --git a/src/packages/map_basic.rs b/src/packages/map_basic.rs index c2f45281..a9fbb79a 100644 --- a/src/packages/map_basic.rs +++ b/src/packages/map_basic.rs @@ -58,12 +58,10 @@ mod map_functions { return Ok(true.into()); } - let def_value = Some(false.into()); - for (m1, v1) in map.iter_mut() { if let Some(v2) = map2.get_mut(m1) { let equals = ctx - .call_fn_dynamic_raw(OP_EQUALS, true, false, &mut [v1, v2], def_value.as_ref()) + .call_fn_dynamic_raw(OP_EQUALS, true, false, &mut [v1, v2]) .map(|v| v.as_bool().unwrap_or(false))?; if !equals { diff --git a/src/packages/string_basic.rs b/src/packages/string_basic.rs index 0a716158..f02e2464 100644 --- a/src/packages/string_basic.rs +++ b/src/packages/string_basic.rs @@ -21,7 +21,7 @@ def_package!(crate:BasicStringPackage:"Basic string utilities, including printin #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[inline(always)] fn print_with_func(fn_name: &str, ctx: &NativeCallContext, value: &mut Dynamic) -> ImmutableString { - match ctx.call_fn_dynamic_raw(fn_name, true, false, &mut [value], None) { + match ctx.call_fn_dynamic_raw(fn_name, true, false, &mut [value]) { Ok(result) if result.is::() => result.take_immutable_string().unwrap(), Ok(result) => ctx.engine().map_type_name(result.type_name()).into(), Err(_) => ctx.engine().map_type_name(value.type_name()).into(), diff --git a/src/parser.rs b/src/parser.rs index e0d1073c..bfef0a54 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1347,7 +1347,6 @@ fn parse_unary( Box::new(FnCallExpr { name: op.into(), args, - def_value: Some(false.into()), // NOT operator, when operating on invalid operand, defaults to false ..Default::default() }), pos, @@ -1792,7 +1791,6 @@ fn parse_binary_op( #[cfg(not(feature = "unchecked"))] settings.ensure_level_within_max_limit(state.max_expr_depth)?; - let cmp_def = Some(false.into()); let op = op_token.syntax(); let op_base = FnCallExpr { @@ -1819,28 +1817,16 @@ fn parse_binary_op( | Token::XOr => Expr::FnCall(Box::new(FnCallExpr { args, ..op_base }), pos), // '!=' defaults to true when passed invalid operands - Token::NotEqualsTo => Expr::FnCall( - Box::new(FnCallExpr { - args, - def_value: Some(true.into()), - ..op_base - }), - pos, - ), + Token::NotEqualsTo => Expr::FnCall(Box::new(FnCallExpr { args, ..op_base }), pos), // Comparison operators default to false when passed invalid operands Token::EqualsTo | Token::LessThan | Token::LessThanEqualsTo | Token::GreaterThan - | Token::GreaterThanEqualsTo => Expr::FnCall( - Box::new(FnCallExpr { - args, - def_value: cmp_def, - ..op_base - }), - pos, - ), + | Token::GreaterThanEqualsTo => { + Expr::FnCall(Box::new(FnCallExpr { args, ..op_base }), pos) + } Token::Or => { let rhs = args.pop().unwrap(); diff --git a/tests/ops.rs b/tests/ops.rs index e087a8d0..72c823cb 100644 --- a/tests/ops.rs +++ b/tests/ops.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, INT}; +use rhai::{Engine, EvalAltResult, Scope, INT}; #[test] fn test_ops() -> Result<(), Box> { @@ -10,6 +10,29 @@ fn test_ops() -> Result<(), Box> { Ok(()) } +#[test] +fn test_ops_numbers() -> Result<(), Box> { + let engine = Engine::new(); + + let mut scope = Scope::new(); + + scope.push("x", 42_u16); + + assert!(matches!( + *engine.eval_with_scope::(&mut scope, "x == 42").expect_err("should error"), + EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with("== (u16,") + )); + #[cfg(not(feature = "no_float"))] + assert!(matches!( + *engine.eval_with_scope::(&mut scope, "x == 42.0").expect_err("should error"), + EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with("== (u16,") + )); + + assert!(!engine.eval_with_scope::(&mut scope, r#"x == "hello""#)?); + + Ok(()) +} + #[test] fn test_op_precedence() -> Result<(), Box> { let engine = Engine::new(); From 9d6ad2092c8369e2901f9d8dc189150da391c8a6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 24 Feb 2021 15:56:29 +0800 Subject: [PATCH 11/23] Fix decimal build. --- src/packages/logic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/packages/logic.rs b/src/packages/logic.rs index 9bd3c8ff..8876b5a6 100644 --- a/src/packages/logic.rs +++ b/src/packages/logic.rs @@ -116,7 +116,7 @@ mod logic_functions { let result = result || type_id == TypeId::of::() || type_id == TypeId::of::(); #[cfg(feature = "decimal")] - let result = result || type_id_x == TypeId::of::(); + let result = result || type_id == TypeId::of::(); result } From 37540fda1241a58dc1a2cc274d215fadb030ef84 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 24 Feb 2021 16:17:04 +0800 Subject: [PATCH 12/23] Fix bug with op-assignment. --- benches/eval_expression.rs | 2 +- src/engine.rs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/benches/eval_expression.rs b/benches/eval_expression.rs index ffb0108c..db7a6528 100644 --- a/benches/eval_expression.rs +++ b/benches/eval_expression.rs @@ -126,7 +126,7 @@ fn bench_eval_loop_number(bench: &mut Bencher) { #[bench] fn bench_eval_loop_strings_build(bench: &mut Bencher) { let script = r#" - let s = 0; + let s = "hello"; for x in range(0, 10000) { s += "x"; } diff --git a/src/engine.rs b/src/engine.rs index 1dec6e27..1cafc84b 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1967,11 +1967,12 @@ impl Engine { { // Expand to `var = var op rhs` let op = &op[..op.len() - 1]; // extract operator without = + let hash_fn = + calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())).unwrap(); // Run function - let (value, _) = self.call_native_fn( - mods, state, lib, op, hash_fn, args, false, false, op_pos, - )?; + let (value, _) = self + .call_native_fn(mods, state, lib, op, hash_fn, args, true, false, op_pos)?; *args[0] = value.flatten(); } From baaa0461bf8d89a43b9823f58f4eeab2551abf22 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 24 Feb 2021 22:40:18 +0800 Subject: [PATCH 13/23] Limit Dynamic parameters to 16. --- src/engine.rs | 12 ++++++------ src/fn_call.rs | 24 ++++++++---------------- src/optimize.rs | 1 - 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 1cafc84b..07740bad 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -193,6 +193,8 @@ pub const MAX_EXPR_DEPTH: usize = 64; #[cfg(not(debug_assertions))] pub const MAX_FUNCTION_EXPR_DEPTH: usize = 32; +pub const MAX_DYNAMIC_PARAMETERS: usize = 16; + pub const KEYWORD_PRINT: &str = "print"; pub const KEYWORD_DEBUG: &str = "debug"; pub const KEYWORD_TYPE_OF: &str = "type_of"; @@ -1679,9 +1681,7 @@ impl Engine { let pos = rhs.position(); if self - .call_native_fn( - mods, state, lib, OP_EQUALS, hash_fn, args, false, false, pos, - )? + .call_native_fn(mods, state, lib, OP_EQUALS, hash_fn, args, false, pos)? .0 .as_bool() .unwrap_or(false) @@ -1961,7 +1961,7 @@ impl Engine { let hash_fn = calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())).unwrap(); - match self.call_native_fn(mods, state, lib, op, hash_fn, args, true, false, op_pos) { + match self.call_native_fn(mods, state, lib, op, hash_fn, args, true, op_pos) { Ok(_) => (), Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) => { @@ -1971,8 +1971,8 @@ impl Engine { calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())).unwrap(); // Run function - let (value, _) = self - .call_native_fn(mods, state, lib, op, hash_fn, args, true, false, op_pos)?; + let (value, _) = + self.call_native_fn(mods, state, lib, op, hash_fn, args, true, op_pos)?; *args[0] = value.flatten(); } diff --git a/src/fn_call.rs b/src/fn_call.rs index edb27f13..9b730c7f 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -4,6 +4,7 @@ use crate::ast::{Expr, Stmt}; use crate::engine::{ search_imports, Imports, State, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_FN_PTR_CALL, KEYWORD_FN_PTR_CURRY, KEYWORD_IS_DEF_VAR, KEYWORD_PRINT, KEYWORD_TYPE_OF, + MAX_DYNAMIC_PARAMETERS, }; use crate::fn_native::FnCallArgs; use crate::module::NamespaceRef; @@ -172,7 +173,6 @@ impl Engine { hash_fn: NonZeroU64, args: &mut FnCallArgs, is_ref: bool, - pub_only: bool, pos: Position, ) -> Result<(Dynamic, bool), Box> { self.inc_operations(state, pos)?; @@ -185,14 +185,15 @@ impl Engine { .entry(hash_fn) .or_insert_with(|| { let num_args = args.len(); + let max_bitmask = 1usize << args.len().min(MAX_DYNAMIC_PARAMETERS); let mut hash = hash_fn; let mut bitmask = 1usize; // Bitmask of which parameter to replace with `Dynamic` loop { - //lib.get_fn(hash, pub_only).or_else(|| + //lib.get_fn(hash, false).or_else(|| match self .global_namespace - .get_fn(hash, pub_only) + .get_fn(hash, false) .cloned() .map(|f| (f, None)) .or_else(|| { @@ -208,8 +209,8 @@ impl Engine { // Specific version found Some(f) => return Some(f), - // No parameters - _ if num_args == 0 => return None, + // Stop when all permutations are exhausted + _ if bitmask >= max_bitmask => return None, // Try all permutations with `Dynamic` wildcards _ => { @@ -228,11 +229,6 @@ impl Engine { ) .unwrap(); - // Stop when all permutations are exhausted - if hash == hash_fn { - return None; - } - bitmask += 1; } } @@ -764,16 +760,12 @@ impl Engine { Ok((result, false)) } else { // Native function call - self.call_native_fn( - mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, pos, - ) + self.call_native_fn(mods, state, lib, fn_name, hash_fn, args, is_ref, pos) } } // Native function call - _ => self.call_native_fn( - mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, pos, - ), + _ => self.call_native_fn(mods, state, lib, fn_name, hash_fn, args, is_ref, pos), } } diff --git a/src/optimize.rs b/src/optimize.rs index e6fb62d2..64b6140d 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -149,7 +149,6 @@ fn call_fn_with_constant_arguments( hash_fn.unwrap(), arg_values.iter_mut().collect::>().as_mut(), false, - true, Position::NONE, ) .ok() From 02057ef1d2b828f905533c25b30f3029ca78bd0a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 24 Feb 2021 23:23:32 +0800 Subject: [PATCH 14/23] Avoid double checking of builtin's. --- src/fn_call.rs | 30 +++++++++++++----------------- src/token.rs | 12 ++++++++++++ 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index 9b730c7f..c39d90cd 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -21,6 +21,7 @@ use crate::stdlib::{ string::ToString, vec::Vec, }; +use crate::token::is_assignment_operator; use crate::utils::combine_hashes; use crate::{ calc_native_fn_hash, calc_script_fn_hash, Dynamic, Engine, EvalAltResult, FnPtr, @@ -285,25 +286,20 @@ impl Engine { // See if it is built in. if args.len() == 2 && !args[0].is_variant() && !args[1].is_variant() { - match run_builtin_binary_op(fn_name, args[0], args[1])? { - Some(v) => return Ok((v, false)), - None => (), - } + if is_assignment_operator(fn_name) { + if is_ref { + // Op-assignment + let (first, second) = args.split_first_mut().unwrap(); - // Op-assignment? - if is_ref { - match fn_name { - _ if fn_name.len() <= 1 => (), - "==" | "!=" | ">=" | "<=" => (), - _ if fn_name.ends_with('=') => { - let (first, second) = args.split_first_mut().unwrap(); - - match run_builtin_op_assignment(fn_name, first, second[0])? { - Some(_) => return Ok((Dynamic::UNIT, false)), - None => (), - } + match run_builtin_op_assignment(fn_name, first, second[0])? { + Some(_) => return Ok((Dynamic::UNIT, false)), + None => (), } - _ => (), + } + } else { + match run_builtin_binary_op(fn_name, args[0], args[1])? { + Some(v) => return Ok((v, false)), + None => (), } } } diff --git a/src/token.rs b/src/token.rs index 7221027c..7e2c7b42 100644 --- a/src/token.rs +++ b/src/token.rs @@ -1657,24 +1657,36 @@ pub fn is_valid_identifier(name: impl Iterator) -> bool { first_alphabetic } +/// Is a text string an assignment operator? +pub fn is_assignment_operator(op: &str) -> bool { + match op { + "+=" | "-=" | "*=" | "/=" | "<<=" | ">>=" | "&=" | "|=" | "^=" | "%=" | "**=" => true, + _ => false, + } +} + +/// Is a character valid to start an identifier? #[cfg(feature = "unicode-xid-ident")] #[inline(always)] pub fn is_id_first_alphabetic(x: char) -> bool { unicode_xid::UnicodeXID::is_xid_start(x) } +/// Is a character valid for an identifier? #[cfg(feature = "unicode-xid-ident")] #[inline(always)] pub fn is_id_continue(x: char) -> bool { unicode_xid::UnicodeXID::is_xid_continue(x) } +/// Is a character valid to start an identifier? #[cfg(not(feature = "unicode-xid-ident"))] #[inline(always)] pub fn is_id_first_alphabetic(x: char) -> bool { x.is_ascii_alphabetic() } +/// Is a character valid for an identifier? #[cfg(not(feature = "unicode-xid-ident"))] #[inline(always)] pub fn is_id_continue(x: char) -> bool { From 49e5382ab0a17339dff144845dc7ee424ae86aca Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 25 Feb 2021 10:59:11 +0800 Subject: [PATCH 15/23] Do not return default for comparisons between same types. --- RELEASES.md | 1 + src/fn_call.rs | 109 +++++++++++++++++++++++++---------------- tests/mismatched_op.rs | 15 +++++- 3 files changed, 80 insertions(+), 45 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index e85c6e56..7158e27c 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -14,6 +14,7 @@ Breaking changes * For plugin functions, constants passed to methods (i.e. `&mut` parameter) now raise an error unless the functions are marked with `#[rhai_fn(pure)]`. * Visibility (i.e. `pub` or not) for generated _plugin_ modules now follow the visibility of the underlying module. +* Comparison operators between the sames types now throw errors when they're not defined instead of returning the default. Only comparing between _different_ types will return the default. * Default stack-overflow and top-level expression nesting limits for release builds are lowered to 64 from 128. New features diff --git a/src/fn_call.rs b/src/fn_call.rs index c39d90cd..78c77702 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -21,7 +21,6 @@ use crate::stdlib::{ string::ToString, vec::Vec, }; -use crate::token::is_assignment_operator; use crate::utils::combine_hashes; use crate::{ calc_native_fn_hash, calc_script_fn_hash, Dynamic, Engine, EvalAltResult, FnPtr, @@ -174,6 +173,7 @@ impl Engine { hash_fn: NonZeroU64, args: &mut FnCallArgs, is_ref: bool, + is_op_assignment: bool, pos: Position, ) -> Result<(Dynamic, bool), Box> { self.inc_operations(state, pos)?; @@ -286,15 +286,16 @@ impl Engine { // See if it is built in. if args.len() == 2 && !args[0].is_variant() && !args[1].is_variant() { - if is_assignment_operator(fn_name) { - if is_ref { - // Op-assignment - let (first, second) = args.split_first_mut().unwrap(); + // Op-assignment? + if is_op_assignment { + if !is_ref { + unreachable!("op-assignments must have ref argument"); + } + let (first, second) = args.split_first_mut().unwrap(); - match run_builtin_op_assignment(fn_name, first, second[0])? { - Some(_) => return Ok((Dynamic::UNIT, false)), - None => (), - } + match run_builtin_op_assignment(fn_name, first, second[0])? { + Some(_) => return Ok((Dynamic::UNIT, false)), + None => (), } } else { match run_builtin_binary_op(fn_name, args[0], args[1])? { @@ -756,12 +757,14 @@ impl Engine { Ok((result, false)) } else { // Native function call - self.call_native_fn(mods, state, lib, fn_name, hash_fn, args, is_ref, pos) + self.call_native_fn( + mods, state, lib, fn_name, hash_fn, args, is_ref, false, pos, + ) } } // Native function call - _ => self.call_native_fn(mods, state, lib, fn_name, hash_fn, args, is_ref, pos), + _ => self.call_native_fn(mods, state, lib, fn_name, hash_fn, args, is_ref, false, pos), } } @@ -1368,17 +1371,29 @@ pub fn run_builtin_binary_op( x: &Dynamic, y: &Dynamic, ) -> Result, Box> { - let first_type = x.type_id(); - let second_type = y.type_id(); + let type1 = x.type_id(); + let type2 = y.type_id(); - let type_id = (first_type, second_type); + if x.is_variant() || y.is_variant() { + // One of the operands is a custom type, so it is never built-in + return Ok(match op { + "!=" if type1 != type2 => Some(Dynamic::TRUE), + "==" | ">" | ">=" | "<" | "<=" if type1 != type2 => Some(Dynamic::FALSE), + _ => None, + }); + } + + let types_pair = (type1, type2); #[cfg(not(feature = "no_float"))] - if let Some((x, y)) = if type_id == (TypeId::of::(), TypeId::of::()) { + if let Some((x, y)) = if types_pair == (TypeId::of::(), TypeId::of::()) { + // FLOAT op FLOAT Some((x.clone().cast::(), y.clone().cast::())) - } else if type_id == (TypeId::of::(), TypeId::of::()) { + } else if types_pair == (TypeId::of::(), TypeId::of::()) { + // FLOAT op INT Some((x.clone().cast::(), y.clone().cast::() as FLOAT)) - } else if type_id == (TypeId::of::(), TypeId::of::()) { + } else if types_pair == (TypeId::of::(), TypeId::of::()) { + // INT op FLOAT Some((x.clone().cast::() as FLOAT, y.clone().cast::())) } else { None @@ -1402,16 +1417,19 @@ pub fn run_builtin_binary_op( #[cfg(feature = "decimal")] if let Some((x, y)) = if type_id == (TypeId::of::(), TypeId::of::()) { + // Decimal op Decimal Some(( *x.read_lock::().unwrap(), *y.read_lock::().unwrap(), )) } else if type_id == (TypeId::of::(), TypeId::of::()) { + // Decimal op INT Some(( *x.read_lock::().unwrap(), y.clone().cast::().into(), )) } else if type_id == (TypeId::of::(), TypeId::of::()) { + // INT op Decimal Some(( x.clone().cast::().into(), *y.read_lock::().unwrap(), @@ -1452,8 +1470,9 @@ pub fn run_builtin_binary_op( } } - if second_type != first_type { - if type_id == (TypeId::of::(), TypeId::of::()) { + if type2 != type1 { + // char op string + if types_pair == (TypeId::of::(), TypeId::of::()) { let x = x.clone().cast::(); let y = &*y.read_lock::().unwrap(); @@ -1462,8 +1481,8 @@ pub fn run_builtin_binary_op( _ => return Ok(None), } } - - if type_id == (TypeId::of::(), TypeId::of::()) { + // string op char + if types_pair == (TypeId::of::(), TypeId::of::()) { let x = &*x.read_lock::().unwrap(); let y = y.clone().cast::(); @@ -1472,7 +1491,7 @@ pub fn run_builtin_binary_op( _ => return Ok(None), } } - + // Default comparison operators for different types return Ok(match op { "!=" => Some(Dynamic::TRUE), "==" | ">" | ">=" | "<" | "<=" => Some(Dynamic::FALSE), @@ -1480,7 +1499,9 @@ pub fn run_builtin_binary_op( }); } - if first_type == TypeId::of::() { + // Beyond here, type1 == type2 + + if type1 == TypeId::of::() { let x = x.clone().cast::(); let y = y.clone().cast::(); @@ -1526,7 +1547,7 @@ pub fn run_builtin_binary_op( } } - if first_type == TypeId::of::() { + if type1 == TypeId::of::() { let x = x.clone().cast::(); let y = y.clone().cast::(); @@ -1540,7 +1561,7 @@ pub fn run_builtin_binary_op( } } - if first_type == TypeId::of::() { + if type1 == TypeId::of::() { let x = &*x.read_lock::().unwrap(); let y = &*y.read_lock::().unwrap(); @@ -1556,7 +1577,7 @@ pub fn run_builtin_binary_op( } } - if first_type == TypeId::of::() { + if type1 == TypeId::of::() { let x = x.clone().cast::(); let y = y.clone().cast::(); @@ -1572,7 +1593,7 @@ pub fn run_builtin_binary_op( } } - if first_type == TypeId::of::<()>() { + if type1 == TypeId::of::<()>() { match op { "==" => return Ok(Some(true.into())), "!=" | ">" | ">=" | "<" | "<=" => return Ok(Some(false.into())), @@ -1580,11 +1601,7 @@ pub fn run_builtin_binary_op( } } - Ok(match op { - "!=" => Some(Dynamic::TRUE), - "==" | ">" | ">=" | "<" | "<=" => Some(Dynamic::FALSE), - _ => None, - }) + Ok(None) } /// Build in common operator assignment implementations to avoid the cost of calling a registered function. @@ -1593,16 +1610,18 @@ pub fn run_builtin_op_assignment( x: &mut Dynamic, y: &Dynamic, ) -> Result, Box> { - let first_type = x.type_id(); - let second_type = y.type_id(); + let type1 = x.type_id(); + let type2 = y.type_id(); - let type_id = (first_type, second_type); + let types_pair = (type1, type2); #[cfg(not(feature = "no_float"))] - if let Some((mut x, y)) = if type_id == (TypeId::of::(), TypeId::of::()) { + if let Some((mut x, y)) = if types_pair == (TypeId::of::(), TypeId::of::()) { + // FLOAT op= FLOAT let y = y.clone().cast::(); Some((x.write_lock::().unwrap(), y)) - } else if type_id == (TypeId::of::(), TypeId::of::()) { + } else if types_pair == (TypeId::of::(), TypeId::of::()) { + // FLOAT op= INT let y = y.clone().cast::() as FLOAT; Some((x.write_lock::().unwrap(), y)) } else { @@ -1621,9 +1640,11 @@ pub fn run_builtin_op_assignment( #[cfg(feature = "decimal")] if let Some((mut x, y)) = if type_id == (TypeId::of::(), TypeId::of::()) { + // Decimal op= Decimal let y = *y.read_lock::().unwrap(); Some((x.write_lock::().unwrap(), y)) } else if type_id == (TypeId::of::(), TypeId::of::()) { + // Decimal op= INT let y = y.clone().cast::().into(); Some((x.write_lock::().unwrap(), y)) } else { @@ -1652,8 +1673,8 @@ pub fn run_builtin_op_assignment( } } - if second_type != first_type { - if type_id == (TypeId::of::(), TypeId::of::()) { + if type2 != type1 { + if types_pair == (TypeId::of::(), TypeId::of::()) { let y = y.read_lock::().unwrap().deref().clone(); let mut x = x.write_lock::().unwrap(); @@ -1666,7 +1687,9 @@ pub fn run_builtin_op_assignment( return Ok(None); } - if first_type == TypeId::of::() { + // Beyond here, type1 == type2 + + if type1 == TypeId::of::() { let y = y.clone().cast::(); let mut x = x.write_lock::().unwrap(); @@ -1706,7 +1729,7 @@ pub fn run_builtin_op_assignment( } } - if first_type == TypeId::of::() { + if type1 == TypeId::of::() { let y = y.clone().cast::(); let mut x = x.write_lock::().unwrap(); @@ -1717,7 +1740,7 @@ pub fn run_builtin_op_assignment( } } - if first_type == TypeId::of::() { + if type1 == TypeId::of::() { let y = y.read_lock::().unwrap().deref().clone(); let mut x = x.write_lock::().unwrap(); @@ -1727,7 +1750,7 @@ pub fn run_builtin_op_assignment( } } - if first_type == TypeId::of::() { + if type1 == TypeId::of::() { let y = y.read_lock::().unwrap().deref().clone(); let mut x = x.write_lock::().unwrap(); diff --git a/tests/mismatched_op.rs b/tests/mismatched_op.rs index 5ef401f0..eea4f693 100644 --- a/tests/mismatched_op.rs +++ b/tests/mismatched_op.rs @@ -12,7 +12,7 @@ fn test_mismatched_op() { #[test] #[cfg(not(feature = "no_object"))] -fn test_mismatched_op_custom_type() { +fn test_mismatched_op_custom_type() -> Result<(), Box> { #[derive(Debug, Clone)] struct TestStruct { x: INT, @@ -30,9 +30,18 @@ fn test_mismatched_op_custom_type() { .register_type_with_name::("TestStruct") .register_fn("new_ts", TestStruct::new); + assert!(matches!(*engine.eval::(r" + let x = new_ts(); + let y = new_ts(); + x == y + ").expect_err("should error"), + EvalAltResult::ErrorFunctionNotFound(f, _) if f == "== (TestStruct, TestStruct)")); + + assert!(!engine.eval::("new_ts() == 42")?); + assert!(matches!( *engine.eval::("60 + new_ts()").expect_err("should error"), - EvalAltResult::ErrorFunctionNotFound(err, _) if err == format!("+ ({}, TestStruct)", std::any::type_name::()) + EvalAltResult::ErrorFunctionNotFound(f, _) if f == format!("+ ({}, TestStruct)", std::any::type_name::()) )); assert!(matches!( @@ -40,4 +49,6 @@ fn test_mismatched_op_custom_type() { EvalAltResult::ErrorMismatchOutputType(need, actual, _) if need == "TestStruct" && actual == std::any::type_name::() )); + + Ok(()) } From 9495d3f73309fd9743549acb226ad2a236be3442 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 25 Feb 2021 10:59:22 +0800 Subject: [PATCH 16/23] Separate op-assignment with other function calls. --- src/engine.rs | 10 ++++++---- src/optimize.rs | 1 + src/packages/logic.rs | 7 ++++++- src/token.rs | 8 -------- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 07740bad..ae295998 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1681,7 +1681,9 @@ impl Engine { let pos = rhs.position(); if self - .call_native_fn(mods, state, lib, OP_EQUALS, hash_fn, args, false, pos)? + .call_native_fn( + mods, state, lib, OP_EQUALS, hash_fn, args, false, false, pos, + )? .0 .as_bool() .unwrap_or(false) @@ -1961,7 +1963,7 @@ impl Engine { let hash_fn = calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())).unwrap(); - match self.call_native_fn(mods, state, lib, op, hash_fn, args, true, op_pos) { + match self.call_native_fn(mods, state, lib, op, hash_fn, args, true, true, op_pos) { Ok(_) => (), Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) => { @@ -1971,8 +1973,8 @@ impl Engine { calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())).unwrap(); // Run function - let (value, _) = - self.call_native_fn(mods, state, lib, op, hash_fn, args, true, op_pos)?; + let (value, _) = self + .call_native_fn(mods, state, lib, op, hash_fn, args, true, false, op_pos)?; *args[0] = value.flatten(); } diff --git a/src/optimize.rs b/src/optimize.rs index 64b6140d..64609e60 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -149,6 +149,7 @@ fn call_fn_with_constant_arguments( hash_fn.unwrap(), arg_values.iter_mut().collect::>().as_mut(), false, + false, Position::NONE, ) .ok() diff --git a/src/packages/logic.rs b/src/packages/logic.rs index 8876b5a6..fdd6e0a3 100644 --- a/src/packages/logic.rs +++ b/src/packages/logic.rs @@ -145,7 +145,12 @@ mod logic_functions { } Err(Box::new(EvalAltResult::ErrorFunctionNotFound( - format!("{} ({}, {})", ctx.fn_name(), x.type_name(), y.type_name()), + format!( + "{} ({}, {})", + ctx.fn_name(), + ctx.engine().map_type_name(x.type_name()), + ctx.engine().map_type_name(y.type_name()) + ), Position::NONE, ))) } diff --git a/src/token.rs b/src/token.rs index 7e2c7b42..ea25f37a 100644 --- a/src/token.rs +++ b/src/token.rs @@ -1657,14 +1657,6 @@ pub fn is_valid_identifier(name: impl Iterator) -> bool { first_alphabetic } -/// Is a text string an assignment operator? -pub fn is_assignment_operator(op: &str) -> bool { - match op { - "+=" | "-=" | "*=" | "/=" | "<<=" | ">>=" | "&=" | "|=" | "^=" | "%=" | "**=" => true, - _ => false, - } -} - /// Is a character valid to start an identifier? #[cfg(feature = "unicode-xid-ident")] #[inline(always)] From 1c1dfc701f1722aebb9def75a1deacf1246dc9ae Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 25 Feb 2021 11:03:54 +0800 Subject: [PATCH 17/23] Fix Decimal. --- src/fn_call.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index 78c77702..a7fc94ba 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -1416,19 +1416,19 @@ pub fn run_builtin_binary_op( } #[cfg(feature = "decimal")] - if let Some((x, y)) = if type_id == (TypeId::of::(), TypeId::of::()) { + if let Some((x, y)) = if types_pair == (TypeId::of::(), TypeId::of::()) { // Decimal op Decimal Some(( *x.read_lock::().unwrap(), *y.read_lock::().unwrap(), )) - } else if type_id == (TypeId::of::(), TypeId::of::()) { + } else if types_pair == (TypeId::of::(), TypeId::of::()) { // Decimal op INT Some(( *x.read_lock::().unwrap(), y.clone().cast::().into(), )) - } else if type_id == (TypeId::of::(), TypeId::of::()) { + } else if types_pair == (TypeId::of::(), TypeId::of::()) { // INT op Decimal Some(( x.clone().cast::().into(), @@ -1639,11 +1639,11 @@ pub fn run_builtin_op_assignment( } #[cfg(feature = "decimal")] - if let Some((mut x, y)) = if type_id == (TypeId::of::(), TypeId::of::()) { + if let Some((mut x, y)) = if types_pair == (TypeId::of::(), TypeId::of::()) { // Decimal op= Decimal let y = *y.read_lock::().unwrap(); Some((x.write_lock::().unwrap(), y)) - } else if type_id == (TypeId::of::(), TypeId::of::()) { + } else if types_pair == (TypeId::of::(), TypeId::of::()) { // Decimal op= INT let y = y.clone().cast::().into(); Some((x.write_lock::().unwrap(), y)) From f03983a9caad78f42e5fafb26d3810b939727295 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 25 Feb 2021 11:04:01 +0800 Subject: [PATCH 18/23] Expose Engine::map_type_name. --- src/engine.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index ae295998..5d1957d8 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -2573,9 +2573,12 @@ impl Engine { Ok(()) } - /// Map a type_name into a pretty-print name + /// Pretty-print a type name. + /// + /// If a type is registered via [`register_type_with_name`][Engine::register_type_with_name], + /// the type name provided for the registration will be used. #[inline(always)] - pub(crate) fn map_type_name<'a>(&'a self, name: &'a str) -> &'a str { + pub fn map_type_name<'a>(&'a self, name: &'a str) -> &'a str { self.type_names .get(name) .map(String::as_str) From 3f4dba9dbc4c998164cb14f226ce9bb6ef2bb73a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 25 Feb 2021 13:29:49 +0800 Subject: [PATCH 19/23] Build in operators between string and char. --- RELEASES.md | 4 +- src/fn_call.rs | 114 +++++++++++++++++++++++--------- src/packages/string_more.rs | 44 ++++++++----- src/utils.rs | 127 +++++++++++++++++++++++++++++++++++- tests/decrement.rs | 8 +-- tests/increment.rs | 2 +- tests/ops.rs | 14 +++- tests/string.rs | 13 ++++ 8 files changed, 271 insertions(+), 55 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 7158e27c..fc50ab94 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -22,7 +22,6 @@ New features * Functions are now allowed to have `Dynamic` arguments. * `#[rhai_fn(pure)]` attribute to mark a plugin function with `&mut` parameter as _pure_ so constants can be passed to it. Without it, passing a constant value into the `&mut` parameter will now raise an error. -* Comparisons between `FLOAT`/[`Decimal`](https://crates.io/crates/rust_decimal) and `INT` are now built in. Enhancements ------------ @@ -31,8 +30,11 @@ Enhancements * Error position in `eval` statements is now wrapped in an `EvalAltResult::ErrorInFunctionCall`. * `Position` now implements `Add` and `AddAssign`. * `Scope` now implements `IntoIterator`. +* Strings now have the `-`/`-=` operators and the `remove` method to delete a sub-string/character. * Strings now have the `split_rev` method and variations of `split` with maximum number of segments. * Arrays now have the `split` method. +* Comparisons between `FLOAT`/[`Decimal`](https://crates.io/crates/rust_decimal) and `INT` are now built in. +* Comparisons between string and `char` are now built in. Version 0.19.12 diff --git a/src/fn_call.rs b/src/fn_call.rs index a7fc94ba..26956964 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -17,7 +17,6 @@ use crate::stdlib::{ iter::{empty, once}, mem, num::NonZeroU64, - ops::Deref, string::ToString, vec::Vec, }; @@ -1470,28 +1469,60 @@ pub fn run_builtin_binary_op( } } + // char op string + if types_pair == (TypeId::of::(), TypeId::of::()) { + let x = x.clone().cast::(); + let y = &*y.read_lock::().unwrap(); + + match op { + "+" => return Ok(Some(format!("{}{}", x, y).into())), + "==" | "!=" | ">" | ">=" | "<" | "<=" => { + let s1 = [x, '\0']; + let mut y = y.chars(); + let s2 = [y.next().unwrap_or('\0'), y.next().unwrap_or('\0')]; + + match op { + "==" => return Ok(Some((s1 == s2).into())), + "!=" => return Ok(Some((s1 != s2).into())), + ">" => return Ok(Some((s1 > s2).into())), + ">=" => return Ok(Some((s1 >= s2).into())), + "<" => return Ok(Some((s1 < s2).into())), + "<=" => return Ok(Some((s1 <= s2).into())), + _ => unreachable!(), + } + } + _ => return Ok(None), + } + } + // string op char + if types_pair == (TypeId::of::(), TypeId::of::()) { + let x = &*x.read_lock::().unwrap(); + let y = y.clone().cast::(); + + match op { + "+" => return Ok(Some((x + y).into())), + "-" => return Ok(Some((x - y).into())), + "==" | "!=" | ">" | ">=" | "<" | "<=" => { + let mut x = x.chars(); + let s1 = [x.next().unwrap_or('\0'), x.next().unwrap_or('\0')]; + let s2 = [y, '\0']; + + match op { + "==" => return Ok(Some((s1 == s2).into())), + "!=" => return Ok(Some((s1 != s2).into())), + ">" => return Ok(Some((s1 > s2).into())), + ">=" => return Ok(Some((s1 >= s2).into())), + "<" => return Ok(Some((s1 < s2).into())), + "<=" => return Ok(Some((s1 <= s2).into())), + _ => unreachable!(), + } + } + _ => return Ok(None), + } + } + + // Default comparison operators for different types if type2 != type1 { - // char op string - if types_pair == (TypeId::of::(), TypeId::of::()) { - let x = x.clone().cast::(); - let y = &*y.read_lock::().unwrap(); - - match op { - "+" => return Ok(Some(format!("{}{}", x, y).into())), - _ => return Ok(None), - } - } - // string op char - if types_pair == (TypeId::of::(), TypeId::of::()) { - let x = &*x.read_lock::().unwrap(); - let y = y.clone().cast::(); - - match op { - "+" => return Ok(Some((x + y).into())), - _ => return Ok(None), - } - } - // Default comparison operators for different types return Ok(match op { "!=" => Some(Dynamic::TRUE), "==" | ">" | ">=" | "<" | "<=" => Some(Dynamic::FALSE), @@ -1567,6 +1598,7 @@ pub fn run_builtin_binary_op( match op { "+" => return Ok(Some((x + y).into())), + "-" => return Ok(Some((x - y).into())), "==" => return Ok(Some((x == y).into())), "!=" => return Ok(Some((x != y).into())), ">" => return Ok(Some((x > y).into())), @@ -1673,17 +1705,34 @@ pub fn run_builtin_op_assignment( } } - if type2 != type1 { - if types_pair == (TypeId::of::(), TypeId::of::()) { - let y = y.read_lock::().unwrap().deref().clone(); - let mut x = x.write_lock::().unwrap(); + // string op= char + if types_pair == (TypeId::of::(), TypeId::of::()) { + let y = y.clone().cast::(); + let mut x = x.write_lock::().unwrap(); - match op { - "+=" => return Ok(Some(*x += y)), - _ => return Ok(None), - } + match op { + "+=" => return Ok(Some(*x += y)), + "-=" => return Ok(Some(*x -= y)), + _ => return Ok(None), } + } + // char op= string + if types_pair == (TypeId::of::(), TypeId::of::()) { + let y = y.read_lock::().unwrap(); + let mut ch = x.read_lock::().unwrap().to_string(); + let mut x = x.write_lock::().unwrap(); + match op { + "+=" => { + ch.push_str(y.as_str()); + return Ok(Some(*x = ch.into())); + } + _ => return Ok(None), + } + } + + // No built-in op-assignments for different types. + if type2 != type1 { return Ok(None); } @@ -1741,7 +1790,7 @@ pub fn run_builtin_op_assignment( } if type1 == TypeId::of::() { - let y = y.read_lock::().unwrap().deref().clone(); + let y = y.clone().cast::(); let mut x = x.write_lock::().unwrap(); match op { @@ -1751,11 +1800,12 @@ pub fn run_builtin_op_assignment( } if type1 == TypeId::of::() { - let y = y.read_lock::().unwrap().deref().clone(); + let y = &*y.read_lock::().unwrap(); let mut x = x.write_lock::().unwrap(); match op { "+=" => return Ok(Some(*x += y)), + "-=" => return Ok(Some(*x -= y)), _ => return Ok(None), } } diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index 7925c0ea..b800d90d 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -20,7 +20,7 @@ def_package!(crate:MoreStringPackage:"Additional string utilities, including str mod string_functions { use crate::ImmutableString; - #[rhai_fn(name = "+")] + #[rhai_fn(name = "+", name = "append")] pub fn add_append(string: &str, item: Dynamic) -> ImmutableString { format!("{}{}", string, item).into() } @@ -42,6 +42,13 @@ mod string_functions { pub fn len(string: &str) -> INT { string.chars().count() as INT } + pub fn remove(string: &mut ImmutableString, sub_string: ImmutableString) { + *string -= sub_string; + } + #[rhai_fn(name = "remove")] + pub fn remove_char(string: &mut ImmutableString, character: char) { + *string -= character; + } pub fn clear(string: &mut ImmutableString) { string.make_mut().clear(); } @@ -64,15 +71,15 @@ mod string_functions { } #[rhai_fn(name = "contains")] - pub fn contains_char(string: &str, ch: char) -> bool { - string.contains(ch) + pub fn contains_char(string: &str, character: char) -> bool { + string.contains(character) } pub fn contains(string: &str, find_string: &str) -> bool { string.contains(find_string) } #[rhai_fn(name = "index_of")] - pub fn index_of_char_starting_from(string: &str, ch: char, start: INT) -> INT { + pub fn index_of_char_starting_from(string: &str, character: char, start: INT) -> INT { let start = if start < 0 { 0 } else if start as usize >= string.chars().count() { @@ -86,14 +93,14 @@ mod string_functions { }; string[start..] - .find(ch) + .find(character) .map(|index| string[0..start + index].chars().count() as INT) .unwrap_or(-1 as INT) } #[rhai_fn(name = "index_of")] - pub fn index_of_char(string: &str, ch: char) -> INT { + pub fn index_of_char(string: &str, character: char) -> INT { string - .find(ch) + .find(character) .map(|index| string[0..index].chars().count() as INT) .unwrap_or(-1 as INT) } @@ -196,26 +203,33 @@ mod string_functions { pub fn replace_string_with_char( string: &mut ImmutableString, find_string: &str, - substitute_char: char, + substitute_character: char, ) { *string = string - .replace(find_string, &substitute_char.to_string()) + .replace(find_string, &substitute_character.to_string()) .into(); } #[rhai_fn(name = "replace")] pub fn replace_char_with_string( string: &mut ImmutableString, - find_char: char, + find_character: char, substitute_string: &str, ) { *string = string - .replace(&find_char.to_string(), substitute_string) + .replace(&find_character.to_string(), substitute_string) .into(); } #[rhai_fn(name = "replace")] - pub fn replace_char(string: &mut ImmutableString, find_char: char, substitute_char: char) { + pub fn replace_char( + string: &mut ImmutableString, + find_character: char, + substitute_character: char, + ) { *string = string - .replace(&find_char.to_string(), &substitute_char.to_string()) + .replace( + &find_character.to_string(), + &substitute_character.to_string(), + ) .into(); } @@ -224,7 +238,7 @@ mod string_functions { _ctx: NativeCallContext, string: &mut ImmutableString, len: INT, - ch: char, + character: char, ) -> Result> { // Check if string will be over max size limit #[cfg(not(feature = "unchecked"))] @@ -243,7 +257,7 @@ mod string_functions { let p = string.make_mut(); for _ in 0..(len as usize - orig_len) { - p.push(ch); + p.push(character); } #[cfg(not(feature = "unchecked"))] diff --git a/src/utils.rs b/src/utils.rs index 7cbed35c..a6a2a0eb 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -12,7 +12,7 @@ use crate::stdlib::{ hash::{BuildHasher, Hash, Hasher}, iter::{empty, FromIterator}, num::NonZeroU64, - ops::{Add, AddAssign, Deref, DerefMut}, + ops::{Add, AddAssign, Deref, DerefMut, Sub, SubAssign}, str::FromStr, string::{String, ToString}, vec::Vec, @@ -468,6 +468,13 @@ impl Add for &ImmutableString { } } +impl AddAssign for ImmutableString { + #[inline(always)] + fn add_assign(&mut self, rhs: String) { + self.make_mut().push_str(&rhs); + } +} + impl Add for ImmutableString { type Output = Self; @@ -496,6 +503,124 @@ impl AddAssign for ImmutableString { } } +impl Sub for ImmutableString { + type Output = Self; + + #[inline] + fn sub(self, rhs: Self) -> Self::Output { + if rhs.is_empty() { + self + } else if self.is_empty() { + rhs + } else { + self.replace(rhs.as_str(), "").into() + } + } +} + +impl Sub for &ImmutableString { + type Output = ImmutableString; + + #[inline] + fn sub(self, rhs: Self) -> Self::Output { + if rhs.is_empty() { + self.clone() + } else if self.is_empty() { + rhs.clone() + } else { + self.replace(rhs.as_str(), "").into() + } + } +} + +impl SubAssign<&ImmutableString> for ImmutableString { + #[inline] + fn sub_assign(&mut self, rhs: &ImmutableString) { + if !rhs.is_empty() { + if self.is_empty() { + self.0 = rhs.0.clone(); + } else { + self.0 = self.replace(rhs.as_str(), "").into(); + } + } + } +} + +impl SubAssign for ImmutableString { + #[inline] + fn sub_assign(&mut self, rhs: ImmutableString) { + if !rhs.is_empty() { + if self.is_empty() { + self.0 = rhs.0; + } else { + self.0 = self.replace(rhs.as_str(), "").into(); + } + } + } +} + +impl Sub for ImmutableString { + type Output = Self; + + #[inline] + fn sub(self, rhs: String) -> Self::Output { + if rhs.is_empty() { + self + } else if self.is_empty() { + rhs.into() + } else { + self.replace(&rhs, "").into() + } + } +} + +impl Sub for &ImmutableString { + type Output = ImmutableString; + + #[inline] + fn sub(self, rhs: String) -> Self::Output { + if rhs.is_empty() { + self.clone() + } else if self.is_empty() { + rhs.into() + } else { + self.replace(&rhs, "").into() + } + } +} + +impl SubAssign for ImmutableString { + #[inline(always)] + fn sub_assign(&mut self, rhs: String) { + self.0 = self.replace(&rhs, "").into(); + } +} + +impl Sub for ImmutableString { + type Output = Self; + + #[inline(always)] + fn sub(self, rhs: char) -> Self::Output { + self.replace(rhs, "").into() + } +} + +impl Sub for &ImmutableString { + type Output = ImmutableString; + + #[inline(always)] + fn sub(self, rhs: char) -> Self::Output { + self.replace(rhs, "").into() + } +} + +impl SubAssign for ImmutableString { + #[inline(always)] + fn sub_assign(&mut self, rhs: char) { + self.0 = self.replace(rhs, "").into(); + } +} + impl> PartialEq for ImmutableString { #[inline(always)] fn eq(&self, other: &S) -> bool { diff --git a/tests/decrement.rs b/tests/decrement.rs index 0691eddf..25a80b2a 100644 --- a/tests/decrement.rs +++ b/tests/decrement.rs @@ -6,10 +6,10 @@ fn test_decrement() -> Result<(), Box> { assert_eq!(engine.eval::("let x = 10; x -= 7; x")?, 3); - assert!(matches!( - *engine.eval::(r#"let s = "test"; s -= "ing"; s"#).expect_err("expects error"), - EvalAltResult::ErrorFunctionNotFound(err, _) if err == "- (&str | ImmutableString | String, &str | ImmutableString | String)" - )); + assert_eq!( + engine.eval::(r#"let s = "test"; s -= 's'; s"#)?, + "tet" + ); Ok(()) } diff --git a/tests/increment.rs b/tests/increment.rs index f622ab21..4ab59117 100644 --- a/tests/increment.rs +++ b/tests/increment.rs @@ -7,7 +7,7 @@ fn test_increment() -> Result<(), Box> { assert_eq!(engine.eval::("let x = 1; x += 2; x")?, 3); assert_eq!( - engine.eval::("let s = \"test\"; s += \"ing\"; s")?, + engine.eval::(r#"let s = "test"; s += "ing"; s"#)?, "testing" ); diff --git a/tests/ops.rs b/tests/ops.rs index 72c823cb..290c9ada 100644 --- a/tests/ops.rs +++ b/tests/ops.rs @@ -34,7 +34,19 @@ fn test_ops_numbers() -> Result<(), Box> { } #[test] -fn test_op_precedence() -> Result<(), Box> { +fn test_ops_strings() -> Result<(), Box> { + let engine = Engine::new(); + + assert!(engine.eval::(r#""hello" > 'c'"#)?); + assert!(engine.eval::(r#""" < 'c'"#)?); + assert!(engine.eval::(r#"'x' > "hello""#)?); + assert!(engine.eval::(r#""hello" > "foo""#)?); + + Ok(()) +} + +#[test] +fn test_ops_precedence() -> Result<(), Box> { let engine = Engine::new(); assert_eq!( diff --git a/tests/string.rs b/tests/string.rs index cd4aa6cb..6880c195 100644 --- a/tests/string.rs +++ b/tests/string.rs @@ -132,6 +132,19 @@ fn test_string_substring() -> Result<(), Box> { "❤❤ hello! ❤❤❤" ); + assert_eq!( + engine.eval::( + r#"let x = "\u2764\u2764\u2764 hello! \u2764\u2764\u2764"; x -= 'l'; x"# + )?, + "❤❤❤ heo! ❤❤❤" + ); + + assert_eq!( + engine.eval::( + r#"let x = "\u2764\u2764\u2764 hello! \u2764\u2764\u2764"; x -= "\u2764\u2764"; x"# + )?, + "❤ hello! ❤" + ); assert_eq!( engine.eval::( r#"let x = "\u2764\u2764\u2764 hello! \u2764\u2764\u2764"; x.index_of('\u2764')"# From 4638983afda3aa80c6a27c59f1864603c52703dc Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 25 Feb 2021 17:52:56 +0800 Subject: [PATCH 20/23] Rename RELEASES to CHANGELOG. --- RELEASES.md => CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename RELEASES.md => CHANGELOG.md (99%) diff --git a/RELEASES.md b/CHANGELOG.md similarity index 99% rename from RELEASES.md rename to CHANGELOG.md index fc50ab94..4206d411 100644 --- a/RELEASES.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ Breaking changes * For plugin functions, constants passed to methods (i.e. `&mut` parameter) now raise an error unless the functions are marked with `#[rhai_fn(pure)]`. * Visibility (i.e. `pub` or not) for generated _plugin_ modules now follow the visibility of the underlying module. -* Comparison operators between the sames types now throw errors when they're not defined instead of returning the default. Only comparing between _different_ types will return the default. +* Comparison operators between the sames types or different _numeric_ types now throw errors when they're not defined instead of returning the default. Only comparing between _different_ types will return the default. * Default stack-overflow and top-level expression nesting limits for release builds are lowered to 64 from 128. New features From 2f78626a21d25d8532adf9bb4112153ee475a518 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 25 Feb 2021 17:53:01 +0800 Subject: [PATCH 21/23] Update README. --- codegen/README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/codegen/README.md b/codegen/README.md index 5099d3a4..075f2929 100644 --- a/codegen/README.md +++ b/codegen/README.md @@ -1,5 +1,10 @@ Procedural Macros for Plugins ============================= +![Rhai logo](https://rhai.rs/book/images/logo/rhai-banner-transparent-colour.svg) + This crate holds procedural macros for code generation, supporting the plugins system for [Rhai](https://github.com/rhaiscript/rhai). + +This crate is automatically referenced by the [Rhai crate](https://crates.io/crates/rhai). +Normally it should not be used directly. From 6aa0be546fe897ccaa6c91f529a4c9340ab4cb45 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 26 Feb 2021 11:21:05 +0800 Subject: [PATCH 22/23] Replace String::from("...") with "...".into(). --- tests/side_effects.rs | 2 +- tests/string.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/side_effects.rs b/tests/side_effects.rs index 69e33de1..b7ba49f6 100644 --- a/tests/side_effects.rs +++ b/tests/side_effects.rs @@ -66,7 +66,7 @@ fn test_side_effects_command() -> Result<(), Box> { #[test] fn test_side_effects_print() -> Result<(), Box> { - let result = Arc::new(RwLock::new(String::from(""))); + let result = Arc::new(RwLock::new(String::new())); let mut engine = Engine::new(); diff --git a/tests/string.rs b/tests/string.rs index 6880c195..a4f80c07 100644 --- a/tests/string.rs +++ b/tests/string.rs @@ -53,8 +53,8 @@ fn test_string() -> Result<(), Box> { fn test_string_dynamic() -> Result<(), Box> { let engine = Engine::new(); let mut scope = Scope::new(); - scope.push("x", Dynamic::from("foo")); - scope.push("y", String::from("foo")); + scope.push("x", "foo"); + scope.push("y", "foo"); scope.push("z", "foo"); assert!(engine.eval_with_scope::(&mut scope, r#"x == "foo""#)?); From d935401b03eaa51c75bd6ed7b5c65b532f350cc9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 26 Feb 2021 11:21:23 +0800 Subject: [PATCH 23/23] Allow evaluating AST in Engine::call_fn_dynamic. --- CHANGELOG.md | 2 ++ src/engine_api.rs | 64 +++++++++++++++++++++++++++++------------------ 2 files changed, 42 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4206d411..97b799ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Breaking changes * Visibility (i.e. `pub` or not) for generated _plugin_ modules now follow the visibility of the underlying module. * Comparison operators between the sames types or different _numeric_ types now throw errors when they're not defined instead of returning the default. Only comparing between _different_ types will return the default. * Default stack-overflow and top-level expression nesting limits for release builds are lowered to 64 from 128. +* `Engine::call_fn_dynamic` takes an additional parameter to optionally evaluate the given `AST` before calling the function. New features ------------ @@ -35,6 +36,7 @@ Enhancements * Arrays now have the `split` method. * Comparisons between `FLOAT`/[`Decimal`](https://crates.io/crates/rust_decimal) and `INT` are now built in. * Comparisons between string and `char` are now built in. +* `Engine::call_fn_dynamic` can now optionally evaluate the given `AST` before calling the function. Version 0.19.12 diff --git a/src/engine_api.rs b/src/engine_api.rs index 808a2eff..e98ace88 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1613,6 +1613,13 @@ impl Engine { /// Call a script function defined in an [`AST`] with multiple arguments. /// Arguments are passed as a tuple. /// + /// ## Warning + /// + /// The [`AST`] is _not_ evaluated before calling the function. The function is called as-is. + /// + /// If the [`AST`] needs to be evaluated before calling the function (usually to load external modules), + /// use [`call_fn_dynamic`][Engine::call_fn_dynamic]. + /// /// # Example /// /// ``` @@ -1633,11 +1640,11 @@ impl Engine { /// scope.push("foo", 42_i64); /// /// // Call the script-defined function - /// let result: i64 = engine.call_fn(&mut scope, &ast, "add", ( String::from("abc"), 123_i64 ) )?; + /// let result: i64 = engine.call_fn(&mut scope, &ast, "add", ( "abc", 123_i64 ) )?; /// assert_eq!(result, 168); /// - /// let result: i64 = engine.call_fn(&mut scope, &ast, "add1", ( String::from("abc"), ) )?; - /// // ^^^^^^^^^^^^^^^^^^^^^^^^ tuple of one + /// let result: i64 = engine.call_fn(&mut scope, &ast, "add1", ( "abc", ) )?; + /// // ^^^^^^^^^^ tuple of one /// assert_eq!(result, 46); /// /// let result: i64 = engine.call_fn(&mut scope, &ast, "bar", () )?; @@ -1659,8 +1666,7 @@ impl Engine { args.parse(&mut arg_values); let mut args: crate::StaticVec<_> = arg_values.as_mut().iter_mut().collect(); - let result = - self.call_fn_dynamic_raw(scope, &[ast.lib()], name, &mut None, args.as_mut())?; + let result = self.call_fn_dynamic_raw(scope, ast, false, name, &mut None, args.as_mut())?; let typ = self.map_type_name(result.type_name()); @@ -1676,6 +1682,9 @@ impl Engine { /// Call a script function defined in an [`AST`] with multiple [`Dynamic`] arguments /// and optionally a value for binding to the `this` pointer. /// + /// There is also an option to evaluate the [`AST`] (e.g. to configuration the environment) + /// before calling the function. + /// /// # WARNING /// /// All the arguments are _consumed_, meaning that they're replaced by `()`. @@ -1704,19 +1713,19 @@ impl Engine { /// scope.push("foo", 42_i64); /// /// // Call the script-defined function - /// let result = engine.call_fn_dynamic(&mut scope, &ast, "add", None, [ String::from("abc").into(), 123_i64.into() ])?; - /// // ^^^^ no 'this' pointer + /// let result = engine.call_fn_dynamic(&mut scope, &ast, false, "add", None, [ "abc".into(), 123_i64.into() ])?; + /// // ^^^^ no 'this' pointer /// assert_eq!(result.cast::(), 168); /// - /// let result = engine.call_fn_dynamic(&mut scope, &ast, "add1", None, [ String::from("abc").into() ])?; + /// let result = engine.call_fn_dynamic(&mut scope, &ast, false, "add1", None, [ "abc".into() ])?; /// assert_eq!(result.cast::(), 46); /// - /// let result = engine.call_fn_dynamic(&mut scope, &ast, "bar", None, [])?; + /// let result = engine.call_fn_dynamic(&mut scope, &ast, false, "bar", None, [])?; /// assert_eq!(result.cast::(), 21); /// /// let mut value: Dynamic = 1_i64.into(); - /// let result = engine.call_fn_dynamic(&mut scope, &ast, "action", Some(&mut value), [ 41_i64.into() ])?; - /// // ^^^^^^^^^^^^^^^^ binding the 'this' pointer + /// let result = engine.call_fn_dynamic(&mut scope, &ast, false, "action", Some(&mut value), [ 41_i64.into() ])?; + /// // ^^^^^^^^^^^^^^^^ binding the 'this' pointer /// assert_eq!(value.as_int().unwrap(), 42); /// # } /// # Ok(()) @@ -1727,14 +1736,15 @@ impl Engine { pub fn call_fn_dynamic( &self, scope: &mut Scope, - lib: impl AsRef, + ast: &AST, + eval_ast: bool, name: &str, mut this_ptr: Option<&mut Dynamic>, mut arg_values: impl AsMut<[Dynamic]>, ) -> Result> { let mut args: crate::StaticVec<_> = arg_values.as_mut().iter_mut().collect(); - self.call_fn_dynamic_raw(scope, &[lib.as_ref()], name, &mut this_ptr, args.as_mut()) + self.call_fn_dynamic_raw(scope, ast, eval_ast, name, &mut this_ptr, args.as_mut()) } /// Call a script function defined in an [`AST`] with multiple [`Dynamic`] arguments. /// @@ -1749,18 +1759,24 @@ impl Engine { pub(crate) fn call_fn_dynamic_raw( &self, scope: &mut Scope, - lib: &[&crate::Module], + ast: &AST, + eval_ast: bool, name: &str, this_ptr: &mut Option<&mut Dynamic>, args: &mut FnCallArgs, ) -> Result> { - let fn_def = lib - .iter() - .find_map(|&m| m.get_script_fn(name, args.len(), true)) - .ok_or_else(|| EvalAltResult::ErrorFunctionNotFound(name.into(), Position::NONE))?; + let state = &mut Default::default(); + let mods = &mut (&self.global_sub_modules).into(); + let lib = &[ast.lib()]; - let mut state = Default::default(); - let mut mods = (&self.global_sub_modules).into(); + if eval_ast { + self.eval_global_statements(scope, mods, state, ast.statements(), lib, 0)?; + } + + let fn_def = ast + .lib() + .get_script_fn(name, args.len(), true) + .ok_or_else(|| EvalAltResult::ErrorFunctionNotFound(name.into(), Position::NONE))?; // Check for data race. if cfg!(not(feature = "no_closure")) { @@ -1769,8 +1785,8 @@ impl Engine { self.call_script_fn( scope, - &mut mods, - &mut state, + mods, + state, lib, this_ptr, fn_def, @@ -1933,7 +1949,7 @@ impl Engine { /// # use std::sync::Arc; /// use rhai::Engine; /// - /// let result = Arc::new(RwLock::new(String::from(""))); + /// let result = Arc::new(RwLock::new(String::new())); /// /// let mut engine = Engine::new(); /// @@ -1962,7 +1978,7 @@ impl Engine { /// # use std::sync::Arc; /// use rhai::Engine; /// - /// let result = Arc::new(RwLock::new(String::from(""))); + /// let result = Arc::new(RwLock::new(String::new())); /// /// let mut engine = Engine::new(); ///