Convert for loop to iterator.

This commit is contained in:
Stephen Chung 2021-11-15 14:30:00 +08:00
parent de906053ed
commit 98707912e0
6 changed files with 115 additions and 131 deletions

View File

@ -4,15 +4,11 @@ Rhai Release Notes
Version 1.2.0 Version 1.2.0
============= =============
Breaking changes Breaking changes for scripts
---------------- ---------------------------
* As originally intended, function calls with a bang (`!`) now operates directly on the caller's scope, allowing variables inside the scope to be mutated. * As originally intended, function calls with a bang (`!`) now operates directly on the caller's scope, allowing variables inside the scope to be mutated.
* As originally intended, `Engine::XXX_with_scope` API's now properly propagate constants within the provided scope also to _functions_ in the script.
Bug fixes
---------
* `Engine::XXX_with_scope` API's now properly propagate constants within the provided scope also to _functions_ in the script.
New features New features
------------ ------------

View File

@ -2029,11 +2029,6 @@ impl Engine {
0, 0,
); );
// Remove arguments
if !rewind_scope && !args.is_empty() {
scope.remove_range(orig_scope_len, args.len())
}
result result
} }
/// Optimize the [`AST`] with constants defined in an external Scope. /// Optimize the [`AST`] with constants defined in an external Scope.

View File

@ -1898,15 +1898,15 @@ impl Engine {
let mut arg_values = StaticVec::with_capacity(args.len()); let mut arg_values = StaticVec::with_capacity(args.len());
let mut first_arg_pos = Position::NONE; let mut first_arg_pos = Position::NONE;
for index in 0..args.len() { args.iter().try_for_each(|expr| {
let (value, pos) = self.get_arg_value( self.get_arg_value(scope, mods, state, lib, this_ptr, level, expr, constants)
scope, mods, state, lib, this_ptr, level, args, constants, index, .map(|(value, pos)| {
)?; if arg_values.is_empty() {
arg_values.push(value.flatten()); first_arg_pos = pos
if index == 0 { }
first_arg_pos = pos arg_values.push(value.flatten());
} })
} })?;
idx_values.push((arg_values, first_arg_pos).into()); idx_values.push((arg_values, first_arg_pos).into());
} }
@ -1942,15 +1942,17 @@ impl Engine {
let mut arg_values = StaticVec::with_capacity(args.len()); let mut arg_values = StaticVec::with_capacity(args.len());
let mut first_arg_pos = Position::NONE; let mut first_arg_pos = Position::NONE;
for index in 0..args.len() { args.iter().try_for_each(|expr| {
let (value, pos) = self.get_arg_value( self.get_arg_value(
scope, mods, state, lib, this_ptr, level, args, constants, index, scope, mods, state, lib, this_ptr, level, expr, constants,
)?; )
arg_values.push(value.flatten()); .map(|(value, pos)| {
if index == 0 { if arg_values.is_empty() {
first_arg_pos = pos; first_arg_pos = pos
} }
} arg_values.push(value.flatten());
})
})?;
(arg_values, first_arg_pos).into() (arg_values, first_arg_pos).into()
} }
@ -2225,7 +2227,7 @@ impl Engine {
let mut pos = *pos; let mut pos = *pos;
let mut result: Dynamic = self.const_empty_string().into(); let mut result: Dynamic = self.const_empty_string().into();
for expr in x.iter() { x.iter().try_for_each(|expr| {
let item = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; let item = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?;
self.eval_op_assignment( self.eval_op_assignment(
@ -2243,8 +2245,8 @@ impl Engine {
pos = expr.position(); pos = expr.position();
self.check_data_size(&result) self.check_data_size(&result)
.map_err(|err| err.fill_position(pos))?; .map_err(|err| err.fill_position(pos))
} })?;
assert!( assert!(
result.is::<ImmutableString>(), result.is::<ImmutableString>(),
@ -2257,24 +2259,21 @@ impl Engine {
#[cfg(not(feature = "no_index"))] #[cfg(not(feature = "no_index"))]
Expr::Array(x, _) => { Expr::Array(x, _) => {
let mut arr = Array::with_capacity(x.len()); let mut arr = Array::with_capacity(x.len());
for item in x.as_ref() { x.iter().try_for_each(|item| {
arr.push( self.eval_expr(scope, mods, state, lib, this_ptr, item, level)
self.eval_expr(scope, mods, state, lib, this_ptr, item, level)? .map(|value| arr.push(value.flatten()))
.flatten(), })?;
);
}
Ok(arr.into()) Ok(arr.into())
} }
#[cfg(not(feature = "no_object"))] #[cfg(not(feature = "no_object"))]
Expr::Map(x, _) => { Expr::Map(x, _) => {
let mut map = x.1.clone(); let mut map = x.1.clone();
for (Ident { name: key, .. }, expr) in &x.0 { x.0.iter().try_for_each(|(Ident { name: key, .. }, expr)| {
let value_ref = map.get_mut(key.as_str()).expect("contains all keys"); let value_ref = map.get_mut(key.as_str()).expect("contains all keys");
*value_ref = self self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)
.eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .map(|value| *value_ref = value.flatten())
.flatten(); })?;
}
Ok(map.into()) Ok(map.into())
} }
@ -3119,19 +3118,20 @@ impl Engine {
// Export statement // Export statement
#[cfg(not(feature = "no_module"))] #[cfg(not(feature = "no_module"))]
Stmt::Export(list, _) => { Stmt::Export(list, _) => {
for (Ident { name, pos, .. }, Ident { name: rename, .. }) in list.as_ref() { list.iter().try_for_each(
// Mark scope variables as public |(Ident { name, pos, .. }, Ident { name: rename, .. })| {
if let Some((index, _)) = scope.get_index(name) { // Mark scope variables as public
scope.add_entry_alias( if let Some((index, _)) = scope.get_index(name) {
index, scope.add_entry_alias(
if rename.is_empty() { name } else { rename }.clone(), index,
); if rename.is_empty() { name } else { rename }.clone(),
} else { );
return Err( Ok(()) as Result<_, Box<EvalAltResult>>
EvalAltResult::ErrorVariableNotFound(name.to_string(), *pos).into() } else {
); Err(EvalAltResult::ErrorVariableNotFound(name.to_string(), *pos).into())
} }
} },
)?;
Ok(Dynamic::UNIT) Ok(Dynamic::UNIT)
} }

View File

@ -471,6 +471,8 @@ impl Engine {
/// Call a script-defined function. /// Call a script-defined function.
/// ///
/// If `rewind_scope` is `false`, arguments are removed from the scope but new variables are not.
///
/// # WARNING /// # WARNING
/// ///
/// Function call arguments may be _consumed_ when the function requires them to be passed by value. /// Function call arguments may be _consumed_ when the function requires them to be passed by value.
@ -548,7 +550,7 @@ impl Engine {
// Merge in encapsulated environment, if any // Merge in encapsulated environment, if any
let mut lib_merged = StaticVec::with_capacity(lib.len() + 1); let mut lib_merged = StaticVec::with_capacity(lib.len() + 1);
let (unified_lib, unified) = if let Some(ref env_lib) = fn_def.lib { let (unified, is_unified) = if let Some(ref env_lib) = fn_def.lib {
state.push_fn_resolution_cache(); state.push_fn_resolution_cache();
lib_merged.push(env_lib.as_ref()); lib_merged.push(env_lib.as_ref());
lib_merged.extend(lib.iter().cloned()); lib_merged.extend(lib.iter().cloned());
@ -572,7 +574,7 @@ impl Engine {
scope, scope,
mods, mods,
state, state,
unified_lib, unified,
this_ptr, this_ptr,
body, body,
true, true,
@ -604,10 +606,14 @@ impl Engine {
// Remove all local variables // Remove all local variables
if rewind_scope { if rewind_scope {
scope.rewind(prev_scope_len); scope.rewind(prev_scope_len);
} else if !args.is_empty() {
// Remove arguments only, leaving new variables in the scope
scope.remove_range(prev_scope_len, args.len())
} }
mods.truncate(prev_mods_len); mods.truncate(prev_mods_len);
if unified { if is_unified {
state.pop_fn_resolution_cache(); state.pop_fn_resolution_cache();
} }
@ -1052,12 +1058,11 @@ impl Engine {
lib: &[&Module], lib: &[&Module],
this_ptr: &mut Option<&mut Dynamic>, this_ptr: &mut Option<&mut Dynamic>,
level: usize, level: usize,
args_expr: &[Expr], arg_expr: &Expr,
constants: &[Dynamic], constants: &[Dynamic],
index: usize,
) -> Result<(Dynamic, Position), Box<EvalAltResult>> { ) -> Result<(Dynamic, Position), Box<EvalAltResult>> {
match args_expr[index] { match arg_expr {
Expr::Stack(slot, pos) => Ok((constants[slot].clone(), pos)), Expr::Stack(slot, pos) => Ok((constants[*slot].clone(), *pos)),
ref arg => self ref arg => self
.eval_expr(scope, mods, state, lib, this_ptr, arg, level) .eval_expr(scope, mods, state, lib, this_ptr, arg, level)
.map(|v| (v, arg.position())), .map(|v| (v, arg.position())),
@ -1075,23 +1080,23 @@ impl Engine {
fn_name: &str, fn_name: &str,
args_expr: &[Expr], args_expr: &[Expr],
constants: &[Dynamic], constants: &[Dynamic],
mut hashes: FnCallHashes, hashes: FnCallHashes,
pos: Position, pos: Position,
capture_scope: bool, capture_scope: bool,
level: usize, level: usize,
) -> RhaiResult { ) -> RhaiResult {
// Handle call() - Redirect function call let mut a_expr = args_expr;
let redirected; let mut total_args = a_expr.len();
let mut args_expr = args_expr;
let mut total_args = args_expr.len();
let mut curry = StaticVec::new(); let mut curry = StaticVec::new();
let mut name = fn_name; let mut name = fn_name;
let mut hashes = hashes;
let redirected; // Handle call() - Redirect function call
match name { match name {
// Handle call() // Handle call()
KEYWORD_FN_PTR_CALL if total_args >= 1 => { KEYWORD_FN_PTR_CALL if total_args >= 1 => {
let (arg, arg_pos) = self.get_arg_value( let (arg, arg_pos) = self.get_arg_value(
scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, scope, mods, state, lib, this_ptr, level, &a_expr[0], constants,
)?; )?;
if !arg.is::<FnPtr>() { if !arg.is::<FnPtr>() {
@ -1109,7 +1114,7 @@ impl Engine {
name = &redirected; name = &redirected;
// Skip the first argument // Skip the first argument
args_expr = &args_expr[1..]; a_expr = &a_expr[1..];
total_args -= 1; total_args -= 1;
// Recalculate hash // Recalculate hash
@ -1123,7 +1128,7 @@ impl Engine {
// Handle Fn() // Handle Fn()
KEYWORD_FN_PTR if total_args == 1 => { KEYWORD_FN_PTR if total_args == 1 => {
let (arg, arg_pos) = self.get_arg_value( let (arg, arg_pos) = self.get_arg_value(
scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, scope, mods, state, lib, this_ptr, level, &a_expr[0], constants,
)?; )?;
// Fn - only in function call style // Fn - only in function call style
@ -1138,7 +1143,7 @@ impl Engine {
// Handle curry() // Handle curry()
KEYWORD_FN_PTR_CURRY if total_args > 1 => { KEYWORD_FN_PTR_CURRY if total_args > 1 => {
let (arg, arg_pos) = self.get_arg_value( let (arg, arg_pos) = self.get_arg_value(
scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, scope, mods, state, lib, this_ptr, level, &a_expr[0], constants,
)?; )?;
if !arg.is::<FnPtr>() { if !arg.is::<FnPtr>() {
@ -1151,12 +1156,10 @@ impl Engine {
let (name, mut fn_curry) = arg.cast::<FnPtr>().take_data(); let (name, mut fn_curry) = arg.cast::<FnPtr>().take_data();
// Append the new curried arguments to the existing list. // Append the new curried arguments to the existing list.
for index in 1..args_expr.len() { a_expr.iter().skip(1).try_for_each(|expr| {
let (value, _) = self.get_arg_value( self.get_arg_value(scope, mods, state, lib, this_ptr, level, expr, constants)
scope, mods, state, lib, this_ptr, level, args_expr, constants, index, .map(|(value, _)| fn_curry.push(value))
)?; })?;
fn_curry.push(value);
}
return Ok(FnPtr::new_unchecked(name, fn_curry).into()); return Ok(FnPtr::new_unchecked(name, fn_curry).into());
} }
@ -1165,7 +1168,7 @@ impl Engine {
#[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "no_closure"))]
crate::engine::KEYWORD_IS_SHARED if total_args == 1 => { crate::engine::KEYWORD_IS_SHARED if total_args == 1 => {
let (arg, _) = self.get_arg_value( let (arg, _) = self.get_arg_value(
scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, scope, mods, state, lib, this_ptr, level, &a_expr[0], constants,
)?; )?;
return Ok(arg.is_shared().into()); return Ok(arg.is_shared().into());
} }
@ -1174,7 +1177,7 @@ impl Engine {
#[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_function"))]
crate::engine::KEYWORD_IS_DEF_FN if total_args == 2 => { crate::engine::KEYWORD_IS_DEF_FN if total_args == 2 => {
let (arg, arg_pos) = self.get_arg_value( let (arg, arg_pos) = self.get_arg_value(
scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, scope, mods, state, lib, this_ptr, level, &a_expr[0], constants,
)?; )?;
let fn_name = arg let fn_name = arg
@ -1182,7 +1185,7 @@ impl Engine {
.map_err(|typ| self.make_type_mismatch_err::<ImmutableString>(typ, arg_pos))?; .map_err(|typ| self.make_type_mismatch_err::<ImmutableString>(typ, arg_pos))?;
let (arg, arg_pos) = self.get_arg_value( let (arg, arg_pos) = self.get_arg_value(
scope, mods, state, lib, this_ptr, level, args_expr, constants, 1, scope, mods, state, lib, this_ptr, level, &a_expr[1], constants,
)?; )?;
let num_params = arg let num_params = arg
@ -1201,7 +1204,7 @@ impl Engine {
// Handle is_def_var() // Handle is_def_var()
KEYWORD_IS_DEF_VAR if total_args == 1 => { KEYWORD_IS_DEF_VAR if total_args == 1 => {
let (arg, arg_pos) = self.get_arg_value( let (arg, arg_pos) = self.get_arg_value(
scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, scope, mods, state, lib, this_ptr, level, &a_expr[0], constants,
)?; )?;
let var_name = arg let var_name = arg
.into_immutable_string() .into_immutable_string()
@ -1214,7 +1217,7 @@ impl Engine {
// eval - only in function call style // eval - only in function call style
let prev_len = scope.len(); let prev_len = scope.len();
let (value, pos) = self.get_arg_value( let (value, pos) = self.get_arg_value(
scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, scope, mods, state, lib, this_ptr, level, &a_expr[0], constants,
)?; )?;
let script = &value let script = &value
.into_immutable_string() .into_immutable_string()
@ -1246,19 +1249,19 @@ impl Engine {
} }
// Normal function call - except for Fn, curry, call and eval (handled above) // Normal function call - except for Fn, curry, call and eval (handled above)
let mut arg_values = StaticVec::with_capacity(args_expr.len()); let mut arg_values = StaticVec::with_capacity(a_expr.len());
let mut args = StaticVec::with_capacity(args_expr.len() + curry.len()); let mut args = StaticVec::with_capacity(a_expr.len() + curry.len());
let mut is_ref_mut = false; let mut is_ref_mut = false;
// Capture parent scope? // Capture parent scope?
//
// If so, do it separately because we cannot convert the first argument (if it is a simple
// variable access) to &mut because `scope` is needed.
if capture_scope && !scope.is_empty() { if capture_scope && !scope.is_empty() {
// func(..., ...) a_expr.iter().try_for_each(|expr| {
for index in 0..args_expr.len() { self.get_arg_value(scope, mods, state, lib, this_ptr, level, expr, constants)
let (value, _) = self.get_arg_value( .map(|(value, _)| arg_values.push(value.flatten()))
scope, mods, state, lib, this_ptr, level, args_expr, constants, index, })?;
)?;
arg_values.push(value.flatten());
}
args.extend(curry.iter_mut()); args.extend(curry.iter_mut());
args.extend(arg_values.iter_mut()); args.extend(arg_values.iter_mut());
@ -1273,22 +1276,20 @@ impl Engine {
} }
// Call with blank scope // Call with blank scope
if args_expr.is_empty() && curry.is_empty() { if a_expr.is_empty() && curry.is_empty() {
// No arguments // No arguments
} else { } else {
// If the first argument is a variable, and there is no curried arguments, // If the first argument is a variable, and there is no curried arguments,
// convert to method-call style in order to leverage potential &mut first argument and // convert to method-call style in order to leverage potential &mut first argument and
// avoid cloning the value // avoid cloning the value
if curry.is_empty() && !args_expr.is_empty() && args_expr[0].is_variable_access(false) { if curry.is_empty() && !a_expr.is_empty() && a_expr[0].is_variable_access(false) {
// func(x, ...) -> x.func(...) // func(x, ...) -> x.func(...)
let (first_expr, rest_expr) = args_expr.split_first().expect("not empty"); let (first_expr, rest_expr) = a_expr.split_first().expect("not empty");
for index in 0..rest_expr.len() { rest_expr.iter().try_for_each(|expr| {
let (value, _) = self.get_arg_value( self.get_arg_value(scope, mods, state, lib, this_ptr, level, expr, constants)
scope, mods, state, lib, this_ptr, level, rest_expr, constants, index, .map(|(value, _)| arg_values.push(value.flatten()))
)?; })?;
arg_values.push(value.flatten());
}
let (mut target, _pos) = let (mut target, _pos) =
self.search_namespace(scope, mods, state, lib, this_ptr, first_expr)?; self.search_namespace(scope, mods, state, lib, this_ptr, first_expr)?;
@ -1317,12 +1318,10 @@ impl Engine {
} }
} else { } else {
// func(..., ...) // func(..., ...)
for index in 0..args_expr.len() { a_expr.iter().try_for_each(|expr| {
let (value, _) = self.get_arg_value( self.get_arg_value(scope, mods, state, lib, this_ptr, level, expr, constants)
scope, mods, state, lib, this_ptr, level, args_expr, constants, index, .map(|(value, _)| arg_values.push(value.flatten()))
)?; })?;
arg_values.push(value.flatten());
}
args.extend(curry.iter_mut()); args.extend(curry.iter_mut());
args.extend(arg_values.iter_mut()); args.extend(arg_values.iter_mut());
} }
@ -1362,16 +1361,12 @@ impl Engine {
// &mut first argument and avoid cloning the value // &mut first argument and avoid cloning the value
if !args_expr.is_empty() && args_expr[0].is_variable_access(true) { if !args_expr.is_empty() && args_expr[0].is_variable_access(true) {
// func(x, ...) -> x.func(...) // func(x, ...) -> x.func(...)
for index in 0..args_expr.len() { arg_values.push(Dynamic::UNIT);
if index == 0 {
arg_values.push(Dynamic::UNIT); args_expr.iter().skip(1).try_for_each(|expr| {
} else { self.get_arg_value(scope, mods, state, lib, this_ptr, level, expr, constants)
let (value, _) = self.get_arg_value( .map(|(value, _)| arg_values.push(value.flatten()))
scope, mods, state, lib, this_ptr, level, args_expr, constants, index, })?;
)?;
arg_values.push(value.flatten());
}
}
// Get target reference to first argument // Get target reference to first argument
let (target, _pos) = let (target, _pos) =
@ -1398,12 +1393,10 @@ impl Engine {
} }
} else { } else {
// func(..., ...) or func(mod::x, ...) // func(..., ...) or func(mod::x, ...)
for index in 0..args_expr.len() { args_expr.iter().try_for_each(|expr| {
let (value, _) = self.get_arg_value( self.get_arg_value(scope, mods, state, lib, this_ptr, level, expr, constants)
scope, mods, state, lib, this_ptr, level, args_expr, constants, index, .map(|(value, _)| arg_values.push(value.flatten()))
)?; })?;
arg_values.push(value.flatten());
}
args.extend(arg_values.iter_mut()); args.extend(arg_values.iter_mut());
} }
} }

View File

@ -999,13 +999,14 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, chaining: bool) {
x.args.iter_mut().for_each(|a| optimize_expr(a, state, false)); x.args.iter_mut().for_each(|a| optimize_expr(a, state, false));
// Move constant arguments // Move constant arguments
for arg in x.args.iter_mut() { let constants = &mut x.constants;
x.args.iter_mut().for_each(|arg| {
if let Some(value) = arg.get_literal_value() { if let Some(value) = arg.get_literal_value() {
state.set_dirty(); state.set_dirty();
x.constants.push(value); constants.push(value);
*arg = Expr::Stack(x.constants.len()-1, arg.position()); *arg = Expr::Stack(constants.len()-1, arg.position());
} }
} });
} }
// Eagerly call functions // Eagerly call functions

View File

@ -60,9 +60,8 @@ impl Serialize for Dynamic {
#[cfg(not(feature = "no_object"))] #[cfg(not(feature = "no_object"))]
Union::Map(ref m, _, _) => { Union::Map(ref m, _, _) => {
let mut map = ser.serialize_map(Some(m.len()))?; let mut map = ser.serialize_map(Some(m.len()))?;
for (k, v) in m.iter() { m.iter()
map.serialize_entry(k.as_str(), v)?; .try_for_each(|(k, v)| map.serialize_entry(k.as_str(), v))?;
}
map.end() map.end()
} }
Union::FnPtr(ref f, _, _) => ser.serialize_str(f.fn_name()), Union::FnPtr(ref f, _, _) => ser.serialize_str(f.fn_name()),