From 6a6c5f30de4488b7b4e40f7c562b04e49e356bf8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 19 Mar 2020 19:53:42 +0800 Subject: [PATCH 1/8] Add eval function. --- README.md | 35 +++++++++++++++ src/api.rs | 117 +++++++++++++++++++++++++++++++----------------- src/engine.rs | 108 +++++++++++++++++++++++++++++++++----------- src/optimize.rs | 22 ++++++--- src/parser.rs | 36 +++++++-------- src/scope.rs | 4 +- 6 files changed, 229 insertions(+), 93 deletions(-) diff --git a/README.md b/README.md index a53651d9..cc21e307 100644 --- a/README.md +++ b/README.md @@ -1515,3 +1515,38 @@ let engine = rhai::Engine::new(); // Turn off the optimizer engine.set_optimization_level(rhai::OptimizationLevel::None); ``` + +`eval` - or "How to Shoot Yourself in the Foot even Easier" +--------------------------------------------------------- + +Saving the best for last: in addition to script optimizations, there is the ever-dreaded... `eval` function! + +```rust +let x = 10; + +fn foo(x) { x += 12; x } + +let script = "let y = x;"; // build a script +script += "y += foo(y);"; +script += "x + y"; + +let result = eval(script); + +print("Answer: " + result); // prints 42 + +print("x = " + x); // prints 10 (functions call arguments are passed by value) +print("y = " + y); // prints 32 (variables defined in 'eval' persist) + +eval("{ let z = y }"); // to keep a variable local, use a statement block + +print("z = " + z); // error - variable 'z' not found +``` + +For those who subscribe to the (very sensible) motto of ["`eval` is **evil**"](http://linterrors.com/js/eval-is-evil), +disable `eval` by overriding it, probably with something that throws. + +```rust +fn eval(script) { throw "eval is evil! I refuse to run " + script } + +let x = eval("40 + 2"); // 'eval' here throws "eval is evil! I refuse to run 40 + 2" +``` diff --git a/src/api.rs b/src/api.rs index 34be8687..d6f592ba 100644 --- a/src/api.rs +++ b/src/api.rs @@ -2,7 +2,7 @@ use crate::any::{Any, AnyExt, Dynamic}; use crate::call::FuncArgs; -use crate::engine::{Engine, FnAny, FnSpec}; +use crate::engine::{Engine, FnAny, FnSpec, FUNC_GETTER, FUNC_SETTER}; use crate::error::ParseError; use crate::fn_register::RegisterFn; use crate::parser::{lex, parse, FnDef, Position, AST}; @@ -173,8 +173,8 @@ impl<'e> Engine<'e> { name: &str, callback: impl Fn(&mut T) -> U + 'static, ) { - let get_name = "get$".to_string() + name; - self.register_fn(&get_name, callback); + let get_fn_name = format!("{}{}", FUNC_GETTER, name); + self.register_fn(&get_fn_name, callback); } /// Register a setter function for a member of a registered type with the `Engine`. @@ -215,8 +215,8 @@ impl<'e> Engine<'e> { name: &str, callback: impl Fn(&mut T, U) -> () + 'static, ) { - let set_name = "set$".to_string() + name; - self.register_fn(&set_name, callback); + let set_fn_name = format!("{}{}", FUNC_SETTER, name); + self.register_fn(&set_fn_name, callback); } /// Shorthand for registering both getter and setter functions @@ -264,7 +264,7 @@ impl<'e> Engine<'e> { self.register_set(name, set_fn); } - /// Compile a string into an `AST`, which can be used later for evaluations. + /// Compile a string into an `AST`, which can be used later for evaluation. /// /// # Example /// @@ -274,7 +274,7 @@ impl<'e> Engine<'e> { /// /// let mut engine = Engine::new(); /// - /// // Compile a script to an AST and store it for later evaluations + /// // Compile a script to an AST and store it for later evaluation /// let ast = engine.compile("40 + 2")?; /// /// for _ in 0..42 { @@ -287,27 +287,40 @@ impl<'e> Engine<'e> { self.compile_with_scope(&Scope::new(), input) } - /// Compile a string into an `AST` using own scope, which can be used later for evaluations. - /// The scope is useful for passing constants into the script for optimization. + /// Compile a string into an `AST` using own scope, which can be used later for evaluation. + /// The scope is useful for passing constants into the script for optimization + /// when using `OptimizationLevel::Full`. /// /// # Example /// /// ``` /// # fn main() -> Result<(), rhai::EvalAltResult> { - /// use rhai::{Engine, Scope}; + /// # #[cfg(not(feature = "no_optimize"))] + /// # { + /// use rhai::{Engine, Scope, OptimizationLevel}; /// /// let mut engine = Engine::new(); /// + /// // Set optimization level to 'Full' so the Engine can fold constants + /// // into function calls and operators. + /// engine.set_optimization_level(OptimizationLevel::Full); + /// /// // Create initialized scope /// let mut scope = Scope::new(); /// scope.push_constant("x", 42_i64); // 'x' is a constant /// - /// // Compile a script to an AST and store it for later evaluations + /// // Compile a script to an AST and store it for later evaluation. + /// // Notice that `Full` optimization is on, so constants are folded + /// // into function calls and operators. /// let ast = engine.compile_with_scope(&mut scope, - /// "if x > 40 { x } else { 0 }" + /// "if x > 40 { x } else { 0 }" // all 'x' are replaced with 42 /// )?; /// + /// // Normally this would have failed because no scope is passed into the 'eval_ast' + /// // call and so the variable 'x' does not exist. Here, it passes because the script + /// // has been optimized and all references to 'x' are already gone. /// assert_eq!(engine.eval_ast::(&ast)?, 42); + /// # } /// # Ok(()) /// # } /// ``` @@ -329,7 +342,7 @@ impl<'e> Engine<'e> { .map(|_| contents) } - /// Compile a script file into an `AST`, which can be used later for evaluations. + /// Compile a script file into an `AST`, which can be used later for evaluation. /// /// # Example /// @@ -339,7 +352,7 @@ impl<'e> Engine<'e> { /// /// let mut engine = Engine::new(); /// - /// // Compile a script file to an AST and store it for later evaluations + /// // Compile a script file to an AST and store it for later evaluation. /// // Notice that a PathBuf is required which can easily be constructed from a string. /// let ast = engine.compile_file("script.rhai".into())?; /// @@ -354,26 +367,33 @@ impl<'e> Engine<'e> { self.compile_file_with_scope(&Scope::new(), path) } - /// Compile a script file into an `AST` using own scope, which can be used later for evaluations. - /// The scope is useful for passing constants into the script for optimization. + /// Compile a script file into an `AST` using own scope, which can be used later for evaluation. + /// The scope is useful for passing constants into the script for optimization + /// when using `OptimizationLevel::Full`. /// /// # Example /// /// ```no_run /// # fn main() -> Result<(), rhai::EvalAltResult> { - /// use rhai::{Engine, Scope}; + /// # #[cfg(not(feature = "no_optimize"))] + /// # { + /// use rhai::{Engine, Scope, OptimizationLevel}; /// /// let mut engine = Engine::new(); /// + /// // Set optimization level to 'Full' so the Engine can fold constants. + /// engine.set_optimization_level(OptimizationLevel::Full); + /// /// // Create initialized scope /// let mut scope = Scope::new(); /// scope.push_constant("x", 42_i64); // 'x' is a constant /// - /// // Compile a script to an AST and store it for later evaluations + /// // Compile a script to an AST and store it for later evaluation. /// // Notice that a PathBuf is required which can easily be constructed from a string. /// let ast = engine.compile_file_with_scope(&mut scope, "script.rhai".into())?; /// /// let result = engine.eval_ast::(&ast)?; + /// # } /// # Ok(()) /// # } /// ``` @@ -491,13 +511,13 @@ impl<'e> Engine<'e> { /// /// # Example /// - /// ```no_run + /// ``` /// # fn main() -> Result<(), rhai::EvalAltResult> { /// use rhai::Engine; /// /// let mut engine = Engine::new(); /// - /// // Compile a script to an AST and store it for later evaluations + /// // Compile a script to an AST and store it for later evaluation /// let ast = engine.compile("40 + 2")?; /// /// // Evaluate it @@ -514,22 +534,25 @@ impl<'e> Engine<'e> { /// /// # Example /// - /// ```no_run + /// ``` /// # fn main() -> Result<(), rhai::EvalAltResult> { /// use rhai::{Engine, Scope}; /// /// let mut engine = Engine::new(); /// - /// // Compile a script to an AST and store it for later evaluations + /// // Compile a script to an AST and store it for later evaluation /// let ast = engine.compile("x + 2")?; /// /// // Create initialized scope /// let mut scope = Scope::new(); /// scope.push("x", 40_i64); /// + /// // Compile a script to an AST and store it for later evaluation + /// let ast = engine.compile("x = x + 2; x")?; + /// /// // Evaluate it - /// assert_eq!(engine.eval_with_scope::(&mut scope, "x = x + 2; x")?, 42); - /// assert_eq!(engine.eval_with_scope::(&mut scope, "x = x + 2; x")?, 44); + /// assert_eq!(engine.eval_ast_with_scope::(&mut scope, &ast)?, 42); + /// assert_eq!(engine.eval_ast_with_scope::(&mut scope, &ast)?, 44); /// /// // The variable in the scope is modified /// assert_eq!(scope.get_value::("x").expect("variable x should exist"), 44); @@ -541,12 +564,32 @@ impl<'e> Engine<'e> { scope: &mut Scope, ast: &AST, ) -> Result { + self.eval_ast_with_scope_raw(scope, false, ast) + .and_then(|out| { + out.downcast::().map(|v| *v).map_err(|a| { + EvalAltResult::ErrorMismatchOutputType( + self.map_type_name((*a).type_name()).to_string(), + Position::none(), + ) + }) + }) + } + + pub(crate) fn eval_ast_with_scope_raw( + &mut self, + scope: &mut Scope, + retain_functions: bool, + ast: &AST, + ) -> Result { fn eval_ast_internal( engine: &mut Engine, scope: &mut Scope, + retain_functions: bool, ast: &AST, ) -> Result { - engine.clear_functions(); + if !retain_functions { + engine.clear_functions(); + } let statements = { let AST(statements, functions) = ast; @@ -560,24 +603,17 @@ impl<'e> Engine<'e> { result = engine.eval_stmt(scope, stmt)?; } - engine.clear_functions(); + if !retain_functions { + engine.clear_functions(); + } Ok(result) } - eval_ast_internal(self, scope, ast) - .or_else(|err| match err { - EvalAltResult::Return(out, _) => Ok(out), - _ => Err(err), - }) - .and_then(|out| { - out.downcast::().map(|v| *v).map_err(|a| { - EvalAltResult::ErrorMismatchOutputType( - self.map_type_name((*a).type_name()).to_string(), - Position::eof(), - ) - }) - }) + eval_ast_internal(self, scope, retain_functions, ast).or_else(|err| match err { + EvalAltResult::Return(out, _) => Ok(out), + _ => Err(err), + }) } /// Evaluate a file, but throw away the result and only return error (if any). @@ -729,9 +765,10 @@ impl<'e> Engine<'e> { name: &str, mut values: Vec, ) -> Result { + let mut scope = Scope::new(); let values: Vec<_> = values.iter_mut().map(Dynamic::as_mut).collect(); - let result = engine.call_fn_raw(name, values, None, Position::none()); + let result = engine.call_fn_raw(&mut scope, name, values, None, Position::none()); result } diff --git a/src/engine.rs b/src/engine.rs index 9ea647f8..bd389312 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -38,6 +38,7 @@ pub(crate) const KEYWORD_PRINT: &'static str = "print"; pub(crate) const KEYWORD_DEBUG: &'static str = "debug"; pub(crate) const KEYWORD_DUMP_AST: &'static str = "dump_ast"; pub(crate) const KEYWORD_TYPE_OF: &'static str = "type_of"; +pub(crate) const KEYWORD_EVAL: &'static str = "eval"; pub(crate) const FUNC_GETTER: &'static str = "get$"; pub(crate) const FUNC_SETTER: &'static str = "set$"; @@ -160,6 +161,7 @@ impl Engine<'_> { /// registered with the `Engine` or written in Rhai pub(crate) fn call_fn_raw( &mut self, + scope: &mut Scope, fn_name: &str, args: FnCallArgs, def_val: Option<&Dynamic>, @@ -185,10 +187,15 @@ impl Engine<'_> { // Evaluate // Convert return statement to return value - return match self.eval_stmt(&mut scope, &fn_def.body) { - Err(EvalAltResult::Return(x, _)) => Ok(x), - other => other, - }; + return self + .eval_stmt(&mut scope, &fn_def.body) + .or_else(|mut err| match err { + EvalAltResult::Return(x, _) => Ok(x), + _ => { + err.set_position(pos); + Err(err) + } + }); } let spec = FnSpec { @@ -196,24 +203,26 @@ impl Engine<'_> { args: Some(args.iter().map(|a| Any::type_id(&**a)).collect()), }; + // Argument must be a string + fn cast_to_string<'a>(r: &'a Variant, pos: Position) -> Result<&'a str, EvalAltResult> { + r.downcast_ref::() + .map(String::as_str) + .ok_or_else(|| EvalAltResult::ErrorMismatchOutputType(r.type_name().into(), pos)) + } + // Search built-in's and external functions if let Some(func) = self.ext_functions.get(&spec) { // Run external function let result = func(args, pos)?; // See if the function match print/debug (which requires special processing) - let callback = match fn_name { - KEYWORD_PRINT => self.on_print.as_mut(), - KEYWORD_DEBUG => self.on_debug.as_mut(), - _ => return Ok(result), - }; + match fn_name { + KEYWORD_PRINT => self.on_print.as_mut()(cast_to_string(result.as_ref(), pos)?), + KEYWORD_DEBUG => self.on_debug.as_mut()(cast_to_string(result.as_ref(), pos)?), + _ => (), + } - let val = &result - .downcast::() - .map(|s| *s) - .unwrap_or("error: not a string".into()); - - return Ok(callback(val).into_dynamic()); + return Ok(result); } if fn_name == KEYWORD_TYPE_OF && args.len() == 1 { @@ -224,6 +233,32 @@ impl Engine<'_> { .into_dynamic()); } + if fn_name == KEYWORD_EVAL && args.len() == 1 { + // Handle `eval` function + let script = cast_to_string(args[0], pos)?; + + #[cfg(not(feature = "no_optimize"))] + let ast = { + let orig_optimization_level = self.optimization_level; + + self.set_optimization_level(OptimizationLevel::None); + let ast = self.compile(script); + self.set_optimization_level(orig_optimization_level); + + ast.map_err(EvalAltResult::ErrorParsing)? + }; + + #[cfg(feature = "no_optimize")] + let ast = self.compile(script).map_err(EvalAltResult::ErrorParsing)?; + + return Ok(self + .eval_ast_with_scope_raw(scope, true, &ast) + .map_err(|mut err| { + err.set_position(pos); + err + })?); + } + if fn_name.starts_with(FUNC_GETTER) { // Getter function not found return Err(EvalAltResult::ErrorDotExpr( @@ -283,14 +318,14 @@ impl Engine<'_> { .chain(values.iter_mut().map(Dynamic::as_mut)) .collect(); - self.call_fn_raw(fn_name, args, def_val.as_ref(), *pos) + self.call_fn_raw(scope, fn_name, args, def_val.as_ref(), *pos) } // xxx.id Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); - self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos) + self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos) } // xxx.idx_lhs[idx_expr] @@ -301,7 +336,7 @@ impl Engine<'_> { Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); ( - self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos)?, + self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos)?, *pos, ) } @@ -329,7 +364,7 @@ impl Engine<'_> { Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); - self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos) + self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos) .and_then(|mut v| self.get_dot_val_helper(scope, v.as_mut(), rhs)) } // xxx.idx_lhs[idx_expr].rhs @@ -340,7 +375,7 @@ impl Engine<'_> { Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); ( - self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos)?, + self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos)?, *pos, ) } @@ -642,7 +677,13 @@ impl Engine<'_> { Expr::Property(id, pos) => { let set_fn_name = format!("{}{}", FUNC_SETTER, id); - self.call_fn_raw(&set_fn_name, vec![this_ptr, new_val.as_mut()], None, *pos) + self.call_fn_raw( + scope, + &set_fn_name, + vec![this_ptr, new_val.as_mut()], + None, + *pos, + ) } // xxx.lhs[idx_expr] @@ -653,14 +694,20 @@ impl Engine<'_> { Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); - self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos) + self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos) .and_then(|v| { let idx = self.eval_index_value(scope, idx_expr)?; Self::update_indexed_value(v, idx as usize, new_val, val_pos) }) .and_then(|mut v| { let set_fn_name = format!("{}{}", FUNC_SETTER, id); - self.call_fn_raw(&set_fn_name, vec![this_ptr, v.as_mut()], None, *pos) + self.call_fn_raw( + scope, + &set_fn_name, + vec![this_ptr, v.as_mut()], + None, + *pos, + ) }) } @@ -677,7 +724,7 @@ impl Engine<'_> { Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); - self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos) + self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos) .and_then(|mut v| { self.set_dot_val_helper(scope, v.as_mut(), rhs, new_val, val_pos) .map(|_| v) // Discard Ok return value @@ -685,7 +732,13 @@ impl Engine<'_> { .and_then(|mut v| { let set_fn_name = format!("{}{}", FUNC_SETTER, id); - self.call_fn_raw(&set_fn_name, vec![this_ptr, v.as_mut()], None, *pos) + self.call_fn_raw( + scope, + &set_fn_name, + vec![this_ptr, v.as_mut()], + None, + *pos, + ) }) } @@ -697,7 +750,7 @@ impl Engine<'_> { Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); - self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos) + self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos) .and_then(|v| { let idx = self.eval_index_value(scope, idx_expr)?; let (mut target, _) = @@ -718,6 +771,7 @@ impl Engine<'_> { let set_fn_name = format!("{}{}", FUNC_SETTER, id); self.call_fn_raw( + scope, &set_fn_name, vec![this_ptr, v.as_mut()], None, @@ -936,6 +990,7 @@ impl Engine<'_> { // Redirect call to `print` self.call_fn_raw( + scope, KEYWORD_PRINT, vec![result.into_dynamic().as_mut()], None, @@ -951,6 +1006,7 @@ impl Engine<'_> { .collect::, _>>()?; self.call_fn_raw( + scope, fn_name, values.iter_mut().map(|b| b.as_mut()).collect(), def_val.as_ref(), diff --git a/src/optimize.rs b/src/optimize.rs index 7ac461ec..35f22957 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -2,7 +2,8 @@ use crate::any::{Any, Dynamic}; use crate::engine::{ - Engine, FnCallArgs, KEYWORD_DEBUG, KEYWORD_DUMP_AST, KEYWORD_PRINT, KEYWORD_TYPE_OF, + Engine, FnCallArgs, KEYWORD_DEBUG, KEYWORD_DUMP_AST, KEYWORD_EVAL, KEYWORD_PRINT, + KEYWORD_TYPE_OF, }; use crate::parser::{map_dynamic_to_expr, Expr, FnDef, ReturnType, Stmt, AST}; use crate::scope::{Scope, ScopeEntry, VariableType}; @@ -287,7 +288,7 @@ fn optimize_stmt<'a>(stmt: Stmt, state: &mut State<'a>, preserve_result: bool) - /// Optimize an expression. fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { // These keywords are handled specially - const SKIP_FUNC_KEYWORDS: [&str; 3] = [KEYWORD_PRINT, KEYWORD_DEBUG, KEYWORD_DUMP_AST]; + const DONT_EVAL_KEYWORDS: [&str; 3] = [KEYWORD_PRINT, KEYWORD_DEBUG, KEYWORD_EVAL]; match expr { // ( stmt ) @@ -431,10 +432,16 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { ), }, - // Do not optimize anything within built-in function keywords - Expr::FunctionCall(id, args, def_value, pos) if SKIP_FUNC_KEYWORDS.contains(&id.as_str())=> + // Do not optimize anything within dump_ast + Expr::FunctionCall(id, args, def_value, pos) if id == KEYWORD_DUMP_AST => Expr::FunctionCall(id, args, def_value, pos), + // Do not optimize anything within built-in function keywords + Expr::FunctionCall(id, args, def_value, pos) if DONT_EVAL_KEYWORDS.contains(&id.as_str())=> + Expr::FunctionCall(id, + args.into_iter().map(|a| optimize_expr(a, state)).collect(), + def_value, pos), + // Eagerly call functions Expr::FunctionCall(id, args, def_value, pos) if state.engine.optimization_level == OptimizationLevel::Full // full optimizations @@ -467,9 +474,13 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { }) ).flatten().unwrap_or_else(|| Expr::FunctionCall(id, args, 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, + args.into_iter().map(|a| optimize_expr(a, state)).collect(), + def_value, pos), + // constant-name Expr::Variable(ref name, _) if state.contains_constant(name) => { state.set_dirty(); @@ -480,6 +491,7 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { .expect("should find constant in scope!") .clone() } + // All other expressions - skip expr => expr, } diff --git a/src/parser.rs b/src/parser.rs index 0f62ee1e..10abc3a8 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -469,6 +469,10 @@ pub enum Token { UnaryMinus, Multiply, Divide, + Modulo, + PowerOf, + LeftShift, + RightShift, SemiColon, Colon, Comma, @@ -482,15 +486,18 @@ pub enum Token { Else, While, Loop, + For, + In, LessThan, GreaterThan, - Bang, LessThanEqualsTo, GreaterThanEqualsTo, EqualsTo, NotEqualsTo, + Bang, Pipe, Or, + XOr, Ampersand, And, #[cfg(not(feature = "no_function"))] @@ -507,15 +514,8 @@ pub enum Token { AndAssign, OrAssign, XOrAssign, - LeftShift, - RightShift, - XOr, - Modulo, ModuloAssign, - PowerOf, PowerOfAssign, - For, - In, LexError(LexError), } @@ -2210,31 +2210,27 @@ fn parse_stmt<'a>( Ok(Stmt::Break(pos)) } (Token::Break, pos) => return Err(ParseError::new(PERR::LoopBreak, *pos)), - (token @ Token::Return, _) | (token @ Token::Throw, _) => { + (token @ Token::Return, pos) | (token @ Token::Throw, pos) => { let return_type = match token { Token::Return => ReturnType::Return, Token::Throw => ReturnType::Exception, _ => panic!("token should be return or throw"), }; + let pos = *pos; input.next(); match input.peek() { // `return`/`throw` at EOF None => Ok(Stmt::ReturnWithVal(None, return_type, Position::eof())), // `return;` or `throw;` - Some((Token::SemiColon, pos)) => { - let pos = *pos; - Ok(Stmt::ReturnWithVal(None, return_type, pos)) - } + Some((Token::SemiColon, _)) => Ok(Stmt::ReturnWithVal(None, return_type, pos)), // `return` or `throw` with expression - Some((_, pos)) => { - let pos = *pos; - Ok(Stmt::ReturnWithVal( - Some(Box::new(parse_expr(input)?)), - return_type, - pos, - )) + Some((_, _)) => { + let expr = parse_expr(input)?; + let pos = expr.position(); + + Ok(Stmt::ReturnWithVal(Some(Box::new(expr)), return_type, pos)) } } } diff --git a/src/scope.rs b/src/scope.rs index 6cb1c4af..dcd35091 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -71,7 +71,7 @@ impl<'a> Scope<'a> { } /// Add (push) a new variable to the Scope. - pub fn push>, T: Any>(&mut self, name: K, value: T) { + pub fn push>, T: Any + Clone>(&mut self, name: K, value: T) { let value = value.into_dynamic(); // Map into constant expressions @@ -91,7 +91,7 @@ impl<'a> Scope<'a> { /// Constants propagation is a technique used to optimize an AST. /// However, in order to be used for optimization, constants must be in one of the recognized types: /// `INT` (default to `i64`, `i32` if `only_i32`), `f64`, `String`, `char` and `bool`. - pub fn push_constant>, T: Any>(&mut self, name: K, value: T) { + pub fn push_constant>, T: Any + Clone>(&mut self, name: K, value: T) { let value = value.into_dynamic(); // Map into constant expressions From 702b2010f2741da5380aaeb7c26e7850dea6c9bd Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 19 Mar 2020 20:55:49 +0800 Subject: [PATCH 2/8] Add contains to Scope. --- src/scope.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/scope.rs b/src/scope.rs index dcd35091..b7d43c97 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -139,6 +139,16 @@ impl<'a> Scope<'a> { self.0.truncate(size); } + /// Does the scope contain the variable? + pub fn contains(&self, key: &str) -> bool { + self.0 + .iter() + .enumerate() + .rev() // Always search a Scope in reverse order + .find(|(_, ScopeEntry { name, .. })| name == key) + .is_some() + } + /// Find a variable in the Scope, starting from the last. pub fn get(&self, key: &str) -> Option<(usize, &str, VariableType, Dynamic)> { self.0 From 36b7124dd57253c0a8ac2a63d1f47b514a969288 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 19 Mar 2020 20:55:53 +0800 Subject: [PATCH 3/8] Add eval test. --- tests/eval.rs | 61 ++++++++++++++++++++++++++++++++++++++++++++++ tests/var_scope.rs | 5 +--- 2 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 tests/eval.rs diff --git a/tests/eval.rs b/tests/eval.rs new file mode 100644 index 00000000..24559d83 --- /dev/null +++ b/tests/eval.rs @@ -0,0 +1,61 @@ +use rhai::{Engine, EvalAltResult, Scope, INT}; + +#[test] +fn test_eval() -> Result<(), EvalAltResult> { + let mut engine = Engine::new(); + let mut scope = Scope::new(); + + assert_eq!( + engine.eval_with_scope::( + &mut scope, + r#" + let x = 10; + + fn foo(x) { x += 12; x } + + let script = "let y = x;"; // build a script + script += "y += foo(y);"; + script += "x + y"; + + eval(script) + "# + )?, + 42 + ); + + assert_eq!( + scope + .get_value::("x") + .expect("variable x should exist"), + 10 + ); + + assert_eq!( + scope + .get_value::("y") + .expect("variable y should exist"), + 32 + ); + + assert!(!scope.contains("z")); + + Ok(()) +} + +#[test] +fn test_eval_override() -> Result<(), EvalAltResult> { + let mut engine = Engine::new(); + + assert_eq!( + engine.eval::( + r#" + fn eval(x) { x } // reflect the script back + + eval("40 + 2") + "# + )?, + "40 + 2" + ); + + Ok(()) +} diff --git a/tests/var_scope.rs b/tests/var_scope.rs index bdb4674e..7b573fd8 100644 --- a/tests/var_scope.rs +++ b/tests/var_scope.rs @@ -42,10 +42,7 @@ fn test_scope_eval() -> Result<(), EvalAltResult> { assert_eq!( scope .get_value::("y") - .ok_or(EvalAltResult::ErrorRuntime( - "variable y not found".into(), - Default::default() - ))?, + .expect("variable y should exist"), 1 ); From 283602cdd827b3ea4adbbd8473617f08c7c827d3 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 19 Mar 2020 22:29:02 +0800 Subject: [PATCH 4/8] Do not call function when optimizing if there is a script-defined function overridding it. --- src/optimize.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/optimize.rs b/src/optimize.rs index 35f22957..43664681 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -438,15 +438,19 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { // Do not optimize anything within built-in function keywords Expr::FunctionCall(id, args, def_value, pos) if DONT_EVAL_KEYWORDS.contains(&id.as_str())=> - Expr::FunctionCall(id, - args.into_iter().map(|a| optimize_expr(a, state)).collect(), - def_value, pos), + Expr::FunctionCall(id, args.into_iter().map(|a| optimize_expr(a, state)).collect(), def_value, pos), // Eagerly call functions Expr::FunctionCall(id, args, def_value, pos) if state.engine.optimization_level == OptimizationLevel::Full // full optimizations && args.iter().all(|expr| expr.is_constant()) // all arguments are constants => { + // First search in script-defined functions (can override built-in) + if state.engine.script_functions.binary_search_by(|f| f.compare(&id, args.len())).is_ok() { + // 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); + } + let mut arg_values: Vec<_> = args.iter().map(Expr::get_constant_value).collect(); let call_args: FnCallArgs = arg_values.iter_mut().map(Dynamic::as_mut).collect(); @@ -477,19 +481,14 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { // 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, args.into_iter().map(|a| optimize_expr(a, state)).collect(), def_value, pos), // constant-name Expr::Variable(ref name, _) if state.contains_constant(name) => { state.set_dirty(); // Replace constant with value - state - .find_constant(name) - .expect("should find constant in scope!") - .clone() + state.find_constant(name).expect("should find constant in scope!").clone() } // All other expressions - skip From 16ea8f416e605bb71dd27ce20e0e38aeed0b3d4e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 19 Mar 2020 22:29:24 +0800 Subject: [PATCH 5/8] Revise section on function side effects because Rhai functions cannot mutate state. --- README.md | 61 ++++++++++++++++++++++++++----------------------------- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index cc21e307..a641e333 100644 --- a/README.md +++ b/README.md @@ -1304,7 +1304,8 @@ for entry in log { Script optimization =================== -Rhai includes an _optimizer_ that tries to optimize a script after parsing. This can reduce resource utilization and increase execution speed. +Rhai includes an _optimizer_ that tries to optimize a script after parsing. +This can reduce resource utilization and increase execution speed. Script optimization can be turned off via the [`no_optimize`] feature. For example, in the following: @@ -1315,7 +1316,7 @@ For example, in the following: 123; // eliminated - no effect "hello"; // eliminated - no effect [1, 2, x, x*2, 5]; // eliminated - no effect - foo(42); // NOT eliminated - the function 'foo' may have side effects + foo(42); // NOT eliminated - functions calls are not touched 666 // NOT eliminated - this is the return value of the block, // and the block is the last one // so this is the return value of the whole script @@ -1349,14 +1350,12 @@ are spliced into the script text in order to turn on/off certain sections. For fixed script texts, the constant values can be provided in a user-defined [`Scope`] object to the [`Engine`] for use in compilation and evaluation. -Beware, however, that most operators are actually function calls, and those functions can be overridden, -so they are not optimized away: +Beware, however, that most operators are actually function calls, and those are not optimized away: ```rust const DECISION = 1; -if DECISION == 1 { // NOT optimized away because you can define - : // your own '==' function to override the built-in default! +if DECISION == 1 { // NOT optimized away because it requires a call to the '==' function : } else if DECISION == 2 { // same here, NOT optimized away : @@ -1367,8 +1366,8 @@ if DECISION == 1 { // NOT optimized away because you can define } ``` -because no operator functions will be run (in order not to trigger side effects) during the optimization process -(unless the optimization level is set to [`OptimizationLevel::Full`]). So, instead, do this: +because no operator functions will be run during the optimization process (unless the optimization level is +set to [`OptimizationLevel::Full`]). So, instead, do this: ```rust const DECISION_1 = true; @@ -1405,11 +1404,14 @@ There are actually three levels of optimizations: `None`, `Simple` and `Full`. * `None` is obvious - no optimization on the AST is performed. -* `Simple` (default) performs relatively _safe_ optimizations without causing side effects - (i.e. it only relies on static analysis and will not actually perform any function calls). +* `Simple` (default) relies exclusively on static analysis, performing relatively _safe_ optimizations only. + In particular, no function calls will be made to determine the output value. This also means that most comparison + operators and constant arithmetic expressions are untouched. -* `Full` is _much_ more aggressive, _including_ running functions on constant arguments to determine their result. - One benefit to this is that many more optimization opportunities arise, especially with regards to comparison operators. +* `Full` is _much_ more aggressive. Functions _will_ be run, when passed constant arguments, to determine their results. + One benefit is that many more optimization opportunities arise, especially with regards to comparison operators. + Nevertheless, the majority of scripts do not excessively rely on constants, and that is why this optimization level + is opt-in; only scripts that are machine-generated tend to have constants spliced in at generation time. An [`Engine`]'s optimization level is set via a call to `set_optimization_level`: @@ -1419,15 +1421,15 @@ engine.set_optimization_level(rhai::OptimizationLevel::Full); ``` When the optimization level is [`OptimizationLevel::Full`], the [`Engine`] assumes all functions to be _pure_ and will _eagerly_ -evaluated all function calls with constant arguments, using the result to replace the call. This also applies to all operators -(which are implemented as functions). For instance, the same example above: +call all functions when passed constant arguments, using the results to replace the actual calls. This also affects all operators +because most of them are implemented as functions. For instance, the same example above: ```rust // When compiling the following with OptimizationLevel::Full... const DECISION = 1; - // this condition is now eliminated because 'DECISION == 1' -if DECISION == 1 { // is a function call to the '==' function, and it returns 'true' + // this condition is now eliminated because 'DECISION == 1' is a +if DECISION == 1 { // function call to the '==' function with constant arguments, and it returns 'true' print("hello!"); // this block is promoted to the parent level } else { print("boo!"); // this block is eliminated because it is never reached @@ -1446,25 +1448,20 @@ let x = (1 + 2) * 3 - 4 / 5 % 6; // <- will be replaced by 'let x = 9' let y = (1 > 2) || (3 <= 4); // <- will be replaced by 'let y = true' ``` -Function side effect considerations ----------------------------------- - -All of Rhai's built-in functions (and operators which are implemented as functions) are _pure_ (i.e. they do not mutate state -nor cause side any effects, with the exception of `print` and `debug` which are handled specially) so using [`OptimizationLevel::Full`] -is usually quite safe _unless_ you register your own types and functions. - -If custom functions are registered, they _may_ be called (or maybe not, if the calls happen to lie within a pruned code block). -If custom functions are registered to replace built-in operators, they will also be called when the operators are used (in an `if` -statement, for example) and cause side-effects. - Function volatility considerations --------------------------------- -Even if a custom function does not mutate state nor cause side effects, it may still be _volatile_, i.e. it _depends_ on the external -environment and not _pure_. A perfect example is a function that gets the current time - obviously each run will return a different value! -The optimizer, when using [`OptimizationLevel::Full`], _assumes_ that all functions are _pure_, so when it finds constant arguments. -This may cause the script to behave differently from the intended semantics because essentially the result of each function call will -always be the same value. +Rhai functions never mutate state nor cause side any effects (except `print` and `debug` which are handled specially). +The only functions allowed to mutate state are custom type getters, setters and methods, and functions calls involving custom types +are never optimized. So using [`OptimizationLevel::Full`] is usually quite safe. + +However, even if a function cannot mutate state nor cause side effects, it may still be _volatile_, i.e. it may +_depend_ on the external environment and not be _pure_. A perfect example is a function that gets the current time - +obviously each run will return a different value! + +The optimizer, when using [`OptimizationLevel::Full`], _assumes_ that all functions are _pure_ when it finds constant arguments. +This may cause the script to behave differently from the intended semantics because essentially the result of each function call +will always be the same value. Therefore, **avoid using [`OptimizationLevel::Full`]** if you intend to register non-_pure_ custom types and/or functions. From ed996e71d65e5ba20367e83788d9cd5fabc66f92 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 20 Mar 2020 19:27:02 +0800 Subject: [PATCH 6/8] Enable dot operations on constant variables. --- README.md | 44 +++- src/api.rs | 3 +- src/engine.rs | 501 ++++++++++++++++++++++++------------------ src/scope.rs | 30 ++- tests/get_set.rs | 2 +- tests/side_effects.rs | 39 ++++ tests/types.rs | 4 +- 7 files changed, 385 insertions(+), 238 deletions(-) create mode 100644 tests/side_effects.rs diff --git a/README.md b/README.md index a641e333..aa478554 100644 --- a/README.md +++ b/README.md @@ -273,10 +273,11 @@ type_of('c') == "char"; type_of(42) == "i64"; let x = 123; -x.type_of() == "i64"; +x.type_of(); // error - 'type_of' cannot use postfix notation +type_of(x) == "i64"; x = 99.999; -x.type_of() == "f64"; +type_of(x) == "f64"; x = "hello"; if type_of(x) == "string" { @@ -538,12 +539,12 @@ with a special "pretty-print" name, [`type_of`] will return that name instead. engine.register_type::(); engine.register_fn("new_ts", TestStruct::new); let x = new_ts(); -print(x.type_of()); // prints "path::to::module::TestStruct" +print(type_of(x)); // prints "path::to::module::TestStruct" engine.register_type_with_name::("Hello"); engine.register_fn("new_ts", TestStruct::new); let x = new_ts(); -print(x.type_of()); // prints "Hello" +print(type_of(x)); // prints "Hello" ``` Getters and setters @@ -1527,16 +1528,35 @@ let script = "let y = x;"; // build a script script += "y += foo(y);"; script += "x + y"; -let result = eval(script); +let result = eval(script); // <- look, JS, we can also do this! print("Answer: " + result); // prints 42 -print("x = " + x); // prints 10 (functions call arguments are passed by value) -print("y = " + y); // prints 32 (variables defined in 'eval' persist) +print("x = " + x); // prints 10 - functions call arguments are passed by value +print("y = " + y); // prints 32 - variables defined in 'eval' persist! eval("{ let z = y }"); // to keep a variable local, use a statement block print("z = " + z); // error - variable 'z' not found + +"print(42)".eval(); // nope - just like 'type_of' postfix notation doesn't work +``` + +Script segments passed to `eval` execute inside the current [`Scope`], so they can access and modify _everything_, +including all variables that are visible at that position in code! It is almost as if the script segments were +physically pasted in at the position of the `eval` call. + +```rust +let script = "x += 32"; +let x = 10; +eval(script); // variable 'x' in the current scope is visible! +print(x); // prints 42 + +// The above is equivalent to: +let script = "x += 32"; +let x = 10; +x += 32; +print(x); ``` For those who subscribe to the (very sensible) motto of ["`eval` is **evil**"](http://linterrors.com/js/eval-is-evil), @@ -1547,3 +1567,13 @@ fn eval(script) { throw "eval is evil! I refuse to run " + script } let x = eval("40 + 2"); // 'eval' here throws "eval is evil! I refuse to run 40 + 2" ``` + +Or override it from Rust: + +```rust +fn alt_eval(script: String) -> Result<(), EvalAltResult> { + Err(format!("eval is evil! I refuse to run {}", script).into()) +} + +engine.register_result_fn("eval", alt_eval); +``` diff --git a/src/api.rs b/src/api.rs index d6f592ba..850ff357 100644 --- a/src/api.rs +++ b/src/api.rs @@ -765,10 +765,9 @@ impl<'e> Engine<'e> { name: &str, mut values: Vec, ) -> Result { - let mut scope = Scope::new(); let values: Vec<_> = values.iter_mut().map(Dynamic::as_mut).collect(); - let result = engine.call_fn_raw(&mut scope, name, values, None, Position::none()); + let result = engine.call_fn_raw(name, values, None, Position::none()); result } diff --git a/src/engine.rs b/src/engine.rs index bd389312..d0c81ba4 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -3,7 +3,7 @@ use crate::any::{Any, AnyExt, Dynamic, Variant}; use crate::parser::{Expr, FnDef, Position, ReturnType, Stmt}; use crate::result::EvalAltResult; -use crate::scope::{Scope, VariableType}; +use crate::scope::{Scope, ScopeSource, VariableType}; #[cfg(not(feature = "no_optimize"))] use crate::optimize::OptimizationLevel; @@ -161,7 +161,6 @@ impl Engine<'_> { /// registered with the `Engine` or written in Rhai pub(crate) fn call_fn_raw( &mut self, - scope: &mut Scope, fn_name: &str, args: FnCallArgs, def_val: Option<&Dynamic>, @@ -225,40 +224,6 @@ impl Engine<'_> { return Ok(result); } - if fn_name == KEYWORD_TYPE_OF && args.len() == 1 { - // Handle `type_of` function - return Ok(self - .map_type_name(args[0].type_name()) - .to_string() - .into_dynamic()); - } - - if fn_name == KEYWORD_EVAL && args.len() == 1 { - // Handle `eval` function - let script = cast_to_string(args[0], pos)?; - - #[cfg(not(feature = "no_optimize"))] - let ast = { - let orig_optimization_level = self.optimization_level; - - self.set_optimization_level(OptimizationLevel::None); - let ast = self.compile(script); - self.set_optimization_level(orig_optimization_level); - - ast.map_err(EvalAltResult::ErrorParsing)? - }; - - #[cfg(feature = "no_optimize")] - let ast = self.compile(script).map_err(EvalAltResult::ErrorParsing)?; - - return Ok(self - .eval_ast_with_scope_raw(scope, true, &ast) - .map_err(|mut err| { - err.set_position(pos); - err - })?); - } - if fn_name.starts_with(FUNC_GETTER) { // Getter function not found return Err(EvalAltResult::ErrorDotExpr( @@ -299,13 +264,38 @@ impl Engine<'_> { )) } - /// Chain-evaluate a dot setter + /// Chain-evaluate a dot setter. + /// + /// Either `src` or `target` should be `Some`. + /// + /// If `target` is `Some`, then it is taken as the reference to use for `this`. + /// + /// Otherwise, if `src` is `Some`, then it holds a name and index into `scope`; using `get_mut` on + /// `scope` can retrieve a mutable reference to the variable's value to use as `this`. fn get_dot_val_helper( &mut self, scope: &mut Scope, - this_ptr: &mut Variant, + src: Option, + target: Option<&mut Variant>, dot_rhs: &Expr, ) -> Result { + // Get the `this` reference. Either `src` or `target` should be `Some`. + fn get_this_ptr<'a>( + scope: &'a mut Scope, + src: Option, + target: Option<&'a mut Variant>, + ) -> &'a mut Variant { + if let Some(t) = target { + // If `target` is `Some`, then it is returned. + t + } else { + // Otherwise, if `src` is `Some`, then it holds a name and index into `scope`; + // using `get_mut` on `scope` to retrieve a mutable reference for return. + let ScopeSource { name, idx, .. } = src.expect("expected source in scope"); + scope.get_mut(name, idx).as_mut() + } + } + match dot_rhs { // xxx.fn_name(args) Expr::FunctionCall(fn_name, arg_expr_list, def_val, pos) => { @@ -314,36 +304,40 @@ impl Engine<'_> { .map(|arg_expr| self.eval_expr(scope, arg_expr)) .collect::, _>>()?; + let this_ptr = get_this_ptr(scope, src, target); + let args = once(this_ptr) .chain(values.iter_mut().map(Dynamic::as_mut)) .collect(); - self.call_fn_raw(scope, fn_name, args, def_val.as_ref(), *pos) + self.call_fn_raw(fn_name, args, def_val.as_ref(), *pos) } // xxx.id Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); - - self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos) + let this_ptr = get_this_ptr(scope, src, target); + self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos) } // xxx.idx_lhs[idx_expr] #[cfg(not(feature = "no_index"))] - Expr::Index(idx_lhs, idx_expr, idx_pos) => { + Expr::Index(idx_lhs, idx_expr, op_pos) => { let (val, _) = match idx_lhs.as_ref() { // xxx.id[idx_expr] Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); + let this_ptr = get_this_ptr(scope, src, target); ( - self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos)?, + self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos)?, *pos, ) } // xxx.???[???][idx_expr] - Expr::Index(_, _, _) => { - (self.get_dot_val_helper(scope, this_ptr, idx_lhs)?, *idx_pos) - } + Expr::Index(_, _, _) => ( + self.get_dot_val_helper(scope, src, target, idx_lhs)?, + *op_pos, + ), // Syntax error _ => { return Err(EvalAltResult::ErrorDotExpr( @@ -354,7 +348,7 @@ impl Engine<'_> { }; let idx = self.eval_index_value(scope, idx_expr)?; - self.get_indexed_value(&val, idx, idx_expr.position(), *idx_pos) + self.get_indexed_value(&val, idx, idx_expr.position(), *op_pos) .map(|(v, _)| v) } @@ -363,26 +357,31 @@ impl Engine<'_> { // xxx.id.rhs Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); + let this_ptr = get_this_ptr(scope, src, target); - self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos) - .and_then(|mut v| self.get_dot_val_helper(scope, v.as_mut(), rhs)) + self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos) + .and_then(|mut v| { + self.get_dot_val_helper(scope, None, Some(v.as_mut()), rhs) + }) } // xxx.idx_lhs[idx_expr].rhs #[cfg(not(feature = "no_index"))] - Expr::Index(idx_lhs, idx_expr, idx_pos) => { + 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 get_fn_name = format!("{}{}", FUNC_GETTER, id); + let this_ptr = get_this_ptr(scope, src, target); ( - self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos)?, + self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos)?, *pos, ) } // xxx.???[???][idx_expr].rhs - Expr::Index(_, _, _) => { - (self.get_dot_val_helper(scope, this_ptr, idx_lhs)?, *idx_pos) - } + Expr::Index(_, _, _) => ( + self.get_dot_val_helper(scope, src, target, idx_lhs)?, + *op_pos, + ), // Syntax error _ => { return Err(EvalAltResult::ErrorDotExpr( @@ -393,8 +392,10 @@ impl Engine<'_> { }; let idx = self.eval_index_value(scope, idx_expr)?; - self.get_indexed_value(&val, idx, idx_expr.position(), *idx_pos) - .and_then(|(mut v, _)| self.get_dot_val_helper(scope, v.as_mut(), rhs)) + self.get_indexed_value(&val, idx, idx_expr.position(), *op_pos) + .and_then(|(mut v, _)| { + self.get_dot_val_helper(scope, None, Some(v.as_mut()), rhs) + }) } // Syntax error _ => Err(EvalAltResult::ErrorDotExpr( @@ -421,28 +422,33 @@ impl Engine<'_> { match dot_lhs { // id.??? Expr::Variable(id, pos) => { - let (src_idx, _, mut target) = Self::search_scope(scope, id, Ok, *pos)?; - let val = self.get_dot_val_helper(scope, target.as_mut(), dot_rhs); + let (ScopeSource { idx, var_type, .. }, _) = + Self::search_scope(scope, id, Ok, *pos)?; - // In case the expression mutated `target`, we need to update it back into the scope because it is cloned. - *scope.get_mut(id, src_idx) = target; + // This is a variable property access (potential function call). + // Use a direct index into `scope` to directly mutate the variable value. + let src = ScopeSource { + name: id, + idx, + var_type, + }; - val + self.get_dot_val_helper(scope, Some(src), None, dot_rhs) } // idx_lhs[idx_expr].??? #[cfg(not(feature = "no_index"))] - Expr::Index(idx_lhs, idx_expr, idx_pos) => { + Expr::Index(idx_lhs, idx_expr, op_pos) => { let (src_type, src, idx, mut target) = - self.eval_index_expr(scope, idx_lhs, idx_expr, *idx_pos)?; - let val = self.get_dot_val_helper(scope, target.as_mut(), dot_rhs); + self.eval_index_expr(scope, idx_lhs, idx_expr, *op_pos)?; + let val = self.get_dot_val_helper(scope, None, Some(target.as_mut()), dot_rhs); // In case the expression mutated `target`, we need to update it back into the scope because it is cloned. - if let Some((id, var_type, src_idx)) = src { - match var_type { + if let Some(src) = src { + match src.var_type { VariableType::Constant => { return Err(EvalAltResult::ErrorAssignmentToConstant( - id.to_string(), + src.name.to_string(), idx_lhs.position(), )); } @@ -450,11 +456,9 @@ impl Engine<'_> { Self::update_indexed_var_in_scope( src_type, scope, - id, - src_idx, + src, idx, - target, - dot_rhs.position(), + (target, dot_rhs.position()), )?; } } @@ -466,22 +470,22 @@ impl Engine<'_> { // {expr}.??? expr => { let mut target = self.eval_expr(scope, expr)?; - self.get_dot_val_helper(scope, target.as_mut(), dot_rhs) + self.get_dot_val_helper(scope, None, Some(target.as_mut()), dot_rhs) } } } /// Search for a variable within the scope, returning its value and index inside the Scope - fn search_scope( - scope: &Scope, + fn search_scope<'a, T>( + scope: &'a Scope, id: &str, - map: impl FnOnce(Dynamic) -> Result, + convert: impl FnOnce(Dynamic) -> Result, begin: Position, - ) -> Result<(usize, VariableType, T), EvalAltResult> { + ) -> Result<(ScopeSource<'a>, T), EvalAltResult> { scope .get(id) .ok_or_else(|| EvalAltResult::ErrorVariableNotFound(id.into(), begin)) - .and_then(move |(idx, _, var_type, val)| map(val).map(|v| (idx, var_type, v))) + .and_then(move |(src, value)| convert(value).map(|v| (src, v))) } /// Evaluate the value of an index (must evaluate to INT) @@ -503,8 +507,8 @@ impl Engine<'_> { &self, val: &Dynamic, idx: INT, - val_pos: Position, idx_pos: Position, + op_pos: Position, ) -> Result<(Dynamic, IndexSourceType), EvalAltResult> { if val.is::() { // val_array[idx] @@ -514,9 +518,9 @@ impl Engine<'_> { arr.get(idx as usize) .cloned() .map(|v| (v, IndexSourceType::Array)) - .ok_or_else(|| EvalAltResult::ErrorArrayBounds(arr.len(), idx, val_pos)) + .ok_or_else(|| EvalAltResult::ErrorArrayBounds(arr.len(), idx, idx_pos)) } else { - Err(EvalAltResult::ErrorArrayBounds(arr.len(), idx, val_pos)) + Err(EvalAltResult::ErrorArrayBounds(arr.len(), idx, idx_pos)) } } else if val.is::() { // val_string[idx] @@ -527,20 +531,20 @@ impl Engine<'_> { .nth(idx as usize) .map(|ch| (ch.into_dynamic(), IndexSourceType::String)) .ok_or_else(|| { - EvalAltResult::ErrorStringBounds(s.chars().count(), idx, val_pos) + EvalAltResult::ErrorStringBounds(s.chars().count(), idx, idx_pos) }) } else { Err(EvalAltResult::ErrorStringBounds( s.chars().count(), idx, - val_pos, + idx_pos, )) } } else { // Error - cannot be indexed Err(EvalAltResult::ErrorIndexingType( self.map_type_name(val.type_name()).to_string(), - idx_pos, + op_pos, )) } } @@ -552,16 +556,8 @@ impl Engine<'_> { scope: &mut Scope, lhs: &'a Expr, idx_expr: &Expr, - idx_pos: Position, - ) -> Result< - ( - IndexSourceType, - Option<(&'a str, VariableType, usize)>, - usize, - Dynamic, - ), - EvalAltResult, - > { + op_pos: Position, + ) -> Result<(IndexSourceType, Option>, usize, Dynamic), EvalAltResult> { let idx = self.eval_index_value(scope, idx_expr)?; match lhs { @@ -569,13 +565,17 @@ impl Engine<'_> { Expr::Variable(id, _) => Self::search_scope( scope, &id, - |val| self.get_indexed_value(&val, idx, idx_expr.position(), idx_pos), + |val| self.get_indexed_value(&val, idx, idx_expr.position(), op_pos), lhs.position(), ) - .map(|(src_idx, var_type, (val, src_type))| { + .map(|(src, (val, src_type))| { ( src_type, - Some((id.as_str(), var_type, src_idx)), + Some(ScopeSource { + name: &id, + var_type: src.var_type, + idx: src.idx, + }), idx as usize, val, ) @@ -585,7 +585,7 @@ impl Engine<'_> { expr => { let val = self.eval_expr(scope, expr)?; - self.get_indexed_value(&val, idx, idx_expr.position(), idx_pos) + self.get_indexed_value(&val, idx, idx_expr.position(), op_pos) .map(|(v, _)| (IndexSourceType::Expression, None, idx as usize, v)) } } @@ -610,26 +610,26 @@ impl Engine<'_> { fn update_indexed_var_in_scope( src_type: IndexSourceType, scope: &mut Scope, - id: &str, - src_idx: usize, + src: ScopeSource, idx: usize, - new_val: Dynamic, - val_pos: Position, + new_val: (Dynamic, Position), ) -> Result { match src_type { // array_id[idx] = val IndexSourceType::Array => { - let arr = scope.get_mut_by_type::(id, src_idx); - Ok((arr[idx as usize] = new_val).into_dynamic()) + let arr = scope.get_mut_by_type::(src.name, src.idx); + Ok((arr[idx as usize] = new_val.0).into_dynamic()) } // string_id[idx] = val IndexSourceType::String => { - let s = scope.get_mut_by_type::(id, src_idx); + let s = scope.get_mut_by_type::(src.name, src.idx); + let pos = new_val.1; // Value must be a character let ch = *new_val + .0 .downcast::() - .map_err(|_| EvalAltResult::ErrorCharMismatch(val_pos))?; + .map_err(|_| EvalAltResult::ErrorCharMismatch(pos))?; Ok(Self::str_replace_char(s, idx as usize, ch).into_dynamic()) } @@ -669,52 +669,44 @@ impl Engine<'_> { scope: &mut Scope, this_ptr: &mut Variant, dot_rhs: &Expr, - mut new_val: Dynamic, - val_pos: Position, + new_val: (&mut Dynamic, Position), ) -> Result { match dot_rhs { // xxx.id Expr::Property(id, pos) => { let set_fn_name = format!("{}{}", FUNC_SETTER, id); - self.call_fn_raw( - scope, - &set_fn_name, - vec![this_ptr, new_val.as_mut()], - None, - *pos, - ) + self.call_fn_raw(&set_fn_name, vec![this_ptr, new_val.0.as_mut()], None, *pos) } // xxx.lhs[idx_expr] // TODO - Allow chaining of indexing! #[cfg(not(feature = "no_index"))] - Expr::Index(lhs, idx_expr, idx_pos) => match lhs.as_ref() { + Expr::Index(lhs, idx_expr, op_pos) => match lhs.as_ref() { // xxx.id[idx_expr] Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); - self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos) + self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos) .and_then(|v| { let idx = self.eval_index_value(scope, idx_expr)?; - Self::update_indexed_value(v, idx as usize, new_val, val_pos) + Self::update_indexed_value( + v, + idx as usize, + new_val.0.clone(), + new_val.1, + ) }) .and_then(|mut v| { let set_fn_name = format!("{}{}", FUNC_SETTER, id); - self.call_fn_raw( - scope, - &set_fn_name, - vec![this_ptr, v.as_mut()], - None, - *pos, - ) + self.call_fn_raw(&set_fn_name, vec![this_ptr, v.as_mut()], None, *pos) }) } // All others - syntax error for setters chain _ => Err(EvalAltResult::ErrorDotExpr( "for assignment".to_string(), - *idx_pos, + *op_pos, )), }, @@ -724,45 +716,34 @@ impl Engine<'_> { Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); - self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos) + self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos) .and_then(|mut v| { - self.set_dot_val_helper(scope, v.as_mut(), rhs, new_val, val_pos) + self.set_dot_val_helper(scope, v.as_mut(), rhs, new_val) .map(|_| v) // Discard Ok return value }) .and_then(|mut v| { let set_fn_name = format!("{}{}", FUNC_SETTER, id); - self.call_fn_raw( - scope, - &set_fn_name, - vec![this_ptr, v.as_mut()], - None, - *pos, - ) + self.call_fn_raw(&set_fn_name, vec![this_ptr, v.as_mut()], None, *pos) }) } // xxx.lhs[idx_expr].rhs // TODO - Allow chaining of indexing! #[cfg(not(feature = "no_index"))] - Expr::Index(lhs, idx_expr, idx_pos) => match lhs.as_ref() { + Expr::Index(lhs, idx_expr, op_pos) => match lhs.as_ref() { // xxx.id[idx_expr].rhs Expr::Property(id, pos) => { let get_fn_name = format!("{}{}", FUNC_GETTER, id); - self.call_fn_raw(scope, &get_fn_name, vec![this_ptr], None, *pos) + self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos) .and_then(|v| { let idx = self.eval_index_value(scope, idx_expr)?; let (mut target, _) = - self.get_indexed_value(&v, idx, idx_expr.position(), *idx_pos)?; + self.get_indexed_value(&v, idx, idx_expr.position(), *op_pos)?; - self.set_dot_val_helper( - scope, - target.as_mut(), - rhs, - new_val, - val_pos, - )?; + let val_pos = new_val.1; + self.set_dot_val_helper(scope, target.as_mut(), rhs, new_val)?; // In case the expression mutated `target`, we need to update it back into the scope because it is cloned. Self::update_indexed_value(v, idx as usize, target, val_pos) @@ -771,7 +752,6 @@ impl Engine<'_> { let set_fn_name = format!("{}{}", FUNC_SETTER, id); self.call_fn_raw( - scope, &set_fn_name, vec![this_ptr, v.as_mut()], None, @@ -783,7 +763,7 @@ impl Engine<'_> { // All others - syntax error for setters chain _ => Err(EvalAltResult::ErrorDotExpr( "for assignment".to_string(), - *idx_pos, + *op_pos, )), }, @@ -808,14 +788,14 @@ impl Engine<'_> { scope: &mut Scope, dot_lhs: &Expr, dot_rhs: &Expr, - new_val: Dynamic, - val_pos: Position, + new_val: (&mut Dynamic, Position), op_pos: Position, ) -> Result { match dot_lhs { // id.??? Expr::Variable(id, pos) => { - let (src_idx, var_type, mut target) = Self::search_scope(scope, id, Ok, *pos)?; + let (ScopeSource { idx, var_type, .. }, mut target) = + Self::search_scope(scope, id, Ok, *pos)?; match var_type { VariableType::Constant => { @@ -827,11 +807,10 @@ impl Engine<'_> { _ => (), } - let val = - self.set_dot_val_helper(scope, target.as_mut(), dot_rhs, new_val, val_pos); + let val = self.set_dot_val_helper(scope, target.as_mut(), dot_rhs, new_val); // In case the expression mutated `target`, we need to update it back into the scope because it is cloned. - *scope.get_mut(id, src_idx) = target; + *scope.get_mut(id, idx) = target; val } @@ -839,24 +818,28 @@ impl Engine<'_> { // lhs[idx_expr].??? // TODO - Allow chaining of indexing! #[cfg(not(feature = "no_index"))] - Expr::Index(lhs, idx_expr, idx_pos) => { + Expr::Index(lhs, idx_expr, op_pos) => { let (src_type, src, idx, mut target) = - self.eval_index_expr(scope, lhs, idx_expr, *idx_pos)?; - let val = - self.set_dot_val_helper(scope, target.as_mut(), dot_rhs, new_val, val_pos); + self.eval_index_expr(scope, lhs, idx_expr, *op_pos)?; + let val_pos = new_val.1; + let val = self.set_dot_val_helper(scope, target.as_mut(), dot_rhs, new_val); // In case the expression mutated `target`, we need to update it back into the scope because it is cloned. - if let Some((id, var_type, src_idx)) = src { - match var_type { + if let Some(src) = src { + match src.var_type { VariableType::Constant => { return Err(EvalAltResult::ErrorAssignmentToConstant( - id.to_string(), + src.name.to_string(), lhs.position(), )); } VariableType::Normal => { Self::update_indexed_var_in_scope( - src_type, scope, id, src_idx, idx, target, val_pos, + src_type, + scope, + src, + idx, + (target, val_pos), )?; } } @@ -882,15 +865,13 @@ impl Engine<'_> { Expr::IntegerConstant(i, _) => Ok(i.into_dynamic()), Expr::StringConstant(s, _) => Ok(s.into_dynamic()), Expr::CharConstant(c, _) => Ok(c.into_dynamic()), - Expr::Variable(id, pos) => { - Self::search_scope(scope, id, Ok, *pos).map(|(_, _, val)| val) - } + Expr::Variable(id, pos) => Self::search_scope(scope, id, Ok, *pos).map(|(_, val)| val), Expr::Property(_, _) => panic!("unexpected property."), // lhs[idx_expr] #[cfg(not(feature = "no_index"))] - Expr::Index(lhs, idx_expr, idx_pos) => self - .eval_index_expr(scope, lhs, idx_expr, *idx_pos) + Expr::Index(lhs, idx_expr, op_pos) => self + .eval_index_expr(scope, lhs, idx_expr, *op_pos) .map(|(_, _, _, x)| x), // Statement block @@ -898,43 +879,55 @@ impl Engine<'_> { // lhs = rhs Expr::Assignment(lhs, rhs, op_pos) => { - let rhs_val = self.eval_expr(scope, rhs)?; + let mut rhs_val = self.eval_expr(scope, rhs)?; match lhs.as_ref() { // name = rhs Expr::Variable(name, pos) => match scope.get(name) { - Some((idx, _, VariableType::Normal, _)) => { + Some(( + ScopeSource { + idx, + var_type: VariableType::Normal, + .. + }, + _, + )) => { *scope.get_mut(name, idx) = rhs_val.clone(); Ok(rhs_val) } - Some((_, _, VariableType::Constant, _)) => Err( - EvalAltResult::ErrorAssignmentToConstant(name.to_string(), *op_pos), - ), + Some(( + ScopeSource { + var_type: VariableType::Constant, + .. + }, + _, + )) => Err(EvalAltResult::ErrorAssignmentToConstant( + name.to_string(), + *op_pos, + )), _ => Err(EvalAltResult::ErrorVariableNotFound(name.clone(), *pos)), }, // idx_lhs[idx_expr] = rhs #[cfg(not(feature = "no_index"))] - Expr::Index(idx_lhs, idx_expr, idx_pos) => { + Expr::Index(idx_lhs, idx_expr, op_pos) => { let (src_type, src, idx, _) = - self.eval_index_expr(scope, idx_lhs, idx_expr, *idx_pos)?; + self.eval_index_expr(scope, idx_lhs, idx_expr, *op_pos)?; - if let Some((id, var_type, src_idx)) = src { - match var_type { + if let Some(src) = src { + match src.var_type { VariableType::Constant => { return Err(EvalAltResult::ErrorAssignmentToConstant( - id.to_string(), + src.name.to_string(), idx_lhs.position(), )); } VariableType::Normal => Ok(Self::update_indexed_var_in_scope( src_type, scope, - &id, - src_idx, + src, idx, - rhs_val, - rhs.position(), + (rhs_val, rhs.position()), )?), } } else { @@ -945,9 +938,13 @@ impl Engine<'_> { } // dot_lhs.dot_rhs = rhs - Expr::Dot(dot_lhs, dot_rhs, _) => { - self.set_dot_val(scope, dot_lhs, dot_rhs, rhs_val, rhs.position(), *op_pos) - } + Expr::Dot(dot_lhs, dot_rhs, _) => self.set_dot_val( + scope, + dot_lhs, + dot_rhs, + (&mut rhs_val, rhs.position()), + *op_pos, + ), // Error assignment to constant expr if expr.is_constant() => Err(EvalAltResult::ErrorAssignmentToConstant( @@ -973,45 +970,111 @@ impl Engine<'_> { Ok(Box::new(arr)) } - // Dump AST - Expr::FunctionCall(fn_name, args_expr_list, _, pos) if fn_name == KEYWORD_DUMP_AST => { - let pos = if args_expr_list.len() == 0 { - *pos - } else { - args_expr_list[0].position() - }; - - // Change the argument to a debug dump of the expressions - let result = args_expr_list - .into_iter() - .map(|expr| format!("{:#?}", expr)) - .collect::>() - .join("\n"); - - // Redirect call to `print` - self.call_fn_raw( - scope, - KEYWORD_PRINT, - vec![result.into_dynamic().as_mut()], - None, - pos, - ) - } - - // Normal function call Expr::FunctionCall(fn_name, args_expr_list, def_val, pos) => { - let mut values = args_expr_list - .iter() - .map(|expr| self.eval_expr(scope, expr)) - .collect::, _>>()?; + // Has a system function an override? + fn has_override(engine: &Engine, name: &str) -> bool { + let spec = FnSpec { + name: name.into(), + args: Some(vec![TypeId::of::()]), + }; - self.call_fn_raw( - scope, - fn_name, - values.iter_mut().map(|b| b.as_mut()).collect(), - def_val.as_ref(), - *pos, - ) + engine.ext_functions.contains_key(&spec) + || engine + .script_functions + .binary_search_by(|f| f.compare(name, 1)) + .is_ok() + } + + match fn_name.as_str() { + // Dump AST + KEYWORD_DUMP_AST => { + let pos = if args_expr_list.len() == 0 { + *pos + } else { + args_expr_list[0].position() + }; + + // Change the argument to a debug dump of the expressions + let result = args_expr_list + .into_iter() + .map(|expr| format!("{:#?}", expr)) + .collect::>() + .join("\n"); + + // Redirect call to `print` + self.call_fn_raw( + KEYWORD_PRINT, + vec![result.into_dynamic().as_mut()], + None, + pos, + ) + } + + // type_of + KEYWORD_TYPE_OF + if args_expr_list.len() == 1 && !has_override(self, KEYWORD_TYPE_OF) => + { + let r = self.eval_expr(scope, &args_expr_list[0])?; + Ok(self + .map_type_name((*r).type_name()) + .to_string() + .into_dynamic()) + } + + // eval + KEYWORD_EVAL + if args_expr_list.len() == 1 && !has_override(self, KEYWORD_EVAL) => + { + let pos = args_expr_list[0].position(); + let r = self.eval_expr(scope, &args_expr_list[0])?; + + let script = + r.downcast_ref::() + .map(String::as_str) + .ok_or_else(|| { + EvalAltResult::ErrorMismatchOutputType( + r.type_name().into(), + pos, + ) + })?; + + #[cfg(not(feature = "no_optimize"))] + let ast = { + let orig_optimization_level = self.optimization_level; + + self.set_optimization_level(OptimizationLevel::None); + let ast = self.compile(script); + self.set_optimization_level(orig_optimization_level); + + ast.map_err(EvalAltResult::ErrorParsing)? + }; + + #[cfg(feature = "no_optimize")] + let ast = self.compile(script).map_err(EvalAltResult::ErrorParsing)?; + + return Ok(self.eval_ast_with_scope_raw(scope, true, &ast).map_err( + |mut err| { + err.set_position(pos); + err + }, + )?); + } + + // Normal function call + _ => { + let mut values = args_expr_list + .iter() + .map(|expr| self.eval_expr(scope, expr)) + .collect::, _>>()?; + + self.call_fn_raw( + fn_name, + values.iter_mut().map(|b| b.as_mut()).collect(), + def_val.as_ref(), + *pos, + ) + } + } } Expr::And(lhs, rhs) => Ok(Box::new( diff --git a/src/scope.rs b/src/scope.rs index b7d43c97..a40a2589 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -31,6 +31,13 @@ pub struct ScopeEntry<'a> { pub expr: Option, } +/// Information about a particular entry in the Scope. +pub(crate) struct ScopeSource<'a> { + pub name: &'a str, + pub idx: usize, + pub var_type: VariableType, +} + /// A type containing information about the current scope. /// Useful for keeping state between `Engine` evaluation runs. /// @@ -150,7 +157,7 @@ impl<'a> Scope<'a> { } /// Find a variable in the Scope, starting from the last. - pub fn get(&self, key: &str) -> Option<(usize, &str, VariableType, Dynamic)> { + pub(crate) fn get(&self, key: &str) -> Option<(ScopeSource, Dynamic)> { self.0 .iter() .enumerate() @@ -165,7 +172,16 @@ impl<'a> Scope<'a> { value, .. }, - )| (i, name.as_ref(), *var_type, value.clone()), + )| { + ( + ScopeSource { + name: name.as_ref(), + idx: i, + var_type: *var_type, + }, + value.clone(), + ) + }, ) } @@ -184,11 +200,11 @@ impl<'a> Scope<'a> { pub(crate) fn get_mut(&mut self, name: &str, index: usize) -> &mut Dynamic { let entry = self.0.get_mut(index).expect("invalid index in Scope"); - assert_ne!( - entry.var_type, - VariableType::Constant, - "get mut of constant variable" - ); + // assert_ne!( + // entry.var_type, + // VariableType::Constant, + // "get mut of constant variable" + // ); assert_eq!(entry.name, name, "incorrect key at Scope entry"); &mut entry.value diff --git a/tests/get_set.rs b/tests/get_set.rs index 7ea064b1..a1c64d25 100644 --- a/tests/get_set.rs +++ b/tests/get_set.rs @@ -91,7 +91,7 @@ fn test_big_get_set() -> Result<(), EvalAltResult> { ); assert_eq!( - engine.eval::("let a = new_tp(); a.type_of()")?, + engine.eval::("let a = new_tp(); type_of(a)")?, "TestParent" ); diff --git a/tests/side_effects.rs b/tests/side_effects.rs new file mode 100644 index 00000000..edfc0009 --- /dev/null +++ b/tests/side_effects.rs @@ -0,0 +1,39 @@ +use rhai::{Engine, EvalAltResult, RegisterFn, Scope}; +use std::cell::Cell; +use std::rc::Rc; + +#[derive(Debug, Clone)] +struct CommandWrapper { + value: Rc>, +} + +impl CommandWrapper { + pub fn set_value(&mut self, x: i64) { + let val = self.value.get(); + self.value.set(val + x); + } +} + +#[test] +fn test_side_effects() -> Result<(), EvalAltResult> { + let mut engine = Engine::new(); + let mut scope = Scope::new(); + + let payload = Rc::new(Cell::new(12)); + assert_eq!(payload.get(), 12); + + let command = CommandWrapper { + value: payload.clone(), + }; + + scope.push_constant("Command", command); + + engine.register_type_with_name::("CommandType"); + engine.register_fn("action", CommandWrapper::set_value); + + engine.eval_with_scope::<()>(&mut scope, "Command.action(30)")?; + + assert_eq!(payload.get(), 42); + + Ok(()) +} diff --git a/tests/types.rs b/tests/types.rs index fd9cda32..0101a120 100644 --- a/tests/types.rs +++ b/tests/types.rs @@ -23,10 +23,10 @@ fn test_type_of() -> Result<(), EvalAltResult> { assert_eq!(engine.eval::(r#"type_of("hello")"#)?, "string"); #[cfg(not(feature = "only_i32"))] - assert_eq!(engine.eval::("let x = 123; x.type_of()")?, "i64"); + assert_eq!(engine.eval::("let x = 123; type_of(x)")?, "i64"); #[cfg(feature = "only_i32")] - assert_eq!(engine.eval::("let x = 123; x.type_of()")?, "i32"); + assert_eq!(engine.eval::("let x = 123; type_of(x)")?, "i32"); Ok(()) } From 1d98f6534231b55358147f5f7e7e0ee761097239 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 20 Mar 2020 19:50:58 +0800 Subject: [PATCH 7/8] Disallow statement expressions in if and while guards to reduce code confusion. --- src/engine.rs | 4 ++-- src/error.rs | 9 ++++++-- src/parser.rs | 64 ++++++++++++++++++++++++++++++++------------------- src/result.rs | 12 +++++----- 4 files changed, 55 insertions(+), 34 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index d0c81ba4..8735d842 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1159,7 +1159,7 @@ impl Engine<'_> { Stmt::IfElse(guard, if_body, else_body) => self .eval_expr(scope, guard)? .downcast::() - .map_err(|_| EvalAltResult::ErrorIfGuard(guard.position())) + .map_err(|_| EvalAltResult::ErrorLogicGuard(guard.position())) .and_then(|guard_val| { if *guard_val { self.eval_stmt(scope, if_body) @@ -1186,7 +1186,7 @@ impl Engine<'_> { return Ok(().into_dynamic()); } } - Err(_) => return Err(EvalAltResult::ErrorIfGuard(guard.position())), + Err(_) => return Err(EvalAltResult::ErrorLogicGuard(guard.position())), } }, diff --git a/src/error.rs b/src/error.rs index 87e2e2d5..2826c465 100644 --- a/src/error.rs +++ b/src/error.rs @@ -47,7 +47,7 @@ pub enum ParseErrorType { /// Error in the script text. Wrapped value is the error message. BadInput(String), /// The script ends prematurely. - InputPastEndOfFile, + UnexpectedEOF, /// An unknown operator is encountered. Wrapped value is the operator. UnknownOperator(String), /// An open `(` is missing the corresponding closing `)`. @@ -72,6 +72,8 @@ pub enum ParseErrorType { ForbiddenConstantExpr(String), /// Missing a variable name after the `let`, `const` or `for` keywords. VariableExpected, + /// Missing an expression. + ExprExpected(String), /// A `for` statement is missing the `in` keyword. MissingIn, /// Defining a function `fn` in an appropriate place (e.g. inside another function). @@ -119,7 +121,7 @@ impl ParseError { pub(crate) fn desc(&self) -> &str { match self.0 { ParseErrorType::BadInput(ref p) => p, - ParseErrorType::InputPastEndOfFile => "Script is incomplete", + ParseErrorType::UnexpectedEOF => "Script is incomplete", ParseErrorType::UnknownOperator(_) => "Unknown operator", ParseErrorType::MissingRightParen(_) => "Expecting ')'", ParseErrorType::MissingLeftBrace => "Expecting '{'", @@ -134,6 +136,7 @@ impl ParseError { ParseErrorType::ForbiddenConstantExpr(_) => "Expecting a constant", ParseErrorType::MissingIn => "Expecting 'in'", ParseErrorType::VariableExpected => "Expecting name of a variable", + ParseErrorType::ExprExpected(_) => "Expecting an expression", #[cfg(not(feature = "no_function"))] ParseErrorType::FnMissingName => "Expecting name in function declaration", #[cfg(not(feature = "no_function"))] @@ -168,6 +171,8 @@ impl fmt::Display for ParseError { write!(f, "{}", if s.is_empty() { self.desc() } else { s })? } + ParseErrorType::ExprExpected(ref s) => write!(f, "Expecting {} expression", s)?, + #[cfg(not(feature = "no_function"))] ParseErrorType::FnMissingParams(ref s) => { write!(f, "Expecting parameters for function '{}'", s)? diff --git a/src/parser.rs b/src/parser.rs index 10abc3a8..d068f8d4 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1601,23 +1601,21 @@ fn parse_array_literal<'a>( /// Parse a primary expression. fn parse_primary<'a>(input: &mut Peekable>) -> Result { - // { - block statement as expression - match input.peek() { - Some((Token::LeftBrace, pos)) => { + let token = match input + .peek() + .ok_or_else(|| ParseError::new(PERR::UnexpectedEOF, Position::eof()))? + { + // { - block statement as expression + (Token::LeftBrace, pos) => { let pos = *pos; return parse_block(input, false).map(|block| Expr::Stmt(Box::new(block), pos)); } - _ => (), - } - - let token = input.next(); + _ => input.next().expect("should be a token"), + }; let mut can_be_indexed = false; - #[allow(unused_mut)] - let mut root_expr = match token - .ok_or_else(|| ParseError::new(PERR::InputPastEndOfFile, Position::eof()))? - { + let mut root_expr = match token { #[cfg(not(feature = "no_float"))] (Token::FloatConstant(x), pos) => Ok(Expr::FloatConstant(x, pos)), @@ -1666,7 +1664,7 @@ fn parse_primary<'a>(input: &mut Peekable>) -> Result(input: &mut Peekable>) -> Result { match input .peek() - .ok_or_else(|| ParseError::new(PERR::InputPastEndOfFile, Position::eof()))? + .ok_or_else(|| ParseError::new(PERR::UnexpectedEOF, Position::eof()))? { // -expr (Token::UnaryMinus, pos) => { @@ -1978,6 +1976,22 @@ fn parse_expr<'a>(input: &mut Peekable>) -> Result( + input: &mut Peekable>, + type_name: &str, +) -> Result<(), ParseError> { + match input + .peek() + .ok_or_else(|| ParseError(PERR::ExprExpected(type_name.to_string()), Position::eof()))? + { + // Disallow statement expressions + (Token::LeftBrace, pos) => Err(ParseError(PERR::ExprExpected(type_name.to_string()), *pos)), + // No need to check for others at this time - leave it for the expr parser + _ => Ok(()), + } +} + /// Parse an if statement. fn parse_if<'a>( input: &mut Peekable>, @@ -1986,19 +2000,20 @@ fn parse_if<'a>( // if ... input.next(); - // if guard { body } + // if guard { if_body } + ensure_not_statement_expr(input, "a boolean")?; let guard = parse_expr(input)?; let if_body = parse_block(input, breakable)?; - // if guard { body } else ... + // if guard { if_body } else ... let else_body = if matches!(input.peek(), Some((Token::Else, _))) { input.next(); Some(Box::new(if matches!(input.peek(), Some((Token::If, _))) { - // if guard { body } else if ... + // if guard { if_body } else if ... parse_if(input, breakable)? } else { - // if guard { body } else { else-body } + // if guard { if_body } else { else-body } parse_block(input, breakable)? })) } else { @@ -2014,6 +2029,7 @@ fn parse_while<'a>(input: &mut Peekable>) -> Result(input: &mut Peekable>) -> Result( input: &mut Peekable>, breakable: bool, ) -> Result { - match input - .peek() - .ok_or_else(|| ParseError::new(PERR::InputPastEndOfFile, Position::eof()))? - { + let token = match input.peek() { + Some(token) => token, + None => return Ok(Stmt::Noop(Position::eof())), + }; + + match token { // Semicolon - empty statement (Token::SemiColon, pos) => Ok(Stmt::Noop(*pos)), @@ -2244,10 +2263,7 @@ fn parse_stmt<'a>( /// Parse a function definition. #[cfg(not(feature = "no_function"))] fn parse_fn<'a>(input: &mut Peekable>) -> Result { - let pos = input - .next() - .ok_or_else(|| ParseError::new(PERR::InputPastEndOfFile, Position::eof()))? - .1; + let pos = input.next().expect("should be fn").1; let name = match input .next() diff --git a/src/result.rs b/src/result.rs index 7bee4a31..06f5f075 100644 --- a/src/result.rs +++ b/src/result.rs @@ -40,8 +40,8 @@ pub enum EvalAltResult { ErrorIndexingType(String, Position), /// Trying to index into an array or string with an index that is not `i64`. ErrorIndexExpr(Position), - /// The guard expression in an `if` statement does not return a boolean value. - ErrorIfGuard(Position), + /// The guard expression in an `if` or `while` statement does not return a boolean value. + ErrorLogicGuard(Position), /// The `for` statement encounters a type that is not an iterator. ErrorFor(Position), /// Usage of an unknown variable. Wrapped value is the name of the variable. @@ -93,7 +93,7 @@ impl EvalAltResult { } Self::ErrorStringBounds(0, _, _) => "Indexing of empty string", Self::ErrorStringBounds(_, _, _) => "String index out of bounds", - Self::ErrorIfGuard(_) => "If guard expects boolean expression", + Self::ErrorLogicGuard(_) => "Boolean expression expected", Self::ErrorFor(_) => "For loop expects array or range", Self::ErrorVariableNotFound(_, _) => "Variable not found", Self::ErrorAssignmentToUnknownLHS(_) => { @@ -123,7 +123,7 @@ impl fmt::Display for EvalAltResult { Self::ErrorVariableNotFound(s, pos) => write!(f, "{}: '{}' ({})", desc, s, pos), Self::ErrorIndexingType(_, pos) => write!(f, "{} ({})", desc, pos), Self::ErrorIndexExpr(pos) => write!(f, "{} ({})", desc, pos), - Self::ErrorIfGuard(pos) => write!(f, "{} ({})", desc, pos), + Self::ErrorLogicGuard(pos) => write!(f, "{} ({})", desc, pos), Self::ErrorFor(pos) => write!(f, "{} ({})", desc, pos), Self::ErrorAssignmentToUnknownLHS(pos) => write!(f, "{} ({})", desc, pos), Self::ErrorAssignmentToConstant(s, pos) => write!(f, "{}: '{}' ({})", desc, s, pos), @@ -222,7 +222,7 @@ impl EvalAltResult { | Self::ErrorStringBounds(_, _, pos) | Self::ErrorIndexingType(_, pos) | Self::ErrorIndexExpr(pos) - | Self::ErrorIfGuard(pos) + | Self::ErrorLogicGuard(pos) | Self::ErrorFor(pos) | Self::ErrorVariableNotFound(_, pos) | Self::ErrorAssignmentToUnknownLHS(pos) @@ -250,7 +250,7 @@ impl EvalAltResult { | Self::ErrorStringBounds(_, _, ref mut pos) | Self::ErrorIndexingType(_, ref mut pos) | Self::ErrorIndexExpr(ref mut pos) - | Self::ErrorIfGuard(ref mut pos) + | Self::ErrorLogicGuard(ref mut pos) | Self::ErrorFor(ref mut pos) | Self::ErrorVariableNotFound(_, ref mut pos) | Self::ErrorAssignmentToUnknownLHS(ref mut pos) From 083b3147be31c4f6b14ae7fb94130f62c886aeb2 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 21 Mar 2020 00:23:13 +0800 Subject: [PATCH 8/8] Fix test_eval. --- tests/eval.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/eval.rs b/tests/eval.rs index 24559d83..fc4424a4 100644 --- a/tests/eval.rs +++ b/tests/eval.rs @@ -3,6 +3,23 @@ use rhai::{Engine, EvalAltResult, Scope, INT}; #[test] fn test_eval() -> Result<(), EvalAltResult> { let mut engine = Engine::new(); + + assert_eq!( + engine.eval::( + r#" + eval("40 + 2") + "# + )?, + 42 + ); + + Ok(()) +} + +#[test] +#[cfg(not(feature = "no_function"))] +fn test_eval_function() -> Result<(), EvalAltResult> { + let mut engine = Engine::new(); let mut scope = Scope::new(); assert_eq!( @@ -43,6 +60,7 @@ fn test_eval() -> Result<(), EvalAltResult> { } #[test] +#[cfg(not(feature = "no_function"))] fn test_eval_override() -> Result<(), EvalAltResult> { let mut engine = Engine::new();