From 33d3e3490873d2bb5f4a803d4d67827c0ba25d78 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 26 Apr 2020 18:04:07 +0800 Subject: [PATCH 01/13] Deep linking for dot/index chains. --- README.md | 2 +- src/any.rs | 14 +- src/engine.rs | 947 ++++++++++++++++++++------------------------------ src/error.rs | 3 - src/parser.rs | 200 +++++------ src/scope.rs | 32 +- 6 files changed, 482 insertions(+), 716 deletions(-) diff --git a/README.md b/README.md index 3b302577..0e8d4b77 100644 --- a/README.md +++ b/README.md @@ -1947,7 +1947,7 @@ Properties and methods in a Rust custom type registered with the [`Engine`] can ```rust let a = new_ts(); // constructor function a.field = 500; // property access -a.update(); // method call +a.update(); // method call, 'a' can be changed update(a); // this works, but 'a' is unchanged because only // a COPY of 'a' is passed to 'update' by VALUE diff --git a/src/any.rs b/src/any.rs index f718b01c..46ae6044 100644 --- a/src/any.rs +++ b/src/any.rs @@ -212,7 +212,7 @@ impl fmt::Display for Dynamic { #[cfg(not(feature = "no_float"))] Union::Float(value) => write!(f, "{}", value), Union::Array(value) => write!(f, "{:?}", value), - Union::Map(value) => write!(f, "{:?}", value), + Union::Map(value) => write!(f, "#{:?}", value), Union::Variant(_) => write!(f, "?"), } } @@ -229,7 +229,7 @@ impl fmt::Debug for Dynamic { #[cfg(not(feature = "no_float"))] Union::Float(value) => write!(f, "{:?}", value), Union::Array(value) => write!(f, "{:?}", value), - Union::Map(value) => write!(f, "{:?}", value), + Union::Map(value) => write!(f, "#{:?}", value), Union::Variant(_) => write!(f, ""), } } @@ -268,16 +268,6 @@ fn cast_box(item: Box) -> Result> { } impl Dynamic { - /// Get a reference to the inner `Union`. - pub(crate) fn get_ref(&self) -> &Union { - &self.0 - } - - /// Get a mutable reference to the inner `Union`. - pub(crate) fn get_mut(&mut self) -> &mut Union { - &mut self.0 - } - /// Create a `Dynamic` from any type. A `Dynamic` value is simply returned as is. /// /// Beware that you need to pass in an `Array` type for it to be recognized as an `Array`. diff --git a/src/engine.rs b/src/engine.rs index b3ed775d..a0d568ca 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -5,7 +5,7 @@ use crate::calc_fn_hash; use crate::error::ParseErrorType; use crate::optimize::OptimizationLevel; use crate::packages::{CorePackage, Package, PackageLibrary, StandardPackage}; -use crate::parser::{Expr, FnDef, ReturnType, Stmt, INT}; +use crate::parser::{Expr, FnDef, ReturnType, Stmt}; use crate::result::EvalAltResult; use crate::scope::{EntryRef as ScopeSource, EntryType as ScopeEntryType, Scope}; use crate::token::Position; @@ -13,10 +13,12 @@ use crate::token::Position; use crate::stdlib::{ any::TypeId, boxed::Box, + cell::RefCell, collections::HashMap, format, hash::{Hash, Hasher}, iter::once, + mem, ops::{Deref, DerefMut}, rc::Rc, string::{String, ToString}, @@ -74,65 +76,75 @@ const FUNCTIONS_COUNT: usize = 512; #[cfg(any(feature = "only_i32", feature = "only_i64"))] const FUNCTIONS_COUNT: usize = 256; -/// A type encapsulating an index value, which may be an integer or a string key. -#[derive(Debug, Eq, PartialEq, Hash, Clone)] -enum IndexValue { - Num(usize), - Str(String), -} - -impl IndexValue { - fn from_num(idx: INT) -> Self { - Self::Num(idx as usize) - } - fn from_str(name: String) -> Self { - Self::Str(name) - } - fn as_num(self) -> usize { - match self { - Self::Num(n) => n, - _ => panic!("index value is numeric"), - } - } - fn as_str(self) -> String { - match self { - Self::Str(s) => s, - _ => panic!("index value is string"), - } - } -} - -/// A type encapsulating the target of a update action. -/// The reason we need this is because we cannot hold a mutable reference to a variable in -/// the current `Scope` while evaluating expressions requiring access to the same `Scope`. -/// So we cannot use a single `&mut Dynamic` everywhere; instead, we hold enough information -/// to find the variable from the `Scope` when we need to update it. -#[derive(Debug)] +/// A type that encapsulates a mutation target for an expression with side effects. enum Target<'a> { - /// The update target is a variable stored in the current `Scope`. - Scope(ScopeSource<'a>), - /// The update target is a `Dynamic` value stored somewhere. - Value(&'a mut Dynamic), + /// The target is a mutable reference to a `Dynamic` value somewhere. + Ref(&'a mut Dynamic), + /// The target is a variable stored in the current `Scope`. + Scope(&'a RefCell), + /// The target is a temporary `Dynamic` value (i.e. the mutation can cause no side effects). + Value(Dynamic), + /// The target is a character inside a String. + StringChar(Box<(&'a mut Dynamic, usize, Dynamic)>), } -impl<'a> Target<'a> { - fn get_mut(self, scope: &'a mut Scope) -> &'a mut Dynamic { +impl Target<'_> { + /// Get the value of the `Target` as a `Dynamic`. + pub fn into_dynamic(self) -> Dynamic { match self { - Self::Value(t) => t, - Self::Scope(src) => scope.get_mut(src), + Target::Ref(r) => r.clone(), + Target::Scope(r) => r.borrow().clone(), + Target::Value(v) => v, + Target::StringChar(s) => s.2, } } -} -impl<'a> From> for Target<'a> { - fn from(src: ScopeSource<'a>) -> Self { - Self::Scope(src) + /// Update the value of the `Target`. + pub fn set_value(&mut self, new_val: Dynamic, pos: Position) -> Result<(), Box> { + match self { + Target::Scope(r) => *r.borrow_mut() = new_val, + Target::Ref(r) => **r = new_val, + Target::Value(_) => { + return Err(Box::new(EvalAltResult::ErrorAssignmentToUnknownLHS(pos))) + } + Target::StringChar(x) => match x.0 { + Dynamic(Union::Str(s)) => { + // Replace the character at the specified index position + let new_ch = new_val + .as_char() + .map_err(|_| EvalAltResult::ErrorCharMismatch(pos))?; + + let mut chars: Vec = s.chars().collect(); + let ch = *chars.get(x.1).expect("string index out of bounds"); + + // See if changed - if so, update the String + if ch != new_ch { + chars[x.1] = new_ch; + s.clear(); + chars.iter().for_each(|&ch| s.push(ch)); + } + } + _ => panic!("should be String"), + }, + } + + Ok(()) } } +impl<'a> From<&'a RefCell> for Target<'a> { + fn from(value: &'a RefCell) -> Self { + Self::Scope(value) + } +} impl<'a> From<&'a mut Dynamic> for Target<'a> { fn from(value: &'a mut Dynamic) -> Self { - Self::Value(value) + Self::Ref(value) + } +} +impl> From for Target<'_> { + fn from(value: T) -> Self { + Self::Value(value.into()) } } @@ -386,85 +398,12 @@ fn search_scope<'a>( scope: &'a Scope, id: &str, begin: Position, -) -> Result<(ScopeSource<'a>, Dynamic), Box> { - scope +) -> Result<(&'a RefCell, ScopeEntryType), Box> { + let (entry, _) = scope .get(id) - .ok_or_else(|| Box::new(EvalAltResult::ErrorVariableNotFound(id.into(), begin))) -} + .ok_or_else(|| Box::new(EvalAltResult::ErrorVariableNotFound(id.into(), begin)))?; -/// Replace a character at an index position in a mutable string -fn str_replace_char(s: &mut String, idx: usize, new_ch: char) { - let mut chars: Vec = s.chars().collect(); - let ch = *chars.get(idx).expect("string index out of bounds"); - - // See if changed - if so, update the String - if ch != new_ch { - chars[idx] = new_ch; - s.clear(); - chars.iter().for_each(|&ch| s.push(ch)); - } -} - -/// Update the value at an index position -fn update_indexed_val( - mut target: Dynamic, - idx: IndexValue, - new_val: Dynamic, - pos: Position, -) -> Result> { - match target.get_mut() { - Union::Array(arr) => { - arr[idx.as_num()] = new_val; - } - Union::Map(map) => { - map.insert(idx.as_str(), new_val); - } - Union::Str(s) => { - // Value must be a character - let ch = new_val - .as_char() - .map_err(|_| EvalAltResult::ErrorCharMismatch(pos))?; - str_replace_char(s, idx.as_num(), ch); - } - // All other variable types should be an error - _ => panic!("invalid type for indexing: {}", target.type_name()), - } - - Ok(target) -} - -/// Update the value at an index position in a variable inside the scope -fn update_indexed_scope_var( - scope: &mut Scope, - src: ScopeSource, - idx: IndexValue, - new_val: Dynamic, - pos: Position, -) -> Result> { - let target = scope.get_mut(src); - - match target.get_mut() { - // array_id[idx] = val - Union::Array(arr) => { - arr[idx.as_num()] = new_val; - } - // map_id[idx] = val - Union::Map(map) => { - map.insert(idx.as_str(), new_val); - } - // string_id[idx] = val - Union::Str(s) => { - // Value must be a character - let ch = new_val - .as_char() - .map_err(|_| EvalAltResult::ErrorCharMismatch(pos))?; - str_replace_char(s, idx.as_num(), ch); - } - // All other variable types should be an error - _ => panic!("invalid type for indexing: {}", target.type_name()), - } - - Ok(().into()) + Ok((&scope.get_ref(entry), entry.typ)) } impl Engine { @@ -692,8 +631,7 @@ impl Engine { || fn_lib.map_or(false, |lib| lib.has_function(name, 1)) } - // Perform an actual function call, taking care of special functions such as `type_of` - // and property getter/setter for maps. + // Perform an actual function call, taking care of special functions fn exec_fn_call( &self, fn_lib: Option<&FunctionsLib>, @@ -717,27 +655,7 @@ impl Engine { ))) } - _ => { - // Map property access? - if let Some(prop) = extract_prop_from_getter(fn_name) { - if let Dynamic(Union::Map(map)) = args[0] { - return Ok(map.get(prop).cloned().unwrap_or_else(|| ().into())); - } - } - - // Map property update - if let Some(prop) = extract_prop_from_setter(fn_name) { - let (arg, value) = args.split_at_mut(1); - - if let Dynamic(Union::Map(map)) = arg[0] { - map.insert(prop.to_string(), value[0].clone()); - return Ok(().into()); - } - } - - // Normal function call - self.call_fn_raw(None, fn_lib, fn_name, args, def_val, pos, level) - } + _ => self.call_fn_raw(None, fn_lib, fn_name, args, def_val, pos, level), } } @@ -784,203 +702,334 @@ impl Engine { .map_err(|err| EvalAltResult::set_position(err, pos)) } - /// Chain-evaluate a dot setter. - fn dot_get_helper( + /// Chain-evaluate a dot/index chain. + fn eval_dot_index_chain_helper( &self, - scope: &mut Scope, fn_lib: Option<&FunctionsLib>, - target: Target, - dot_rhs: &Expr, + mut target: Target, + rhs: &Expr, + idx_list: &mut [Dynamic], + idx_more: &mut Vec, + is_index: bool, + op_pos: Position, level: usize, - ) -> Result> { - match dot_rhs { - // xxx.fn_name(arg_expr_list) - Expr::FunctionCall(fn_name, arg_exprs, def_val, pos) => { - let mut arg_values = arg_exprs - .iter() - .map(|arg_expr| self.eval_expr(scope, fn_lib, arg_expr, level)) - .collect::, _>>()?; - let mut args: Vec<_> = once(target.get_mut(scope)) - .chain(arg_values.iter_mut()) - .collect(); - let def_val = def_val.as_ref(); - self.exec_fn_call(fn_lib, fn_name, &mut args, def_val, *pos, 0) + mut new_val: Option, + ) -> Result<(Dynamic, bool), Box> { + // Store a copy of the RefMut from `borrow_mut` since it is a temporary value + let mut scope_base = match target { + Target::Scope(r) => Some(r.borrow_mut()), + Target::Ref(_) | Target::Value(_) | Target::StringChar(_) => None, + }; + // Get a reference to the mutation target Dynamic + let obj = match target { + Target::Scope(_) => scope_base.as_mut().unwrap().deref_mut(), + Target::Ref(r) => r, + Target::Value(ref mut r) => r, + Target::StringChar(ref mut x) => &mut x.2, + }; + + // Pop the last index value + let mut idx_val; + let mut idx_fixed = idx_list; + + if let Some(val) = idx_more.pop() { + // Values in variable list + idx_val = val; + } else { + // No more value in variable list, pop from fixed list + let len = idx_fixed.len(); + let splits = idx_fixed.split_at_mut(len - 1); + + idx_val = mem::replace(splits.1.get_mut(0).unwrap(), ().into()); + idx_fixed = splits.0; + } + + if is_index { + match rhs { + // xxx[idx].dot_rhs... + Expr::Dot(idx, idx_rhs, pos) | + // xxx[idx][dot_rhs]... + Expr::Index(idx, idx_rhs, pos) => { + let is_index = matches!(rhs, Expr::Index(_,_,_)); + + let indexed_val = self.get_indexed_mut(obj, idx_val, idx.position(), op_pos, false)?; + self.eval_dot_index_chain_helper( + fn_lib, indexed_val, idx_rhs.as_ref(), idx_fixed, idx_more, is_index, *pos, level, new_val + ) + } + // xxx[rhs] = new_val + _ if new_val.is_some() => { + let mut indexed_val = self.get_indexed_mut(obj, idx_val, rhs.position(), op_pos, true)?; + indexed_val.set_value(new_val.unwrap(), rhs.position())?; + Ok((().into(), true)) + } + // xxx[rhs] + _ => self + .get_indexed_mut(obj, idx_val, rhs.position(), op_pos, false) + .map(|v| (v.into_dynamic(), false)) } - - // xxx.id - Expr::Property(id, pos) => { - let fn_name = make_getter(id); - let mut args = [target.get_mut(scope)]; - self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0) - } - - // xxx.idx_lhs[idx_expr] - Expr::Index(idx_lhs, idx_expr, op_pos) => { - let lhs_value = match idx_lhs.as_ref() { - // xxx.id[idx_expr] - Expr::Property(id, pos) => { - let fn_name = make_getter(id); - let mut args = [target.get_mut(scope)]; - self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0)? + } else { + match rhs { + // xxx.fn_name(arg_expr_list) + Expr::FunctionCall(fn_name, _, def_val, pos) => { + let mut args = once(obj) + .chain(idx_val.downcast_mut::().unwrap().iter_mut()) + .collect::>(); + let def_val = def_val.as_ref(); + // A function call is assumed to have side effects, so the value is changed + self.exec_fn_call(fn_lib, fn_name, &mut args, def_val, *pos, 0).map(|v| (v, true)) + } + // {xxx:map}.id + Expr::Property(id, pos) if obj.is::() => { + let mut indexed_val = + self.get_indexed_mut(obj, id.to_string().into(), *pos, op_pos, new_val.is_some())?; + if let Some(new_val) = new_val { + indexed_val.set_value(new_val, rhs.position())?; + Ok((().into(), true)) + } else { + Ok((indexed_val.into_dynamic(), false)) } - // xxx.???[???][idx_expr] - Expr::Index(_, _, _) => { - // Chain the indexing - self.dot_get_helper(scope, fn_lib, target, idx_lhs, level)? - } - // Syntax error - _ => { - return Err(Box::new(EvalAltResult::ErrorDotExpr( - "".to_string(), - dot_rhs.position(), - ))) - } - }; - - self.get_indexed_val(scope, fn_lib, &lhs_value, idx_expr, *op_pos, level, false) - .map(|(val, _)| val) - } - - // xxx.dot_lhs.rhs - Expr::Dot(dot_lhs, rhs, _) => match dot_lhs.as_ref() { - // xxx.id.rhs + } + // xxx.id = ??? + Expr::Property(id, pos) if new_val.is_some() => { + let fn_name = make_setter(id); + let mut args = [obj, new_val.as_mut().unwrap()]; + self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0).map(|v| (v, true)) + } + // xxx.id Expr::Property(id, pos) => { let fn_name = make_getter(id); - let mut args = [target.get_mut(scope)]; - self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0) - .and_then(|mut val| { - self.dot_get_helper(scope, fn_lib, (&mut val).into(), rhs, level) - }) + let mut args = [obj]; + self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0).map(|v| (v, false)) } - // xxx.idx_lhs[idx_expr].rhs - Expr::Index(idx_lhs, idx_expr, op_pos) => { - let val = match idx_lhs.as_ref() { - // xxx.id[idx_expr].rhs - Expr::Property(id, pos) => { - let fn_name = make_getter(id); - let mut args = [target.get_mut(scope)]; - self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0)? - } - // xxx.???[???][idx_expr].rhs - Expr::Index(_, _, _) => { - self.dot_get_helper(scope, fn_lib, target, idx_lhs, level)? - } - // Syntax error - _ => { - return Err(Box::new(EvalAltResult::ErrorDotExpr( - "".to_string(), - dot_rhs.position(), - ))) - } - }; + // {xxx:map}.idx_lhs[idx_expr] + Expr::Index(dot_lhs, dot_rhs, pos) | + // {xxx:map}.dot_lhs.rhs + Expr::Dot(dot_lhs, dot_rhs, pos) if obj.is::() => { + let is_index = matches!(rhs, Expr::Index(_,_,_)); - self.get_indexed_val(scope, fn_lib, &val, idx_expr, *op_pos, level, false) - .and_then(|(mut val, _)| { - self.dot_get_helper(scope, fn_lib, (&mut val).into(), rhs, level) - }) + let indexed_val = if let Expr::Property(id, pos) = dot_lhs.as_ref() { + self.get_indexed_mut(obj, id.to_string().into(), *pos, op_pos, false)? + } else { + // Syntax error + return Err(Box::new(EvalAltResult::ErrorDotExpr( + "".to_string(), + rhs.position(), + ))); + }; + self.eval_dot_index_chain_helper( + fn_lib, indexed_val, dot_rhs, idx_fixed, idx_more, is_index, *pos, level, new_val + ) + } + // xxx.idx_lhs[idx_expr] + Expr::Index(dot_lhs, dot_rhs, pos) | + // xxx.dot_lhs.rhs + Expr::Dot(dot_lhs, dot_rhs, pos) => { + let is_index = matches!(rhs, Expr::Index(_,_,_)); + let mut buf: Dynamic = ().into(); + let mut args = [obj, &mut buf]; + + let mut indexed_val = if let Expr::Property(id, pos) = dot_lhs.as_ref() { + let fn_name = make_getter(id); + self.exec_fn_call(fn_lib, &fn_name, &mut args[..1], None, *pos, 0)? + } else { + // Syntax error + return Err(Box::new(EvalAltResult::ErrorDotExpr( + "".to_string(), + rhs.position(), + ))); + }; + let (result, changed) = self.eval_dot_index_chain_helper( + fn_lib, (&mut indexed_val).into(), dot_rhs, idx_fixed, idx_more, is_index, *pos, level, new_val + )?; + + // Feed the value back via a setter just in case it has been updated + if changed { + if let Expr::Property(id, pos) = dot_lhs.as_ref() { + let fn_name = make_setter(id); + args[1] = &mut indexed_val; + self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0)?; + } + } + + Ok((result, changed)) } // Syntax error _ => Err(Box::new(EvalAltResult::ErrorDotExpr( "".to_string(), - dot_lhs.position(), + rhs.position(), ))), - }, - - // Syntax error - _ => Err(Box::new(EvalAltResult::ErrorDotExpr( - "".to_string(), - dot_rhs.position(), - ))), + } } } - /// Evaluate a dot chain getter - fn dot_get( + /// Evaluate a dot/index chain + fn eval_dot_index_chain( &self, scope: &mut Scope, fn_lib: Option<&FunctionsLib>, dot_lhs: &Expr, dot_rhs: &Expr, + is_index: bool, + op_pos: Position, level: usize, + new_val: Option, ) -> Result> { + // Keep four levels of index values in fixed array to reduce allocations + let mut idx_list: [Dynamic; 4] = [().into(), ().into(), ().into(), ().into()]; + // Spill over additional levels into a variable list + let idx_more: &mut Vec = &mut Vec::new(); + + let size = + self.eval_indexed_chain(scope, fn_lib, dot_rhs, &mut idx_list, idx_more, 0, level)?; + + let idx_list = if size < idx_list.len() { + &mut idx_list[0..size] + } else { + &mut idx_list + }; + match dot_lhs { - // id.??? + // id.??? or id[???] Expr::Variable(id, pos) => { - let (entry, _) = search_scope(scope, id, *pos)?; + let (target, typ) = search_scope(scope, id, *pos)?; - // Avoid referencing scope which is used below as mut - let entry = ScopeSource { name: id, ..entry }; - - // This is a variable property access (potential function call). - // Use a direct index into `scope` to directly mutate the variable value. - self.dot_get_helper(scope, fn_lib, entry.into(), dot_rhs, level) - } - - // idx_lhs[idx_expr].??? - Expr::Index(idx_lhs, idx_expr, op_pos) => { - let (src, index, mut val) = - self.eval_index_expr(scope, fn_lib, idx_lhs, idx_expr, *op_pos, level)?; - let value = self.dot_get_helper(scope, fn_lib, (&mut val).into(), dot_rhs, level); - - // In case the expression mutated `target`, we need to update it back into the scope because it is cloned. - if let Some(src) = src { - match src.typ { - ScopeEntryType::Constant => { - return Err(Box::new(EvalAltResult::ErrorAssignmentToConstant( - src.name.to_string(), - idx_lhs.position(), - ))); - } - - ScopeEntryType::Normal => { - update_indexed_scope_var(scope, src, index, val, dot_rhs.position())?; - } + // Constants cannot be modified + match typ { + ScopeEntryType::Constant if new_val.is_some() => { + return Err(Box::new(EvalAltResult::ErrorAssignmentToConstant( + id.to_string(), + *pos, + ))); } + _ => (), } - value + self.eval_dot_index_chain_helper( + fn_lib, + target.into(), + dot_rhs, + idx_list, + idx_more, + is_index, + op_pos, + level, + new_val, + ) + .map(|(v, _)| v) } - - // {expr}.??? + // {expr}.??? = ??? or {expr}[???] = ??? + expr if new_val.is_some() => { + return Err(Box::new(EvalAltResult::ErrorAssignmentToUnknownLHS( + expr.position(), + ))); + } + // {expr}.??? or {expr}[???] expr => { - let mut val = self.eval_expr(scope, fn_lib, expr, level)?; - self.dot_get_helper(scope, fn_lib, (&mut val).into(), dot_rhs, level) + let val = self.eval_expr(scope, fn_lib, expr, level)?; + + self.eval_dot_index_chain_helper( + fn_lib, + val.into(), + dot_rhs, + idx_list, + idx_more, + is_index, + op_pos, + level, + new_val, + ) + .map(|(v, _)| v) } } } - /// Get the value at the indexed position of a base type - fn get_indexed_val( + fn eval_indexed_chain( &self, scope: &mut Scope, fn_lib: Option<&FunctionsLib>, - val: &Dynamic, - idx_expr: &Expr, - op_pos: Position, + expr: &Expr, + list: &mut [Dynamic], + more: &mut Vec, + size: usize, level: usize, - only_index: bool, - ) -> Result<(Dynamic, IndexValue), Box> { - let idx_pos = idx_expr.position(); + ) -> Result> { + let size = match expr { + Expr::FunctionCall(_, arg_exprs, _, _) => { + let arg_values = arg_exprs + .iter() + .map(|arg_expr| self.eval_expr(scope, fn_lib, arg_expr, level)) + .collect::, _>>()?; + + if size < list.len() { + list[size] = arg_values.into(); + } else { + more.push(arg_values.into()); + } + size + 1 + } + Expr::Property(_, _) => { + // Placeholder + if size < list.len() { + list[size] = ().into(); + } else { + more.push(().into()); + } + size + 1 + } + Expr::Index(lhs, rhs, _) | Expr::Dot(lhs, rhs, _) => { + // Evaluate in left-to-right order + let lhs_val = match lhs.as_ref() { + Expr::Property(_, _) => ().into(), // Placeholder + _ => self.eval_expr(scope, fn_lib, lhs, level)?, + }; + + // Push in reverse order + let size = self.eval_indexed_chain(scope, fn_lib, rhs, list, more, size, level)?; + + if size < list.len() { + list[size] = lhs_val; + } else { + more.push(lhs_val); + } + size + 1 + } + _ => { + let val = self.eval_expr(scope, fn_lib, expr, level)?; + if size < list.len() { + list[size] = val; + } else { + more.push(val); + } + size + 1 + } + }; + Ok(size) + } + + /// Get the value at the indexed position of a base type + fn get_indexed_mut<'a>( + &self, + val: &'a mut Dynamic, + idx: Dynamic, + idx_pos: Position, + op_pos: Position, + create: bool, + ) -> Result, Box> { let type_name = self.map_type_name(val.type_name()); - match val.get_ref() { - Union::Array(arr) => { + match val { + Dynamic(Union::Array(arr)) => { // val_array[idx] - let index = self - .eval_expr(scope, fn_lib, idx_expr, level)? + let index = idx .as_int() - .map_err(|_| EvalAltResult::ErrorNumericIndexExpr(idx_expr.position()))?; + .map_err(|_| EvalAltResult::ErrorNumericIndexExpr(idx_pos))?; let arr_len = arr.len(); if index >= 0 { - arr.get(index as usize) - .map(|v| { - ( - if only_index { ().into() } else { v.clone() }, - IndexValue::from_num(index), - ) - }) + arr.get_mut(index as usize) + .map(Target::from) .ok_or_else(|| { Box::new(EvalAltResult::ErrorArrayBounds(arr_len, index, idx_pos)) }) @@ -991,37 +1040,31 @@ impl Engine { } } - Union::Map(map) => { + Dynamic(Union::Map(map)) => { // val_map[idx] - let index = self - .eval_expr(scope, fn_lib, idx_expr, level)? + let index = idx .take_string() - .map_err(|_| EvalAltResult::ErrorStringIndexExpr(idx_expr.position()))?; + .map_err(|_| EvalAltResult::ErrorStringIndexExpr(idx_pos))?; - Ok(( - map.get(&index) - .map(|v| if only_index { ().into() } else { v.clone() }) - .unwrap_or_else(|| ().into()), - IndexValue::from_str(index), - )) + Ok(if create { + map.entry(index).or_insert(().into()).into() + } else { + map.get_mut(&index).map(Target::from).unwrap_or_else(|| Target::from(())) + }) } - Union::Str(s) => { + Dynamic(Union::Str(s)) => { // val_string[idx] - let index = self - .eval_expr(scope, fn_lib, idx_expr, level)? + let index = idx .as_int() - .map_err(|_| EvalAltResult::ErrorNumericIndexExpr(idx_expr.position()))?; + .map_err(|_| EvalAltResult::ErrorNumericIndexExpr(idx_pos))?; let num_chars = s.chars().count(); if index >= 0 { - s.chars() - .nth(index as usize) - .map(|ch| (ch.into(), IndexValue::from_num(index))) - .ok_or_else(|| { - Box::new(EvalAltResult::ErrorStringBounds(num_chars, index, idx_pos)) - }) + let index = index as usize; + let ch = s.chars().nth(index).unwrap(); + Ok(Target::StringChar(Box::new((val, index, ch.into())))) } else { Err(Box::new(EvalAltResult::ErrorStringBounds( num_chars, index, idx_pos, @@ -1037,221 +1080,6 @@ impl Engine { } } - /// Evaluate an index expression - fn eval_index_expr<'a>( - &self, - scope: &mut Scope, - fn_lib: Option<&FunctionsLib>, - lhs: &'a Expr, - idx_expr: &Expr, - op_pos: Position, - level: usize, - ) -> Result<(Option>, IndexValue, Dynamic), Box> { - match lhs { - // id[idx_expr] - Expr::Variable(name, _) => { - let (ScopeSource { typ, index, .. }, val) = - search_scope(scope, &name, lhs.position())?; - let (val, idx) = - self.get_indexed_val(scope, fn_lib, &val, idx_expr, op_pos, level, false)?; - - Ok((Some(ScopeSource { name, typ, index }), idx, val)) - } - - // (expr)[idx_expr] - expr => { - let val = self.eval_expr(scope, fn_lib, expr, level)?; - self.get_indexed_val(scope, fn_lib, &val, idx_expr, op_pos, level, false) - .map(|(val, index)| (None, index, val)) - } - } - } - - /// Chain-evaluate a dot setter - fn dot_set_helper( - &self, - scope: &mut Scope, - fn_lib: Option<&FunctionsLib>, - this_ptr: &mut Dynamic, - dot_rhs: &Expr, - new_val: &mut Dynamic, - val_pos: Position, - level: usize, - ) -> Result> { - match dot_rhs { - // xxx.id - Expr::Property(id, pos) => { - let mut args = [this_ptr, new_val]; - self.exec_fn_call(fn_lib, &make_setter(id), &mut args, None, *pos, 0) - } - - // xxx.lhs[idx_expr] - // TODO - Allow chaining of indexing! - Expr::Index(lhs, idx_expr, op_pos) => match lhs.as_ref() { - // xxx.id[idx_expr] - Expr::Property(id, pos) => { - let fn_name = make_getter(id); - self.exec_fn_call(fn_lib, &fn_name, &mut [this_ptr], None, *pos, 0) - .and_then(|val| { - let (_, index) = self.get_indexed_val( - scope, fn_lib, &val, idx_expr, *op_pos, level, true, - )?; - - update_indexed_val(val, index, new_val.clone(), val_pos) - }) - .and_then(|mut val| { - let fn_name = make_setter(id); - let mut args = [this_ptr, &mut val]; - self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0) - }) - } - - // All others - syntax error for setters chain - _ => Err(Box::new(EvalAltResult::ErrorDotExpr( - "for assignment".to_string(), - *op_pos, - ))), - }, - - // xxx.lhs.{...} - Expr::Dot(lhs, rhs, _) => match lhs.as_ref() { - // xxx.id.rhs - Expr::Property(id, pos) => { - let fn_name = make_getter(id); - self.exec_fn_call(fn_lib, &fn_name, &mut [this_ptr], None, *pos, 0) - .and_then(|mut val| { - self.dot_set_helper( - scope, fn_lib, &mut val, rhs, new_val, val_pos, level, - )?; - Ok(val) - }) - .and_then(|mut val| { - let fn_name = make_setter(id); - let mut args = [this_ptr, &mut val]; - self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0) - }) - } - - // xxx.lhs[idx_expr].rhs - // TODO - Allow chaining of indexing! - Expr::Index(lhs, idx_expr, op_pos) => match lhs.as_ref() { - // xxx.id[idx_expr].rhs - Expr::Property(id, pos) => { - let fn_name = make_getter(id); - self.exec_fn_call(fn_lib, &fn_name, &mut [this_ptr], None, *pos, 0) - .and_then(|v| { - let (mut value, index) = self.get_indexed_val( - scope, fn_lib, &v, idx_expr, *op_pos, level, false, - )?; - - self.dot_set_helper( - scope, fn_lib, &mut value, rhs, new_val, val_pos, level, - )?; - - // In case the expression mutated `target`, we need to update it back into the scope because it is cloned. - update_indexed_val(v, index, value, val_pos) - }) - .and_then(|mut v| { - let fn_name = make_setter(id); - let mut args = [this_ptr, &mut v]; - self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0) - }) - } - - // All others - syntax error for setters chain - _ => Err(Box::new(EvalAltResult::ErrorDotExpr( - "for assignment".to_string(), - *op_pos, - ))), - }, - - // All others - syntax error for setters chain - _ => Err(Box::new(EvalAltResult::ErrorDotExpr( - "for assignment".to_string(), - lhs.position(), - ))), - }, - - // Syntax error - _ => Err(Box::new(EvalAltResult::ErrorDotExpr( - "for assignment".to_string(), - dot_rhs.position(), - ))), - } - } - - // Evaluate a dot chain setter - fn dot_set( - &self, - scope: &mut Scope, - fn_lib: Option<&FunctionsLib>, - dot_lhs: &Expr, - dot_rhs: &Expr, - new_val: &mut Dynamic, - val_pos: Position, - op_pos: Position, - level: usize, - ) -> Result> { - match dot_lhs { - // id.??? - Expr::Variable(id, pos) => { - let (src, mut target) = search_scope(scope, id, *pos)?; - - match src.typ { - ScopeEntryType::Constant => Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant(id.to_string(), op_pos), - )), - _ => { - // Avoid referencing scope which is used below as mut - let entry = ScopeSource { name: id, ..src }; - let this_ptr = &mut target; - let value = self.dot_set_helper( - scope, fn_lib, this_ptr, dot_rhs, new_val, val_pos, level, - ); - - // In case the expression mutated `target`, we need to update it back into the scope because it is cloned. - *scope.get_mut(entry) = target; - - value - } - } - } - - // lhs[idx_expr].??? - // TODO - Allow chaining of indexing! - Expr::Index(lhs, idx_expr, op_pos) => { - let (src, index, mut target) = - self.eval_index_expr(scope, fn_lib, lhs, idx_expr, *op_pos, level)?; - let this_ptr = &mut target; - let value = - self.dot_set_helper(scope, fn_lib, this_ptr, dot_rhs, new_val, val_pos, level); - - // In case the expression mutated `target`, we need to update it back into the scope because it is cloned. - if let Some(src) = src { - match src.typ { - ScopeEntryType::Constant => { - return Err(Box::new(EvalAltResult::ErrorAssignmentToConstant( - src.name.to_string(), - lhs.position(), - ))); - } - ScopeEntryType::Normal => { - update_indexed_scope_var(scope, src, index, target, val_pos)?; - } - } - } - - value - } - - // Syntax error - _ => Err(Box::new(EvalAltResult::ErrorDotExpr( - "for assignment".to_string(), - dot_lhs.position(), - ))), - } - } - // Evaluate an 'in' expression fn eval_in_expr( &self, @@ -1319,7 +1147,9 @@ impl Engine { Expr::FloatConstant(f, _) => Ok((*f).into()), Expr::StringConstant(s, _) => Ok(s.to_string().into()), Expr::CharConstant(c, _) => Ok((*c).into()), - Expr::Variable(id, pos) => search_scope(scope, id, *pos).map(|(_, val)| val), + Expr::Variable(id, pos) => { + search_scope(scope, id, *pos).map(|(v, _)| v.borrow().clone()) + } Expr::Property(_, _) => panic!("unexpected property."), // Statement block @@ -1327,7 +1157,7 @@ impl Engine { // lhs = rhs Expr::Assignment(lhs, rhs, op_pos) => { - let mut rhs_val = self.eval_expr(scope, fn_lib, rhs, level)?; + let rhs_val = self.eval_expr(scope, fn_lib, rhs, level)?; match lhs.as_ref() { // name = rhs @@ -1350,7 +1180,7 @@ impl Engine { )) => { // Avoid referencing scope which is used below as mut let entry = ScopeSource { name, ..entry }; - *scope.get_mut(entry) = rhs_val.clone(); + *scope.get_ref(entry).borrow_mut() = rhs_val.clone(); Ok(rhs_val) } @@ -1369,39 +1199,19 @@ impl Engine { // idx_lhs[idx_expr] = rhs #[cfg(not(feature = "no_index"))] Expr::Index(idx_lhs, idx_expr, op_pos) => { - let (src, index, _) = - self.eval_index_expr(scope, fn_lib, idx_lhs, idx_expr, *op_pos, level)?; - - if let Some(src) = src { - match src.typ { - ScopeEntryType::Constant => { - Err(Box::new(EvalAltResult::ErrorAssignmentToConstant( - src.name.to_string(), - idx_lhs.position(), - ))) - } - ScopeEntryType::Normal => { - let pos = rhs.position(); - Ok(update_indexed_scope_var(scope, src, index, rhs_val, pos)?) - } - } - } else { - Err(Box::new(EvalAltResult::ErrorAssignmentToUnknownLHS( - idx_lhs.position(), - ))) - } + let new_val = Some(rhs_val); + self.eval_dot_index_chain( + scope, fn_lib, idx_lhs, idx_expr, true, *op_pos, level, new_val, + ) } - // dot_lhs.dot_rhs = rhs #[cfg(not(feature = "no_object"))] Expr::Dot(dot_lhs, dot_rhs, _) => { - let new_val = &mut rhs_val; - let val_pos = rhs.position(); - self.dot_set( - scope, fn_lib, dot_lhs, dot_rhs, new_val, val_pos, *op_pos, level, + let new_val = Some(rhs_val); + self.eval_dot_index_chain( + scope, fn_lib, dot_lhs, dot_rhs, false, *op_pos, level, new_val, ) } - // Error assignment to constant expr if expr.is_constant() => { Err(Box::new(EvalAltResult::ErrorAssignmentToConstant( @@ -1419,12 +1229,15 @@ impl Engine { // lhs[idx_expr] #[cfg(not(feature = "no_index"))] - Expr::Index(lhs, idx_expr, op_pos) => self - .eval_index_expr(scope, fn_lib, lhs, idx_expr, *op_pos, level) - .map(|(_, _, x)| x), + Expr::Index(lhs, idx_expr, op_pos) => { + self.eval_dot_index_chain(scope, fn_lib, lhs, idx_expr, true, *op_pos, level, None) + } + // lhs.dot_rhs #[cfg(not(feature = "no_object"))] - Expr::Dot(lhs, rhs, _) => self.dot_get(scope, fn_lib, lhs, rhs, level), + Expr::Dot(lhs, dot_rhs, op_pos) => { + self.eval_dot_index_chain(scope, fn_lib, lhs, dot_rhs, false, *op_pos, level, None) + } #[cfg(not(feature = "no_index"))] Expr::Array(contents, _) => { @@ -1618,7 +1431,7 @@ impl Engine { }; for a in iter_fn(arr) { - *scope.get_mut(entry) = a; + *scope.get_ref(entry).borrow_mut() = a; match self.eval_stmt(scope, fn_lib, body, level) { Ok(_) => (), diff --git a/src/error.rs b/src/error.rs index d87c3722..b08e5975 100644 --- a/src/error.rs +++ b/src/error.rs @@ -100,8 +100,6 @@ pub enum ParseErrorType { FnMissingBody(String), /// Assignment to an inappropriate LHS (left-hand-side) expression. AssignmentToInvalidLHS, - /// Assignment to a copy of a value. - AssignmentToCopy, /// Assignment to an a constant variable. AssignmentToConstant(String), /// Break statement not inside a loop. @@ -150,7 +148,6 @@ impl ParseError { ParseErrorType::FnMissingBody(_) => "Expecting body statement block for function declaration", ParseErrorType::WrongFnDefinition => "Function definitions must be at global level and cannot be inside a block or another function", ParseErrorType::AssignmentToInvalidLHS => "Cannot assign to this expression", - ParseErrorType::AssignmentToCopy => "Cannot assign to this expression because it will only be changing a copy of the value", ParseErrorType::AssignmentToConstant(_) => "Cannot assign to a constant variable.", ParseErrorType::LoopBreak => "Break statement should only be used inside a loop" } diff --git a/src/parser.rs b/src/parser.rs index 15343778..a9f79250 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -476,34 +476,41 @@ impl Expr { /// Is a particular token allowed as a postfix operator to this expression? pub fn is_valid_postfix(&self, token: &Token) -> bool { match self { - Expr::IntegerConstant(_, _) - | Expr::FloatConstant(_, _) - | Expr::CharConstant(_, _) - | Expr::In(_, _, _) - | Expr::And(_, _, _) - | Expr::Or(_, _, _) - | Expr::True(_) - | Expr::False(_) - | Expr::Unit(_) => false, + Self::IntegerConstant(_, _) + | Self::FloatConstant(_, _) + | Self::CharConstant(_, _) + | Self::In(_, _, _) + | Self::And(_, _, _) + | Self::Or(_, _, _) + | Self::True(_) + | Self::False(_) + | Self::Unit(_) => false, - Expr::StringConstant(_, _) - | Expr::Stmt(_, _) - | Expr::FunctionCall(_, _, _, _) - | Expr::Assignment(_, _, _) - | Expr::Dot(_, _, _) - | Expr::Index(_, _, _) - | Expr::Array(_, _) - | Expr::Map(_, _) => match token { + Self::StringConstant(_, _) + | Self::Stmt(_, _) + | Self::FunctionCall(_, _, _, _) + | Self::Assignment(_, _, _) + | Self::Dot(_, _, _) + | Self::Index(_, _, _) + | Self::Array(_, _) + | Self::Map(_, _) => match token { Token::LeftBracket => true, _ => false, }, - Expr::Variable(_, _) | Expr::Property(_, _) => match token { + Self::Variable(_, _) | Self::Property(_, _) => match token { Token::LeftBracket | Token::LeftParen => true, _ => false, }, } } + + pub(crate) fn into_property(self) -> Self { + match self { + Self::Variable(id, pos) => Self::Property(id, pos), + _ => self, + } + } } /// Consume a particular token, checking that it is the expected one. @@ -734,7 +741,17 @@ fn parse_index_expr<'a>( match input.peek().unwrap() { (Token::RightBracket, _) => { eat_token(input, Token::RightBracket); - Ok(Expr::Index(lhs, Box::new(idx_expr), pos)) + + let idx_expr = Box::new(idx_expr); + + match input.peek().unwrap() { + (Token::LeftBracket, _) => { + let follow_pos = eat_token(input, Token::LeftBracket); + let follow = parse_index_expr(idx_expr, input, follow_pos, allow_stmt_expr)?; + Ok(Expr::Index(lhs, Box::new(follow), pos)) + } + _ => Ok(Expr::Index(lhs, idx_expr, pos)), + } } (Token::LexError(err), pos) => return Err(PERR::BadInput(err.to_string()).into_err(*pos)), (_, pos) => Err(PERR::MissingToken( @@ -1005,71 +1022,6 @@ fn parse_unary<'a>( } } -/// Parse an assignment. -fn parse_assignment(lhs: Expr, rhs: Expr, pos: Position) -> Result> { - // Is the LHS in a valid format for an assignment target? - fn valid_assignment_chain(expr: &Expr, is_top: bool) -> Option> { - match expr { - // var - Expr::Variable(_, _) => { - assert!(is_top, "property expected but gets variable"); - None - } - // property - Expr::Property(_, _) => { - assert!(!is_top, "variable expected but gets property"); - None - } - - // idx_lhs[...] - Expr::Index(idx_lhs, _, pos) => match idx_lhs.as_ref() { - // var[...] - Expr::Variable(_, _) => { - assert!(is_top, "property expected but gets variable"); - None - } - // property[...] - Expr::Property(_, _) => { - assert!(!is_top, "variable expected but gets property"); - None - } - // ???[...][...] - Expr::Index(_, _, _) => Some(ParseErrorType::AssignmentToCopy.into_err(*pos)), - // idx_lhs[...] - _ => Some(ParseErrorType::AssignmentToInvalidLHS.into_err(*pos)), - }, - - // dot_lhs.dot_rhs - Expr::Dot(dot_lhs, dot_rhs, pos) => match dot_lhs.as_ref() { - // var.dot_rhs - Expr::Variable(_, _) if is_top => valid_assignment_chain(dot_rhs, false), - // property.dot_rhs - Expr::Property(_, _) if !is_top => valid_assignment_chain(dot_rhs, false), - // idx_lhs[...].dot_rhs - Expr::Index(idx_lhs, _, _) => match idx_lhs.as_ref() { - // var[...].dot_rhs - Expr::Variable(_, _) if is_top => valid_assignment_chain(dot_rhs, false), - // property[...].dot_rhs - Expr::Property(_, _) if !is_top => valid_assignment_chain(dot_rhs, false), - // ???[...][...].dot_rhs - Expr::Index(_, _, _) => Some(ParseErrorType::AssignmentToCopy.into_err(*pos)), - // idx_lhs[...].dot_rhs - _ => Some(ParseErrorType::AssignmentToCopy.into_err(idx_lhs.position())), - }, - - expr => panic!("unexpected dot expression {:#?}", expr), - }, - - _ => Some(ParseErrorType::AssignmentToInvalidLHS.into_err(expr.position())), - } - } - - match valid_assignment_chain(&lhs, true) { - None => Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs), pos)), - Some(err) => Err(err), - } -} - fn parse_assignment_stmt<'a>( input: &mut Peekable>, lhs: Expr, @@ -1077,7 +1029,7 @@ fn parse_assignment_stmt<'a>( ) -> Result> { let pos = eat_token(input, Token::Equals); let rhs = parse_expr(input, allow_stmt_expr)?; - parse_assignment(lhs, rhs, pos) + Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs), pos)) } /// Parse an operator-assignment expression. @@ -1108,15 +1060,51 @@ fn parse_op_assignment_stmt<'a>( let rhs = parse_expr(input, allow_stmt_expr)?; // lhs op= rhs -> lhs = op(lhs, rhs) - parse_assignment( - lhs, - Expr::FunctionCall(op.into(), vec![lhs_copy, rhs], None, pos), - pos, - ) + let rhs_expr = Expr::FunctionCall(op.into(), vec![lhs_copy, rhs], None, pos); + Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs_expr), pos)) } -/// Parse an 'in' expression. -fn parse_in_expr(lhs: Expr, rhs: Expr, op_pos: Position) -> Result> { +/// Make a dot expression. +fn make_dot_expr(lhs: Expr, rhs: Expr, op_pos: Position, is_index: bool) -> Expr { + match (lhs, rhs) { + // idx_lhs[idx_rhs].rhs + (Expr::Index(idx_lhs, idx_rhs, idx_pos), rhs) => Expr::Index( + idx_lhs, + Box::new(make_dot_expr(*idx_rhs, rhs, op_pos, true)), + idx_pos, + ), + // lhs.id + (lhs, rhs @ Expr::Variable(_, _)) | (lhs, rhs @ Expr::Property(_, _)) => { + let lhs = if is_index { lhs.into_property() } else { lhs }; + Expr::Dot(Box::new(lhs), Box::new(rhs.into_property()), op_pos) + } + // lhs.dot_lhs.dot_rhs + (lhs, Expr::Dot(dot_lhs, dot_rhs, dot_pos)) => Expr::Dot( + Box::new(lhs), + Box::new(Expr::Dot( + Box::new(dot_lhs.into_property()), + Box::new(dot_rhs.into_property()), + dot_pos, + )), + op_pos, + ), + // lhs.idx_lhs[idx_rhs] + (lhs, Expr::Index(idx_lhs, idx_rhs, idx_pos)) => Expr::Dot( + Box::new(lhs), + Box::new(Expr::Index( + Box::new(idx_lhs.into_property()), + Box::new(idx_rhs.into_property()), + idx_pos, + )), + op_pos, + ), + // lhs.rhs + (lhs, rhs) => Expr::Dot(Box::new(lhs), Box::new(rhs.into_property()), op_pos), + } +} + +/// Make an 'in' expression. +fn make_in_expr(lhs: Expr, rhs: Expr, op_pos: Position) -> Result> { match (&lhs, &rhs) { (_, Expr::IntegerConstant(_, pos)) | (_, Expr::FloatConstant(_, pos)) @@ -1321,34 +1309,10 @@ fn parse_binary_op<'a>( Token::Pipe => Expr::FunctionCall("|".into(), vec![current_lhs, rhs], None, pos), Token::XOr => Expr::FunctionCall("^".into(), vec![current_lhs, rhs], None, pos), - Token::In => parse_in_expr(current_lhs, rhs, pos)?, + Token::In => make_in_expr(current_lhs, rhs, pos)?, #[cfg(not(feature = "no_object"))] - Token::Period => { - fn check_property(expr: Expr) -> Result> { - match expr { - // xxx.lhs.rhs - Expr::Dot(lhs, rhs, pos) => Ok(Expr::Dot( - Box::new(check_property(*lhs)?), - Box::new(check_property(*rhs)?), - pos, - )), - // xxx.lhs[idx] - Expr::Index(lhs, idx, pos) => { - Ok(Expr::Index(Box::new(check_property(*lhs)?), idx, pos)) - } - // xxx.id - Expr::Variable(id, pos) => Ok(Expr::Property(id, pos)), - // xxx.prop - expr @ Expr::Property(_, _) => Ok(expr), - // xxx.fn() - expr @ Expr::FunctionCall(_, _, _, _) => Ok(expr), - expr => Err(PERR::PropertyExpected.into_err(expr.position())), - } - } - - Expr::Dot(Box::new(current_lhs), Box::new(check_property(rhs)?), pos) - } + Token::Period => make_dot_expr(current_lhs, rhs, pos, false), token => return Err(PERR::UnknownOperator(token.syntax().into()).into_err(pos)), }; diff --git a/src/scope.rs b/src/scope.rs index 6abfb40f..6eeff2bd 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -4,7 +4,7 @@ use crate::any::{Dynamic, Variant}; use crate::parser::{map_dynamic_to_expr, Expr}; use crate::token::Position; -use crate::stdlib::{borrow::Cow, iter, vec::Vec}; +use crate::stdlib::{borrow::Cow, cell::RefCell, iter, vec::Vec}; /// Type of an entry in the Scope. #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -23,7 +23,7 @@ pub struct Entry<'a> { /// Type of the entry. pub typ: EntryType, /// Current value of the entry. - pub value: Dynamic, + pub value: RefCell, /// A constant expression if the initial value matches one of the recognized types. pub expr: Option, } @@ -226,15 +226,17 @@ impl<'a> Scope<'a> { value: Dynamic, map_expr: bool, ) { + let expr = if map_expr { + map_dynamic_to_expr(value.clone(), Position::none()) + } else { + None + }; + self.0.push(Entry { name: name.into(), typ: entry_type, - value: value.clone(), - expr: if map_expr { - map_dynamic_to_expr(value, Position::none()) - } else { - None - }, + value: value.into(), + expr, }); } @@ -311,7 +313,7 @@ impl<'a> Scope<'a> { index, typ: *typ, }, - value.clone(), + value.borrow().clone(), )) } else { None @@ -337,7 +339,7 @@ impl<'a> Scope<'a> { .iter() .rev() .find(|Entry { name: key, .. }| name == key) - .and_then(|Entry { value, .. }| value.downcast_ref::().cloned()) + .and_then(|Entry { value, .. }| value.borrow().downcast_ref::().cloned()) } /// Update the value of the named entry. @@ -377,14 +379,14 @@ impl<'a> Scope<'a> { .. }, _, - )) => self.0.get_mut(index).unwrap().value = Dynamic::from(value), + )) => *self.0.get_mut(index).unwrap().value.borrow_mut() = Dynamic::from(value), None => self.push(name, value), } } /// Get a mutable reference to an entry in the Scope. - pub(crate) fn get_mut(&mut self, key: EntryRef) -> &mut Dynamic { - let entry = self.0.get_mut(key.index).expect("invalid index in Scope"); + pub(crate) fn get_ref(&self, key: EntryRef) -> &RefCell { + let entry = self.0.get(key.index).expect("invalid index in Scope"); assert_eq!(entry.typ, key.typ, "entry type not matched"); // assert_ne!( @@ -394,7 +396,7 @@ impl<'a> Scope<'a> { // ); assert_eq!(entry.name, key.name, "incorrect key at Scope entry"); - &mut entry.value + &entry.value } /// Get an iterator to entries in the Scope. @@ -418,7 +420,7 @@ where .extend(iter.into_iter().map(|(name, typ, value)| Entry { name: name.into(), typ, - value, + value: value.into(), expr: None, })); } From 619b627d54ef389ea20ff5007a765c88b36b5aff Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 26 Apr 2020 18:37:41 +0800 Subject: [PATCH 02/13] Modify for mutliple levels of indexing. --- benches/eval_array.rs | 2 +- scripts/mat_mul.rhai | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/benches/eval_array.rs b/benches/eval_array.rs index 687f22f5..d97e1164 100644 --- a/benches/eval_array.rs +++ b/benches/eval_array.rs @@ -51,7 +51,7 @@ fn bench_eval_array_large_set(bench: &mut Bencher) { let script = r#"let x = [ 1, 2.345, "hello", true, [ 1, 2, 3, [ "hey", [ "deeply", "nested" ], "jude" ] ] ]; - x[4] = 42 + x[4][3][1][1] = 42 "#; let mut engine = Engine::new(); diff --git a/scripts/mat_mul.rhai b/scripts/mat_mul.rhai index c7c00ae9..59011e22 100644 --- a/scripts/mat_mul.rhai +++ b/scripts/mat_mul.rhai @@ -16,9 +16,7 @@ fn mat_gen(n) { for i in range(0, n) { for j in range(0, n) { - let foo = m[i]; - foo[j] = tmp * (i.to_float() - j.to_float()) * (i.to_float() + j.to_float()); - m[i] = foo; + m[i][j] = tmp * (i.to_float() - j.to_float()) * (i.to_float() + j.to_float()); } } @@ -34,9 +32,7 @@ fn mat_mul(a, b) { for i in range(0, n) { for j in range(0, p) { - let foo = b2[j]; - foo[i] = b[i][j]; - b2[j] = foo; + b2[j][i] = b[i][j]; } } From f5c7d7cd0df3df74ef5ec5c209afd83562e19f16 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 26 Apr 2020 19:37:32 +0800 Subject: [PATCH 03/13] Reduce memory footprint of Target. --- src/engine.rs | 9 +++++---- src/parser.rs | 30 +++++++++++++++++------------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index a0d568ca..82f3e3d4 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -83,7 +83,7 @@ enum Target<'a> { /// The target is a variable stored in the current `Scope`. Scope(&'a RefCell), /// The target is a temporary `Dynamic` value (i.e. the mutation can cause no side effects). - Value(Dynamic), + Value(Box), /// The target is a character inside a String. StringChar(Box<(&'a mut Dynamic, usize, Dynamic)>), } @@ -94,7 +94,7 @@ impl Target<'_> { match self { Target::Ref(r) => r.clone(), Target::Scope(r) => r.borrow().clone(), - Target::Value(v) => v, + Target::Value(v) => *v, Target::StringChar(s) => s.2, } } @@ -144,7 +144,7 @@ impl<'a> From<&'a mut Dynamic> for Target<'a> { } impl> From for Target<'_> { fn from(value: T) -> Self { - Self::Value(value.into()) + Self::Value(Box::new(value.into())) } } @@ -724,7 +724,7 @@ impl Engine { let obj = match target { Target::Scope(_) => scope_base.as_mut().unwrap().deref_mut(), Target::Ref(r) => r, - Target::Value(ref mut r) => r, + Target::Value(ref mut r) => r.as_mut(), Target::StringChar(ref mut x) => &mut x.2, }; @@ -777,6 +777,7 @@ impl Engine { .collect::>(); let def_val = def_val.as_ref(); // A function call is assumed to have side effects, so the value is changed + // TODO - Remove assumption of side effects by checking whether the first parameter is &mut self.exec_fn_call(fn_lib, fn_name, &mut args, def_val, *pos, 0).map(|v| (v, true)) } // {xxx:map}.id diff --git a/src/parser.rs b/src/parser.rs index a9f79250..36cf4fd0 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -505,6 +505,7 @@ impl Expr { } } + /// Convert a `Variable` into a `Property`. All other variants are untouched. pub(crate) fn into_property(self) -> Self { match self { Self::Variable(id, pos) => Self::Property(id, pos), @@ -626,9 +627,10 @@ fn parse_call_expr<'a, S: Into> + Display>( } } -/// Parse an indexing expression. -fn parse_index_expr<'a>( - lhs: Box, +/// Parse an indexing chain. +/// Indexing binds to the right, so this call parses all possible levels of indexing following in the input. +fn parse_index_chain<'a>( + lhs: Expr, input: &mut Peekable>, pos: Position, allow_stmt_expr: bool, @@ -645,7 +647,7 @@ fn parse_index_expr<'a>( )) .into_err(*pos)) } - Expr::IntegerConstant(_, pos) => match *lhs { + Expr::IntegerConstant(_, pos) => match lhs { Expr::Array(_, _) | Expr::StringConstant(_, _) => (), Expr::Map(_, _) => { @@ -674,7 +676,7 @@ fn parse_index_expr<'a>( }, // lhs[string] - Expr::StringConstant(_, pos) => match *lhs { + Expr::StringConstant(_, pos) => match lhs { Expr::Map(_, _) => (), Expr::Array(_, _) | Expr::StringConstant(_, _) => { @@ -742,15 +744,18 @@ fn parse_index_expr<'a>( (Token::RightBracket, _) => { eat_token(input, Token::RightBracket); - let idx_expr = Box::new(idx_expr); - + // Any more indexing following? match input.peek().unwrap() { + // If another indexing level, right-bind it (Token::LeftBracket, _) => { let follow_pos = eat_token(input, Token::LeftBracket); - let follow = parse_index_expr(idx_expr, input, follow_pos, allow_stmt_expr)?; - Ok(Expr::Index(lhs, Box::new(follow), pos)) + // Recursively parse the indexing chain, right-binding each + let follow = parse_index_chain(idx_expr, input, follow_pos, allow_stmt_expr)?; + // Indexing binds to right + Ok(Expr::Index(Box::new(lhs), Box::new(follow), pos)) } - _ => Ok(Expr::Index(lhs, idx_expr, pos)), + // Otherwise terminate the indexing chain + _ => Ok(Expr::Index(Box::new(lhs), Box::new(idx_expr), pos)), } } (Token::LexError(err), pos) => return Err(PERR::BadInput(err.to_string()).into_err(*pos)), @@ -943,9 +948,7 @@ fn parse_primary<'a>( parse_call_expr(id, input, pos, allow_stmt_expr)? } // Indexing - (expr, Token::LeftBracket) => { - parse_index_expr(Box::new(expr), input, pos, allow_stmt_expr)? - } + (expr, Token::LeftBracket) => parse_index_chain(expr, input, pos, allow_stmt_expr)?, // Unknown postfix operator (expr, token) => panic!("unknown postfix operator {:?} for {:?}", token, expr), } @@ -1068,6 +1071,7 @@ fn parse_op_assignment_stmt<'a>( fn make_dot_expr(lhs: Expr, rhs: Expr, op_pos: Position, is_index: bool) -> Expr { match (lhs, rhs) { // idx_lhs[idx_rhs].rhs + // Attach dot chain to the bottom level of indexing chain (Expr::Index(idx_lhs, idx_rhs, idx_pos), rhs) => Expr::Index( idx_lhs, Box::new(make_dot_expr(*idx_rhs, rhs, op_pos, true)), From ce121ed6af6778dbf0fb3f1834f716a0afb2c21b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 26 Apr 2020 21:48:49 +0800 Subject: [PATCH 04/13] Add comments and minor refactor. --- src/engine.rs | 64 +++++++++++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 82f3e3d4..6e8e5fa1 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -703,6 +703,8 @@ impl Engine { } /// Chain-evaluate a dot/index chain. + /// Index values are pre-calculated and stored in `idx_list`. + /// Any spill-overs are stored in `idx_more`. fn eval_dot_index_chain_helper( &self, fn_lib: Option<&FunctionsLib>, @@ -730,18 +732,18 @@ impl Engine { // Pop the last index value let mut idx_val; - let mut idx_fixed = idx_list; + let mut idx_list = idx_list; if let Some(val) = idx_more.pop() { // Values in variable list idx_val = val; } else { // No more value in variable list, pop from fixed list - let len = idx_fixed.len(); - let splits = idx_fixed.split_at_mut(len - 1); + let len = idx_list.len(); + let splits = idx_list.split_at_mut(len - 1); idx_val = mem::replace(splits.1.get_mut(0).unwrap(), ().into()); - idx_fixed = splits.0; + idx_list = splits.0; } if is_index { @@ -754,7 +756,7 @@ impl Engine { let indexed_val = self.get_indexed_mut(obj, idx_val, idx.position(), op_pos, false)?; self.eval_dot_index_chain_helper( - fn_lib, indexed_val, idx_rhs.as_ref(), idx_fixed, idx_more, is_index, *pos, level, new_val + fn_lib, indexed_val, idx_rhs.as_ref(), idx_list, idx_more, is_index, *pos, level, new_val ) } // xxx[rhs] = new_val @@ -780,16 +782,18 @@ impl Engine { // TODO - Remove assumption of side effects by checking whether the first parameter is &mut self.exec_fn_call(fn_lib, fn_name, &mut args, def_val, *pos, 0).map(|v| (v, true)) } + // {xxx:map}.id = ??? + Expr::Property(id, pos) if obj.is::() && new_val.is_some() => { + let mut indexed_val = + self.get_indexed_mut(obj, id.to_string().into(), *pos, op_pos, true)?; + indexed_val.set_value(new_val.unwrap(), rhs.position())?; + Ok((().into(), true)) + } // {xxx:map}.id Expr::Property(id, pos) if obj.is::() => { - let mut indexed_val = - self.get_indexed_mut(obj, id.to_string().into(), *pos, op_pos, new_val.is_some())?; - if let Some(new_val) = new_val { - indexed_val.set_value(new_val, rhs.position())?; - Ok((().into(), true)) - } else { - Ok((indexed_val.into_dynamic(), false)) - } + let indexed_val = + self.get_indexed_mut(obj, id.to_string().into(), *pos, op_pos, false)?; + Ok((indexed_val.into_dynamic(), false)) } // xxx.id = ??? Expr::Property(id, pos) if new_val.is_some() => { @@ -819,7 +823,7 @@ impl Engine { ))); }; self.eval_dot_index_chain_helper( - fn_lib, indexed_val, dot_rhs, idx_fixed, idx_more, is_index, *pos, level, new_val + fn_lib, indexed_val, dot_rhs, idx_list, idx_more, is_index, *pos, level, new_val ) } // xxx.idx_lhs[idx_expr] @@ -830,7 +834,7 @@ impl Engine { let mut buf: Dynamic = ().into(); let mut args = [obj, &mut buf]; - let mut indexed_val = if let Expr::Property(id, pos) = dot_lhs.as_ref() { + let indexed_val = &mut (if let Expr::Property(id, pos) = dot_lhs.as_ref() { let fn_name = make_getter(id); self.exec_fn_call(fn_lib, &fn_name, &mut args[..1], None, *pos, 0)? } else { @@ -839,16 +843,16 @@ impl Engine { "".to_string(), rhs.position(), ))); - }; + }); let (result, changed) = self.eval_dot_index_chain_helper( - fn_lib, (&mut indexed_val).into(), dot_rhs, idx_fixed, idx_more, is_index, *pos, level, new_val + fn_lib, indexed_val.into(), dot_rhs, idx_list, idx_more, is_index, *pos, level, new_val )?; // Feed the value back via a setter just in case it has been updated if changed { if let Expr::Property(id, pos) = dot_lhs.as_ref() { let fn_name = make_setter(id); - args[1] = &mut indexed_val; + args[1] = indexed_val; self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0)?; } } @@ -864,7 +868,7 @@ impl Engine { } } - /// Evaluate a dot/index chain + /// Evaluate a dot/index chain. fn eval_dot_index_chain( &self, scope: &mut Scope, @@ -945,6 +949,11 @@ impl Engine { } } + /// Evaluate a chain of indexes and store the results in a list. + /// The first few results are stored in the array `list` which is of fixed length. + /// Any spill-overs are stored in `more`, which is dynamic. + /// The fixed length array is used to avoid an allocation in the overwhelming cases of just a few levels of indexing. + /// The total number of values is returned. fn eval_indexed_chain( &self, scope: &mut Scope, @@ -955,7 +964,7 @@ impl Engine { size: usize, level: usize, ) -> Result> { - let size = match expr { + match expr { Expr::FunctionCall(_, arg_exprs, _, _) => { let arg_values = arg_exprs .iter() @@ -967,21 +976,21 @@ impl Engine { } else { more.push(arg_values.into()); } - size + 1 + Ok(size + 1) } Expr::Property(_, _) => { - // Placeholder + // Store a placeholder - no need to copy the property name if size < list.len() { list[size] = ().into(); } else { more.push(().into()); } - size + 1 + Ok(size + 1) } Expr::Index(lhs, rhs, _) | Expr::Dot(lhs, rhs, _) => { // Evaluate in left-to-right order let lhs_val = match lhs.as_ref() { - Expr::Property(_, _) => ().into(), // Placeholder + Expr::Property(_, _) => ().into(), // Store a placeholder in case of a property _ => self.eval_expr(scope, fn_lib, lhs, level)?, }; @@ -993,7 +1002,7 @@ impl Engine { } else { more.push(lhs_val); } - size + 1 + Ok(size + 1) } _ => { let val = self.eval_expr(scope, fn_lib, expr, level)?; @@ -1002,10 +1011,9 @@ impl Engine { } else { more.push(val); } - size + 1 + Ok(size + 1) } - }; - Ok(size) + } } /// Get the value at the indexed position of a base type From 07c5abcc02733549e7d1b3c4be769aa1a7196aca Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 27 Apr 2020 09:36:31 +0800 Subject: [PATCH 05/13] Remove RefCell in Scope. --- src/engine.rs | 30 +++++++----------------------- src/scope.rs | 16 ++++++++-------- 2 files changed, 15 insertions(+), 31 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 6e8e5fa1..e7aacb79 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -13,7 +13,6 @@ use crate::token::Position; use crate::stdlib::{ any::TypeId, boxed::Box, - cell::RefCell, collections::HashMap, format, hash::{Hash, Hasher}, @@ -80,8 +79,6 @@ const FUNCTIONS_COUNT: usize = 256; enum Target<'a> { /// The target is a mutable reference to a `Dynamic` value somewhere. Ref(&'a mut Dynamic), - /// The target is a variable stored in the current `Scope`. - Scope(&'a RefCell), /// The target is a temporary `Dynamic` value (i.e. the mutation can cause no side effects). Value(Box), /// The target is a character inside a String. @@ -93,7 +90,6 @@ impl Target<'_> { pub fn into_dynamic(self) -> Dynamic { match self { Target::Ref(r) => r.clone(), - Target::Scope(r) => r.borrow().clone(), Target::Value(v) => *v, Target::StringChar(s) => s.2, } @@ -102,7 +98,6 @@ impl Target<'_> { /// Update the value of the `Target`. pub fn set_value(&mut self, new_val: Dynamic, pos: Position) -> Result<(), Box> { match self { - Target::Scope(r) => *r.borrow_mut() = new_val, Target::Ref(r) => **r = new_val, Target::Value(_) => { return Err(Box::new(EvalAltResult::ErrorAssignmentToUnknownLHS(pos))) @@ -132,11 +127,6 @@ impl Target<'_> { } } -impl<'a> From<&'a RefCell> for Target<'a> { - fn from(value: &'a RefCell) -> Self { - Self::Scope(value) - } -} impl<'a> From<&'a mut Dynamic> for Target<'a> { fn from(value: &'a mut Dynamic) -> Self { Self::Ref(value) @@ -395,15 +385,15 @@ fn default_print(s: &str) { /// Search for a variable within the scope, returning its value and index inside the Scope fn search_scope<'a>( - scope: &'a Scope, + scope: &'a mut Scope, id: &str, begin: Position, -) -> Result<(&'a RefCell, ScopeEntryType), Box> { - let (entry, _) = scope +) -> Result<(&'a mut Dynamic, ScopeEntryType), Box> { + let (ScopeSource { typ, index, .. }, _) = scope .get(id) .ok_or_else(|| Box::new(EvalAltResult::ErrorVariableNotFound(id.into(), begin)))?; - Ok((&scope.get_ref(entry), entry.typ)) + Ok((scope.get_mut(ScopeSource { name: id, typ, index }), typ)) } impl Engine { @@ -717,14 +707,8 @@ impl Engine { level: usize, mut new_val: Option, ) -> Result<(Dynamic, bool), Box> { - // Store a copy of the RefMut from `borrow_mut` since it is a temporary value - let mut scope_base = match target { - Target::Scope(r) => Some(r.borrow_mut()), - Target::Ref(_) | Target::Value(_) | Target::StringChar(_) => None, - }; // Get a reference to the mutation target Dynamic let obj = match target { - Target::Scope(_) => scope_base.as_mut().unwrap().deref_mut(), Target::Ref(r) => r, Target::Value(ref mut r) => r.as_mut(), Target::StringChar(ref mut x) => &mut x.2, @@ -1157,7 +1141,7 @@ impl Engine { Expr::StringConstant(s, _) => Ok(s.to_string().into()), Expr::CharConstant(c, _) => Ok((*c).into()), Expr::Variable(id, pos) => { - search_scope(scope, id, *pos).map(|(v, _)| v.borrow().clone()) + search_scope(scope, id, *pos).map(|(v, _)| v.clone()) } Expr::Property(_, _) => panic!("unexpected property."), @@ -1189,7 +1173,7 @@ impl Engine { )) => { // Avoid referencing scope which is used below as mut let entry = ScopeSource { name, ..entry }; - *scope.get_ref(entry).borrow_mut() = rhs_val.clone(); + *scope.get_mut(entry) = rhs_val.clone(); Ok(rhs_val) } @@ -1440,7 +1424,7 @@ impl Engine { }; for a in iter_fn(arr) { - *scope.get_ref(entry).borrow_mut() = a; + *scope.get_mut(entry) = a; match self.eval_stmt(scope, fn_lib, body, level) { Ok(_) => (), diff --git a/src/scope.rs b/src/scope.rs index 6eeff2bd..cea43327 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -4,7 +4,7 @@ use crate::any::{Dynamic, Variant}; use crate::parser::{map_dynamic_to_expr, Expr}; use crate::token::Position; -use crate::stdlib::{borrow::Cow, cell::RefCell, iter, vec::Vec}; +use crate::stdlib::{borrow::Cow, iter, vec::Vec}; /// Type of an entry in the Scope. #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -23,7 +23,7 @@ pub struct Entry<'a> { /// Type of the entry. pub typ: EntryType, /// Current value of the entry. - pub value: RefCell, + pub value: Dynamic, /// A constant expression if the initial value matches one of the recognized types. pub expr: Option, } @@ -313,7 +313,7 @@ impl<'a> Scope<'a> { index, typ: *typ, }, - value.borrow().clone(), + value.clone(), )) } else { None @@ -339,7 +339,7 @@ impl<'a> Scope<'a> { .iter() .rev() .find(|Entry { name: key, .. }| name == key) - .and_then(|Entry { value, .. }| value.borrow().downcast_ref::().cloned()) + .and_then(|Entry { value, .. }| value.downcast_ref::().cloned()) } /// Update the value of the named entry. @@ -379,14 +379,14 @@ impl<'a> Scope<'a> { .. }, _, - )) => *self.0.get_mut(index).unwrap().value.borrow_mut() = Dynamic::from(value), + )) => self.0.get_mut(index).unwrap().value = Dynamic::from(value), None => self.push(name, value), } } /// Get a mutable reference to an entry in the Scope. - pub(crate) fn get_ref(&self, key: EntryRef) -> &RefCell { - let entry = self.0.get(key.index).expect("invalid index in Scope"); + pub(crate) fn get_mut(&mut self, key: EntryRef) -> &mut Dynamic { + let entry = self.0.get_mut(key.index).expect("invalid index in Scope"); assert_eq!(entry.typ, key.typ, "entry type not matched"); // assert_ne!( @@ -396,7 +396,7 @@ impl<'a> Scope<'a> { // ); assert_eq!(entry.name, key.name, "incorrect key at Scope entry"); - &entry.value + &mut entry.value } /// Get an iterator to entries in the Scope. From 5afeba6fd1961d331ae9c480694302c546b39257 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 27 Apr 2020 11:24:58 +0800 Subject: [PATCH 06/13] No need to return value for Scope::get. --- src/engine.rs | 14 +++++------- src/scope.rs | 59 +++++++++++++++++---------------------------------- 2 files changed, 25 insertions(+), 48 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index e7aacb79..9611a9ce 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -389,7 +389,7 @@ fn search_scope<'a>( id: &str, begin: Position, ) -> Result<(&'a mut Dynamic, ScopeEntryType), Box> { - let (ScopeSource { typ, index, .. }, _) = scope + let ScopeSource { typ, index, .. } = scope .get(id) .ok_or_else(|| Box::new(EvalAltResult::ErrorVariableNotFound(id.into(), begin)))?; @@ -1162,28 +1162,24 @@ impl Engine { ))) } - Some(( + Some( entry @ ScopeSource { typ: ScopeEntryType::Normal, .. - }, - _, - )) => { + }) => { // Avoid referencing scope which is used below as mut let entry = ScopeSource { name, ..entry }; *scope.get_mut(entry) = rhs_val.clone(); Ok(rhs_val) } - Some(( + Some( ScopeSource { typ: ScopeEntryType::Constant, .. - }, - _, - )) => Err(Box::new(EvalAltResult::ErrorAssignmentToConstant( + }) => Err(Box::new(EvalAltResult::ErrorAssignmentToConstant( name.to_string(), *op_pos, ))), diff --git a/src/scope.rs b/src/scope.rs index cea43327..5dd23e73 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -291,35 +291,22 @@ impl<'a> Scope<'a> { } /// Find an entry in the Scope, starting from the last. - pub(crate) fn get(&self, name: &str) -> Option<(EntryRef, Dynamic)> { + pub(crate) fn get(&self, name: &str) -> Option { self.0 .iter() .enumerate() .rev() // Always search a Scope in reverse order - .find_map( - |( - index, - Entry { + .find_map(|(index, Entry { name: key, typ, .. })| { + if name == key { + Some(EntryRef { name: key, - typ, - value, - .. - }, - )| { - if name == key { - Some(( - EntryRef { - name: key, - index, - typ: *typ, - }, - value.clone(), - )) - } else { - None - } - }, - ) + index, + typ: *typ, + }) + } else { + None + } + }) } /// Get the value of an entry in the Scope, starting from the last. @@ -365,21 +352,15 @@ impl<'a> Scope<'a> { /// ``` pub fn set_value(&mut self, name: &'a str, value: T) { match self.get(name) { - Some(( - EntryRef { - typ: EntryType::Constant, - .. - }, - _, - )) => panic!("variable {} is constant", name), - Some(( - EntryRef { - index, - typ: EntryType::Normal, - .. - }, - _, - )) => self.0.get_mut(index).unwrap().value = Dynamic::from(value), + Some(EntryRef { + typ: EntryType::Constant, + .. + }) => panic!("variable {} is constant", name), + Some(EntryRef { + index, + typ: EntryType::Normal, + .. + }) => self.0.get_mut(index).unwrap().value = Dynamic::from(value), None => self.push(name, value), } } From 0d5a36edc8a9725dee38fef140932c5f89876869 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 27 Apr 2020 12:42:39 +0800 Subject: [PATCH 07/13] Increase prime numbers limit to 100K. --- scripts/primes.rhai | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/primes.rhai b/scripts/primes.rhai index a389999f..22defcb4 100644 --- a/scripts/primes.rhai +++ b/scripts/primes.rhai @@ -2,7 +2,7 @@ let now = timestamp(); -const MAX_NUMBER_TO_CHECK = 10_000; // 1229 primes <= 10000 +const MAX_NUMBER_TO_CHECK = 100_000; // 9592 primes <= 100000 let prime_mask = []; prime_mask.pad(MAX_NUMBER_TO_CHECK, true); From b3e46597907004135c0d9a540c2fad66fe379a09 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 27 Apr 2020 20:25:20 +0800 Subject: [PATCH 08/13] Encapsulate index values into StaticVec type. --- src/engine.rs | 202 +++++++++++++++++++++++++------------------------- 1 file changed, 103 insertions(+), 99 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 9611a9ce..0ff8351c 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -60,6 +60,13 @@ pub const MAX_CALL_STACK_DEPTH: usize = 28; #[cfg(not(debug_assertions))] pub const MAX_CALL_STACK_DEPTH: usize = 256; +#[cfg(not(feature = "only_i32"))] +#[cfg(not(feature = "only_i64"))] +const FUNCTIONS_COUNT: usize = 512; + +#[cfg(any(feature = "only_i32", feature = "only_i64"))] +const FUNCTIONS_COUNT: usize = 256; + pub const KEYWORD_PRINT: &str = "print"; pub const KEYWORD_DEBUG: &str = "debug"; pub const KEYWORD_TYPE_OF: &str = "type_of"; @@ -68,13 +75,6 @@ pub const FUNC_TO_STRING: &str = "to_string"; pub const FUNC_GETTER: &str = "get$"; pub const FUNC_SETTER: &str = "set$"; -#[cfg(not(feature = "only_i32"))] -#[cfg(not(feature = "only_i64"))] -const FUNCTIONS_COUNT: usize = 512; - -#[cfg(any(feature = "only_i32", feature = "only_i64"))] -const FUNCTIONS_COUNT: usize = 256; - /// A type that encapsulates a mutation target for an expression with side effects. enum Target<'a> { /// The target is a mutable reference to a `Dynamic` value somewhere. @@ -82,6 +82,7 @@ enum Target<'a> { /// The target is a temporary `Dynamic` value (i.e. the mutation can cause no side effects). Value(Box), /// The target is a character inside a String. + /// This is necessary because directly pointing to a char inside a String is impossible. StringChar(Box<(&'a mut Dynamic, usize, Dynamic)>), } @@ -138,6 +139,55 @@ impl> From for Target<'_> { } } +/// A type to hold a number of `Dynamic` values in static storage for speed, +/// and any spill-overs in a `Vec`. +struct StaticVec { + /// Total number of values held. + len: usize, + /// Static storage. + list: [Dynamic; 4], + /// Dynamic storage. + more: Vec, +} + +impl StaticVec { + /// Create a new `StaticVec`. + pub fn new() -> Self { + Self { + len: 0, + list: [().into(), ().into(), ().into(), ().into()], + more: Vec::new(), + } + } + /// Push a new value to the end of this `StaticVec`. + pub fn push>(&mut self, value: T) { + if self.len >= self.list.len() { + self.more.push(value.into()); + } else { + self.list[self.len] = value.into(); + } + self.len += 1; + } + /// Pop a value from the end of this `StaticVec`. + /// + /// # Panics + /// + /// Panics if the `StaticVec` is empty. + pub fn pop(&mut self) -> Dynamic { + let result = if self.len <= 0 { + panic!("nothing to pop!") + } else if self.len <= self.list.len() { + mem::replace(self.list.get_mut(self.len - 1).unwrap(), ().into()) + } else { + self.more.pop().unwrap() + }; + + self.len -= 1; + + result + } +} + /// A type that holds a library (`HashMap`) of script-defined functions. /// /// Since script-defined functions have `Dynamic` parameters, functions with the same name @@ -389,11 +439,18 @@ fn search_scope<'a>( id: &str, begin: Position, ) -> Result<(&'a mut Dynamic, ScopeEntryType), Box> { - let ScopeSource { typ, index, .. } = scope + let ScopeSource { typ, index, .. } = scope .get(id) .ok_or_else(|| Box::new(EvalAltResult::ErrorVariableNotFound(id.into(), begin)))?; - Ok((scope.get_mut(ScopeSource { name: id, typ, index }), typ)) + Ok(( + scope.get_mut(ScopeSource { + name: id, + typ, + index, + }), + typ, + )) } impl Engine { @@ -536,8 +593,7 @@ impl Engine { // Raise error let types_list: Vec<_> = args .iter() - .map(|x| x.type_name()) - .map(|name| self.map_type_name(name)) + .map(|name| self.map_type_name(name.type_name())) .collect(); Err(Box::new(EvalAltResult::ErrorFunctionNotFound( @@ -611,12 +667,12 @@ impl Engine { // Has a system function an override? fn has_override(&self, fn_lib: Option<&FunctionsLib>, name: &str) -> bool { - let hash = &calc_fn_hash(name, once(TypeId::of::())); + let hash = calc_fn_hash(name, once(TypeId::of::())); // First check registered functions - self.functions.contains_key(hash) + self.functions.contains_key(&hash) // Then check packages - || self.packages.iter().any(|p| p.functions.contains_key(hash)) + || self.packages.iter().any(|p| p.functions.contains_key(&hash)) // Then check script-defined functions || fn_lib.map_or(false, |lib| lib.has_function(name, 1)) } @@ -693,15 +749,12 @@ impl Engine { } /// Chain-evaluate a dot/index chain. - /// Index values are pre-calculated and stored in `idx_list`. - /// Any spill-overs are stored in `idx_more`. fn eval_dot_index_chain_helper( &self, fn_lib: Option<&FunctionsLib>, mut target: Target, rhs: &Expr, - idx_list: &mut [Dynamic], - idx_more: &mut Vec, + idx_values: &mut StaticVec, is_index: bool, op_pos: Position, level: usize, @@ -715,20 +768,7 @@ impl Engine { }; // Pop the last index value - let mut idx_val; - let mut idx_list = idx_list; - - if let Some(val) = idx_more.pop() { - // Values in variable list - idx_val = val; - } else { - // No more value in variable list, pop from fixed list - let len = idx_list.len(); - let splits = idx_list.split_at_mut(len - 1); - - idx_val = mem::replace(splits.1.get_mut(0).unwrap(), ().into()); - idx_list = splits.0; - } + let mut idx_val = idx_values.pop(); if is_index { match rhs { @@ -740,7 +780,7 @@ impl Engine { let indexed_val = self.get_indexed_mut(obj, idx_val, idx.position(), op_pos, false)?; self.eval_dot_index_chain_helper( - fn_lib, indexed_val, idx_rhs.as_ref(), idx_list, idx_more, is_index, *pos, level, new_val + fn_lib, indexed_val, idx_rhs.as_ref(), idx_values, is_index, *pos, level, new_val ) } // xxx[rhs] = new_val @@ -758,9 +798,9 @@ impl Engine { match rhs { // xxx.fn_name(arg_expr_list) Expr::FunctionCall(fn_name, _, def_val, pos) => { - let mut args = once(obj) + let mut args: Vec<_> = once(obj) .chain(idx_val.downcast_mut::().unwrap().iter_mut()) - .collect::>(); + .collect(); let def_val = def_val.as_ref(); // A function call is assumed to have side effects, so the value is changed // TODO - Remove assumption of side effects by checking whether the first parameter is &mut @@ -768,14 +808,14 @@ impl Engine { } // {xxx:map}.id = ??? Expr::Property(id, pos) if obj.is::() && new_val.is_some() => { - let mut indexed_val = + let mut indexed_val = self.get_indexed_mut(obj, id.to_string().into(), *pos, op_pos, true)?; indexed_val.set_value(new_val.unwrap(), rhs.position())?; Ok((().into(), true)) } // {xxx:map}.id Expr::Property(id, pos) if obj.is::() => { - let indexed_val = + let indexed_val = self.get_indexed_mut(obj, id.to_string().into(), *pos, op_pos, false)?; Ok((indexed_val.into_dynamic(), false)) } @@ -807,7 +847,7 @@ impl Engine { ))); }; self.eval_dot_index_chain_helper( - fn_lib, indexed_val, dot_rhs, idx_list, idx_more, is_index, *pos, level, new_val + fn_lib, indexed_val, dot_rhs, idx_values, is_index, *pos, level, new_val ) } // xxx.idx_lhs[idx_expr] @@ -829,7 +869,7 @@ impl Engine { ))); }); let (result, changed) = self.eval_dot_index_chain_helper( - fn_lib, indexed_val.into(), dot_rhs, idx_list, idx_more, is_index, *pos, level, new_val + fn_lib, indexed_val.into(), dot_rhs, idx_values, is_index, *pos, level, new_val )?; // Feed the value back via a setter just in case it has been updated @@ -864,19 +904,9 @@ impl Engine { level: usize, new_val: Option, ) -> Result> { - // Keep four levels of index values in fixed array to reduce allocations - let mut idx_list: [Dynamic; 4] = [().into(), ().into(), ().into(), ().into()]; - // Spill over additional levels into a variable list - let idx_more: &mut Vec = &mut Vec::new(); + let idx_values = &mut StaticVec::new(); - let size = - self.eval_indexed_chain(scope, fn_lib, dot_rhs, &mut idx_list, idx_more, 0, level)?; - - let idx_list = if size < idx_list.len() { - &mut idx_list[0..size] - } else { - &mut idx_list - }; + self.eval_indexed_chain(scope, fn_lib, dot_rhs, idx_values, 0, level)?; match dot_lhs { // id.??? or id[???] @@ -898,8 +928,7 @@ impl Engine { fn_lib, target.into(), dot_rhs, - idx_list, - idx_more, + idx_values, is_index, op_pos, level, @@ -921,8 +950,7 @@ impl Engine { fn_lib, val.into(), dot_rhs, - idx_list, - idx_more, + idx_values, is_index, op_pos, level, @@ -943,11 +971,10 @@ impl Engine { scope: &mut Scope, fn_lib: Option<&FunctionsLib>, expr: &Expr, - list: &mut [Dynamic], - more: &mut Vec, + idx_values: &mut StaticVec, size: usize, level: usize, - ) -> Result> { + ) -> Result<(), Box> { match expr { Expr::FunctionCall(_, arg_exprs, _, _) => { let arg_values = arg_exprs @@ -955,22 +982,10 @@ impl Engine { .map(|arg_expr| self.eval_expr(scope, fn_lib, arg_expr, level)) .collect::, _>>()?; - if size < list.len() { - list[size] = arg_values.into(); - } else { - more.push(arg_values.into()); - } - Ok(size + 1) - } - Expr::Property(_, _) => { - // Store a placeholder - no need to copy the property name - if size < list.len() { - list[size] = ().into(); - } else { - more.push(().into()); - } - Ok(size + 1) + idx_values.push(arg_values) } + // Store a placeholder - no need to copy the property name + Expr::Property(_, _) => idx_values.push(()), Expr::Index(lhs, rhs, _) | Expr::Dot(lhs, rhs, _) => { // Evaluate in left-to-right order let lhs_val = match lhs.as_ref() { @@ -979,25 +994,14 @@ impl Engine { }; // Push in reverse order - let size = self.eval_indexed_chain(scope, fn_lib, rhs, list, more, size, level)?; + self.eval_indexed_chain(scope, fn_lib, rhs, idx_values, size, level)?; - if size < list.len() { - list[size] = lhs_val; - } else { - more.push(lhs_val); - } - Ok(size + 1) - } - _ => { - let val = self.eval_expr(scope, fn_lib, expr, level)?; - if size < list.len() { - list[size] = val; - } else { - more.push(val); - } - Ok(size + 1) + idx_values.push(lhs_val); } + _ => idx_values.push(self.eval_expr(scope, fn_lib, expr, level)?), } + + Ok(()) } /// Get the value at the indexed position of a base type @@ -1042,7 +1046,9 @@ impl Engine { Ok(if create { map.entry(index).or_insert(().into()).into() } else { - map.get_mut(&index).map(Target::from).unwrap_or_else(|| Target::from(())) + map.get_mut(&index) + .map(Target::from) + .unwrap_or_else(|| Target::from(())) }) } @@ -1140,9 +1146,7 @@ impl Engine { Expr::FloatConstant(f, _) => Ok((*f).into()), Expr::StringConstant(s, _) => Ok(s.to_string().into()), Expr::CharConstant(c, _) => Ok((*c).into()), - Expr::Variable(id, pos) => { - search_scope(scope, id, *pos).map(|(v, _)| v.clone()) - } + Expr::Variable(id, pos) => search_scope(scope, id, *pos).map(|(v, _)| v.clone()), Expr::Property(_, _) => panic!("unexpected property."), // Statement block @@ -1168,18 +1172,18 @@ impl Engine { ScopeSource { typ: ScopeEntryType::Normal, .. - }) => { + }, + ) => { // Avoid referencing scope which is used below as mut let entry = ScopeSource { name, ..entry }; *scope.get_mut(entry) = rhs_val.clone(); Ok(rhs_val) } - Some( - ScopeSource { - typ: ScopeEntryType::Constant, - .. - }) => Err(Box::new(EvalAltResult::ErrorAssignmentToConstant( + Some(ScopeSource { + typ: ScopeEntryType::Constant, + .. + }) => Err(Box::new(EvalAltResult::ErrorAssignmentToConstant( name.to_string(), *op_pos, ))), From 43fdf3f9628aa46ebb8fb4bde37a6c7e63d57eb3 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 27 Apr 2020 20:43:55 +0800 Subject: [PATCH 09/13] FunctionsLib always exist. --- src/api.rs | 7 +++--- src/engine.rs | 59 ++++++++++++++++++++++----------------------------- src/scope.rs | 5 +---- 3 files changed, 29 insertions(+), 42 deletions(-) diff --git a/src/api.rs b/src/api.rs index 64f28b42..b0bb779c 100644 --- a/src/api.rs +++ b/src/api.rs @@ -804,7 +804,7 @@ impl Engine { ast.0 .iter() .try_fold(().into(), |_, stmt| { - self.eval_stmt(scope, Some(ast.1.as_ref()), stmt, 0) + self.eval_stmt(scope, ast.1.as_ref(), stmt, 0) }) .or_else(|err| match *err { EvalAltResult::Return(out, _) => Ok(out), @@ -867,7 +867,7 @@ impl Engine { ast.0 .iter() .try_fold(().into(), |_, stmt| { - self.eval_stmt(scope, Some(ast.1.as_ref()), stmt, 0) + self.eval_stmt(scope, ast.1.as_ref(), stmt, 0) }) .map_or_else( |err| match *err { @@ -930,8 +930,7 @@ impl Engine { .get_function(name, args.len()) .ok_or_else(|| Box::new(EvalAltResult::ErrorFunctionNotFound(name.to_string(), pos)))?; - let result = - self.call_fn_from_lib(Some(scope), Some(&fn_lib), fn_def, &mut args, pos, 0)?; + let result = self.call_fn_from_lib(Some(scope), fn_lib, fn_def, &mut args, pos, 0)?; let return_type = self.map_type_name(result.type_name()); diff --git a/src/engine.rs b/src/engine.rs index 0ff8351c..b2c8e731 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -436,21 +436,14 @@ fn default_print(s: &str) { /// Search for a variable within the scope, returning its value and index inside the Scope fn search_scope<'a>( scope: &'a mut Scope, - id: &str, + name: &str, begin: Position, ) -> Result<(&'a mut Dynamic, ScopeEntryType), Box> { let ScopeSource { typ, index, .. } = scope - .get(id) - .ok_or_else(|| Box::new(EvalAltResult::ErrorVariableNotFound(id.into(), begin)))?; + .get(name) + .ok_or_else(|| Box::new(EvalAltResult::ErrorVariableNotFound(name.into(), begin)))?; - Ok(( - scope.get_mut(ScopeSource { - name: id, - typ, - index, - }), - typ, - )) + Ok((scope.get_mut(ScopeSource { name, typ, index }), typ)) } impl Engine { @@ -512,7 +505,7 @@ impl Engine { pub(crate) fn call_fn_raw( &self, scope: Option<&mut Scope>, - fn_lib: Option<&FunctionsLib>, + fn_lib: &FunctionsLib, fn_name: &str, args: &mut FnCallArgs, def_val: Option<&Dynamic>, @@ -525,10 +518,10 @@ impl Engine { } #[cfg(feature = "no_function")] - const fn_lib: Option<&FunctionsLib> = None; + const fn_lib: &FunctionsLib = None; // First search in script-defined functions (can override built-in) - if let Some(fn_def) = fn_lib.and_then(|lib| lib.get_function(fn_name, args.len())) { + if let Some(fn_def) = fn_lib.get_function(fn_name, args.len()) { return self.call_fn_from_lib(scope, fn_lib, fn_def, args, pos, level); } @@ -606,7 +599,7 @@ impl Engine { pub(crate) fn call_fn_from_lib( &self, scope: Option<&mut Scope>, - fn_lib: Option<&FunctionsLib>, + fn_lib: &FunctionsLib, fn_def: &FnDef, args: &mut FnCallArgs, pos: Position, @@ -666,7 +659,7 @@ impl Engine { } // Has a system function an override? - fn has_override(&self, fn_lib: Option<&FunctionsLib>, name: &str) -> bool { + fn has_override(&self, fn_lib: &FunctionsLib, name: &str) -> bool { let hash = calc_fn_hash(name, once(TypeId::of::())); // First check registered functions @@ -674,13 +667,13 @@ impl Engine { // Then check packages || self.packages.iter().any(|p| p.functions.contains_key(&hash)) // Then check script-defined functions - || fn_lib.map_or(false, |lib| lib.has_function(name, 1)) + || fn_lib.has_function(name, 1) } // Perform an actual function call, taking care of special functions fn exec_fn_call( &self, - fn_lib: Option<&FunctionsLib>, + fn_lib: &FunctionsLib, fn_name: &str, args: &mut [&mut Dynamic], def_val: Option<&Dynamic>, @@ -709,7 +702,7 @@ impl Engine { fn eval_script_expr( &self, scope: &mut Scope, - fn_lib: Option<&FunctionsLib>, + fn_lib: &FunctionsLib, script: &Dynamic, pos: Position, ) -> Result> { @@ -732,15 +725,13 @@ impl Engine { ))); } - if let Some(lib) = fn_lib { - #[cfg(feature = "sync")] - { - ast.1 = Arc::new(lib.clone()); - } - #[cfg(not(feature = "sync"))] - { - ast.1 = Rc::new(lib.clone()); - } + #[cfg(feature = "sync")] + { + ast.1 = Arc::new(fn_lib.clone()); + } + #[cfg(not(feature = "sync"))] + { + ast.1 = Rc::new(fn_lib.clone()); } // Evaluate the AST @@ -751,7 +742,7 @@ impl Engine { /// Chain-evaluate a dot/index chain. fn eval_dot_index_chain_helper( &self, - fn_lib: Option<&FunctionsLib>, + fn_lib: &FunctionsLib, mut target: Target, rhs: &Expr, idx_values: &mut StaticVec, @@ -896,7 +887,7 @@ impl Engine { fn eval_dot_index_chain( &self, scope: &mut Scope, - fn_lib: Option<&FunctionsLib>, + fn_lib: &FunctionsLib, dot_lhs: &Expr, dot_rhs: &Expr, is_index: bool, @@ -969,7 +960,7 @@ impl Engine { fn eval_indexed_chain( &self, scope: &mut Scope, - fn_lib: Option<&FunctionsLib>, + fn_lib: &FunctionsLib, expr: &Expr, idx_values: &mut StaticVec, size: usize, @@ -1083,7 +1074,7 @@ impl Engine { fn eval_in_expr( &self, scope: &mut Scope, - fn_lib: Option<&FunctionsLib>, + fn_lib: &FunctionsLib, lhs: &Expr, rhs: &Expr, level: usize, @@ -1136,7 +1127,7 @@ impl Engine { fn eval_expr( &self, scope: &mut Scope, - fn_lib: Option<&FunctionsLib>, + fn_lib: &FunctionsLib, expr: &Expr, level: usize, ) -> Result> { @@ -1325,7 +1316,7 @@ impl Engine { pub(crate) fn eval_stmt( &self, scope: &mut Scope, - fn_lib: Option<&FunctionsLib>, + fn_lib: &FunctionsLib, stmt: &Stmt, level: usize, ) -> Result> { diff --git a/src/scope.rs b/src/scope.rs index 5dd23e73..f34905e6 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -392,10 +392,7 @@ impl Default for Scope<'_> { } } -impl<'a, K> iter::Extend<(K, EntryType, Dynamic)> for Scope<'a> -where - K: Into>, -{ +impl<'a, K: Into>> iter::Extend<(K, EntryType, Dynamic)> for Scope<'a> { fn extend>(&mut self, iter: T) { self.0 .extend(iter.into_iter().map(|(name, typ, value)| Entry { From c2bb1f48c2c8d1002a7264dbaf78a13b7d5ea454 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 27 Apr 2020 21:14:34 +0800 Subject: [PATCH 10/13] Reduce size of scope entry. --- src/optimize.rs | 4 ++-- src/scope.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/optimize.rs b/src/optimize.rs index 324fb03a..3f00e711 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -646,12 +646,12 @@ fn optimize<'a>( .filter(|ScopeEntry { typ, expr, .. }| { // Get all the constants with definite constant expressions *typ == ScopeEntryType::Constant - && expr.as_ref().map(Expr::is_constant).unwrap_or(false) + && expr.as_ref().map(|v| v.is_constant()).unwrap_or(false) }) .for_each(|ScopeEntry { name, expr, .. }| { state.push_constant( name.as_ref(), - expr.as_ref().expect("should be Some(expr)").clone(), + (**expr.as_ref().expect("should be Some(expr)")).clone(), ) }); diff --git a/src/scope.rs b/src/scope.rs index f34905e6..de57e8c2 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -25,7 +25,7 @@ pub struct Entry<'a> { /// Current value of the entry. pub value: Dynamic, /// A constant expression if the initial value matches one of the recognized types. - pub expr: Option, + pub expr: Option>, } /// Information about a particular entry in the Scope. @@ -227,7 +227,7 @@ impl<'a> Scope<'a> { map_expr: bool, ) { let expr = if map_expr { - map_dynamic_to_expr(value.clone(), Position::none()) + map_dynamic_to_expr(value.clone(), Position::none()).map(Box::new) } else { None }; From d043300ae29ba15efe225d5f39f81ab58af4fd65 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 27 Apr 2020 21:28:31 +0800 Subject: [PATCH 11/13] Reduce unnecessary Option's. --- src/api.rs | 14 ++++-------- src/engine.rs | 59 +++++++++++++++++++++++---------------------------- 2 files changed, 31 insertions(+), 42 deletions(-) diff --git a/src/api.rs b/src/api.rs index b0bb779c..0f367f14 100644 --- a/src/api.rs +++ b/src/api.rs @@ -146,14 +146,8 @@ impl Engine { /// ``` #[cfg(not(feature = "no_object"))] pub fn register_type_with_name(&mut self, name: &str) { - if self.type_names.is_none() { - self.type_names = Some(HashMap::new()); - } - // Add the pretty-print type name into the map self.type_names - .as_mut() - .unwrap() .insert(type_name::().to_string(), name.to_string()); } @@ -994,7 +988,7 @@ impl Engine { /// ``` #[cfg(feature = "sync")] pub fn on_print(&mut self, callback: impl Fn(&str) + Send + Sync + 'static) { - self.on_print = Some(Box::new(callback)); + self.print = Box::new(callback); } /// Override default action of `print` (print to stdout using `println!`) /// @@ -1022,7 +1016,7 @@ impl Engine { /// ``` #[cfg(not(feature = "sync"))] pub fn on_print(&mut self, callback: impl Fn(&str) + 'static) { - self.on_print = Some(Box::new(callback)); + self.print = Box::new(callback); } /// Override default action of `debug` (print to stdout using `println!`) @@ -1051,7 +1045,7 @@ impl Engine { /// ``` #[cfg(feature = "sync")] pub fn on_debug(&mut self, callback: impl Fn(&str) + Send + Sync + 'static) { - self.on_debug = Some(Box::new(callback)); + self.debug = Box::new(callback); } /// Override default action of `debug` (print to stdout using `println!`) /// @@ -1079,6 +1073,6 @@ impl Engine { /// ``` #[cfg(not(feature = "sync"))] pub fn on_debug(&mut self, callback: impl Fn(&str) + 'static) { - self.on_debug = Some(Box::new(callback)); + self.debug = Box::new(callback); } } diff --git a/src/engine.rs b/src/engine.rs index b2c8e731..8023a6a8 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -295,21 +295,21 @@ pub struct Engine { /// A hashmap containing all iterators known to the engine. pub(crate) type_iterators: HashMap>, /// A hashmap mapping type names to pretty-print names. - pub(crate) type_names: Option>, + pub(crate) type_names: HashMap, /// Closure for implementing the `print` command. #[cfg(feature = "sync")] - pub(crate) on_print: Option>, + pub(crate) print: Box, /// Closure for implementing the `print` command. #[cfg(not(feature = "sync"))] - pub(crate) on_print: Option>, + pub(crate) print: Box, /// Closure for implementing the `debug` command. #[cfg(feature = "sync")] - pub(crate) on_debug: Option>, + pub(crate) debug: Box, /// Closure for implementing the `debug` command. #[cfg(not(feature = "sync"))] - pub(crate) on_debug: Option>, + pub(crate) debug: Box, /// Optimize the AST after compilation. pub(crate) optimization_level: OptimizationLevel, @@ -327,11 +327,11 @@ impl Default for Engine { packages: Vec::new(), functions: HashMap::with_capacity(FUNCTIONS_COUNT), type_iterators: HashMap::new(), - type_names: None, + type_names: HashMap::new(), // default print/debug implementations - on_print: Some(Box::new(default_print)), - on_debug: Some(Box::new(default_print)), + print: Box::new(default_print), + debug: Box::new(default_print), // optimization level #[cfg(feature = "no_optimize")] @@ -459,9 +459,9 @@ impl Engine { packages: Vec::new(), functions: HashMap::with_capacity(FUNCTIONS_COUNT / 2), type_iterators: HashMap::new(), - type_names: None, - on_print: None, - on_debug: None, + type_names: HashMap::new(), + print: Box::new(|_| {}), + debug: Box::new(|_| {}), #[cfg(feature = "no_optimize")] optimization_level: OptimizationLevel::None, @@ -539,25 +539,20 @@ impl Engine { // See if the function match print/debug (which requires special processing) return Ok(match fn_name { - KEYWORD_PRINT if self.on_print.is_some() => { - self.on_print.as_ref().unwrap()(result.as_str().map_err(|type_name| { - Box::new(EvalAltResult::ErrorMismatchOutputType( - type_name.into(), - pos, - )) - })?) - .into() - } - KEYWORD_DEBUG if self.on_debug.is_some() => { - self.on_debug.as_ref().unwrap()(result.as_str().map_err(|type_name| { - Box::new(EvalAltResult::ErrorMismatchOutputType( - type_name.into(), - pos, - )) - })?) - .into() - } - KEYWORD_PRINT | KEYWORD_DEBUG => ().into(), + KEYWORD_PRINT => (self.print)(result.as_str().map_err(|type_name| { + Box::new(EvalAltResult::ErrorMismatchOutputType( + type_name.into(), + pos, + )) + })?) + .into(), + KEYWORD_DEBUG => (self.debug)(result.as_str().map_err(|type_name| { + Box::new(EvalAltResult::ErrorMismatchOutputType( + type_name.into(), + pos, + )) + })?) + .into(), _ => result, }); } @@ -1493,8 +1488,8 @@ impl Engine { /// Map a type_name into a pretty-print name pub(crate) fn map_type_name<'a>(&'a self, name: &'a str) -> &'a str { self.type_names - .as_ref() - .and_then(|list| list.get(name).map(String::as_str)) + .get(name) + .map(String::as_str) .unwrap_or(name) } } From d3a97dc86b1e8e738939021b197d560b37575104 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 27 Apr 2020 22:49:09 +0800 Subject: [PATCH 12/13] Remove EntryRef from Scope. --- src/engine.rs | 40 ++++++++++------------------------------ src/scope.rs | 35 ++++++++--------------------------- 2 files changed, 18 insertions(+), 57 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 8023a6a8..4fd38ab6 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -7,7 +7,7 @@ use crate::optimize::OptimizationLevel; use crate::packages::{CorePackage, Package, PackageLibrary, StandardPackage}; use crate::parser::{Expr, FnDef, ReturnType, Stmt}; use crate::result::EvalAltResult; -use crate::scope::{EntryRef as ScopeSource, EntryType as ScopeEntryType, Scope}; +use crate::scope::{EntryType as ScopeEntryType, Scope}; use crate::token::Position; use crate::stdlib::{ @@ -439,11 +439,11 @@ fn search_scope<'a>( name: &str, begin: Position, ) -> Result<(&'a mut Dynamic, ScopeEntryType), Box> { - let ScopeSource { typ, index, .. } = scope + let (index, typ) = scope .get(name) .ok_or_else(|| Box::new(EvalAltResult::ErrorVariableNotFound(name.into(), begin)))?; - Ok((scope.get_mut(ScopeSource { name, typ, index }), typ)) + Ok((scope.get_mut(index), typ)) } impl Engine { @@ -517,9 +517,6 @@ impl Engine { return Err(Box::new(EvalAltResult::ErrorStackOverflow(pos))); } - #[cfg(feature = "no_function")] - const fn_lib: &FunctionsLib = None; - // First search in script-defined functions (can override built-in) if let Some(fn_def) = fn_lib.get_function(fn_name, args.len()) { return self.call_fn_from_lib(scope, fn_lib, fn_def, args, pos, level); @@ -1152,27 +1149,15 @@ impl Engine { ))) } - Some( - entry - @ - ScopeSource { - typ: ScopeEntryType::Normal, - .. - }, - ) => { + Some((index, ScopeEntryType::Normal)) => { // Avoid referencing scope which is used below as mut - let entry = ScopeSource { name, ..entry }; - *scope.get_mut(entry) = rhs_val.clone(); + *scope.get_mut(index) = rhs_val.clone(); Ok(rhs_val) } - Some(ScopeSource { - typ: ScopeEntryType::Constant, - .. - }) => Err(Box::new(EvalAltResult::ErrorAssignmentToConstant( - name.to_string(), - *op_pos, - ))), + Some((_, ScopeEntryType::Constant)) => Err(Box::new( + EvalAltResult::ErrorAssignmentToConstant(name.to_string(), *op_pos), + )), }, // idx_lhs[idx_expr] = rhs @@ -1402,15 +1387,10 @@ impl Engine { }) { // Add the loop variable scope.push(name.clone(), ()); - - let entry = ScopeSource { - name, - index: scope.len() - 1, - typ: ScopeEntryType::Normal, - }; + let index = scope.len() - 1; for a in iter_fn(arr) { - *scope.get_mut(entry) = a; + *scope.get_mut(index) = a; match self.eval_stmt(scope, fn_lib, body, level) { Ok(_) => (), diff --git a/src/scope.rs b/src/scope.rs index de57e8c2..2f6ca0c2 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -28,14 +28,6 @@ pub struct Entry<'a> { pub expr: Option>, } -/// Information about a particular entry in the Scope. -#[derive(Debug, Hash, Copy, Clone)] -pub(crate) struct EntryRef<'a> { - pub name: &'a str, - pub index: usize, - pub typ: EntryType, -} - /// A type containing information about the current scope. /// Useful for keeping state between `Engine` evaluation runs. /// @@ -291,18 +283,14 @@ impl<'a> Scope<'a> { } /// Find an entry in the Scope, starting from the last. - pub(crate) fn get(&self, name: &str) -> Option { + pub(crate) fn get(&self, name: &str) -> Option<(usize, EntryType)> { self.0 .iter() .enumerate() .rev() // Always search a Scope in reverse order .find_map(|(index, Entry { name: key, typ, .. })| { if name == key { - Some(EntryRef { - name: key, - index, - typ: *typ, - }) + Some((index, *typ)) } else { None } @@ -352,30 +340,23 @@ impl<'a> Scope<'a> { /// ``` pub fn set_value(&mut self, name: &'a str, value: T) { match self.get(name) { - Some(EntryRef { - typ: EntryType::Constant, - .. - }) => panic!("variable {} is constant", name), - Some(EntryRef { - index, - typ: EntryType::Normal, - .. - }) => self.0.get_mut(index).unwrap().value = Dynamic::from(value), + Some((_, EntryType::Constant)) => panic!("variable {} is constant", name), + Some((index, EntryType::Normal)) => { + self.0.get_mut(index).unwrap().value = Dynamic::from(value) + } None => self.push(name, value), } } /// Get a mutable reference to an entry in the Scope. - pub(crate) fn get_mut(&mut self, key: EntryRef) -> &mut Dynamic { - let entry = self.0.get_mut(key.index).expect("invalid index in Scope"); - assert_eq!(entry.typ, key.typ, "entry type not matched"); + pub(crate) fn get_mut(&mut self, index: usize) -> &mut Dynamic { + let entry = self.0.get_mut(index).expect("invalid index in Scope"); // assert_ne!( // entry.typ, // EntryType::Constant, // "get mut of constant entry" // ); - assert_eq!(entry.name, key.name, "incorrect key at Scope entry"); &mut entry.value } From 41366d08fe3ccfe0d56ce3835d606a4520309a80 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 27 Apr 2020 22:52:20 +0800 Subject: [PATCH 13/13] Fix tests and packages for all features. --- src/packages/map_basic.rs | 6 ++---- src/packages/string_more.rs | 2 +- src/packages/time_basic.rs | 4 ++-- tests/maps.rs | 1 + 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/packages/map_basic.rs b/src/packages/map_basic.rs index 44009e8a..40e2cec0 100644 --- a/src/packages/map_basic.rs +++ b/src/packages/map_basic.rs @@ -12,12 +12,10 @@ use crate::stdlib::{ }; fn map_get_keys(map: &mut Map) -> Vec { - map.iter() - .map(|(k, _)| k.to_string().into()) - .collect::>() + map.iter().map(|(k, _)| k.to_string().into()).collect() } fn map_get_values(map: &mut Map) -> Vec { - map.iter().map(|(_, v)| v.clone()).collect::>() + map.iter().map(|(_, v)| v.clone()).collect() } #[cfg(not(feature = "no_object"))] diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index 4edef10e..0144792e 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -37,7 +37,7 @@ fn sub_string(s: &mut String, start: INT, len: INT) -> String { len as usize }; - chars[offset..][..len].into_iter().collect::() + chars[offset..][..len].into_iter().collect() } fn crop_string(s: &mut String, start: INT, len: INT) { let offset = if s.is_empty() || len <= 0 { diff --git a/src/packages/time_basic.rs b/src/packages/time_basic.rs index cb0c5f8e..173c3e22 100644 --- a/src/packages/time_basic.rs +++ b/src/packages/time_basic.rs @@ -87,10 +87,10 @@ def_package!(crate:BasicTimePackage:"Basic timing utilities.", lib, { #[cfg(not(feature = "unchecked"))] { if seconds > (MAX_INT as u64) { - return Err(EvalAltResult::ErrorArithmetic( + return Err(Box::new(EvalAltResult::ErrorArithmetic( format!("Integer overflow for timestamp.elapsed(): {}", seconds), Position::none(), - )); + ))); } } return Ok(seconds as INT); diff --git a/tests/maps.rs b/tests/maps.rs index 897f8430..64c5a4c2 100644 --- a/tests/maps.rs +++ b/tests/maps.rs @@ -143,6 +143,7 @@ fn test_map_return() -> Result<(), Box> { } #[test] +#[cfg(not(feature = "no_index"))] fn test_map_for() -> Result<(), Box> { let engine = Engine::new();