From 6351c07bc6caecd2b7f232b057e8f4b7a2829b7f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 28 Apr 2020 19:39:28 +0800 Subject: [PATCH 1/4] Fix compiling for all features. --- src/packages/map_basic.rs | 6 ++---- src/packages/string_more.rs | 2 +- src/packages/time_basic.rs | 4 ++-- src/scope.rs | 4 ++-- tests/maps.rs | 1 + 5 files changed, 8 insertions(+), 9 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/src/scope.rs b/src/scope.rs index 2f6ca0c2..7bb40325 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, boxed::Box, iter, vec::Vec}; /// Type of an entry in the Scope. #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -362,7 +362,7 @@ impl<'a> Scope<'a> { } /// Get an iterator to entries in the Scope. - pub fn iter(&self) -> impl Iterator { + pub(crate) fn iter(&self) -> impl Iterator { self.0.iter().rev() // Always search a Scope in reverse order } } 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(); From 9ad9dd52ab5a860386eacedcab756762786db11b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 28 Apr 2020 19:39:36 +0800 Subject: [PATCH 2/4] Reduce unnecessary Cow's in Expr. --- src/engine.rs | 6 +++--- src/optimize.rs | 20 ++++++++------------ src/parser.rs | 49 +++++++++++++++++++++++++------------------------ 3 files changed, 36 insertions(+), 39 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 4fd38ab6..f38cb8c4 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -893,7 +893,7 @@ impl Engine { match dot_lhs { // id.??? or id[???] - Expr::Variable(id, pos) => { + Expr::Variable(id, _, pos) => { let (target, typ) = search_scope(scope, id, *pos)?; // Constants cannot be modified @@ -1129,7 +1129,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 @@ -1141,7 +1141,7 @@ impl Engine { match lhs.as_ref() { // name = rhs - Expr::Variable(name, pos) => match scope.get(name) { + Expr::Variable(name, _, pos) => match scope.get(name) { None => { return Err(Box::new(EvalAltResult::ErrorVariableNotFound( name.to_string(), diff --git a/src/optimize.rs b/src/optimize.rs index 3f00e711..f61bef36 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -368,12 +368,12 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { //id = id2 = expr2 Expr::Assignment(id2, expr2, pos2) => match (*id, *id2) { // var = var = expr2 -> var = expr2 - (Expr::Variable(var, _), Expr::Variable(var2, _)) if var == var2 => { + (Expr::Variable(var, sp, _), Expr::Variable(var2, sp2, _)) if var == var2 && sp == sp2 => { // Assignment to the same variable - fold state.set_dirty(); Expr::Assignment( - Box::new(Expr::Variable(var, pos)), + Box::new(Expr::Variable(var, sp, pos)), Box::new(optimize_expr(*expr2, state)), pos, ) @@ -397,13 +397,11 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { #[cfg(not(feature = "no_object"))] Expr::Dot(lhs, rhs, pos) => match (*lhs, *rhs) { // map.string - (Expr::Map(items, pos), Expr::Property(s, _)) - if items.iter().all(|(_, x, _)| x.is_pure()) => - { + (Expr::Map(items, pos), Expr::Property(s, _)) if items.iter().all(|(_, x, _)| x.is_pure()) => { // Map literal where everything is pure - promote the indexed item. // All other items can be thrown away. state.set_dirty(); - items.into_iter().find(|(name, _, _)| name == s.as_ref()) + items.into_iter().find(|(name, _, _)| name == &s) .map(|(_, expr, _)| expr.set_position(pos)) .unwrap_or_else(|| Expr::Unit(pos)) } @@ -428,13 +426,11 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { items.remove(i as usize).set_position(pos) } // map[string] - (Expr::Map(items, pos), Expr::StringConstant(s, _)) - if items.iter().all(|(_, x, _)| x.is_pure()) => - { + (Expr::Map(items, pos), Expr::StringConstant(s, _)) if items.iter().all(|(_, x, _)| x.is_pure()) => { // Map literal where everything is pure - promote the indexed item. // All other items can be thrown away. state.set_dirty(); - items.into_iter().find(|(name, _, _)| name == s.as_ref()) + items.into_iter().find(|(name, _, _)| name == &s) .map(|(_, expr, _)| expr.set_position(pos)) .unwrap_or_else(|| Expr::Unit(pos)) } @@ -470,7 +466,7 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { // "xxx" in "xxxxx" (Expr::StringConstant(lhs, pos), Expr::StringConstant(rhs, _)) => { state.set_dirty(); - if rhs.contains(lhs.as_ref()) { + if rhs.contains(&lhs) { Expr::True(pos) } else { Expr::False(pos) @@ -613,7 +609,7 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { Expr::FunctionCall(id, args.into_iter().map(|a| optimize_expr(a, state)).collect(), def_value, pos), // constant-name - Expr::Variable(name, pos) if state.contains_constant(&name) => { + Expr::Variable(name, _, pos) if state.contains_constant(&name) => { state.set_dirty(); // Replace constant with value diff --git a/src/parser.rs b/src/parser.rs index 36cf4fd0..fd58cff0 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -12,7 +12,6 @@ use crate::stdlib::{ boxed::Box, char, collections::HashMap, - fmt::Display, format, iter::Peekable, ops::Add, @@ -196,11 +195,11 @@ pub enum Stmt { /// loop { stmt } Loop(Box), /// for id in expr { stmt } - For(Cow<'static, str>, Box, Box), + For(String, Box, Box), /// let id = expr - Let(Cow<'static, str>, Option>, Position), + Let(String, Option>, Position), /// const id = expr - Const(Cow<'static, str>, Box, Position), + Const(String, Box, Position), /// { stmt; ... } Block(Vec, Position), /// { stmt } @@ -280,14 +279,16 @@ pub enum Expr { /// Character constant. CharConstant(char, Position), /// String constant. - StringConstant(Cow<'static, str>, Position), + StringConstant(String, Position), /// Variable access. - Variable(Cow<'static, str>, Position), + Variable(String, usize, Position), /// Property access. - Property(Cow<'static, str>, Position), + Property(String, Position), /// { stmt } Stmt(Box, Position), /// func(expr, ... ) + /// Use `Cow<'static, str>` because a lot of operators (e.g. `==`, `>=`) are implemented as function calls + /// and the function names are predictable, so no need to allocate a new `String`. FunctionCall(Cow<'static, str>, Vec, Option, Position), /// expr = expr Assignment(Box, Box, Position), @@ -325,7 +326,7 @@ impl Expr { #[cfg(not(feature = "no_float"))] Self::FloatConstant(f, _) => (*f).into(), Self::CharConstant(c, _) => (*c).into(), - Self::StringConstant(s, _) => s.to_string().into(), + Self::StringConstant(s, _) => s.clone().into(), Self::True(_) => true.into(), Self::False(_) => false.into(), Self::Unit(_) => ().into(), @@ -382,7 +383,7 @@ impl Expr { | Self::StringConstant(_, pos) | Self::Array(_, pos) | Self::Map(_, pos) - | Self::Variable(_, pos) + | Self::Variable(_, _, pos) | Self::Property(_, pos) | Self::Stmt(_, pos) | Self::FunctionCall(_, _, _, pos) @@ -408,7 +409,7 @@ impl Expr { | Self::StringConstant(_, pos) | Self::Array(_, pos) | Self::Map(_, pos) - | Self::Variable(_, pos) + | Self::Variable(_, _, pos) | Self::Property(_, pos) | Self::Stmt(_, pos) | Self::FunctionCall(_, _, _, pos) @@ -439,7 +440,7 @@ impl Expr { Self::Stmt(stmt, _) => stmt.is_pure(), - Self::Variable(_, _) => true, + Self::Variable(_, _, _) => true, expr => expr.is_constant(), } @@ -498,7 +499,7 @@ impl Expr { _ => false, }, - Self::Variable(_, _) | Self::Property(_, _) => match token { + Self::Variable(_, _, _) | Self::Property(_, _) => match token { Token::LeftBracket | Token::LeftParen => true, _ => false, }, @@ -508,7 +509,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), + Self::Variable(id, _, pos) => Self::Property(id, pos), _ => self, } } @@ -567,8 +568,8 @@ fn parse_paren_expr<'a>( } /// Parse a function call. -fn parse_call_expr<'a, S: Into> + Display>( - id: S, +fn parse_call_expr<'a>( + id: String, input: &mut Peekable>, begin: Position, allow_stmt_expr: bool, @@ -916,8 +917,8 @@ fn parse_primary<'a>( #[cfg(not(feature = "no_float"))] Token::FloatConstant(x) => Expr::FloatConstant(x, pos), Token::CharConstant(c) => Expr::CharConstant(c, pos), - Token::StringConst(s) => Expr::StringConstant(s.into(), pos), - Token::Identifier(s) => Expr::Variable(s.into(), pos), + Token::StringConst(s) => Expr::StringConstant(s, pos), + Token::Identifier(s) => Expr::Variable(s, 0, pos), Token::LeftParen => parse_paren_expr(input, pos, allow_stmt_expr)?, #[cfg(not(feature = "no_index"))] Token::LeftBracket => parse_array_literal(input, pos, allow_stmt_expr)?, @@ -943,7 +944,7 @@ fn parse_primary<'a>( root_expr = match (root_expr, token) { // Function call - (Expr::Variable(id, pos), Token::LeftParen) + (Expr::Variable(id, _, pos), Token::LeftParen) | (Expr::Property(id, pos), Token::LeftParen) => { parse_call_expr(id, input, pos, allow_stmt_expr)? } @@ -1078,7 +1079,7 @@ fn make_dot_expr(lhs: Expr, rhs: Expr, op_pos: Position, is_index: bool) -> Expr idx_pos, ), // lhs.id - (lhs, rhs @ Expr::Variable(_, _)) | (lhs, rhs @ Expr::Property(_, _)) => { + (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) } @@ -1479,7 +1480,7 @@ fn parse_for<'a>( let expr = parse_expr(input, allow_stmt_expr)?; let body = parse_block(input, true, allow_stmt_expr)?; - Ok(Stmt::For(name.into(), Box::new(expr), Box::new(body))) + Ok(Stmt::For(name, Box::new(expr), Box::new(body))) } /// Parse a variable definition statement. @@ -1505,10 +1506,10 @@ fn parse_let<'a>( match var_type { // let name = expr - ScopeEntryType::Normal => Ok(Stmt::Let(name.into(), Some(Box::new(init_value)), pos)), + ScopeEntryType::Normal => Ok(Stmt::Let(name, Some(Box::new(init_value)), pos)), // const name = { expr:constant } ScopeEntryType::Constant if init_value.is_constant() => { - Ok(Stmt::Const(name.into(), Box::new(init_value), pos)) + Ok(Stmt::Const(name, Box::new(init_value), pos)) } // const name = expr - error ScopeEntryType::Constant => { @@ -1517,7 +1518,7 @@ fn parse_let<'a>( } } else { // let name - Ok(Stmt::Let(name.into(), None, pos)) + Ok(Stmt::Let(name, None, pos)) } } @@ -1840,7 +1841,7 @@ pub fn map_dynamic_to_expr(value: Dynamic, pos: Position) -> Option { Union::Unit(_) => Some(Expr::Unit(pos)), Union::Int(value) => Some(Expr::IntegerConstant(value, pos)), Union::Char(value) => Some(Expr::CharConstant(value, pos)), - Union::Str(value) => Some(Expr::StringConstant((*value).into(), pos)), + Union::Str(value) => Some(Expr::StringConstant((*value).clone(), pos)), Union::Bool(true) => Some(Expr::True(pos)), Union::Bool(false) => Some(Expr::False(pos)), #[cfg(not(feature = "no_index"))] From 304c658f89ea8a78a259f0c061ebe6684670eed3 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 28 Apr 2020 23:05:03 +0800 Subject: [PATCH 3/4] Use scope offset for variable access. --- src/api.rs | 10 +- src/engine.rs | 152 +++++++++++++++--------- src/optimize.rs | 10 +- src/parser.rs | 303 +++++++++++++++++++++++++++++++++--------------- src/scope.rs | 4 +- tests/eval.rs | 7 +- 6 files changed, 328 insertions(+), 158 deletions(-) diff --git a/src/api.rs b/src/api.rs index 0f367f14..9377bc68 100644 --- a/src/api.rs +++ b/src/api.rs @@ -1,7 +1,7 @@ //! Module that defines the extern API of `Engine`. use crate::any::{Dynamic, Variant}; -use crate::engine::{make_getter, make_setter, Engine, Map}; +use crate::engine::{make_getter, make_setter, Engine, Map, State}; use crate::error::ParseError; use crate::fn_call::FuncArgs; use crate::fn_register::RegisterFn; @@ -795,10 +795,12 @@ impl Engine { scope: &mut Scope, ast: &AST, ) -> Result> { + let mut state = State::new(); + ast.0 .iter() .try_fold(().into(), |_, stmt| { - self.eval_stmt(scope, ast.1.as_ref(), stmt, 0) + self.eval_stmt(scope, &mut state, ast.1.as_ref(), stmt, 0) }) .or_else(|err| match *err { EvalAltResult::Return(out, _) => Ok(out), @@ -858,10 +860,12 @@ impl Engine { scope: &mut Scope, ast: &AST, ) -> Result<(), Box> { + let mut state = State::new(); + ast.0 .iter() .try_fold(().into(), |_, stmt| { - self.eval_stmt(scope, ast.1.as_ref(), stmt, 0) + self.eval_stmt(scope, &mut state, ast.1.as_ref(), stmt, 0) }) .map_or_else( |err| match *err { diff --git a/src/engine.rs b/src/engine.rs index f38cb8c4..85d5a031 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -188,6 +188,23 @@ impl StaticVec { } } +/// A type that holds all the current states of the Engine. +#[derive(Debug, Clone, Hash, Copy)] +pub struct State { + /// Normally, access to variables are parsed with a relative offset into the scope to avoid a lookup. + /// In some situation, e.g. after running an `eval` statement, subsequent offsets may become mis-aligned. + /// When that happens, this flag is turned on to force a scope lookup by name. + pub always_search: bool, +} + +impl State { + pub fn new() -> Self { + Self { + always_search: false, + } + } +} + /// A type that holds a library (`HashMap`) of script-defined functions. /// /// Since script-defined functions have `Dynamic` parameters, functions with the same name @@ -439,11 +456,11 @@ fn search_scope<'a>( name: &str, begin: Position, ) -> Result<(&'a mut Dynamic, ScopeEntryType), Box> { - let (index, typ) = scope + let (index, _) = scope .get(name) .ok_or_else(|| Box::new(EvalAltResult::ErrorVariableNotFound(name.into(), begin)))?; - Ok((scope.get_mut(index), typ)) + Ok(scope.get_mut(index)) } impl Engine { @@ -601,6 +618,7 @@ impl Engine { // Extern scope passed in which is not empty Some(scope) if scope.len() > 0 => { let scope_len = scope.len(); + let mut state = State::new(); scope.extend( // Put arguments into scope as variables - variable name is copied @@ -614,7 +632,7 @@ impl Engine { // Evaluate the function at one higher level of call depth let result = self - .eval_stmt(scope, fn_lib, &fn_def.body, level + 1) + .eval_stmt(scope, &mut state, fn_lib, &fn_def.body, level + 1) .or_else(|err| match *err { // Convert return statement to return value EvalAltResult::Return(x, _) => Ok(x), @@ -628,6 +646,7 @@ impl Engine { // No new scope - create internal scope _ => { let mut scope = Scope::new(); + let mut state = State::new(); scope.extend( // Put arguments into scope as variables @@ -640,7 +659,7 @@ impl Engine { // Evaluate the function at one higher level of call depth return self - .eval_stmt(&mut scope, fn_lib, &fn_def.body, level + 1) + .eval_stmt(&mut scope, &mut state, fn_lib, &fn_def.body, level + 1) .or_else(|err| match *err { // Convert return statement to return value EvalAltResult::Return(x, _) => Ok(x), @@ -784,7 +803,7 @@ impl Engine { let mut args: Vec<_> = once(obj) .chain(idx_val.downcast_mut::().unwrap().iter_mut()) .collect(); - let def_val = def_val.as_ref(); + let def_val = def_val.as_deref(); // 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)) @@ -879,6 +898,7 @@ impl Engine { fn eval_dot_index_chain( &self, scope: &mut Scope, + state: &mut State, fn_lib: &FunctionsLib, dot_lhs: &Expr, dot_rhs: &Expr, @@ -889,12 +909,16 @@ impl Engine { ) -> Result> { let idx_values = &mut StaticVec::new(); - self.eval_indexed_chain(scope, fn_lib, dot_rhs, idx_values, 0, level)?; + self.eval_indexed_chain(scope, state, fn_lib, dot_rhs, idx_values, 0, level)?; match dot_lhs { // id.??? or id[???] - Expr::Variable(id, _, pos) => { - let (target, typ) = search_scope(scope, id, *pos)?; + Expr::Variable(id, index, pos) => { + let (target, typ) = if !state.always_search && *index > 0 { + scope.get_mut(scope.len() - *index) + } else { + search_scope(scope, id, *pos)? + }; // Constants cannot be modified match typ { @@ -927,7 +951,7 @@ impl Engine { } // {expr}.??? or {expr}[???] expr => { - let val = self.eval_expr(scope, fn_lib, expr, level)?; + let val = self.eval_expr(scope, state, fn_lib, expr, level)?; self.eval_dot_index_chain_helper( fn_lib, @@ -952,6 +976,7 @@ impl Engine { fn eval_indexed_chain( &self, scope: &mut Scope, + state: &mut State, fn_lib: &FunctionsLib, expr: &Expr, idx_values: &mut StaticVec, @@ -962,7 +987,7 @@ impl Engine { Expr::FunctionCall(_, arg_exprs, _, _) => { let arg_values = arg_exprs .iter() - .map(|arg_expr| self.eval_expr(scope, fn_lib, arg_expr, level)) + .map(|arg_expr| self.eval_expr(scope, state, fn_lib, arg_expr, level)) .collect::, _>>()?; idx_values.push(arg_values) @@ -973,15 +998,15 @@ impl Engine { // Evaluate in left-to-right order let lhs_val = match lhs.as_ref() { Expr::Property(_, _) => ().into(), // Store a placeholder in case of a property - _ => self.eval_expr(scope, fn_lib, lhs, level)?, + _ => self.eval_expr(scope, state, fn_lib, lhs, level)?, }; // Push in reverse order - self.eval_indexed_chain(scope, fn_lib, rhs, idx_values, size, level)?; + self.eval_indexed_chain(scope, state, fn_lib, rhs, idx_values, size, level)?; idx_values.push(lhs_val); } - _ => idx_values.push(self.eval_expr(scope, fn_lib, expr, level)?), + _ => idx_values.push(self.eval_expr(scope, state, fn_lib, expr, level)?), } Ok(()) @@ -1066,13 +1091,14 @@ impl Engine { fn eval_in_expr( &self, scope: &mut Scope, + state: &mut State, fn_lib: &FunctionsLib, lhs: &Expr, rhs: &Expr, level: usize, ) -> Result> { - let mut lhs_value = self.eval_expr(scope, fn_lib, lhs, level)?; - let rhs_value = self.eval_expr(scope, fn_lib, rhs, level)?; + let mut lhs_value = self.eval_expr(scope, state, fn_lib, lhs, level)?; + let rhs_value = self.eval_expr(scope, state, fn_lib, rhs, level)?; match rhs_value { Dynamic(Union::Array(mut rhs_value)) => { @@ -1119,6 +1145,7 @@ impl Engine { fn eval_expr( &self, scope: &mut Scope, + state: &mut State, fn_lib: &FunctionsLib, expr: &Expr, level: usize, @@ -1129,15 +1156,18 @@ impl Engine { Expr::FloatConstant(f, _) => Ok((*f).into()), Expr::StringConstant(s, _) => Ok(s.to_string().into()), Expr::CharConstant(c, _) => Ok((*c).into()), + Expr::Variable(_, index, _) if !state.always_search && *index > 0 => { + Ok(scope.get_mut(scope.len() - *index).0.clone()) + } Expr::Variable(id, _, pos) => search_scope(scope, id, *pos).map(|(v, _)| v.clone()), Expr::Property(_, _) => panic!("unexpected property."), // Statement block - Expr::Stmt(stmt, _) => self.eval_stmt(scope, fn_lib, stmt, level), + Expr::Stmt(stmt, _) => self.eval_stmt(scope, state, fn_lib, stmt, level), // lhs = rhs Expr::Assignment(lhs, rhs, op_pos) => { - let rhs_val = self.eval_expr(scope, fn_lib, rhs, level)?; + let rhs_val = self.eval_expr(scope, state, fn_lib, rhs, level)?; match lhs.as_ref() { // name = rhs @@ -1151,7 +1181,7 @@ impl Engine { Some((index, ScopeEntryType::Normal)) => { // Avoid referencing scope which is used below as mut - *scope.get_mut(index) = rhs_val.clone(); + *scope.get_mut(index).0 = rhs_val.clone(); Ok(rhs_val) } @@ -1165,7 +1195,7 @@ impl Engine { Expr::Index(idx_lhs, idx_expr, op_pos) => { let new_val = Some(rhs_val); self.eval_dot_index_chain( - scope, fn_lib, idx_lhs, idx_expr, true, *op_pos, level, new_val, + scope, state, fn_lib, idx_lhs, idx_expr, true, *op_pos, level, new_val, ) } // dot_lhs.dot_rhs = rhs @@ -1173,7 +1203,7 @@ impl Engine { Expr::Dot(dot_lhs, dot_rhs, _) => { let new_val = Some(rhs_val); self.eval_dot_index_chain( - scope, fn_lib, dot_lhs, dot_rhs, false, *op_pos, level, new_val, + scope, state, fn_lib, dot_lhs, dot_rhs, false, *op_pos, level, new_val, ) } // Error assignment to constant @@ -1193,22 +1223,22 @@ impl Engine { // lhs[idx_expr] #[cfg(not(feature = "no_index"))] - Expr::Index(lhs, idx_expr, op_pos) => { - self.eval_dot_index_chain(scope, fn_lib, lhs, idx_expr, true, *op_pos, level, None) - } + Expr::Index(lhs, idx_expr, op_pos) => self.eval_dot_index_chain( + scope, state, fn_lib, lhs, idx_expr, true, *op_pos, level, None, + ), // lhs.dot_rhs #[cfg(not(feature = "no_object"))] - Expr::Dot(lhs, dot_rhs, op_pos) => { - self.eval_dot_index_chain(scope, fn_lib, lhs, dot_rhs, false, *op_pos, level, None) - } + Expr::Dot(lhs, dot_rhs, op_pos) => self.eval_dot_index_chain( + scope, state, fn_lib, lhs, dot_rhs, false, *op_pos, level, None, + ), #[cfg(not(feature = "no_index"))] Expr::Array(contents, _) => { let mut arr = Array::new(); contents.into_iter().try_for_each(|item| { - self.eval_expr(scope, fn_lib, item, level) + self.eval_expr(scope, state, fn_lib, item, level) .map(|val| arr.push(val)) })?; @@ -1220,9 +1250,10 @@ impl Engine { let mut map = Map::new(); contents.into_iter().try_for_each(|(key, expr, _)| { - self.eval_expr(scope, fn_lib, &expr, level).map(|val| { - map.insert(key.clone(), val); - }) + self.eval_expr(scope, state, fn_lib, &expr, level) + .map(|val| { + map.insert(key.clone(), val); + }) })?; Ok(Dynamic(Union::Map(Box::new(map)))) @@ -1231,7 +1262,7 @@ impl Engine { Expr::FunctionCall(fn_name, arg_exprs, def_val, pos) => { let mut arg_values = arg_exprs .iter() - .map(|expr| self.eval_expr(scope, fn_lib, expr, level)) + .map(|expr| self.eval_expr(scope, state, fn_lib, expr, level)) .collect::, _>>()?; let mut args: Vec<_> = arg_values.iter_mut().collect(); @@ -1242,27 +1273,34 @@ impl Engine { && !self.has_override(fn_lib, KEYWORD_EVAL) { // Evaluate the text string as a script - return self.eval_script_expr(scope, fn_lib, args[0], arg_exprs[0].position()); + let result = + self.eval_script_expr(scope, fn_lib, args[0], arg_exprs[0].position()); + + // IMPORTANT! The eval may define new variables in the current scope! + // We can no longer trust the variable offsets. + state.always_search = true; + + return result; } // Normal function call - let def_val = def_val.as_ref(); + let def_val = def_val.as_deref(); self.exec_fn_call(fn_lib, fn_name, &mut args, def_val, *pos, level) } Expr::In(lhs, rhs, _) => { - self.eval_in_expr(scope, fn_lib, lhs.as_ref(), rhs.as_ref(), level) + self.eval_in_expr(scope, state, fn_lib, lhs.as_ref(), rhs.as_ref(), level) } Expr::And(lhs, rhs, _) => Ok((self - .eval_expr(scope, fn_lib,lhs.as_ref(), level)? + .eval_expr(scope, state, fn_lib, lhs.as_ref(), level)? .as_bool() .map_err(|_| { EvalAltResult::ErrorBooleanArgMismatch("AND".into(), lhs.position()) })? && // Short-circuit using && self - .eval_expr(scope, fn_lib,rhs.as_ref(), level)? + .eval_expr(scope, state, fn_lib, rhs.as_ref(), level)? .as_bool() .map_err(|_| { EvalAltResult::ErrorBooleanArgMismatch("AND".into(), rhs.position()) @@ -1270,14 +1308,14 @@ impl Engine { .into()), Expr::Or(lhs, rhs, _) => Ok((self - .eval_expr(scope,fn_lib, lhs.as_ref(), level)? + .eval_expr(scope, state, fn_lib, lhs.as_ref(), level)? .as_bool() .map_err(|_| { EvalAltResult::ErrorBooleanArgMismatch("OR".into(), lhs.position()) })? || // Short-circuit using || self - .eval_expr(scope,fn_lib, rhs.as_ref(), level)? + .eval_expr(scope, state, fn_lib, rhs.as_ref(), level)? .as_bool() .map_err(|_| { EvalAltResult::ErrorBooleanArgMismatch("OR".into(), rhs.position()) @@ -1296,6 +1334,7 @@ impl Engine { pub(crate) fn eval_stmt( &self, scope: &mut Scope, + state: &mut State, fn_lib: &FunctionsLib, stmt: &Stmt, level: usize, @@ -1306,7 +1345,7 @@ impl Engine { // Expression as statement Stmt::Expr(expr) => { - let result = self.eval_expr(scope, fn_lib, expr, level)?; + let result = self.eval_expr(scope, state, fn_lib, expr, level)?; Ok(if let Expr::Assignment(_, _, _) = *expr.as_ref() { // If it is an assignment, erase the result at the root @@ -1321,24 +1360,28 @@ impl Engine { let prev_len = scope.len(); let result = block.iter().try_fold(().into(), |_, stmt| { - self.eval_stmt(scope, fn_lib, stmt, level) + self.eval_stmt(scope, state, fn_lib, stmt, level) }); scope.rewind(prev_len); + // The impact of an eval statement goes away at the end of a block + // because any new variables introduced will go out of scope + state.always_search = false; + result } // If-else statement Stmt::IfThenElse(guard, if_body, else_body) => self - .eval_expr(scope, fn_lib, guard, level)? + .eval_expr(scope, state, fn_lib, guard, level)? .as_bool() .map_err(|_| Box::new(EvalAltResult::ErrorLogicGuard(guard.position()))) .and_then(|guard_val| { if guard_val { - self.eval_stmt(scope, fn_lib, if_body, level) + self.eval_stmt(scope, state, fn_lib, if_body, level) } else if let Some(stmt) = else_body { - self.eval_stmt(scope, fn_lib, stmt.as_ref(), level) + self.eval_stmt(scope, state, fn_lib, stmt.as_ref(), level) } else { Ok(().into()) } @@ -1346,8 +1389,11 @@ impl Engine { // While loop Stmt::While(guard, body) => loop { - match self.eval_expr(scope, fn_lib, guard, level)?.as_bool() { - Ok(true) => match self.eval_stmt(scope, fn_lib, body, level) { + match self + .eval_expr(scope, state, fn_lib, guard, level)? + .as_bool() + { + Ok(true) => match self.eval_stmt(scope, state, fn_lib, body, level) { Ok(_) => (), Err(err) => match *err { EvalAltResult::ErrorLoopBreak(false, _) => (), @@ -1364,7 +1410,7 @@ impl Engine { // Loop statement Stmt::Loop(body) => loop { - match self.eval_stmt(scope, fn_lib, body, level) { + match self.eval_stmt(scope, state, fn_lib, body, level) { Ok(_) => (), Err(err) => match *err { EvalAltResult::ErrorLoopBreak(false, _) => (), @@ -1376,7 +1422,7 @@ impl Engine { // For loop Stmt::For(name, expr, body) => { - let arr = self.eval_expr(scope, fn_lib, expr, level)?; + let arr = self.eval_expr(scope, state, fn_lib, expr, level)?; let tid = arr.type_id(); if let Some(iter_fn) = self.type_iterators.get(&tid).or_else(|| { @@ -1390,9 +1436,9 @@ impl Engine { let index = scope.len() - 1; for a in iter_fn(arr) { - *scope.get_mut(index) = a; + *scope.get_mut(index).0 = a; - match self.eval_stmt(scope, fn_lib, body, level) { + match self.eval_stmt(scope, state, fn_lib, body, level) { Ok(_) => (), Err(err) => match *err { EvalAltResult::ErrorLoopBreak(false, _) => (), @@ -1422,7 +1468,7 @@ impl Engine { // Return value Stmt::ReturnWithVal(Some(a), ReturnType::Return, pos) => Err(Box::new( - EvalAltResult::Return(self.eval_expr(scope, fn_lib, a, level)?, *pos), + EvalAltResult::Return(self.eval_expr(scope, state, fn_lib, a, level)?, *pos), )), // Empty throw @@ -1432,7 +1478,7 @@ impl Engine { // Throw value Stmt::ReturnWithVal(Some(a), ReturnType::Exception, pos) => { - let val = self.eval_expr(scope, fn_lib, a, level)?; + let val = self.eval_expr(scope, state, fn_lib, a, level)?; Err(Box::new(EvalAltResult::ErrorRuntime( val.take_string().unwrap_or_else(|_| "".to_string()), *pos, @@ -1441,7 +1487,7 @@ impl Engine { // Let statement Stmt::Let(name, Some(expr), _) => { - let val = self.eval_expr(scope, fn_lib, expr, level)?; + let val = self.eval_expr(scope, state, fn_lib, expr, level)?; // TODO - avoid copying variable name in inner block? scope.push_dynamic_value(name.clone(), ScopeEntryType::Normal, val, false); Ok(().into()) @@ -1455,7 +1501,7 @@ impl Engine { // Const statement Stmt::Const(name, expr, _) if expr.is_constant() => { - let val = self.eval_expr(scope, fn_lib, expr, level)?; + let val = self.eval_expr(scope, state, fn_lib, expr, level)?; // TODO - avoid copying variable name in inner block? scope.push_dynamic_value(name.clone(), ScopeEntryType::Constant, val, true); Ok(().into()) diff --git a/src/optimize.rs b/src/optimize.rs index f61bef36..1514b36c 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -559,7 +559,7 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { // Do not call some special keywords Expr::FunctionCall(id, args, def_value, pos) if DONT_EVAL_KEYWORDS.contains(&id.as_ref())=> - Expr::FunctionCall(id, args.into_iter().map(|a| optimize_expr(a, state)).collect(), def_value, pos), + Expr::FunctionCall(id, Box::new(args.into_iter().map(|a| optimize_expr(a, state)).collect()), def_value, pos), // Eagerly call functions Expr::FunctionCall(id, args, def_value, pos) @@ -569,7 +569,7 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { // First search in script-defined functions (can override built-in) if state.fn_lib.iter().find(|(name, len)| name == &id && *len == args.len()).is_some() { // A script-defined function overrides the built-in function - do not make the call - return Expr::FunctionCall(id, args.into_iter().map(|a| optimize_expr(a, state)).collect(), def_value, pos); + return Expr::FunctionCall(id, Box::new(args.into_iter().map(|a| optimize_expr(a, state)).collect()), def_value, pos); } let mut arg_values: Vec<_> = args.iter().map(Expr::get_constant_value).collect(); @@ -591,7 +591,7 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { Some(arg_for_type_of.to_string().into()) } else { // Otherwise use the default value, if any - def_value.clone() + def_value.clone().map(|v| *v) } }).and_then(|result| map_dynamic_to_expr(result, pos)) .map(|expr| { @@ -600,13 +600,13 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { }) ).unwrap_or_else(|| // Optimize function call arguments - Expr::FunctionCall(id, args.into_iter().map(|a| optimize_expr(a, state)).collect(), def_value, pos) + Expr::FunctionCall(id, Box::new(args.into_iter().map(|a| optimize_expr(a, state)).collect()), def_value, pos) ) } // id(args ..) -> optimize function call arguments Expr::FunctionCall(id, args, def_value, pos) => - Expr::FunctionCall(id, args.into_iter().map(|a| optimize_expr(a, state)).collect(), def_value, pos), + Expr::FunctionCall(id, Box::new(args.into_iter().map(|a| optimize_expr(a, state)).collect()), def_value, pos), // constant-name Expr::Variable(name, _, pos) if state.contains_constant(&name) => { diff --git a/src/parser.rs b/src/parser.rs index fd58cff0..5fe913b3 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -183,6 +183,33 @@ pub enum ReturnType { Exception, } +/// A type that encapsulates a local stack with variable names to simulate an actual runtime scope. +struct Stack(Vec); + +impl Stack { + pub fn new() -> Self { + Self(Vec::new()) + } + pub fn len(&self) -> usize { + self.0.len() + } + pub fn push(&mut self, name: String) { + self.0.push(name); + } + pub fn rewind(&mut self, len: usize) { + self.0.truncate(len); + } + pub fn find(&self, name: &str) -> usize { + self.0 + .iter() + .rev() + .enumerate() + .find(|(_, n)| *n == name) + .map(|(i, _)| i + 1) + .unwrap_or(0) + } +} + /// A statement. #[derive(Debug, Clone)] pub enum Stmt { @@ -289,7 +316,12 @@ pub enum Expr { /// func(expr, ... ) /// Use `Cow<'static, str>` because a lot of operators (e.g. `==`, `>=`) are implemented as function calls /// and the function names are predictable, so no need to allocate a new `String`. - FunctionCall(Cow<'static, str>, Vec, Option, Position), + FunctionCall( + Cow<'static, str>, + Box>, + Option>, + Position, + ), /// expr = expr Assignment(Box, Box, Position), /// lhs.rhs @@ -544,6 +576,7 @@ fn match_token(input: &mut Peekable, token: Token) -> Result( input: &mut Peekable>, + stack: &mut Stack, begin: Position, allow_stmt_expr: bool, ) -> Result> { @@ -551,7 +584,7 @@ fn parse_paren_expr<'a>( return Ok(Expr::Unit(begin)); } - let expr = parse_expr(input, allow_stmt_expr)?; + let expr = parse_expr(input, stack, allow_stmt_expr)?; match input.next().unwrap() { // ( xxx ) @@ -569,12 +602,13 @@ fn parse_paren_expr<'a>( /// Parse a function call. fn parse_call_expr<'a>( - id: String, input: &mut Peekable>, + stack: &mut Stack, + id: String, begin: Position, allow_stmt_expr: bool, ) -> Result> { - let mut args_expr_list = Vec::new(); + let mut args = Vec::new(); match input.peek().unwrap() { // id @@ -590,19 +624,19 @@ fn parse_call_expr<'a>( // id() (Token::RightParen, _) => { eat_token(input, Token::RightParen); - return Ok(Expr::FunctionCall(id.into(), args_expr_list, None, begin)); + return Ok(Expr::FunctionCall(id.into(), Box::new(args), None, begin)); } // id... _ => (), } loop { - args_expr_list.push(parse_expr(input, allow_stmt_expr)?); + args.push(parse_expr(input, stack, allow_stmt_expr)?); match input.peek().unwrap() { (Token::RightParen, _) => { eat_token(input, Token::RightParen); - return Ok(Expr::FunctionCall(id.into(), args_expr_list, None, begin)); + return Ok(Expr::FunctionCall(id.into(), Box::new(args), None, begin)); } (Token::Comma, _) => { eat_token(input, Token::Comma); @@ -631,12 +665,13 @@ fn parse_call_expr<'a>( /// 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>, + stack: &mut Stack, + lhs: Expr, pos: Position, allow_stmt_expr: bool, ) -> Result> { - let idx_expr = parse_expr(input, allow_stmt_expr)?; + let idx_expr = parse_expr(input, stack, allow_stmt_expr)?; // Check type of indexing - must be integer or string match &idx_expr { @@ -751,7 +786,8 @@ fn parse_index_chain<'a>( (Token::LeftBracket, _) => { let follow_pos = eat_token(input, Token::LeftBracket); // Recursively parse the indexing chain, right-binding each - let follow = parse_index_chain(idx_expr, input, follow_pos, allow_stmt_expr)?; + let follow = + parse_index_chain(input, stack, idx_expr, follow_pos, allow_stmt_expr)?; // Indexing binds to right Ok(Expr::Index(Box::new(lhs), Box::new(follow), pos)) } @@ -771,6 +807,7 @@ fn parse_index_chain<'a>( /// Parse an array literal. fn parse_array_literal<'a>( input: &mut Peekable>, + stack: &mut Stack, begin: Position, allow_stmt_expr: bool, ) -> Result> { @@ -778,7 +815,7 @@ fn parse_array_literal<'a>( if !match_token(input, Token::RightBracket)? { while !input.peek().unwrap().0.is_eof() { - arr.push(parse_expr(input, allow_stmt_expr)?); + arr.push(parse_expr(input, stack, allow_stmt_expr)?); match input.peek().unwrap() { (Token::Comma, _) => eat_token(input, Token::Comma), @@ -812,6 +849,7 @@ fn parse_array_literal<'a>( /// Parse a map literal. fn parse_map_literal<'a>( input: &mut Peekable>, + stack: &mut Stack, begin: Position, allow_stmt_expr: bool, ) -> Result> { @@ -853,7 +891,7 @@ fn parse_map_literal<'a>( } }; - let expr = parse_expr(input, allow_stmt_expr)?; + let expr = parse_expr(input, stack, allow_stmt_expr)?; map.push((name, expr, pos)); @@ -899,13 +937,14 @@ fn parse_map_literal<'a>( /// Parse a primary expression. fn parse_primary<'a>( input: &mut Peekable>, + stack: &mut Stack, allow_stmt_expr: bool, ) -> Result> { let (token, pos) = match input.peek().unwrap() { // { - block statement as expression (Token::LeftBrace, pos) if allow_stmt_expr => { let pos = *pos; - return parse_block(input, false, allow_stmt_expr) + return parse_block(input, stack, false, allow_stmt_expr) .map(|block| Expr::Stmt(Box::new(block), pos)); } (Token::EOF, pos) => return Err(PERR::UnexpectedEOF.into_err(*pos)), @@ -918,12 +957,15 @@ fn parse_primary<'a>( Token::FloatConstant(x) => Expr::FloatConstant(x, pos), Token::CharConstant(c) => Expr::CharConstant(c, pos), Token::StringConst(s) => Expr::StringConstant(s, pos), - Token::Identifier(s) => Expr::Variable(s, 0, pos), - Token::LeftParen => parse_paren_expr(input, pos, allow_stmt_expr)?, + Token::Identifier(s) => { + let index = stack.find(&s); + Expr::Variable(s, index, pos) + } + Token::LeftParen => parse_paren_expr(input, stack, pos, allow_stmt_expr)?, #[cfg(not(feature = "no_index"))] - Token::LeftBracket => parse_array_literal(input, pos, allow_stmt_expr)?, + Token::LeftBracket => parse_array_literal(input, stack, pos, allow_stmt_expr)?, #[cfg(not(feature = "no_object"))] - Token::MapStart => parse_map_literal(input, pos, allow_stmt_expr)?, + Token::MapStart => parse_map_literal(input, stack, pos, allow_stmt_expr)?, Token::True => Expr::True(pos), Token::False => Expr::False(pos), Token::LexError(err) => return Err(PERR::BadInput(err.to_string()).into_err(pos)), @@ -946,10 +988,12 @@ fn parse_primary<'a>( // Function call (Expr::Variable(id, _, pos), Token::LeftParen) | (Expr::Property(id, pos), Token::LeftParen) => { - parse_call_expr(id, input, pos, allow_stmt_expr)? + parse_call_expr(input, stack, id, pos, allow_stmt_expr)? } // Indexing - (expr, Token::LeftBracket) => parse_index_chain(expr, input, pos, allow_stmt_expr)?, + (expr, Token::LeftBracket) => { + parse_index_chain(input, stack, expr, pos, allow_stmt_expr)? + } // Unknown postfix operator (expr, token) => panic!("unknown postfix operator {:?} for {:?}", token, expr), } @@ -961,6 +1005,7 @@ fn parse_primary<'a>( /// Parse a potential unary operator. fn parse_unary<'a>( input: &mut Peekable>, + stack: &mut Stack, allow_stmt_expr: bool, ) -> Result> { match input.peek().unwrap() { @@ -968,7 +1013,7 @@ fn parse_unary<'a>( (Token::If, pos) => { let pos = *pos; Ok(Expr::Stmt( - Box::new(parse_if(input, false, allow_stmt_expr)?), + Box::new(parse_if(input, stack, false, allow_stmt_expr)?), pos, )) } @@ -976,7 +1021,7 @@ fn parse_unary<'a>( (Token::UnaryMinus, _) => { let pos = eat_token(input, Token::UnaryMinus); - match parse_unary(input, allow_stmt_expr)? { + match parse_unary(input, stack, allow_stmt_expr)? { // Negative integer Expr::IntegerConstant(i, _) => i .checked_neg() @@ -1001,49 +1046,51 @@ fn parse_unary<'a>( Expr::FloatConstant(f, pos) => Ok(Expr::FloatConstant(-f, pos)), // Call negative function - expr => Ok(Expr::FunctionCall("-".into(), vec![expr], None, pos)), + e => Ok(Expr::FunctionCall("-".into(), Box::new(vec![e]), None, pos)), } } // +expr (Token::UnaryPlus, _) => { eat_token(input, Token::UnaryPlus); - parse_unary(input, allow_stmt_expr) + parse_unary(input, stack, allow_stmt_expr) } // !expr (Token::Bang, _) => { let pos = eat_token(input, Token::Bang); Ok(Expr::FunctionCall( "!".into(), - vec![parse_primary(input, allow_stmt_expr)?], - Some(false.into()), // NOT operator, when operating on invalid operand, defaults to false + Box::new(vec![parse_primary(input, stack, allow_stmt_expr)?]), + Some(Box::new(false.into())), // NOT operator, when operating on invalid operand, defaults to false pos, )) } // (Token::EOF, pos) => Err(PERR::UnexpectedEOF.into_err(*pos)), // All other tokens - _ => parse_primary(input, allow_stmt_expr), + _ => parse_primary(input, stack, allow_stmt_expr), } } fn parse_assignment_stmt<'a>( input: &mut Peekable>, + stack: &mut Stack, lhs: Expr, allow_stmt_expr: bool, ) -> Result> { let pos = eat_token(input, Token::Equals); - let rhs = parse_expr(input, allow_stmt_expr)?; + let rhs = parse_expr(input, stack, allow_stmt_expr)?; Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs), pos)) } /// Parse an operator-assignment expression. fn parse_op_assignment_stmt<'a>( input: &mut Peekable>, + stack: &mut Stack, lhs: Expr, allow_stmt_expr: bool, ) -> Result> { let (op, pos) = match *input.peek().unwrap() { - (Token::Equals, _) => return parse_assignment_stmt(input, lhs, allow_stmt_expr), + (Token::Equals, _) => return parse_assignment_stmt(input, stack, lhs, allow_stmt_expr), (Token::PlusAssign, pos) => ("+", pos), (Token::MinusAssign, pos) => ("-", pos), (Token::MultiplyAssign, pos) => ("*", pos), @@ -1061,10 +1108,10 @@ fn parse_op_assignment_stmt<'a>( input.next(); let lhs_copy = lhs.clone(); - let rhs = parse_expr(input, allow_stmt_expr)?; + let rhs = parse_expr(input, stack, allow_stmt_expr)?; // lhs op= rhs -> lhs = op(lhs, rhs) - let rhs_expr = Expr::FunctionCall(op.into(), vec![lhs_copy, rhs], None, pos); + let rhs_expr = Expr::FunctionCall(op.into(), Box::new(vec![lhs_copy, rhs]), None, pos); Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs_expr), pos)) } @@ -1240,6 +1287,7 @@ fn make_in_expr(lhs: Expr, rhs: Expr, op_pos: Position) -> Result( input: &mut Peekable>, + stack: &mut Stack, parent_precedence: u8, lhs: Expr, allow_stmt_expr: bool, @@ -1262,7 +1310,7 @@ fn parse_binary_op<'a>( let (op_token, pos) = input.next().unwrap(); - let rhs = parse_unary(input, allow_stmt_expr)?; + let rhs = parse_unary(input, stack, allow_stmt_expr)?; let next_precedence = input.peek().unwrap().0.precedence(); @@ -1271,48 +1319,90 @@ fn parse_binary_op<'a>( let rhs = if (current_precedence == next_precedence && bind_right) || current_precedence < next_precedence { - parse_binary_op(input, current_precedence, rhs, allow_stmt_expr)? + parse_binary_op(input, stack, current_precedence, rhs, allow_stmt_expr)? } else { // Otherwise bind to left (even if next operator has the same precedence) rhs }; - current_lhs = match op_token { - Token::Plus => Expr::FunctionCall("+".into(), vec![current_lhs, rhs], None, pos), - Token::Minus => Expr::FunctionCall("-".into(), vec![current_lhs, rhs], None, pos), - Token::Multiply => Expr::FunctionCall("*".into(), vec![current_lhs, rhs], None, pos), - Token::Divide => Expr::FunctionCall("/".into(), vec![current_lhs, rhs], None, pos), + let cmp_default = Some(Box::new(false.into())); - Token::LeftShift => Expr::FunctionCall("<<".into(), vec![current_lhs, rhs], None, pos), - Token::RightShift => Expr::FunctionCall(">>".into(), vec![current_lhs, rhs], None, pos), - Token::Modulo => Expr::FunctionCall("%".into(), vec![current_lhs, rhs], None, pos), - Token::PowerOf => Expr::FunctionCall("~".into(), vec![current_lhs, rhs], None, pos), + current_lhs = match op_token { + Token::Plus => { + Expr::FunctionCall("+".into(), Box::new(vec![current_lhs, rhs]), None, pos) + } + Token::Minus => { + Expr::FunctionCall("-".into(), Box::new(vec![current_lhs, rhs]), None, pos) + } + Token::Multiply => { + Expr::FunctionCall("*".into(), Box::new(vec![current_lhs, rhs]), None, pos) + } + Token::Divide => { + Expr::FunctionCall("/".into(), Box::new(vec![current_lhs, rhs]), None, pos) + } + + Token::LeftShift => { + Expr::FunctionCall("<<".into(), Box::new(vec![current_lhs, rhs]), None, pos) + } + Token::RightShift => { + Expr::FunctionCall(">>".into(), Box::new(vec![current_lhs, rhs]), None, pos) + } + Token::Modulo => { + Expr::FunctionCall("%".into(), Box::new(vec![current_lhs, rhs]), None, pos) + } + Token::PowerOf => { + Expr::FunctionCall("~".into(), Box::new(vec![current_lhs, rhs]), None, pos) + } // Comparison operators default to false when passed invalid operands - Token::EqualsTo => { - Expr::FunctionCall("==".into(), vec![current_lhs, rhs], Some(false.into()), pos) - } - Token::NotEqualsTo => { - Expr::FunctionCall("!=".into(), vec![current_lhs, rhs], Some(false.into()), pos) - } - Token::LessThan => { - Expr::FunctionCall("<".into(), vec![current_lhs, rhs], Some(false.into()), pos) - } - Token::LessThanEqualsTo => { - Expr::FunctionCall("<=".into(), vec![current_lhs, rhs], Some(false.into()), pos) - } - Token::GreaterThan => { - Expr::FunctionCall(">".into(), vec![current_lhs, rhs], Some(false.into()), pos) - } - Token::GreaterThanEqualsTo => { - Expr::FunctionCall(">=".into(), vec![current_lhs, rhs], Some(false.into()), pos) - } + Token::EqualsTo => Expr::FunctionCall( + "==".into(), + Box::new(vec![current_lhs, rhs]), + cmp_default, + pos, + ), + Token::NotEqualsTo => Expr::FunctionCall( + "!=".into(), + Box::new(vec![current_lhs, rhs]), + cmp_default, + pos, + ), + Token::LessThan => Expr::FunctionCall( + "<".into(), + Box::new(vec![current_lhs, rhs]), + cmp_default, + pos, + ), + Token::LessThanEqualsTo => Expr::FunctionCall( + "<=".into(), + Box::new(vec![current_lhs, rhs]), + cmp_default, + pos, + ), + Token::GreaterThan => Expr::FunctionCall( + ">".into(), + Box::new(vec![current_lhs, rhs]), + cmp_default, + pos, + ), + Token::GreaterThanEqualsTo => Expr::FunctionCall( + ">=".into(), + Box::new(vec![current_lhs, rhs]), + cmp_default, + pos, + ), Token::Or => Expr::Or(Box::new(current_lhs), Box::new(rhs), pos), Token::And => Expr::And(Box::new(current_lhs), Box::new(rhs), pos), - Token::Ampersand => Expr::FunctionCall("&".into(), vec![current_lhs, rhs], None, pos), - Token::Pipe => Expr::FunctionCall("|".into(), vec![current_lhs, rhs], None, pos), - Token::XOr => Expr::FunctionCall("^".into(), vec![current_lhs, rhs], None, pos), + Token::Ampersand => { + Expr::FunctionCall("&".into(), Box::new(vec![current_lhs, rhs]), None, pos) + } + Token::Pipe => { + Expr::FunctionCall("|".into(), Box::new(vec![current_lhs, rhs]), None, pos) + } + Token::XOr => { + Expr::FunctionCall("^".into(), Box::new(vec![current_lhs, rhs]), None, pos) + } Token::In => make_in_expr(current_lhs, rhs, pos)?, @@ -1327,10 +1417,11 @@ fn parse_binary_op<'a>( /// Parse an expression. fn parse_expr<'a>( input: &mut Peekable>, + stack: &mut Stack, allow_stmt_expr: bool, ) -> Result> { - let lhs = parse_unary(input, allow_stmt_expr)?; - parse_binary_op(input, 1, lhs, allow_stmt_expr) + let lhs = parse_unary(input, stack, allow_stmt_expr)?; + parse_binary_op(input, stack, 1, lhs, allow_stmt_expr) } /// Make sure that the expression is not a statement expression (i.e. wrapped in `{}`). @@ -1380,6 +1471,7 @@ fn ensure_not_assignment<'a>( /// Parse an if statement. fn parse_if<'a>( input: &mut Peekable>, + stack: &mut Stack, breakable: bool, allow_stmt_expr: bool, ) -> Result> { @@ -1388,18 +1480,18 @@ fn parse_if<'a>( // if guard { if_body } ensure_not_statement_expr(input, "a boolean")?; - let guard = parse_expr(input, allow_stmt_expr)?; + let guard = parse_expr(input, stack, allow_stmt_expr)?; ensure_not_assignment(input)?; - let if_body = parse_block(input, breakable, allow_stmt_expr)?; + let if_body = parse_block(input, stack, breakable, allow_stmt_expr)?; // if guard { if_body } else ... let else_body = if match_token(input, Token::Else).unwrap_or(false) { Some(Box::new(if let (Token::If, _) = input.peek().unwrap() { // if guard { if_body } else if ... - parse_if(input, breakable, allow_stmt_expr)? + parse_if(input, stack, breakable, allow_stmt_expr)? } else { // if guard { if_body } else { else-body } - parse_block(input, breakable, allow_stmt_expr)? + parse_block(input, stack, breakable, allow_stmt_expr)? })) } else { None @@ -1415,6 +1507,7 @@ fn parse_if<'a>( /// Parse a while loop. fn parse_while<'a>( input: &mut Peekable>, + stack: &mut Stack, allow_stmt_expr: bool, ) -> Result> { // while ... @@ -1422,9 +1515,9 @@ fn parse_while<'a>( // while guard { body } ensure_not_statement_expr(input, "a boolean")?; - let guard = parse_expr(input, allow_stmt_expr)?; + let guard = parse_expr(input, stack, allow_stmt_expr)?; ensure_not_assignment(input)?; - let body = parse_block(input, true, allow_stmt_expr)?; + let body = parse_block(input, stack, true, allow_stmt_expr)?; Ok(Stmt::While(Box::new(guard), Box::new(body))) } @@ -1432,13 +1525,14 @@ fn parse_while<'a>( /// Parse a loop statement. fn parse_loop<'a>( input: &mut Peekable>, + stack: &mut Stack, allow_stmt_expr: bool, ) -> Result> { // loop ... eat_token(input, Token::Loop); // loop { body } - let body = parse_block(input, true, allow_stmt_expr)?; + let body = parse_block(input, stack, true, allow_stmt_expr)?; Ok(Stmt::Loop(Box::new(body))) } @@ -1446,6 +1540,7 @@ fn parse_loop<'a>( /// Parse a for loop. fn parse_for<'a>( input: &mut Peekable>, + stack: &mut Stack, allow_stmt_expr: bool, ) -> Result> { // for ... @@ -1477,8 +1572,14 @@ fn parse_for<'a>( // for name in expr { body } ensure_not_statement_expr(input, "a boolean")?; - let expr = parse_expr(input, allow_stmt_expr)?; - let body = parse_block(input, true, allow_stmt_expr)?; + let expr = parse_expr(input, stack, allow_stmt_expr)?; + + let prev_len = stack.len(); + stack.push(name.clone()); + + let body = parse_block(input, stack, true, allow_stmt_expr)?; + + stack.rewind(prev_len); Ok(Stmt::For(name, Box::new(expr), Box::new(body))) } @@ -1486,6 +1587,7 @@ fn parse_for<'a>( /// Parse a variable definition statement. fn parse_let<'a>( input: &mut Peekable>, + stack: &mut Stack, var_type: ScopeEntryType, allow_stmt_expr: bool, ) -> Result> { @@ -1502,13 +1604,17 @@ fn parse_let<'a>( // let name = ... if match_token(input, Token::Equals)? { // let name = expr - let init_value = parse_expr(input, allow_stmt_expr)?; + let init_value = parse_expr(input, stack, allow_stmt_expr)?; match var_type { // let name = expr - ScopeEntryType::Normal => Ok(Stmt::Let(name, Some(Box::new(init_value)), pos)), + ScopeEntryType::Normal => { + stack.push(name.clone()); + Ok(Stmt::Let(name, Some(Box::new(init_value)), pos)) + } // const name = { expr:constant } ScopeEntryType::Constant if init_value.is_constant() => { + stack.push(name.clone()); Ok(Stmt::Const(name, Box::new(init_value), pos)) } // const name = expr - error @@ -1525,6 +1631,7 @@ fn parse_let<'a>( /// Parse a statement block. fn parse_block<'a>( input: &mut Peekable>, + stack: &mut Stack, breakable: bool, allow_stmt_expr: bool, ) -> Result> { @@ -1540,10 +1647,11 @@ fn parse_block<'a>( }; let mut statements = Vec::new(); + let prev_len = stack.len(); while !match_token(input, Token::RightBrace)? { // Parse statements inside the block - let stmt = parse_stmt(input, breakable, allow_stmt_expr)?; + let stmt = parse_stmt(input, stack, breakable, allow_stmt_expr)?; // See if it needs a terminating semicolon let need_semicolon = !stmt.is_self_terminated(); @@ -1579,22 +1687,26 @@ fn parse_block<'a>( } } + stack.rewind(prev_len); + Ok(Stmt::Block(statements, pos)) } /// Parse an expression as a statement. fn parse_expr_stmt<'a>( input: &mut Peekable>, + stack: &mut Stack, allow_stmt_expr: bool, ) -> Result> { - let expr = parse_expr(input, allow_stmt_expr)?; - let expr = parse_op_assignment_stmt(input, expr, allow_stmt_expr)?; + let expr = parse_expr(input, stack, allow_stmt_expr)?; + let expr = parse_op_assignment_stmt(input, stack, expr, allow_stmt_expr)?; Ok(Stmt::Expr(Box::new(expr))) } /// Parse a single statement. fn parse_stmt<'a>( input: &mut Peekable>, + stack: &mut Stack, breakable: bool, allow_stmt_expr: bool, ) -> Result> { @@ -1607,16 +1719,16 @@ fn parse_stmt<'a>( // Semicolon - empty statement Token::SemiColon => Ok(Stmt::Noop(*pos)), - Token::LeftBrace => parse_block(input, breakable, allow_stmt_expr), + Token::LeftBrace => parse_block(input, stack, breakable, allow_stmt_expr), // fn ... #[cfg(not(feature = "no_function"))] Token::Fn => Err(PERR::WrongFnDefinition.into_err(*pos)), - Token::If => parse_if(input, breakable, allow_stmt_expr), - Token::While => parse_while(input, allow_stmt_expr), - Token::Loop => parse_loop(input, allow_stmt_expr), - Token::For => parse_for(input, allow_stmt_expr), + Token::If => parse_if(input, stack, breakable, allow_stmt_expr), + Token::While => parse_while(input, stack, allow_stmt_expr), + Token::Loop => parse_loop(input, stack, allow_stmt_expr), + Token::For => parse_for(input, stack, allow_stmt_expr), Token::Continue if breakable => { let pos = eat_token(input, Token::Continue); @@ -1644,23 +1756,24 @@ fn parse_stmt<'a>( (Token::SemiColon, _) => Ok(Stmt::ReturnWithVal(None, return_type, pos)), // `return` or `throw` with expression (_, _) => { - let expr = parse_expr(input, allow_stmt_expr)?; + let expr = parse_expr(input, stack, allow_stmt_expr)?; let pos = expr.position(); Ok(Stmt::ReturnWithVal(Some(Box::new(expr)), return_type, pos)) } } } - Token::Let => parse_let(input, ScopeEntryType::Normal, allow_stmt_expr), - Token::Const => parse_let(input, ScopeEntryType::Constant, allow_stmt_expr), + Token::Let => parse_let(input, stack, ScopeEntryType::Normal, allow_stmt_expr), + Token::Const => parse_let(input, stack, ScopeEntryType::Constant, allow_stmt_expr), - _ => parse_expr_stmt(input, allow_stmt_expr), + _ => parse_expr_stmt(input, stack, allow_stmt_expr), } } /// Parse a function definition. fn parse_fn<'a>( input: &mut Peekable>, + stack: &mut Stack, allow_stmt_expr: bool, ) -> Result> { let pos = input.next().expect("should be fn").1; @@ -1683,7 +1796,10 @@ fn parse_fn<'a>( loop { match input.next().unwrap() { - (Token::Identifier(s), pos) => params.push((s, pos)), + (Token::Identifier(s), pos) => { + stack.push(s.clone()); + params.push((s, pos)) + } (Token::LexError(err), pos) => { return Err(PERR::BadInput(err.to_string()).into_err(pos)) } @@ -1721,7 +1837,7 @@ fn parse_fn<'a>( // Parse function body let body = match input.peek().unwrap() { - (Token::LeftBrace, _) => parse_block(input, false, allow_stmt_expr)?, + (Token::LeftBrace, _) => parse_block(input, stack, false, allow_stmt_expr)?, (_, pos) => return Err(PERR::FnMissingBody(name).into_err(*pos)), }; @@ -1741,7 +1857,8 @@ pub fn parse_global_expr<'a>( scope: &Scope, optimization_level: OptimizationLevel, ) -> Result> { - let expr = parse_expr(input, false)?; + let mut stack = Stack::new(); + let expr = parse_expr(input, &mut stack, false)?; match input.peek().unwrap() { (Token::EOF, _) => (), @@ -1769,20 +1886,22 @@ fn parse_global_level<'a>( ) -> Result<(Vec, HashMap), Box> { let mut statements = Vec::::new(); let mut functions = HashMap::::new(); + let mut stack = Stack::new(); while !input.peek().unwrap().0.is_eof() { // Collect all the function definitions #[cfg(not(feature = "no_function"))] { if let (Token::Fn, _) = input.peek().unwrap() { - let f = parse_fn(input, true)?; + let mut stack = Stack::new(); + let f = parse_fn(input, &mut stack, true)?; functions.insert(calc_fn_def(&f.name, f.params.len()), f); continue; } } // Actual statement - let stmt = parse_stmt(input, false, true)?; + let stmt = parse_stmt(input, &mut stack, false, true)?; let need_semicolon = !stmt.is_self_terminated(); diff --git a/src/scope.rs b/src/scope.rs index 7bb40325..b50d1af3 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -349,7 +349,7 @@ impl<'a> Scope<'a> { } /// Get a mutable reference to an entry in the Scope. - pub(crate) fn get_mut(&mut self, index: usize) -> &mut Dynamic { + pub(crate) fn get_mut(&mut self, index: usize) -> (&mut Dynamic, EntryType) { let entry = self.0.get_mut(index).expect("invalid index in Scope"); // assert_ne!( @@ -358,7 +358,7 @@ impl<'a> Scope<'a> { // "get mut of constant entry" // ); - &mut entry.value + (&mut entry.value, entry.typ) } /// Get an iterator to entries in the Scope. diff --git a/tests/eval.rs b/tests/eval.rs index c9027a25..28618fc5 100644 --- a/tests/eval.rs +++ b/tests/eval.rs @@ -34,10 +34,10 @@ fn test_eval_function() -> Result<(), Box> { script += "y += foo(y);"; script += "x + y"; - eval(script) + eval(script) + x + y "# )?, - 42 + 84 ); assert_eq!( @@ -54,7 +54,8 @@ fn test_eval_function() -> Result<(), Box> { 32 ); - assert!(!scope.contains("z")); + assert!(scope.contains("script")); + assert_eq!(scope.len(), 3); Ok(()) } From 21c3edb31ede6a85dc629329debcea8f14d55cba Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 29 Apr 2020 16:11:54 +0800 Subject: [PATCH 4/4] Streamline. --- src/engine.rs | 24 ++++++++++++++---------- src/optimize.rs | 6 +++--- src/parser.rs | 23 ++++++++++++++++------- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 85d5a031..9c0ed804 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -189,7 +189,7 @@ impl StaticVec { } /// A type that holds all the current states of the Engine. -#[derive(Debug, Clone, Hash, Copy)] +#[derive(Debug, Clone, Copy)] pub struct State { /// Normally, access to variables are parsed with a relative offset into the scope to avoid a lookup. /// In some situation, e.g. after running an `eval` statement, subsequent offsets may become mis-aligned. @@ -198,6 +198,7 @@ pub struct State { } impl State { + /// Create a new `State`. pub fn new() -> Self { Self { always_search: false, @@ -914,10 +915,9 @@ impl Engine { match dot_lhs { // id.??? or id[???] Expr::Variable(id, index, pos) => { - let (target, typ) = if !state.always_search && *index > 0 { - scope.get_mut(scope.len() - *index) - } else { - search_scope(scope, id, *pos)? + let (target, typ) = match index { + Some(i) if !state.always_search => scope.get_mut(scope.len() - i.get()), + _ => search_scope(scope, id, *pos)?, }; // Constants cannot be modified @@ -1156,8 +1156,8 @@ impl Engine { Expr::FloatConstant(f, _) => Ok((*f).into()), Expr::StringConstant(s, _) => Ok(s.to_string().into()), Expr::CharConstant(c, _) => Ok((*c).into()), - Expr::Variable(_, index, _) if !state.always_search && *index > 0 => { - Ok(scope.get_mut(scope.len() - *index).0.clone()) + Expr::Variable(_, Some(index), _) if !state.always_search => { + Ok(scope.get_mut(scope.len() - index.get()).0.clone()) } Expr::Variable(id, _, pos) => search_scope(scope, id, *pos).map(|(v, _)| v.clone()), Expr::Property(_, _) => panic!("unexpected property."), @@ -1272,13 +1272,17 @@ impl Engine { && args.len() == 1 && !self.has_override(fn_lib, KEYWORD_EVAL) { + let prev_len = scope.len(); + // Evaluate the text string as a script let result = self.eval_script_expr(scope, fn_lib, args[0], arg_exprs[0].position()); - // IMPORTANT! The eval may define new variables in the current scope! - // We can no longer trust the variable offsets. - state.always_search = true; + if scope.len() != prev_len { + // IMPORTANT! If the eval defines new variables in the current scope, + // all variable offsets from this point on will be mis-aligned. + state.always_search = true; + } return result; } diff --git a/src/optimize.rs b/src/optimize.rs index 1514b36c..a63075a3 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -729,10 +729,10 @@ pub fn optimize_into_ast( // Optimize the function body let mut body = - optimize(vec![fn_def.body], engine, &Scope::new(), &fn_lib, level); + optimize(vec![*fn_def.body], engine, &Scope::new(), &fn_lib, level); // {} -> Noop - fn_def.body = match body.pop().unwrap_or_else(|| Stmt::Noop(pos)) { + fn_def.body = Box::new(match body.pop().unwrap_or_else(|| Stmt::Noop(pos)) { // { return val; } -> val Stmt::ReturnWithVal(Some(val), ReturnType::Return, _) => Stmt::Expr(val), // { return; } -> () @@ -741,7 +741,7 @@ pub fn optimize_into_ast( } // All others stmt => stmt, - }; + }); } fn_def }) diff --git a/src/parser.rs b/src/parser.rs index 5fe913b3..dccad46e 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -14,6 +14,7 @@ use crate::stdlib::{ collections::HashMap, format, iter::Peekable, + num::NonZeroUsize, ops::Add, rc::Rc, string::{String, ToString}, @@ -169,7 +170,7 @@ pub struct FnDef { /// Names of function parameters. pub params: Vec, /// Function body. - pub body: Stmt, + pub body: Box, /// Position of the function definition. pub pos: Position, } @@ -184,29 +185,37 @@ pub enum ReturnType { } /// A type that encapsulates a local stack with variable names to simulate an actual runtime scope. +#[derive(Debug, Clone)] struct Stack(Vec); impl Stack { + /// Create a new `Stack`. pub fn new() -> Self { Self(Vec::new()) } + /// Get the number of variables in the `Stack`. pub fn len(&self) -> usize { self.0.len() } + /// Push (add) a new variable onto the `Stack`. pub fn push(&mut self, name: String) { self.0.push(name); } + /// Rewind the stack to a previous size. pub fn rewind(&mut self, len: usize) { self.0.truncate(len); } - pub fn find(&self, name: &str) -> usize { + /// Find a variable by name in the `Stack`, searching in reverse. + /// The return value is the offset to be deducted from `Stack::len`, + /// i.e. the top element of the `Stack` is offset 1. + /// Return zero when the variable name is not found in the `Stack`. + pub fn find(&self, name: &str) -> Option { self.0 .iter() .rev() .enumerate() .find(|(_, n)| *n == name) - .map(|(i, _)| i + 1) - .unwrap_or(0) + .and_then(|(i, _)| NonZeroUsize::new(i + 1)) } } @@ -308,7 +317,7 @@ pub enum Expr { /// String constant. StringConstant(String, Position), /// Variable access. - Variable(String, usize, Position), + Variable(String, Option, Position), /// Property access. Property(String, Position), /// { stmt } @@ -1836,10 +1845,10 @@ fn parse_fn<'a>( })?; // Parse function body - let body = match input.peek().unwrap() { + let body = Box::new(match input.peek().unwrap() { (Token::LeftBrace, _) => parse_block(input, stack, false, allow_stmt_expr)?, (_, pos) => return Err(PERR::FnMissingBody(name).into_err(*pos)), - }; + }); let params = params.into_iter().map(|(p, _)| p).collect();