From fd401f304877cb74fa942d073b8183edd05c2aaa Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 24 Dec 2022 19:37:06 +0800 Subject: [PATCH] Add array API with closure variation that binds to this. --- CHANGELOG.md | 5 ++ src/api/deprecated.rs | 4 +- src/eval/chaining.rs | 55 +++++++------------ src/packages/array_basic.rs | 105 +++++++++++++++++++++++++++--------- src/types/fn_ptr.rs | 38 +++++++++++-- tests/arrays.rs | 25 +++++++++ 6 files changed, 162 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a9aa87d..72776dd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,11 @@ Net features * A compact script compresses better than one with liberal whitespaces and comments. * Unlike some uglifiers or minifiers, `Engine::compact_script` does not optimize the script in any way, nor does it rename variables. +### Enhanced array API + +* Array methods that take a function pointer, usually a closure (e.g. `map`, `filter`, `index_of` etc.), can now provide a closure with one few parameter but binds the first parameter to `this`. +* This vastly improves performance when working with arrays of object maps by avoiding unnecessary cloning of large types. + Enhancements ------------ diff --git a/src/api/deprecated.rs b/src/api/deprecated.rs index e4370ca0..4dd7d703 100644 --- a/src/api/deprecated.rs +++ b/src/api/deprecated.rs @@ -674,7 +674,7 @@ pub mod deprecated_array_functions { #[rhai_fn(name = "map", return_raw)] pub fn map_by_fn_name( ctx: NativeCallContext, - array: Array, + array: &mut Array, mapper: &str, ) -> RhaiResultOf { map(ctx, array, FnPtr::new(mapper)?) @@ -712,7 +712,7 @@ pub mod deprecated_array_functions { #[rhai_fn(name = "filter", return_raw)] pub fn filter_by_fn_name( ctx: NativeCallContext, - array: Array, + array: &mut Array, filter_func: &str, ) -> RhaiResultOf { filter(ctx, array, FnPtr::new(filter_func)?) diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 96095960..71d39a43 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -471,13 +471,8 @@ impl Engine { if chain_type == ChainType::Dotting && !x.is_qualified() => { for expr in &x.args { - let arg_value = self.get_arg_value( - global, - caches, - scope, - this_ptr.as_deref_mut(), - expr, - )?; + let tp = this_ptr.as_deref_mut(); + let arg_value = self.get_arg_value(global, caches, scope, tp, expr)?; _arg_values.push(arg_value.0.flatten()); } } @@ -529,7 +524,7 @@ impl Engine { &self, global: &mut GlobalRuntimeState, caches: &mut Caches, - mut this_ptr: Option<&mut Dynamic>, + this_ptr: Option<&mut Dynamic>, root: &Expr, parent: &Expr, target: &mut Target, @@ -540,6 +535,9 @@ impl Engine { let is_ref_mut = target.is_ref(); let op_pos = parent.position(); + #[cfg(feature = "debugging")] + let mut this_ptr = this_ptr; + #[cfg(feature = "debugging")] let scope = &mut Scope::new(); @@ -838,17 +836,13 @@ impl Engine { // {xxx:map}.sub_lhs[expr] | {xxx:map}.sub_lhs.expr Expr::Index(x, ..) | Expr::Dot(x, ..) if target.is_map() => { let _node = &x.lhs; + let mut _this_ptr = this_ptr; + let _tp = _this_ptr.as_deref_mut(); let val_target = &mut match x.lhs { Expr::Property(ref p, pos) => { #[cfg(feature = "debugging")] - self.run_debugger( - global, - caches, - scope, - this_ptr.as_deref_mut(), - _node, - )?; + self.run_debugger(global, caches, scope, _tp, _node)?; let index = &mut p.2.clone().into(); self.get_indexed_mut( @@ -858,13 +852,8 @@ impl Engine { // {xxx:map}.fn_name(arg_expr_list)[expr] | {xxx:map}.fn_name(arg_expr_list).expr Expr::MethodCall(ref x, pos) if !x.is_qualified() => { #[cfg(feature = "debugging")] - let reset = self.run_debugger_with_reset( - global, - caches, - scope, - this_ptr.as_deref_mut(), - _node, - )?; + let reset = self + .run_debugger_with_reset(global, caches, scope, _tp, _node)?; #[cfg(feature = "debugging")] auto_restore!(global if Some(reset) => move |g| g.debugger_mut().reset_status(reset)); @@ -893,25 +882,21 @@ impl Engine { }; self.eval_dot_index_chain_raw( - global, caches, this_ptr, root, rhs, val_target, &x.rhs, idx_values, + global, caches, _this_ptr, root, rhs, val_target, &x.rhs, idx_values, new_val, ) } // xxx.sub_lhs[expr] | xxx.sub_lhs.expr Expr::Index(x, ..) | Expr::Dot(x, ..) => { let _node = &x.lhs; + let mut _this_ptr = this_ptr; + let _tp = _this_ptr.as_deref_mut(); match x.lhs { // xxx.prop[expr] | xxx.prop.expr Expr::Property(ref p, pos) => { #[cfg(feature = "debugging")] - self.run_debugger( - global, - caches, - scope, - this_ptr.as_deref_mut(), - _node, - )?; + self.run_debugger(global, caches, scope, _tp, _node)?; let ((getter, hash_get), (setter, hash_set), name) = &**p; let args = &mut [target.as_mut()]; @@ -943,7 +928,7 @@ impl Engine { let val = &mut (&mut val).into(); let (result, may_be_changed) = self.eval_dot_index_chain_raw( - global, caches, this_ptr, root, rhs, val, &x.rhs, idx_values, + global, caches, _this_ptr, root, rhs, val, &x.rhs, idx_values, new_val, )?; @@ -987,11 +972,7 @@ impl Engine { let val = { #[cfg(feature = "debugging")] let reset = self.run_debugger_with_reset( - global, - caches, - scope, - this_ptr.as_deref_mut(), - _node, + global, caches, scope, _tp, _node, )?; #[cfg(feature = "debugging")] auto_restore!(global if Some(reset) => move |g| g.debugger_mut().reset_status(reset)); @@ -1015,7 +996,7 @@ impl Engine { let val = &mut val.into(); self.eval_dot_index_chain_raw( - global, caches, this_ptr, root, rhs, val, &x.rhs, idx_values, + global, caches, _this_ptr, root, rhs, val, &x.rhs, idx_values, new_val, ) } diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index a5780fca..51af68a0 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -647,6 +647,10 @@ pub mod array_functions { /// Iterate through all the elements in the array, applying a `mapper` function to each element /// in turn, and return the results as a new array. /// + /// # No Function Parameter + /// + /// Array element (mutable) is bound to `this`. + /// /// # Function Parameters /// /// * `element`: copy of array element @@ -666,17 +670,17 @@ pub mod array_functions { /// print(y); // prints "[0, 2, 6, 12, 20]" /// ``` #[rhai_fn(return_raw)] - pub fn map(ctx: NativeCallContext, array: Array, map: FnPtr) -> RhaiResultOf { + pub fn map(ctx: NativeCallContext, array: &mut Array, map: FnPtr) -> RhaiResultOf { if array.is_empty() { - return Ok(array); + return Ok(Array::new()); } let mut ar = Array::with_capacity(array.len()); - for (i, item) in array.into_iter().enumerate() { + for (i, item) in array.iter_mut().enumerate() { let ex = [(i as INT).into()]; - ar.push(map.call_raw_with_extra_args("map", &ctx, None, [item], ex)?); + ar.push(map.call_raw_with_extra_args("map", &ctx, Some(item), [], ex)?); } Ok(ar) @@ -685,6 +689,10 @@ pub mod array_functions { /// Iterate through all the elements in the array, applying a `filter` function to each element /// in turn, and return a copy of all elements (in order) that return `true` as a new array. /// + /// # No Function Parameter + /// + /// Array element (mutable) is bound to `this`. + /// /// # Function Parameters /// /// * `element`: copy of array element @@ -704,22 +712,22 @@ pub mod array_functions { /// print(y); // prints "[12, 20]" /// ``` #[rhai_fn(return_raw)] - pub fn filter(ctx: NativeCallContext, array: Array, filter: FnPtr) -> RhaiResultOf { + pub fn filter(ctx: NativeCallContext, array: &mut Array, filter: FnPtr) -> RhaiResultOf { if array.is_empty() { - return Ok(array); + return Ok(Array::new()); } let mut ar = Array::new(); - for (i, item) in array.into_iter().enumerate() { + for (i, item) in array.iter_mut().enumerate() { let ex = [(i as INT).into()]; if filter - .call_raw_with_extra_args("filter", &ctx, None, [item.clone()], ex)? + .call_raw_with_extra_args("filter", &ctx, Some(item), [], ex)? .as_bool() .unwrap_or(false) { - ar.push(item); + ar.push(item.clone()); } } @@ -871,6 +879,10 @@ pub mod array_functions { /// in turn, and return the index of the first element that returns `true`. /// If no element returns `true`, `-1` is returned. /// + /// # No Function Parameter + /// + /// Array element (mutable) is bound to `this`. + /// /// # Function Parameters /// /// * `element`: copy of array element @@ -907,6 +919,10 @@ pub mod array_functions { /// * If `start` < -length of array, position counts from the beginning of the array. /// * If `start` ≥ length of array, `-1` is returned. /// + /// # No Function Parameter + /// + /// Array element (mutable) is bound to `this`. + /// /// # Function Parameters /// /// * `element`: copy of array element @@ -942,11 +958,11 @@ pub mod array_functions { let (start, ..) = calc_offset_len(array.len(), start, 0); - for (i, item) in array.iter().enumerate().skip(start) { + for (i, item) in array.iter_mut().enumerate().skip(start) { let ex = [(i as INT).into()]; if filter - .call_raw_with_extra_args("index_of", &ctx, None, [item.clone()], ex)? + .call_raw_with_extra_args("index_of", &ctx, Some(item), [], ex)? .as_bool() .unwrap_or(false) { @@ -960,6 +976,10 @@ pub mod array_functions { /// in turn, and return a copy of the first element that returns `true`. If no element returns /// `true`, `()` is returned. /// + /// # No Function Parameter + /// + /// Array element (mutable) is bound to `this`. + /// /// # Function Parameters /// /// * `element`: copy of array element @@ -972,7 +992,7 @@ pub mod array_functions { /// /// print(x.find(|v| v > 3)); // prints 5: 5 > 3 /// - /// x.find(|v| v > 13) ?? print("not found"); // prints "not found": nothing is > 13 + /// print(x.find(|v| v > 13) ?? "not found"); // prints "not found": nothing is > 13 /// /// print(x.find(|v, i| v * i > 13)); // prints 5: 3 * 5 > 13 /// ``` @@ -988,6 +1008,10 @@ pub mod array_functions { /// * If `start` < -length of array, position counts from the beginning of the array. /// * If `start` ≥ length of array, `-1` is returned. /// + /// # No Function Parameter + /// + /// Array element (mutable) is bound to `this`. + /// /// # Function Parameters /// /// * `element`: copy of array element @@ -1000,9 +1024,9 @@ pub mod array_functions { /// /// print(x.find(|v| v > 1, 2)); // prints 3: 3 > 1 /// - /// x.find(|v| v < 2, 3) ?? print("not found"); // prints "not found": nothing < 2 past index 3 + /// print(x.find(|v| v < 2, 3) ?? "not found"); // prints "not found": nothing < 2 past index 3 /// - /// x.find(|v| v > 1, 8) ?? print("not found"); // prints "not found": nothing found past end of array + /// print(x.find(|v| v > 1, 8) ?? "not found"); // prints "not found": nothing found past end of array /// /// print(x.find(|v| v > 1, -3)); // prints 5: -3 = start from index 4 /// @@ -1028,6 +1052,10 @@ pub mod array_functions { /// Iterate through all the elements in the array, applying a `mapper` function to each element /// in turn, and return the first result that is not `()`. Otherwise, `()` is returned. /// + /// # No Function Parameter + /// + /// Array element (mutable) is bound to `this`. + /// /// # Function Parameters /// /// * `element`: copy of array element @@ -1040,7 +1068,9 @@ pub mod array_functions { /// /// print(x.find_map(|v| v.alice)); // prints 1 /// - /// x.find_map(|v| v.dave) ?? print("not found"); // prints "not found" + /// print(x.find_map(|v| v.dave) ?? "not found"); // prints "not found" + /// + /// print(x.find_map(|| this.dave) ?? "not found"); // prints "not found" /// ``` #[rhai_fn(return_raw, pure)] pub fn find_map(ctx: NativeCallContext, array: &mut Array, filter: FnPtr) -> RhaiResult { @@ -1054,6 +1084,10 @@ pub mod array_functions { /// * If `start` < -length of array, position counts from the beginning of the array. /// * If `start` ≥ length of array, `-1` is returned. /// + /// # No Function Parameter + /// + /// Array element (mutable) is bound to `this`. + /// /// # Function Parameters /// /// * `element`: copy of array element @@ -1066,13 +1100,17 @@ pub mod array_functions { /// /// print(x.find_map(|v| v.alice, 2)); // prints 0 /// - /// x.find_map(|v| v.bob, 4) ?? print("not found"); // prints "not found" + /// print(x.find_map(|v| v.bob, 4) ?? "not found"); // prints "not found" /// - /// x.find_map(|v| v.alice, 8) ?? print("not found"); // prints "not found" + /// print(x.find_map(|v| v.alice, 8) ?? "not found"); // prints "not found" + /// + /// print(x.find_map(|| this.alice, 8) ?? "not found"); // prints "not found" /// /// print(x.find_map(|v| v.bob, -4)); // prints 3: -4 = start from index 2 /// /// print(x.find_map(|v| v.alice, -99)); // prints 1: -99 = start from beginning + /// + /// print(x.find_map(|| this.alice, -99)); // prints 1: -99 = start from beginning /// ``` #[rhai_fn(name = "find_map", return_raw, pure)] pub fn find_map_starting_from( @@ -1087,11 +1125,10 @@ pub mod array_functions { let (start, ..) = calc_offset_len(array.len(), start, 0); - for (i, item) in array.iter().enumerate().skip(start) { + for (i, item) in array.iter_mut().enumerate().skip(start) { let ex = [(i as INT).into()]; - let value = - filter.call_raw_with_extra_args("find_map", &ctx, None, [item.clone()], ex)?; + let value = filter.call_raw_with_extra_args("find_map", &ctx, Some(item), [], ex)?; if !value.is_unit() { return Ok(value); @@ -1102,6 +1139,10 @@ pub mod array_functions { } /// Return `true` if any element in the array that returns `true` when applied the `filter` function. /// + /// # No Function Parameter + /// + /// Array element (mutable) is bound to `this`. + /// /// # Function Parameters /// /// * `element`: copy of array element @@ -1124,11 +1165,11 @@ pub mod array_functions { return Ok(false); } - for (i, item) in array.iter().enumerate() { + for (i, item) in array.iter_mut().enumerate() { let ex = [(i as INT).into()]; if filter - .call_raw_with_extra_args("some", &ctx, None, [item.clone()], ex)? + .call_raw_with_extra_args("some", &ctx, Some(item), [], ex)? .as_bool() .unwrap_or(false) { @@ -1140,6 +1181,10 @@ pub mod array_functions { } /// Return `true` if all elements in the array return `true` when applied the `filter` function. /// + /// # No Function Parameter + /// + /// Array element (mutable) is bound to `this`. + /// /// # Function Parameters /// /// * `element`: copy of array element @@ -1162,11 +1207,11 @@ pub mod array_functions { return Ok(true); } - for (i, item) in array.iter().enumerate() { + for (i, item) in array.iter_mut().enumerate() { let ex = [(i as INT).into()]; if !filter - .call_raw_with_extra_args("all", &ctx, None, [item.clone()], ex)? + .call_raw_with_extra_args("all", &ctx, Some(item), [], ex)? .as_bool() .unwrap_or(false) { @@ -1516,6 +1561,10 @@ pub mod array_functions { /// Remove all elements in the array that returns `true` when applied the `filter` function and /// return them as a new array. /// + /// # No Function Parameter + /// + /// Array element (mutable) is bound to `this`. + /// /// # Function Parameters /// /// * `element`: copy of array element @@ -1553,7 +1602,7 @@ pub mod array_functions { let ex = [(i as INT).into()]; if filter - .call_raw_with_extra_args("drain", &ctx, None, [array[x].clone()], ex)? + .call_raw_with_extra_args("drain", &ctx, Some(&mut array[x]), [], ex)? .as_bool() .unwrap_or(false) { @@ -1659,6 +1708,10 @@ pub mod array_functions { /// Remove all elements in the array that do not return `true` when applied the `filter` /// function and return them as a new array. /// + /// # No Function Parameter + /// + /// Array element (mutable) is bound to `this`. + /// /// # Function Parameters /// /// * `element`: copy of array element @@ -1696,7 +1749,7 @@ pub mod array_functions { let ex = [(i as INT).into()]; if filter - .call_raw_with_extra_args("retain", &ctx, None, [array[x].clone()], ex)? + .call_raw_with_extra_args("retain", &ctx, Some(&mut array[x]), [], ex)? .as_bool() .unwrap_or(false) { diff --git a/src/types/fn_ptr.rs b/src/types/fn_ptr.rs index 35098945..952a402c 100644 --- a/src/types/fn_ptr.rs +++ b/src/types/fn_ptr.rs @@ -359,6 +359,9 @@ impl FnPtr { /// Make a call to a function pointer with either a specified number of arguments, or with extra /// arguments attached. /// + /// If `this_ptr` is provided, it is first provided to script-defined functions bound to `this`. + /// When an appropriate function is not found, it is then removed and mapped to the first parameter. + /// /// This is useful for calling predicate closures within an iteration loop where the extra argument /// is the current element's index. /// @@ -380,6 +383,9 @@ impl FnPtr { /// or with extra arguments attached. /// Exported under the `internals` feature only. /// + /// If `this_ptr` is provided, it is first provided to script-defined functions bound to `this`. + /// When an appropriate function is not found, it is then removed and mapped to the first parameter. + /// /// This is useful for calling predicate closures within an iteration loop where the extra /// argument is the current element's index. /// @@ -409,8 +415,14 @@ impl FnPtr { ) -> RhaiResult { #[cfg(not(feature = "no_function"))] if let Some(arity) = self.fn_def().map(|f| f.params.len()) { + if arity == N + 1 && this_ptr.is_some() { + let mut args = FnArgsVec::with_capacity(items.len() + 1); + args.push(this_ptr.as_mut().unwrap().clone()); + args.extend(items); + return self.call_raw(ctx, None, args); + } if arity == N { - return self.call_raw(ctx, None, items); + return self.call_raw(ctx, this_ptr, items); } if arity == N + E { let mut items2 = FnArgsVec::with_capacity(items.len() + extras.len()); @@ -421,12 +433,28 @@ impl FnPtr { } self.call_raw(ctx, this_ptr.as_deref_mut(), items.clone()) + .or_else(|err| match *err { + ERR::ErrorFunctionNotFound(sig, ..) + if this_ptr.is_some() && sig.starts_with(self.fn_name()) => + { + let mut args = FnArgsVec::with_capacity(items.len() + 1); + args.push(this_ptr.as_mut().unwrap().clone()); + args.extend(IntoIterator::into_iter(items.clone())); + self.call_raw(ctx, this_ptr.as_deref_mut(), args) + } + _ => Err(err), + }) .or_else(|err| match *err { ERR::ErrorFunctionNotFound(sig, ..) if sig.starts_with(self.fn_name()) => { - let mut items2 = FnArgsVec::with_capacity(items.len() + extras.len()); - items2.extend(IntoIterator::into_iter(items)); - items2.extend(IntoIterator::into_iter(extras)); - self.call_raw(ctx, this_ptr, items2) + let mut args = FnArgsVec::with_capacity( + items.len() + extras.len() + if this_ptr.is_some() { 1 } else { 0 }, + ); + if let Some(ref mut this_ptr) = this_ptr { + args.push(this_ptr.clone()); + } + args.extend(IntoIterator::into_iter(items)); + args.extend(IntoIterator::into_iter(extras)); + self.call_raw(ctx, this_ptr, args) } _ => Err(err), }) diff --git a/tests/arrays.rs b/tests/arrays.rs index cb27331c..f2343301 100644 --- a/tests/arrays.rs +++ b/tests/arrays.rs @@ -298,6 +298,7 @@ fn test_arrays_map_reduce() -> Result<(), Box> { let engine = Engine::new(); assert_eq!(engine.eval::("[1].map(|x| x + 41)[0]")?, 42); + assert_eq!(engine.eval::("[1].map(|| this + 41)[0]")?, 42); assert_eq!(engine.eval::("([1].map(|x| x + 41))[0]")?, 42); assert_eq!( engine.eval::("let c = 40; let y = 1; [1].map(|x, i| c + x + y + i)[0]")?, @@ -316,6 +317,18 @@ fn test_arrays_map_reduce() -> Result<(), Box> { [3] ); + assert_eq!( + engine + .eval::( + " + let x = [1, 2, 3]; + x.filter(|| this > 2) + " + )? + .into_typed_array::()?, + [3] + ); + assert_eq!( engine .eval::( @@ -340,6 +353,18 @@ fn test_arrays_map_reduce() -> Result<(), Box> { [2, 4, 6] ); + assert_eq!( + engine + .eval::( + " + let x = [1, 2, 3]; + x.map(|| this * 2) + " + )? + .into_typed_array::()?, + [2, 4, 6] + ); + assert_eq!( engine .eval::(