From 024133ae2db0f13563ddfff969181af90445202b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 7 Mar 2020 10:15:42 +0800 Subject: [PATCH] Avoid string copying. --- src/engine.rs | 68 +++++++++++++++++++++-------------------- src/error.rs | 3 ++ src/parser.rs | 76 +++++++++++++++++++++++++++------------------- src/scope.rs | 35 ++++++++++++--------- tests/var_scope.rs | 4 +-- 5 files changed, 106 insertions(+), 80 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index d394c31d..53cb3ddc 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -46,22 +46,22 @@ type IteratorFn = dyn Fn(&Dynamic) -> Box>; /// } /// } /// ``` -pub struct Engine<'a> { +pub struct Engine<'e> { /// A hashmap containing all compiled functions known to the engine - pub(crate) external_functions: HashMap, Arc>, + pub(crate) external_functions: HashMap, Arc>>, /// A hashmap containing all script-defined functions - pub(crate) script_functions: HashMap, Arc>, + pub(crate) script_functions: HashMap, Arc>>, /// A hashmap containing all iterators known to the engine pub(crate) type_iterators: HashMap>, pub(crate) type_names: HashMap, - pub(crate) on_print: Box, - pub(crate) on_debug: Box, + pub(crate) on_print: Box, + pub(crate) on_debug: Box, } -pub enum FnIntExt { +pub enum FnIntExt<'a> { Ext(Box), - Int(FnDef), + Int(FnDef<'a>), } pub type FnAny = dyn Fn(FnCallArgs, Position) -> Result; @@ -104,27 +104,27 @@ impl Engine<'_> { if let Some(f) = fn_def { match *f { - FnIntExt::Ext(ref f) => { - let r = f(args, pos)?; + FnIntExt::Ext(ref func) => { + let result = func(args, pos)?; let callback = match spec.name.as_ref() { KEYWORD_PRINT => self.on_print.as_mut(), KEYWORD_DEBUG => self.on_debug.as_mut(), - _ => return Ok(r), + _ => return Ok(result), }; - Ok(callback( - &r.downcast::() - .map(|s| *s) - .unwrap_or("error: not a string".into()), - ) - .into_dynamic()) + let val = &result + .downcast::() + .map(|s| *s) + .unwrap_or("error: not a string".into()); + + Ok(callback(val).into_dynamic()) } - FnIntExt::Int(ref f) => { - if f.params.len() != args.len() { + FnIntExt::Int(ref func) => { + if func.params.len() != args.len() { return Err(EvalAltResult::ErrorFunctionArgsMismatch( spec.name.into(), - f.params.len(), + func.params.len(), args.len(), pos, )); @@ -133,13 +133,13 @@ impl Engine<'_> { let mut scope = Scope::new(); scope.extend( - f.params + func.params .iter() - .cloned() + .map(|s| s.clone()) .zip(args.iter().map(|x| (*x).into_dynamic())), ); - match self.eval_stmt(&mut scope, &*f.body) { + match self.eval_stmt(&mut scope, &*func.body) { Err(EvalAltResult::Return(x, _)) => Ok(x), other => other, } @@ -319,12 +319,12 @@ impl Engine<'_> { } /// Evaluate an index expression - fn eval_index_expr( + fn eval_index_expr<'a>( &mut self, scope: &mut Scope, - lhs: &Expr, + lhs: &'a Expr, idx_expr: &Expr, - ) -> Result<(IndexSourceType, Option<(String, usize)>, usize, Dynamic), EvalAltResult> { + ) -> Result<(IndexSourceType, Option<(&'a str, usize)>, usize, Dynamic), EvalAltResult> { let idx = self.eval_index_value(scope, idx_expr)?; match lhs { @@ -336,7 +336,7 @@ impl Engine<'_> { lhs.position(), ) .map(|(src_idx, (val, src_type))| { - (src_type, Some((id.clone(), src_idx)), idx as usize, val) + (src_type, Some((id.as_str(), src_idx)), idx as usize, val) }), // (expr)[idx_expr] @@ -371,7 +371,8 @@ impl Engine<'_> { // array_id[idx] = val IndexSourceType::Array => { let arr = scope.get_mut_by_type::(id, src_idx); - (arr[idx as usize] = val).into_dynamic() + arr[idx as usize] = val; + ().into_dynamic() } // string_id[idx] = val @@ -381,7 +382,8 @@ impl Engine<'_> { let ch = *val .downcast::() .expect("char value expected to update an index position in a string"); - Self::str_replace_char(s, idx as usize, ch).into_dynamic() + Self::str_replace_char(s, idx as usize, ch); + ().into_dynamic() } // All other variable types should be an error @@ -419,7 +421,7 @@ impl Engine<'_> { // of the above `clone`. if let Some((id, src_idx)) = src { Self::update_indexed_variable_in_scope( - src_type, scope, &id, src_idx, idx, target, + src_type, scope, id, src_idx, idx, target, ); } @@ -510,7 +512,7 @@ impl Engine<'_> { if let Some((id, src_idx)) = src { Self::update_indexed_variable_in_scope( - src_type, scope, &id, src_idx, idx, target, + src_type, scope, id, src_idx, idx, target, ); } @@ -525,10 +527,10 @@ impl Engine<'_> { /// Evaluate an expression fn eval_expr(&mut self, scope: &mut Scope, expr: &Expr) -> Result { match expr { - Expr::IntegerConstant(i, _) => Ok((*i).into_dynamic()), - Expr::FloatConstant(i, _) => Ok((*i).into_dynamic()), + Expr::IntegerConstant(i, _) => Ok(i.into_dynamic()), + Expr::FloatConstant(f, _) => Ok(f.into_dynamic()), Expr::StringConstant(s, _) => Ok(s.into_dynamic()), - Expr::CharConstant(c, _) => Ok((*c).into_dynamic()), + Expr::CharConstant(c, _) => Ok(c.into_dynamic()), Expr::Identifier(id, pos) => { Self::search_scope(scope, id, Ok, *pos).map(|(_, val)| val) } diff --git a/src/error.rs b/src/error.rs index 3096040e..52ca89fc 100644 --- a/src/error.rs +++ b/src/error.rs @@ -75,6 +75,8 @@ pub enum ParseErrorType { FnMissingName, /// A function definition is missing the parameters list. Wrapped value is the function name. FnMissingParams(String), + /// Assignment to an inappropriate LHS (left-hand-side) expression. + AssignmentToInvalidLHS, } /// Error when parsing a script. @@ -114,6 +116,7 @@ impl Error for ParseError { ParseErrorType::FnMissingName => "Expecting name in function declaration", ParseErrorType::FnMissingParams(_) => "Expecting parameters in function declaration", ParseErrorType::WrongFnDefinition => "Function definitions must be at top level and cannot be inside a block or another function", + ParseErrorType::AssignmentToInvalidLHS => "Assignment to an unsupported left-hand side expression" } } diff --git a/src/parser.rs b/src/parser.rs index 204998b0..18052ee9 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2,7 +2,7 @@ use crate::any::Dynamic; use crate::error::{LexError, ParseError, ParseErrorType}; use std::char; use std::iter::Peekable; -use std::str::Chars; +use std::{borrow::Cow, str::Chars}; type LERR = LexError; type PERR = ParseErrorType; @@ -99,12 +99,12 @@ impl std::fmt::Debug for Position { } /// Compiled AST (abstract syntax tree) of a Rhai script. -pub struct AST(pub(crate) Vec, pub(crate) Vec); +pub struct AST(pub(crate) Vec, pub(crate) Vec>); #[derive(Debug, Clone)] -pub struct FnDef { - pub name: String, - pub params: Vec, +pub struct FnDef<'a> { + pub name: Cow<'a, str>, + pub params: Vec>, pub body: Box, pub pos: Position, } @@ -232,14 +232,14 @@ pub enum Token { } impl Token { - pub fn syntax(&self) -> std::borrow::Cow<'static, str> { + pub fn syntax<'a>(&'a self) -> Cow<'a, str> { use self::Token::*; match *self { - IntegerConstant(ref s) => s.to_string().into(), - FloatConstant(ref s) => s.to_string().into(), - Identifier(ref s) => s.to_string().into(), - CharConstant(ref s) => s.to_string().into(), + IntegerConstant(ref i) => i.to_string().into(), + FloatConstant(ref f) => f.to_string().into(), + Identifier(ref s) => s.into(), + CharConstant(ref c) => c.to_string().into(), LexError(ref err) => err.to_string().into(), ref token => (match token { @@ -301,7 +301,7 @@ impl Token { PowerOfAssign => "~=", For => "for", In => "in", - _ => panic!(), + _ => panic!("operator should be match in outer scope"), }) .into(), } @@ -538,8 +538,7 @@ impl<'a> TokenIterator<'a> { } } - let out: String = result.iter().collect(); - Ok(out) + Ok(result.iter().collect()) } fn inner_next(&mut self) -> Option<(Token, Position)> { @@ -640,20 +639,20 @@ impl<'a> TokenIterator<'a> { }, pos, )); + } else { + let out: String = result.iter().filter(|&&c| c != '_').collect(); + + return Some(( + if let Ok(val) = out.parse::() { + Token::IntegerConstant(val) + } else if let Ok(val) = out.parse::() { + Token::FloatConstant(val) + } else { + Token::LexError(LERR::MalformedNumber(result.iter().collect())) + }, + pos, + )); } - - let out: String = result.iter().filter(|&&c| c != '_').collect(); - - return Some(( - if let Ok(val) = out.parse::() { - Token::IntegerConstant(val) - } else if let Ok(val) = out.parse::() { - Token::FloatConstant(val) - } else { - Token::LexError(LERR::MalformedNumber(result.iter().collect())) - }, - pos, - )); } 'A'..='Z' | 'a'..='z' | '_' => { let mut result = Vec::new(); @@ -687,7 +686,7 @@ impl<'a> TokenIterator<'a> { "fn" => Token::Fn, "for" => Token::For, "in" => Token::In, - x => Token::Identifier(x.into()), + _ => Token::Identifier(out), }, pos, )); @@ -1264,6 +1263,21 @@ fn parse_unary<'a>(input: &mut Peekable>) -> Result Result { + match lhs { + // Only assignments to a variable, and index erxpression and a dot expression is valid LHS + Expr::Identifier(_, _) | Expr::Index(_, _) | Expr::Dot(_, _) => { + Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs))) + } + + // All other LHS cannot be assigned to + _ => Err(ParseError::new( + PERR::AssignmentToInvalidLHS, + lhs.position(), + )), + } +} + fn parse_binary_op<'a>( input: &mut Peekable>, precedence: i8, @@ -1308,7 +1322,7 @@ fn parse_binary_op<'a>( } Token::Divide => Expr::FunctionCall("/".into(), vec![current_lhs, rhs], None, pos), - Token::Equals => Expr::Assignment(Box::new(current_lhs), Box::new(rhs)), + Token::Equals => parse_assignment(current_lhs, rhs)?, Token::PlusAssign => { let lhs_copy = current_lhs.clone(); Expr::Assignment( @@ -1687,7 +1701,7 @@ fn parse_stmt<'a>(input: &mut Peekable>) -> Result(input: &mut Peekable>) -> Result { +fn parse_fn<'a>(input: &mut Peekable>) -> Result, ParseError> { let pos = match input.next() { Some((_, tok_pos)) => tok_pos, _ => return Err(ParseError::new(PERR::InputPastEndOfFile, Position::eof())), @@ -1723,7 +1737,7 @@ fn parse_fn<'a>(input: &mut Peekable>) -> Result break, Some((Token::Comma, _)) => (), Some((Token::Identifier(s), _)) => { - params.push(s); + params.push(s.into()); } Some((_, pos)) => return Err(ParseError::new(PERR::MalformedCallExpr, pos)), None => return Err(ParseError::new(PERR::MalformedCallExpr, Position::eof())), @@ -1734,7 +1748,7 @@ fn parse_fn<'a>(input: &mut Peekable>) -> Result); +pub struct Scope<'a>(Vec<(Cow<'a, str>, Dynamic)>); -impl Scope { +impl<'a> Scope<'a> { /// Create a new Scope. pub fn new() -> Self { Self(Vec::new()) @@ -36,18 +37,18 @@ impl Scope { } /// Add (push) a new variable to the Scope. - pub fn push(&mut self, key: String, value: T) { - self.0.push((key, Box::new(value))); + pub fn push>, T: Any>(&mut self, key: K, value: T) { + self.0.push((key.into(), Box::new(value))); } /// Add (push) a new variable to the Scope. - pub(crate) fn push_dynamic(&mut self, key: String, value: Dynamic) { - self.0.push((key, value)); + pub(crate) fn push_dynamic>>(&mut self, key: K, value: Dynamic) { + self.0.push((key.into(), value)); } /// Remove (pop) the last variable from the Scope. pub fn pop(&mut self) -> Option<(String, Dynamic)> { - self.0.pop() + self.0.pop().map(|(key, value)| (key.to_string(), value)) } /// Truncate (rewind) the Scope to a previous size. @@ -56,13 +57,13 @@ impl Scope { } /// Find a variable in the Scope, starting from the last. - pub fn get(&self, key: &str) -> Option<(usize, String, Dynamic)> { + pub fn get(&self, key: &str) -> Option<(usize, &str, Dynamic)> { self.0 .iter() .enumerate() .rev() // Always search a Scope in reverse order .find(|(_, (name, _))| name == key) - .map(|(i, (name, value))| (i, name.clone(), value.clone())) + .map(|(i, (name, value))| (i, name.as_ref(), value.clone())) } /// Get the value of a variable in the Scope, starting from the last. @@ -97,20 +98,26 @@ impl Scope { self.0 .iter() .rev() // Always search a Scope in reverse order - .map(|(key, value)| (key.as_str(), value)) + .map(|(key, value)| (key.as_ref(), value)) } + /* /// Get a mutable iterator to variables in the Scope. pub fn iter_mut(&mut self) -> impl Iterator { self.0 .iter_mut() .rev() // Always search a Scope in reverse order - .map(|(key, value)| (key.as_str(), value)) + .map(|(key, value)| (key.as_ref(), value)) } + */ } -impl std::iter::Extend<(String, Dynamic)> for Scope { - fn extend>(&mut self, iter: T) { - self.0.extend(iter); +impl<'a, K> std::iter::Extend<(K, Dynamic)> for Scope<'a> +where + K: Into>, +{ + fn extend>(&mut self, iter: T) { + self.0 + .extend(iter.into_iter().map(|(key, value)| (key.into(), value))); } } diff --git a/tests/var_scope.rs b/tests/var_scope.rs index 5522ba67..d1f0c24e 100644 --- a/tests/var_scope.rs +++ b/tests/var_scope.rs @@ -25,8 +25,8 @@ fn test_scope_eval() -> Result<(), EvalAltResult> { // Then push some initialized variables into the state // NOTE: Remember the default numbers used by Rhai are i64 and f64. // Better stick to them or it gets hard to work with other variables in the script. - scope.push("y".into(), 42_i64); - scope.push("z".into(), 999_i64); + scope.push("y", 42_i64); + scope.push("z", 999_i64); // First invocation engine