Encapsulate function calls and handle map property access more efficiently.

README.md

@@ -367,7 +367,7 @@ use rhai::packages::Package                     // load the 'Package' trait to u
 use rhai::packages::CorePackage;                // the 'core' package contains basic functionalities (e.g. arithmetic)
 
 let mut engine = Engine::new_raw();             // create a 'raw' Engine
-let package = CorePackage::new();               // create a package
+let package = CorePackage::new();               // create a package - can be shared among multiple `Engine` instances
 
 engine.load_package(package.get());             // load the package manually
 ```
@@ -377,10 +377,10 @@ The follow packages are available:
 | Package                  | Description                                     | In `CorePackage` | In `StandardPackage` |
 | ------------------------ | ----------------------------------------------- | :--------------: | :------------------: |
 | `BasicArithmeticPackage` | Arithmetic operators (e.g. `+`, `-`, `*`, `/`)  |       Yes        |         Yes          |
-| `BasicIteratorPackage`   | Numeric ranges                                  |       Yes        |         Yes          |
+| `BasicIteratorPackage`   | Numeric ranges (e.g. `range(1, 10)`)            |       Yes        |         Yes          |
 | `LogicPackage`           | Logic and comparison operators (e.g. `==`, `>`) |       Yes        |         Yes          |
 | `BasicStringPackage`     | Basic string functions                          |       Yes        |         Yes          |
-| `BasicTimePackage`       | Basic time functions (e.g. `Instant`)           |       Yes        |         Yes          |
+| `BasicTimePackage`       | Basic time functions (e.g. [timestamps])        |       Yes        |         Yes          |
 | `MoreStringPackage`      | Additional string functions                     |        No        |         Yes          |
 | `BasicMathPackage`       | Basic math functions (e.g. `sin`, `sqrt`)       |        No        |         Yes          |
 | `BasicArrayPackage`      | Basic [array] functions                         |        No        |         Yes          |
@@ -456,7 +456,7 @@ type_of('c') == "char";
 type_of(42) == "i64";
 
 let x = 123;
-x.type_of();                    // <- error: 'type_of' cannot use method-call style
+x.type_of() == "i64";                           // method-call style is also OK
 type_of(x) == "i64";
 
 x = 99.999;
@@ -878,12 +878,12 @@ with a special "pretty-print" name, [`type_of()`] will return that name instead.
 engine.register_type::<TestStruct>();
 engine.register_fn("new_ts", TestStruct::new);
 let x = new_ts();
-print(type_of(x));                                 // prints "path::to::module::TestStruct"
+print(x.type_of());                                 // prints "path::to::module::TestStruct"
 
 engine.register_type_with_name::<TestStruct>("Hello");
 engine.register_fn("new_ts", TestStruct::new);
 let x = new_ts();
-print(type_of(x));                                 // prints "Hello"
+print(x.type_of());                                 // prints "Hello"
 ```
 
 Getters and setters
@@ -1571,7 +1571,7 @@ let json = r#"{
 // Set the second boolean parameter to true in order to map 'null' to '()'
 let map = engine.parse_json(json, true)?;
 
-map.len() == 6;                         // 'map' contains all properties int the JSON string
+map.len() == 6;                         // 'map' contains all properties in the JSON string
 
 // Put the object map into a 'Scope'
 let mut scope = Scope::new();
@@ -1584,7 +1584,10 @@ result == 3;                            // the object map is successfully used i
 
 `timestamp`'s
 -------------
-[`timestamp`]: #timestamp-s
+
+[`timestamp`]: #timestamps
+[timestamp]: #timestamps
+[timestamps]: #timestamps
 
 Timestamps are provided by the [`BasicTimePackage`](#packages) (excluded if using a [raw `Engine`]) via the `timestamp`
 function.
@@ -2246,7 +2249,7 @@ 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', method-call style doesn't work
+"print(42)".eval();         // <- nope... method-call style doesn't work
 ```
 
 Script segments passed to `eval` execute inside the current [`Scope`], so they can access and modify _everything_,

src/engine.rs

@@ -68,6 +68,7 @@ const FUNCTIONS_COUNT: usize = 512;
 #[cfg(any(feature = "only_i32", feature = "only_i64"))]
 const FUNCTIONS_COUNT: usize = 256;
 
+/// A type encapsulating an index value, which may be an integer or a string key.
 #[derive(Debug, Eq, PartialEq, Hash, Clone)]
 enum IndexValue {
     Num(usize),
@@ -95,9 +96,16 @@ impl IndexValue {
     }
 }
 
+/// A type encapsulating the target of a update action.
+/// The reason we need this is because we cannot hold a mutable reference to a variable in
+/// the current `Scope` while evaluating expressions requiring access to the same `Scope`.
+/// So we cannot use a single `&mut Dynamic` everywhere; instead, we hold enough information
+/// to find the variable from the `Scope` when we need to update it.
 #[derive(Debug)]
 enum Target<'a> {
+    /// The update target is a variable stored in the current `Scope`.
     Scope(ScopeSource<'a>),
+    /// The update target is a `Dynamic` value stored somewhere.
     Value(&'a mut Dynamic),
 }
 
@@ -610,34 +618,19 @@ impl Engine {
         }
 
         if let Some(prop) = extract_prop_from_getter(fn_name) {
-            return match args[0] {
-                // Map property access
-                Dynamic(Union::Map(map)) => Ok(map.get(prop).cloned().unwrap_or_else(|| ().into())),
-
             // Getter function not found
-                _ => Err(Box::new(EvalAltResult::ErrorDotExpr(
+            return Err(Box::new(EvalAltResult::ErrorDotExpr(
                 format!("- property '{}' unknown or write-only", prop),
                 pos,
-                ))),
+            )));
-            };
         }
 
         if let Some(prop) = extract_prop_from_setter(fn_name) {
-            let (arg, value) = args.split_at_mut(1);
-
-            return match arg[0] {
-                // Map property update
-                Dynamic(Union::Map(map)) => {
-                    map.insert(prop.to_string(), value[0].clone());
-                    Ok(().into())
-                }
-
             // Setter function not found
-                _ => Err(Box::new(EvalAltResult::ErrorDotExpr(
+            return Err(Box::new(EvalAltResult::ErrorDotExpr(
                 format!("- property '{}' unknown or read-only", prop),
                 pos,
-                ))),
+            )));
-            };
         }
 
         if let Some(val) = def_val {
@@ -658,6 +651,59 @@ impl Engine {
         )))
     }
 
+    // Has a system function an override?
+    fn has_override(&self, fn_lib: Option<&FunctionsLib>, name: &str) -> bool {
+        let hash = &calc_fn_hash(name, once(TypeId::of::<String>()));
+
+        // First check registered functions
+        self.functions.contains_key(hash)
+            // Then check packages
+            || self.packages.iter().any(|p| p.functions.contains_key(hash))
+            // Then check script-defined functions
+            || fn_lib.map_or(false, |lib| lib.has_function(name, 1))
+    }
+
+    // Perform an actual function call, taking care of special functions such as `type_of`
+    // and property getter/setter for maps.
+    fn exec_fn_call(
+        &self,
+        fn_lib: Option<&FunctionsLib>,
+        fn_name: &str,
+        args: &mut [&mut Dynamic],
+        def_val: Option<&Dynamic>,
+        pos: Position,
+        level: usize,
+    ) -> Result<Dynamic, Box<EvalAltResult>> {
+        match fn_name {
+            // type_of
+            KEYWORD_TYPE_OF if args.len() == 1 && !self.has_override(fn_lib, KEYWORD_TYPE_OF) => {
+                Ok(self.map_type_name(args[0].type_name()).to_string().into())
+            }
+
+            _ => {
+                // Map property access?
+                if let Some(prop) = extract_prop_from_getter(fn_name) {
+                    if let Dynamic(Union::Map(map)) = args[0] {
+                        return Ok(map.get(prop).cloned().unwrap_or_else(|| ().into()));
+                    }
+                }
+
+                // Map property update
+                if let Some(prop) = extract_prop_from_setter(fn_name) {
+                    let (arg, value) = args.split_at_mut(1);
+
+                    if let Dynamic(Union::Map(map)) = arg[0] {
+                        map.insert(prop.to_string(), value[0].clone());
+                        return Ok(().into());
+                    }
+                }
+
+                // Normal function call
+                self.call_fn_raw(None, fn_lib, fn_name, args, def_val, pos, level)
+            }
+        }
+    }
+
     /// Chain-evaluate a dot setter.
     fn dot_get_helper(
         &self,
@@ -669,22 +715,23 @@ impl Engine {
     ) -> Result<Dynamic, Box<EvalAltResult>> {
         match dot_rhs {
             // xxx.fn_name(arg_expr_list)
-            Expr::FunctionCall(fn_name, arg_expr_list, def_val, pos) => {
+            Expr::FunctionCall(fn_name, arg_exprs, def_val, pos) => {
-                let mut values = arg_expr_list
+                let mut arg_values = arg_exprs
                     .iter()
                     .map(|arg_expr| self.eval_expr(scope, fn_lib, arg_expr, level))
                     .collect::<Result<Vec<_>, _>>()?;
                 let mut args: Vec<_> = once(target.get_mut(scope))
-                    .chain(values.iter_mut())
+                    .chain(arg_values.iter_mut())
                     .collect();
                 let def_val = def_val.as_ref();
-                self.call_fn_raw(None, fn_lib, fn_name, &mut args, def_val, *pos, 0)
+                self.exec_fn_call(fn_lib, fn_name, &mut args, def_val, *pos, 0)
             }
 
             // xxx.id
             Expr::Property(id, pos) => {
+                let fn_name = make_getter(id);
                 let mut args = [target.get_mut(scope)];
-                self.call_fn_raw(None, fn_lib, &make_getter(id), &mut args, None, *pos, 0)
+                self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0)
             }
 
             // xxx.idx_lhs[idx_expr]
@@ -692,8 +739,9 @@ impl Engine {
                 let lhs_value = match idx_lhs.as_ref() {
                     // xxx.id[idx_expr]
                     Expr::Property(id, pos) => {
+                        let fn_name = make_getter(id);
                         let mut args = [target.get_mut(scope)];
-                        self.call_fn_raw(None, fn_lib, &make_getter(id), &mut args, None, *pos, 0)?
+                        self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0)?
                     }
                     // xxx.???[???][idx_expr]
                     Expr::Index(_, _, _) => {
@@ -717,8 +765,9 @@ impl Engine {
             Expr::Dot(dot_lhs, rhs, _) => match dot_lhs.as_ref() {
                 // xxx.id.rhs
                 Expr::Property(id, pos) => {
+                    let fn_name = make_getter(id);
                     let mut args = [target.get_mut(scope)];
-                    self.call_fn_raw(None, fn_lib, &make_getter(id), &mut args, None, *pos, 0)
+                    self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0)
                         .and_then(|mut val| {
                             self.dot_get_helper(scope, fn_lib, (&mut val).into(), rhs, level)
                         })
@@ -730,7 +779,7 @@ impl Engine {
                         Expr::Property(id, pos) => {
                             let fn_name = make_getter(id);
                             let mut args = [target.get_mut(scope)];
-                            self.call_fn_raw(None, fn_lib, &fn_name, &mut args, None, *pos, 0)?
+                            self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0)?
                         }
                         // xxx.???[???][idx_expr].rhs
                         Expr::Index(_, _, _) => {
@@ -953,7 +1002,7 @@ impl Engine {
             // xxx.id
             Expr::Property(id, pos) => {
                 let mut args = [this_ptr, new_val];
-                self.call_fn_raw(None, fn_lib, &make_setter(id), &mut args, None, *pos, 0)
+                self.exec_fn_call(fn_lib, &make_setter(id), &mut args, None, *pos, 0)
             }
 
             // xxx.lhs[idx_expr]
@@ -962,7 +1011,7 @@ impl Engine {
                 // xxx.id[idx_expr]
                 Expr::Property(id, pos) => {
                     let fn_name = make_getter(id);
-                    self.call_fn_raw(None, fn_lib, &fn_name, &mut [this_ptr], None, *pos, 0)
+                    self.exec_fn_call(fn_lib, &fn_name, &mut [this_ptr], None, *pos, 0)
                         .and_then(|val| {
                             let (_, index) = self.get_indexed_val(
                                 scope, fn_lib, &val, idx_expr, *op_pos, level, true,
@@ -973,7 +1022,7 @@ impl Engine {
                         .and_then(|mut val| {
                             let fn_name = make_setter(id);
                             let mut args = [this_ptr, &mut val];
-                            self.call_fn_raw(None, fn_lib, &fn_name, &mut args, None, *pos, 0)
+                            self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0)
                         })
                 }
 
@@ -989,7 +1038,7 @@ impl Engine {
                 // xxx.id.rhs
                 Expr::Property(id, pos) => {
                     let fn_name = make_getter(id);
-                    self.call_fn_raw(None, fn_lib, &fn_name, &mut [this_ptr], None, *pos, 0)
+                    self.exec_fn_call(fn_lib, &fn_name, &mut [this_ptr], None, *pos, 0)
                         .and_then(|mut val| {
                             self.dot_set_helper(
                                 scope, fn_lib, &mut val, rhs, new_val, val_pos, level,
@@ -999,7 +1048,7 @@ impl Engine {
                         .and_then(|mut val| {
                             let fn_name = make_setter(id);
                             let mut args = [this_ptr, &mut val];
-                            self.call_fn_raw(None, fn_lib, &fn_name, &mut args, None, *pos, 0)
+                            self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0)
                         })
                 }
 
@@ -1009,7 +1058,7 @@ impl Engine {
                     // xxx.id[idx_expr].rhs
                     Expr::Property(id, pos) => {
                         let fn_name = make_getter(id);
-                        self.call_fn_raw(None, fn_lib, &fn_name, &mut [this_ptr], None, *pos, 0)
+                        self.exec_fn_call(fn_lib, &fn_name, &mut [this_ptr], None, *pos, 0)
                             .and_then(|v| {
                                 let (mut value, index) = self.get_indexed_val(
                                     scope, fn_lib, &v, idx_expr, *op_pos, level, false,
@@ -1025,7 +1074,7 @@ impl Engine {
                             .and_then(|mut v| {
                                 let fn_name = make_setter(id);
                                 let mut args = [this_ptr, &mut v];
-                                self.call_fn_raw(None, fn_lib, &fn_name, &mut args, None, *pos, 0)
+                                self.exec_fn_call(fn_lib, &fn_name, &mut args, None, *pos, 0)
                             })
                     }
 
@@ -1322,40 +1371,25 @@ impl Engine {
                 Ok(Dynamic(Union::Map(Box::new(map))))
             }
 
-            Expr::FunctionCall(fn_name, args_expr_list, def_val, pos) => {
+            Expr::FunctionCall(fn_name, arg_exprs, def_val, pos) => {
-                // Has a system function an override?
+                let mut arg_values = arg_exprs
-                fn has_override(
+                    .iter()
-                    engine: &Engine,
+                    .map(|expr| self.eval_expr(scope, fn_lib, expr, level))
-                    fn_lib: Option<&FunctionsLib>,
+                    .collect::<Result<Vec<_>, _>>()?;
-                    name: &str,
-                ) -> bool {
-                    engine
-                        .functions
-                        .contains_key(&calc_fn_hash(name, once(TypeId::of::<String>())))
-                        || fn_lib.map_or(false, |lib| lib.has_function(name, 1))
-                }
 
-                match fn_name.as_ref() {
+                let mut args: Vec<_> = arg_values.iter_mut().collect();
-                    // type_of
+
-                    KEYWORD_TYPE_OF
+                // eval - only in function call style
-                        if args_expr_list.len() == 1
+                if fn_name == KEYWORD_EVAL
-                            && !has_override(self, fn_lib, KEYWORD_TYPE_OF) =>
+                    && args.len() == 1
+                    && !self.has_override(fn_lib, KEYWORD_EVAL)
                 {
-                        let result = self.eval_expr(scope, fn_lib, &args_expr_list[0], level)?;
-                        Ok(self.map_type_name(result.type_name()).to_string().into())
-                    }
-
-                    // eval
-                    KEYWORD_EVAL
-                        if args_expr_list.len() == 1
-                            && !has_override(self, fn_lib, KEYWORD_EVAL) =>
-                    {
-                        let pos = args_expr_list[0].position();
-                        let result = self.eval_expr(scope, fn_lib, &args_expr_list[0], level)?;
-
                     // Get the script text by evaluating the expression
-                        let script = result.as_str().map_err(|type_name| {
+                    let script = args[0].as_str().map_err(|type_name| {
-                            EvalAltResult::ErrorMismatchOutputType(type_name.into(), pos)
+                        EvalAltResult::ErrorMismatchOutputType(
+                            type_name.into(),
+                            arg_exprs[0].position(),
+                        )
                     })?;
 
                     // Compile the script text
@@ -1369,7 +1403,7 @@ impl Engine {
                     // If new functions are defined within the eval string, it is an error
                     if ast.1.len() > 0 {
                         return Err(Box::new(EvalAltResult::ErrorParsing(
-                                ParseErrorType::WrongFnDefinition.into_err(pos),
+                            ParseErrorType::WrongFnDefinition.into_err(*pos),
                         )));
                     }
 
@@ -1385,22 +1419,14 @@ impl Engine {
                     }
 
                     // Evaluate the AST
-                        self.eval_ast_with_scope_raw(scope, &ast)
+                    return self
-                            .map_err(|err| Box::new(err.set_position(pos)))
+                        .eval_ast_with_scope_raw(scope, &ast)
+                        .map_err(|err| Box::new(err.set_position(*pos)));
                 }
 
                 // Normal function call
-                    _ => {
-                        let mut arg_values = args_expr_list
-                            .iter()
-                            .map(|expr| self.eval_expr(scope, fn_lib, expr, level))
-                            .collect::<Result<Vec<_>, _>>()?;
-
-                        let mut args: Vec<_> = arg_values.iter_mut().collect();
                 let def_val = def_val.as_ref();
-                        self.call_fn_raw(None, fn_lib, fn_name, &mut args, def_val, *pos, level)
+                self.exec_fn_call(fn_lib, fn_name, &mut args, def_val, *pos, level)
-                    }
-                }
             }
 
             Expr::In(lhs, rhs, _) => {

src/token.rs

@@ -361,9 +361,11 @@ impl Token {
         use Token::*;
 
         match self {
-            // Equals | PlusAssign | MinusAssign | MultiplyAssign | DivideAssign | LeftShiftAssign
+            // Assignments are not considered expressions - set to zero
-            // | RightShiftAssign | AndAssign | OrAssign | XOrAssign | ModuloAssign
+            Equals | PlusAssign | MinusAssign | MultiplyAssign | DivideAssign | LeftShiftAssign
-            // | PowerOfAssign => 10,
+            | RightShiftAssign | AndAssign | OrAssign | XOrAssign | ModuloAssign
+            | PowerOfAssign => 0,
+
             Or | XOr | Pipe => 40,
 
             And | Ampersand => 50,