From 691e04292fe8b8a29f15c5b2281c535ebf99e594 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 30 Jul 2020 13:28:06 +0800 Subject: [PATCH 1/7] Put externals in ScriptFnDef. --- src/optimize.rs | 30 +++++++++++++-------- src/parser.rs | 71 ++++++++++++++++++++++++++++++------------------- 2 files changed, 63 insertions(+), 38 deletions(-) diff --git a/src/optimize.rs b/src/optimize.rs index b46b7488..e26d1908 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -5,10 +5,10 @@ use crate::calc_fn_hash; use crate::engine::{ Engine, Imports, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT, KEYWORD_TYPE_OF, }; +use crate::fn_native::FnPtr; use crate::module::Module; use crate::parser::{map_dynamic_to_expr, Expr, ScriptFnDef, Stmt, AST}; use crate::scope::{Entry as ScopeEntry, EntryType as ScopeEntryType, Scope}; -use crate::token::is_valid_identifier; use crate::utils::StaticVec; #[cfg(not(feature = "no_function"))] @@ -19,6 +19,7 @@ use crate::parser::CustomExpr; use crate::stdlib::{ boxed::Box, + convert::TryFrom, iter::empty, string::{String, ToString}, vec, @@ -418,7 +419,7 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { // All other items can be thrown away. state.set_dirty(); let pos = m.1; - m.0.into_iter().find(|((name, _), _)| name.as_str() == prop.as_str()) + m.0.into_iter().find(|((name, _), _)| name == prop) .map(|(_, mut expr)| { expr.set_position(pos); expr }) .unwrap_or_else(|| Expr::Unit(pos)) } @@ -495,7 +496,7 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { state.set_dirty(); let ch = a.0.to_string(); - if b.0.iter().find(|((name, _), _)| name.as_str() == ch.as_str()).is_some() { + if b.0.iter().find(|((name, _), _)| name == &ch).is_some() { Expr::True(a.1) } else { Expr::False(a.1) @@ -553,14 +554,19 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { // Fn("...") Expr::FnCall(x) - if x.1.is_none() - && (x.0).0 == KEYWORD_FN_PTR - && x.3.len() == 1 - && matches!(x.3[0], Expr::StringConstant(_)) + if x.1.is_none() + && (x.0).0 == KEYWORD_FN_PTR + && x.3.len() == 1 + && matches!(x.3[0], Expr::StringConstant(_)) => { - match &x.3[0] { - Expr::StringConstant(s) if is_valid_identifier(s.0.chars()) => Expr::FnPointer(s.clone()), - _ => Expr::FnCall(x) + if let Expr::StringConstant(s) = &x.3[0] { + if let Ok(fn_ptr) = FnPtr::try_from(s.0.as_str()) { + Expr::FnPointer(Box::new((fn_ptr.take_data().0, s.1))) + } else { + Expr::FnCall(x) + } + } else { + unreachable!() } } @@ -578,7 +584,7 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { let _has_script_fn = state.lib.iter_fn().find(|(_, _, _, f)| { if !f.is_script() { return false; } let fn_def = f.get_fn_def(); - fn_def.name.as_str() == name && (args.len()..=args.len() + 1).contains(&fn_def.params.len()) + fn_def.name == name && (args.len()..=args.len() + 1).contains(&fn_def.params.len()) }).is_some(); #[cfg(feature = "no_function")] @@ -765,6 +771,8 @@ pub fn optimize_into_ast( access: fn_def.access, body: Default::default(), params: fn_def.params.clone(), + #[cfg(not(feature = "no_capture"))] + externals: fn_def.externals.clone(), pos: fn_def.pos, } .into() diff --git a/src/parser.rs b/src/parser.rs index 3068bad0..abe3c7d8 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -363,6 +363,9 @@ pub struct ScriptFnDef { pub access: FnAccess, /// Names of function parameters. pub params: StaticVec, + /// Access to external variables. + #[cfg(not(feature = "no_capture"))] + pub externals: StaticVec, /// Function body. pub body: Stmt, /// Position of the function definition. @@ -448,7 +451,7 @@ impl<'e> ParseState<'e> { /// The return value is the offset to be deducted from `Stack::len`, /// i.e. the top element of the `ParseState` is offset 1. /// Return `None` when the variable name is not found in the `stack`. - fn access_var(&mut self, name: &str, pos: Position) -> Option { + fn access_var(&mut self, name: &str, _pos: Position) -> Option { let index = self .stack .iter() @@ -459,7 +462,7 @@ impl<'e> ParseState<'e> { #[cfg(not(feature = "no_capture"))] if index.is_none() && !self.externals.contains_key(name) { - self.externals.insert(name.to_string(), pos); + self.externals.insert(name.to_string(), _pos); } index @@ -2848,7 +2851,7 @@ fn parse_stmt( match input.next().unwrap() { (Token::Fn, pos) => { - let mut state = ParseState::new( + let mut new_state = ParseState::new( state.engine, #[cfg(not(feature = "unchecked"))] state.max_function_expr_depth, @@ -2867,7 +2870,7 @@ fn parse_stmt( pos: pos, }; - let func = parse_fn(input, &mut state, lib, access, settings)?; + let func = parse_fn(input, &mut new_state, lib, access, settings)?; // Qualifiers (none) + function name + number of arguments. let hash = calc_fn_hash(empty(), &func.name, func.params.len(), empty()); @@ -3039,12 +3042,23 @@ fn parse_fn( (_, pos) => return Err(PERR::FnMissingBody(name).into_err(*pos)), }; - let params = params.into_iter().map(|(p, _)| p).collect(); + let params: StaticVec<_> = params.into_iter().map(|(p, _)| p).collect(); + + #[cfg(not(feature = "no_capture"))] + let externals = state + .externals + .iter() + .map(|(name, _)| name) + .filter(|name| !params.contains(name)) + .cloned() + .collect(); Ok(ScriptFnDef { name: name.into(), access, params, + #[cfg(not(feature = "no_capture"))] + externals, body, pos: settings.pos, }) @@ -3054,40 +3068,31 @@ fn parse_fn( #[cfg(not(feature = "no_capture"))] fn make_curry_from_externals( fn_expr: Expr, - state: &mut ParseState, - settings: &ParseSettings, + externals: StaticVec<(String, Position)>, + pos: Position, ) -> Expr { - if state.externals.is_empty() { + if externals.is_empty() { return fn_expr; } + let num_externals = externals.len(); let mut args: StaticVec<_> = Default::default(); - state.externals.iter().for_each(|(var_name, pos)| { - args.push(Expr::Variable(Box::new(( - (var_name.clone(), *pos), - None, - 0, - None, - )))); + externals.into_iter().for_each(|(var_name, pos)| { + args.push(Expr::Variable(Box::new(((var_name, pos), None, 0, None)))); }); - let hash = calc_fn_hash( - empty(), - KEYWORD_FN_PTR_CURRY, - state.externals.len(), - empty(), - ); + let hash = calc_fn_hash(empty(), KEYWORD_FN_PTR_CURRY, num_externals, empty()); let fn_call = Expr::FnCall(Box::new(( - (KEYWORD_FN_PTR_CURRY.into(), false, settings.pos), + (KEYWORD_FN_PTR_CURRY.into(), false, pos), None, hash, args, None, ))); - Expr::Dot(Box::new((fn_expr, fn_call, settings.pos))) + Expr::Dot(Box::new((fn_expr, fn_call, pos))) } /// Parse an anonymous function definition. @@ -3160,11 +3165,20 @@ fn parse_anon_fn( #[cfg(feature = "no_capture")] let params: StaticVec<_> = params.into_iter().map(|(v, _)| v).collect(); + // External variables may need to be processed in a consistent order, + // so extract them into a list. + #[cfg(not(feature = "no_capture"))] + let externals: StaticVec<_> = state + .externals + .iter() + .map(|(k, &v)| (k.clone(), v)) + .collect(); + // Add parameters that are auto-curried #[cfg(not(feature = "no_capture"))] - let params: StaticVec<_> = state - .externals - .keys() + let params: StaticVec<_> = externals + .iter() + .map(|(k, _)| k) .cloned() .chain(params.into_iter().map(|(v, _)| v)) .collect(); @@ -3183,10 +3197,13 @@ fn parse_anon_fn( // Create unique function name let fn_name: ImmutableString = format!("{}{:16x}", FN_ANONYMOUS, hash).into(); + // Define the function let script = ScriptFnDef { name: fn_name.clone(), access: FnAccess::Public, params, + #[cfg(not(feature = "no_capture"))] + externals: Default::default(), body, pos: settings.pos, }; @@ -3194,7 +3211,7 @@ fn parse_anon_fn( let expr = Expr::FnPointer(Box::new((fn_name, settings.pos))); #[cfg(not(feature = "no_capture"))] - let expr = make_curry_from_externals(expr, state, &settings); + let expr = make_curry_from_externals(expr, externals, settings.pos); Ok((expr, script)) } From e505a068397a52f44174b27443c3d64e68276982 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 30 Jul 2020 13:28:25 +0800 Subject: [PATCH 2/7] Add comparison operators to ImmutableString. --- src/engine.rs | 8 +++----- src/fn_call.rs | 3 ++- src/packages/map_basic.rs | 6 +++--- src/utils.rs | 43 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 7806382f..2ad03365 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1105,7 +1105,7 @@ impl Engine { .downcast_ref::() .ok_or_else(|| EvalAltResult::ErrorStringIndexExpr(idx_pos))?; - map.get_mut(index.as_str()) + map.get_mut(index) .map(Target::from) .unwrap_or_else(|| Target::from(())) }) @@ -1208,10 +1208,8 @@ impl Engine { #[cfg(not(feature = "no_object"))] Dynamic(Union::Map(rhs_value)) => match lhs_value { // Only allows String or char - Dynamic(Union::Str(s)) => Ok(rhs_value.contains_key(s.as_str()).into()), - Dynamic(Union::Char(c)) => { - Ok(rhs_value.contains_key(c.to_string().as_str()).into()) - } + Dynamic(Union::Str(s)) => Ok(rhs_value.contains_key(&s).into()), + Dynamic(Union::Char(c)) => Ok(rhs_value.contains_key(&c.to_string()).into()), _ => Err(Box::new(EvalAltResult::ErrorInExpr(lhs.position()))), }, Dynamic(Union::Str(rhs_value)) => match lhs_value { diff --git a/src/fn_call.rs b/src/fn_call.rs index 162410f3..861f490b 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -665,7 +665,8 @@ impl Engine { )) }) .and_then(|s| FnPtr::try_from(s)) - .map(Into::::into); + .map(Into::::into) + .map_err(|err| err.new_position(expr.position())); } } diff --git a/src/packages/map_basic.rs b/src/packages/map_basic.rs index cc1aa000..9437bc67 100644 --- a/src/packages/map_basic.rs +++ b/src/packages/map_basic.rs @@ -23,7 +23,7 @@ fn map_get_values(map: &mut Map) -> FuncReturn> { def_package!(crate:BasicMapPackage:"Basic object map utilities.", lib, { lib.set_fn_2_mut( "has", - |map: &mut Map, prop: ImmutableString| Ok(map.contains_key(prop.as_str())), + |map: &mut Map, prop: ImmutableString| Ok(map.contains_key(&prop)), ); lib.set_fn_1_mut("len", |map: &mut Map| Ok(map.len() as INT)); lib.set_fn_1_mut("clear", |map: &mut Map| { @@ -32,7 +32,7 @@ def_package!(crate:BasicMapPackage:"Basic object map utilities.", lib, { }); lib.set_fn_2_mut( "remove", - |x: &mut Map, name: ImmutableString| Ok(x.remove(name.as_str()).unwrap_or_else(|| ().into())), + |x: &mut Map, name: ImmutableString| Ok(x.remove(&name).unwrap_or_else(|| ().into())), ); lib.set_fn_2_mut( "mixin", @@ -47,7 +47,7 @@ def_package!(crate:BasicMapPackage:"Basic object map utilities.", lib, { "fill_with", |map1: &mut Map, map2: Map| { map2.into_iter().for_each(|(key, value)| { - if !map1.contains_key(key.as_str()) { + if !map1.contains_key(&key) { map1.insert(key, value); } }); diff --git a/src/utils.rs b/src/utils.rs index 13e11704..6668a7a9 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -6,6 +6,7 @@ use crate::stdlib::{ any::TypeId, borrow::Borrow, boxed::Box, + cmp::Ordering, fmt, hash::{BuildHasher, Hash, Hasher}, iter::FromIterator, @@ -141,6 +142,12 @@ impl AsRef for ImmutableString { } } +impl Borrow for ImmutableString { + fn borrow(&self) -> &String { + &self.0 + } +} + impl Borrow for ImmutableString { fn borrow(&self) -> &str { self.0.as_str() @@ -348,6 +355,42 @@ impl AddAssign for ImmutableString { } } +impl> PartialEq for ImmutableString { + fn eq(&self, other: &S) -> bool { + self.as_str().eq(other.as_ref()) + } +} + +impl PartialEq for str { + fn eq(&self, other: &ImmutableString) -> bool { + self.eq(other.as_str()) + } +} + +impl PartialEq for String { + fn eq(&self, other: &ImmutableString) -> bool { + self.eq(other.as_str()) + } +} + +impl> PartialOrd for ImmutableString { + fn partial_cmp(&self, other: &S) -> Option { + self.as_str().partial_cmp(other.as_ref()) + } +} + +impl PartialOrd for str { + fn partial_cmp(&self, other: &ImmutableString) -> Option { + self.partial_cmp(other.as_str()) + } +} + +impl PartialOrd for String { + fn partial_cmp(&self, other: &ImmutableString) -> Option { + self.as_str().partial_cmp(other.as_str()) + } +} + impl ImmutableString { /// Consume the `ImmutableString` and convert it into a `String`. /// If there are other references to the same string, a cloned copy is returned. From 98b294c699a163a74551eda7a3a88dfb1e16e9ae Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 30 Jul 2020 18:18:28 +0800 Subject: [PATCH 3/7] Implement capturing. --- RELEASES.md | 4 + doc/src/SUMMARY.md | 13 +- doc/src/advanced.md | 2 + doc/src/language/fn-anon.md | 2 +- doc/src/language/fn-capture.md | 62 +++++++ doc/src/language/fn-closure.md | 2 + doc/src/language/fn-curry.md | 4 +- doc/src/start/features.md | 2 +- src/engine.rs | 49 +++--- src/error.rs | 11 +- src/fn_call.rs | 288 ++++++++++++++++++++------------- src/fn_native.rs | 2 +- src/optimize.rs | 4 +- src/parser.rs | 66 +++++--- src/scope.rs | 13 +- src/token.rs | 14 +- tests/functions.rs | 52 +++++- 17 files changed, 410 insertions(+), 180 deletions(-) create mode 100644 doc/src/language/fn-capture.md diff --git a/RELEASES.md b/RELEASES.md index 7f59cddf..f163f06f 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -9,6 +9,8 @@ This version adds: * Binding the `this` pointer in a function pointer `call`. * Anonymous functions (in Rust closure syntax). Simplifies creation of single-use ad-hoc functions. * Currying of function pointers. +* Auto-currying of anonymous functions. +* Capturing call scope via `func!(...)` syntax. New features ------------ @@ -19,6 +21,8 @@ New features * Anonymous functions are supported in the syntax of a Rust closure, e.g. `|x, y, z| x + y - z`. * Custom syntax now works even without the `internals` feature. * Currying of function pointers is supported via the new `curry` keyword. +* Automatic currying of anonymous functions to capture environment variables. +* Capturing of the calling scope for function call via the `func!(...)` syntax. * `Module::set_indexer_get_set_fn` is added as a shorthand of both `Module::set_indexer_get_fn` and `Module::set_indexer_set_fn`. * New `unicode-xid-ident` feature to allow [Unicode Standard Annex #31](http://www.unicode.org/reports/tr31/) for identifiers. diff --git a/doc/src/SUMMARY.md b/doc/src/SUMMARY.md index 78ea9882..6b966de1 100644 --- a/doc/src/SUMMARY.md +++ b/doc/src/SUMMARY.md @@ -100,21 +100,22 @@ The Rhai Scripting Language 8. [Maximum Call Stack Depth](safety/max-call-stack.md) 9. [Maximum Statement Depth](safety/max-stmt-depth.md) 7. [Advanced Topics](advanced.md) - 1. [Object-Oriented Programming (OOP)](language/oop.md) - 2. [Serialization/Deserialization of `Dynamic` with `serde`](rust/serde.md) - 3. [Script Optimization](engine/optimize/index.md) + 1. [Capture Scope for Function Call](language/fn-capture.md) + 2. [Object-Oriented Programming (OOP)](language/oop.md) + 3. [Serialization/Deserialization of `Dynamic` with `serde`](rust/serde.md) + 4. [Script Optimization](engine/optimize/index.md) 1. [Optimization Levels](engine/optimize/optimize-levels.md) 2. [Re-Optimize an AST](engine/optimize/reoptimize.md) 3. [Eager Function Evaluation](engine/optimize/eager.md) 4. [Side-Effect Considerations](engine/optimize/side-effects.md) 5. [Volatility Considerations](engine/optimize/volatility.md) 6. [Subtle Semantic Changes](engine/optimize/semantics.md) - 4. [Low-Level API](rust/register-raw.md) - 5. [Use as DSL](engine/dsl.md) + 5. [Low-Level API](rust/register-raw.md) + 6. [Use as DSL](engine/dsl.md) 1. [Disable Keywords and/or Operators](engine/disable.md) 2. [Custom Operators](engine/custom-op.md) 3. [Extending with Custom Syntax](engine/custom-syntax.md) - 6. [Eval Statement](language/eval.md) + 7. [Eval Statement](language/eval.md) 8. [Appendix](appendix/index.md) 1. [Keywords](appendix/keywords.md) 2. [Operators and Symbols](appendix/operators.md) diff --git a/doc/src/advanced.md b/doc/src/advanced.md index cd958c73..6b79fe70 100644 --- a/doc/src/advanced.md +++ b/doc/src/advanced.md @@ -5,6 +5,8 @@ Advanced Topics This section covers advanced features such as: +* [Capture the calling scope]({{rootUrl}}/language/fn-capture.md) in a function call. + * Simulated [Object Oriented Programming (OOP)][OOP]. * [`serde`] integration. diff --git a/doc/src/language/fn-anon.md b/doc/src/language/fn-anon.md index a4cb0fee..e76ba59c 100644 --- a/doc/src/language/fn-anon.md +++ b/doc/src/language/fn-anon.md @@ -54,7 +54,7 @@ WARNING - NOT Closures ---------------------- Remember: anonymous functions, though having the same syntax as Rust _closures_, are themselves -**not** closures. In particular, they do not capture their running environment. They are more like +**not** closures. In particular, they do not capture their execution environment. They are more like Rust's function pointers. They do, however, _capture_ variable _values_ from their execution environment, unless the [`no_capture`] diff --git a/doc/src/language/fn-capture.md b/doc/src/language/fn-capture.md new file mode 100644 index 00000000..042675e9 --- /dev/null +++ b/doc/src/language/fn-capture.md @@ -0,0 +1,62 @@ +Capture The Calling Scope for Function Call +========================================== + +{{#include ../links.md}} + + +Peeking Out of The Pure Box +--------------------------- + +Rhai functions are _pure_, meaning that they depend on on their arguments and have no +access to the calling environment. + +When a function accesses a variable that is not defined within that function's scope, +it raises an evaluation error. + +It is possible, through a special syntax, to capture the calling scope - i.e. the scope +that makes the function call - and access variables defined there. + +```rust +fn foo(y) { // function accesses 'x' and 'y', but 'x' is not defined + x += y; // 'x' is modified in this function + x +} + +let x = 1; + +foo(41); // error: variable 'x' not found + +// Calling a function with a '!' causes it to capture the calling scope + +foo!(41) == 42; // the function can access the value of 'x', but cannot change it + +x == 1; // 'x' is still the original value + +x.method!(); // <- syntax error: capturing is not allowed in method-call style + +// Capturing also works for function pointers + +let f = Fn("foo"); + +call!(f, 41) == 42; // must use function-call style + +f.call!(41); // <- syntax error: capturing is not allowed in method-call style +``` + + +No Mutations +------------ + +Variables in the calling scope are accessed as copies. +Changes to them do not reflect back to the calling scope. + +Rhai functions remain _pure_ in the sense that they can never mutate their environment. + + +Caveat Emptor +------------- + +Functions relying on the calling scope is a _Very Bad Idea™_ because it makes code almost impossible +to reason and maintain, as their behaviors are volatile and unpredictable. + +This usage should be at the last resort. diff --git a/doc/src/language/fn-closure.md b/doc/src/language/fn-closure.md index d8749601..1701cc4a 100644 --- a/doc/src/language/fn-closure.md +++ b/doc/src/language/fn-closure.md @@ -1,6 +1,8 @@ Capture External Variables via Automatic Currying ================================================ +{{#include ../links.md}} + Poor Man's Closures ------------------- diff --git a/doc/src/language/fn-curry.md b/doc/src/language/fn-curry.md index 71c8933a..83705175 100644 --- a/doc/src/language/fn-curry.md +++ b/doc/src/language/fn-curry.md @@ -33,7 +33,7 @@ curried.call(2) == 42; // <- de-sugars to 'func.call(21, 2)' Automatic Currying ------------------ -[Anonymous functions] defined via a closure syntax _capture_ external variables that are not shadowed inside -the function's scope. +[Anonymous functions] defined via a closure syntax _capture_ the _values_ of external variables +that are not shadowed inside the function's scope. This is accomplished via [automatic currying]. diff --git a/doc/src/start/features.md b/doc/src/start/features.md index 7a06618a..efeb39c5 100644 --- a/doc/src/start/features.md +++ b/doc/src/start/features.md @@ -23,7 +23,7 @@ more control over what a script can (or cannot) do. | `no_object` | Disable support for [custom types] and [object maps]. | | `no_function` | Disable script-defined [functions]. | | `no_module` | Disable loading external [modules]. | -| `no_capture` | Disable capturing external variables in [anonymous functions]. | +| `no_capture` | Disable [capturing][capture] external variables in [anonymous functions] and [capturing the calling scope]({{rootUrl}}/language/fn-capture.md) in function calls. | | `no_std` | Build for `no-std`. Notice that additional dependencies will be pulled in to replace `std` features. | | `serde` | Enable serialization/deserialization via `serde`. Notice that the [`serde`](https://crates.io/crates/serde) crate will be pulled in together with its dependencies. | | `internals` | Expose internal data structures (e.g. [`AST`] nodes). Beware that Rhai internals are volatile and may change from version to version. | diff --git a/src/engine.rs b/src/engine.rs index 2ad03365..ac264af4 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -686,7 +686,7 @@ impl Engine { let args = &mut [target.as_mut(), &mut idx_val2, &mut new_val]; self.exec_fn_call( - state, lib, FN_IDX_SET, true, 0, args, is_ref, true, false, + state, lib, FN_IDX_SET, 0, args, is_ref, true, false, None, None, level, ) .or_else(|err| match *err { @@ -710,7 +710,7 @@ impl Engine { let args = &mut [target.as_mut(), &mut idx_val2, &mut new_val]; self.exec_fn_call( - state, lib, FN_IDX_SET, true, 0, args, is_ref, true, false, + state, lib, FN_IDX_SET, 0, args, is_ref, true, false, None, None, level, )?; } @@ -732,7 +732,7 @@ impl Engine { match rhs { // xxx.fn_name(arg_expr_list) Expr::FnCall(x) if x.1.is_none() => { - let ((name, native, pos), _, hash, _, def_val) = x.as_ref(); + let ((name, native, _, pos), _, hash, _, def_val) = x.as_ref(); self.make_method_call( state, lib, name, *hash, target, idx_val, *def_val, *native, false, level, @@ -766,7 +766,7 @@ impl Engine { let ((_, _, setter), pos) = x.as_ref(); let mut args = [target.as_mut(), _new_val.as_mut().unwrap()]; self.exec_fn_call( - state, lib, setter, true, 0, &mut args, is_ref, true, false, None, + state, lib, setter, 0, &mut args, is_ref, true, false, None, None, level, ) .map(|(v, _)| (v, true)) @@ -777,7 +777,7 @@ impl Engine { let ((_, getter, _), pos) = x.as_ref(); let mut args = [target.as_mut()]; self.exec_fn_call( - state, lib, getter, true, 0, &mut args, is_ref, true, false, None, + state, lib, getter, 0, &mut args, is_ref, true, false, None, None, level, ) .map(|(v, _)| (v, false)) @@ -795,7 +795,7 @@ impl Engine { } // {xxx:map}.fn_name(arg_expr_list)[expr] | {xxx:map}.fn_name(arg_expr_list).expr Expr::FnCall(x) if x.1.is_none() => { - let ((name, native, pos), _, hash, _, def_val) = x.as_ref(); + let ((name, native, _, pos), _, hash, _, def_val) = x.as_ref(); let (val, _) = self .make_method_call( state, lib, name, *hash, target, idx_val, *def_val, @@ -828,7 +828,7 @@ impl Engine { let args = &mut arg_values[..1]; let (mut val, updated) = self .exec_fn_call( - state, lib, getter, true, 0, args, is_ref, true, false, + state, lib, getter, 0, args, is_ref, true, false, None, None, level, ) .map_err(|err| err.new_position(*pos))?; @@ -848,8 +848,8 @@ impl Engine { // Re-use args because the first &mut parameter will not be consumed arg_values[1] = val; self.exec_fn_call( - state, lib, setter, true, 0, arg_values, is_ref, true, - false, None, level, + state, lib, setter, 0, arg_values, is_ref, true, false, + None, None, level, ) .or_else( |err| match *err { @@ -866,7 +866,7 @@ impl Engine { } // xxx.fn_name(arg_expr_list)[expr] | xxx.fn_name(arg_expr_list).expr Expr::FnCall(x) if x.1.is_none() => { - let ((name, native, pos), _, hash, _, def_val) = x.as_ref(); + let ((name, native, _, pos), _, hash, _, def_val) = x.as_ref(); let (mut val, _) = self .make_method_call( state, lib, name, *hash, target, idx_val, *def_val, @@ -1138,7 +1138,7 @@ impl Engine { let type_name = val.type_name(); let args = &mut [val, &mut _idx]; self.exec_fn_call( - state, _lib, FN_IDX_GET, true, 0, args, is_ref, true, false, None, _level, + state, _lib, FN_IDX_GET, 0, args, is_ref, true, false, None, None, _level, ) .map(|(v, _)| v.into()) .map_err(|err| match *err { @@ -1186,15 +1186,13 @@ impl Engine { let def_value = Some(false); let args = &mut [&mut lhs_value.clone(), value]; - let hashes = ( - // Qualifiers (none) + function name + number of arguments + argument `TypeId`'s. - calc_fn_hash(empty(), op, args.len(), args.iter().map(|a| a.type_id())), - 0, - ); + // Qualifiers (none) + function name + number of arguments + argument `TypeId`'s. + let hash = + calc_fn_hash(empty(), op, args.len(), args.iter().map(|a| a.type_id())); let (r, _) = self .call_fn_raw( - &mut scope, mods, state, lib, op, hashes, args, false, false, false, + &mut scope, mods, state, lib, op, hash, args, false, false, false, def_value, level, ) .map_err(|err| err.new_position(rhs.position()))?; @@ -1301,14 +1299,14 @@ impl Engine { } else if run_builtin_op_assignment(op, lhs_ptr, &rhs_val)?.is_none() { // Not built in, map to `var = var op rhs` let op = &op[..op.len() - 1]; // extract operator without = - let hash = calc_fn_hash(empty(), op, 2, empty()); + // Clone the LHS value let args = &mut [&mut lhs_ptr.clone(), &mut rhs_val]; + // Run function let (value, _) = self .exec_fn_call( - state, lib, op, true, hash, args, false, false, false, None, - level, + state, lib, op, 0, args, false, false, false, None, None, level, ) .map_err(|err| err.new_position(*op_pos))?; // Set value to LHS @@ -1331,13 +1329,12 @@ impl Engine { } else { // Op-assignment - always map to `lhs = lhs op rhs` let op = &op[..op.len() - 1]; // extract operator without = - let hash = calc_fn_hash(empty(), op, 2, empty()); let args = &mut [ &mut self.eval_expr(scope, mods, state, lib, this_ptr, lhs_expr, level)?, &mut rhs_val, ]; self.exec_fn_call( - state, lib, op, true, hash, args, false, false, false, None, level, + state, lib, op, 0, args, false, false, false, None, None, level, ) .map(|(v, _)| v) .map_err(|err| err.new_position(*op_pos))? @@ -1407,20 +1404,20 @@ impl Engine { // Normal function call Expr::FnCall(x) if x.1.is_none() => { - let ((name, native, pos), _, hash, args_expr, def_val) = x.as_ref(); + let ((name, native, capture, pos), _, hash, args_expr, def_val) = x.as_ref(); self.make_function_call( scope, mods, state, lib, this_ptr, name, args_expr, *def_val, *hash, *native, - false, level, + false, *capture, level, ) .map_err(|err| err.new_position(*pos)) } // Module-qualified function call Expr::FnCall(x) if x.1.is_some() => { - let ((name, _, pos), modules, hash, args_expr, def_val) = x.as_ref(); + let ((name, _, capture, pos), modules, hash, args_expr, def_val) = x.as_ref(); self.make_qualified_function_call( scope, mods, state, lib, this_ptr, modules, name, args_expr, *def_val, *hash, - level, + *capture, level, ) .map_err(|err| err.new_position(*pos)) } diff --git a/src/error.rs b/src/error.rs index fc92e5ea..259d6916 100644 --- a/src/error.rs +++ b/src/error.rs @@ -91,6 +91,10 @@ pub enum ParseErrorType { /// /// Never appears under the `no_object` and `no_index` features combination. MalformedInExpr(String), + /// A capturing has syntax error. Wrapped value is the error description (if any). + /// + /// Never appears under the `no_capture` feature. + MalformedCapture(String), /// A map definition has duplicated property names. Wrapped value is the property name. /// /// Never appears under the `no_object` feature. @@ -166,6 +170,7 @@ impl ParseErrorType { Self::MalformedCallExpr(_) => "Invalid expression in function call arguments", Self::MalformedIndexExpr(_) => "Invalid index in indexing expression", Self::MalformedInExpr(_) => "Invalid 'in' expression", + Self::MalformedCapture(_) => "Invalid capturing", Self::DuplicatedProperty(_) => "Duplicated property in object map literal", Self::ForbiddenConstantExpr(_) => "Expecting a constant", Self::PropertyExpected => "Expecting name of a property", @@ -199,9 +204,9 @@ impl fmt::Display for ParseErrorType { } Self::UnknownOperator(s) => write!(f, "{}: '{}'", self.desc(), s), - Self::MalformedIndexExpr(s) => f.write_str(if s.is_empty() { self.desc() } else { s }), - - Self::MalformedInExpr(s) => f.write_str(if s.is_empty() { self.desc() } else { s }), + Self::MalformedIndexExpr(s) | Self::MalformedInExpr(s) | Self::MalformedCapture(s) => { + f.write_str(if s.is_empty() { self.desc() } else { s }) + } Self::DuplicatedProperty(s) => { write!(f, "Duplicated property '{}' for object map literal", s) diff --git a/src/fn_call.rs b/src/fn_call.rs index 861f490b..ca1aef51 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -19,8 +19,9 @@ use crate::utils::StaticVec; #[cfg(not(feature = "no_function"))] use crate::{ - parser::ScriptFnDef, r#unsafe::unsafe_cast_var_name_to_lifetime, - scope::EntryType as ScopeEntryType, + parser::ScriptFnDef, + r#unsafe::unsafe_cast_var_name_to_lifetime, + scope::{Entry as ScopeEntry, EntryType as ScopeEntryType}, }; #[cfg(not(feature = "no_float"))] @@ -105,8 +106,32 @@ fn restore_first_arg<'a>(old_this_ptr: Option<&'a mut Dynamic>, args: &mut FnCal } } +// Add captured variables into scope +#[cfg(not(feature = "no_capture"))] +fn add_captured_variables_into_scope<'s>( + externals: &[String], + captured: &'s Scope<'s>, + scope: &mut Scope<'s>, +) { + externals + .iter() + .map(|var_name| captured.get_entry(var_name)) + .filter(Option::is_some) + .map(Option::unwrap) + .for_each( + |ScopeEntry { + name, typ, value, .. + }| { + match typ { + ScopeEntryType::Normal => scope.push(name.clone(), value.clone()), + ScopeEntryType::Constant => scope.push_constant(name.clone(), value.clone()), + }; + }, + ); +} + impl Engine { - /// Universal method for calling functions either registered with the `Engine` or written in Rhai. + /// Call a native Rust function registered with the `Engine`. /// Position in `EvalAltResult` is `None` and must be set afterwards. /// /// ## WARNING @@ -121,7 +146,7 @@ impl Engine { state: &mut State, lib: &Module, fn_name: &str, - (hash_fn, hash_script): (u64, u64), + hash_fn: u64, args: &mut FnCallArgs, is_ref: bool, _is_method: bool, @@ -131,8 +156,6 @@ impl Engine { ) -> Result<(Dynamic, bool), Box> { self.inc_operations(state)?; - let native_only = hash_script == 0; - // Check for stack overflow #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "unchecked"))] @@ -142,23 +165,13 @@ impl Engine { )); } - let mut this_copy: Dynamic = Default::default(); - let mut old_this_ptr: Option<&mut Dynamic> = None; - // Search for the function - // First search in script-defined functions (can override built-in) - // Then search registered native functions (can override packages) + // First search registered native functions (can override packages) // Then search packages - // NOTE: We skip script functions for global_module and packages, and native functions for lib - let func = if !native_only { - lib.get_fn(hash_script, pub_only) //.or_else(|| lib.get_fn(hash_fn, pub_only)) - } else { - None - } - //.or_else(|| self.global_module.get_fn(hash_script, pub_only)) - .or_else(|| self.global_module.get_fn(hash_fn, pub_only)) - //.or_else(|| self.packages.get_fn(hash_script, pub_only)) - .or_else(|| self.packages.get_fn(hash_fn, pub_only)); + let func = self + .global_module + .get_fn(hash_fn, pub_only) + .or_else(|| self.packages.get_fn(hash_fn, pub_only)); if let Some(func) = func { #[cfg(not(feature = "no_function"))] @@ -166,43 +179,12 @@ impl Engine { #[cfg(feature = "no_function")] let need_normalize = is_ref && func.is_pure(); + let mut this_copy: Dynamic = Default::default(); + let mut old_this_ptr: Option<&mut Dynamic> = None; + // Calling pure function but the first argument is a reference? normalize_first_arg(need_normalize, &mut this_copy, &mut old_this_ptr, args); - #[cfg(not(feature = "no_function"))] - if func.is_script() { - // Run scripted function - let fn_def = func.get_fn_def(); - - // Method call of script function - map first argument to `this` - return if _is_method { - let (first, rest) = args.split_at_mut(1); - Ok(( - self.call_script_fn( - _scope, - _mods, - state, - lib, - &mut Some(first[0]), - fn_name, - fn_def, - rest, - _level, - )?, - false, - )) - } else { - let result = self.call_script_fn( - _scope, _mods, state, lib, &mut None, fn_name, fn_def, args, _level, - )?; - - // Restore the original reference - restore_first_arg(old_this_ptr, args); - - Ok((result, false)) - }; - } - // Run external function let result = func.get_native_fn()(self, lib, args)?; @@ -414,24 +396,23 @@ impl Engine { state: &mut State, lib: &Module, fn_name: &str, - native_only: bool, hash_script: u64, args: &mut FnCallArgs, is_ref: bool, is_method: bool, pub_only: bool, + capture: Option, def_val: Option, level: usize, ) -> Result<(Dynamic, bool), Box> { // Qualifiers (none) + function name + number of arguments + argument `TypeId`'s. let arg_types = args.iter().map(|a| a.type_id()); let hash_fn = calc_fn_hash(empty(), fn_name, args.len(), arg_types); - let hashes = (hash_fn, if native_only { 0 } else { hash_script }); match fn_name { // type_of KEYWORD_TYPE_OF - if args.len() == 1 && !self.has_override(lib, hashes.0, hashes.1, pub_only) => + if args.len() == 1 && !self.has_override(lib, hash_fn, hash_script, pub_only) => { Ok(( self.map_type_name(args[0].type_name()).to_string().into(), @@ -441,7 +422,7 @@ impl Engine { // Fn KEYWORD_FN_PTR - if args.len() == 1 && !self.has_override(lib, hashes.0, hashes.1, pub_only) => + if args.len() == 1 && !self.has_override(lib, hash_fn, hash_script, pub_only) => { Err(Box::new(EvalAltResult::ErrorRuntime( "'Fn' should not be called in method style. Try Fn(...);".into(), @@ -451,7 +432,7 @@ impl Engine { // eval - reaching this point it must be a method-style call KEYWORD_EVAL - if args.len() == 1 && !self.has_override(lib, hashes.0, hashes.1, pub_only) => + if args.len() == 1 && !self.has_override(lib, hash_fn, hash_script, pub_only) => { Err(Box::new(EvalAltResult::ErrorRuntime( "'eval' should not be called in method style. Try eval(...);".into(), @@ -459,12 +440,61 @@ impl Engine { ))) } - // Normal function call + // Normal script function call + #[cfg(not(feature = "no_function"))] + _ if hash_script > 0 && lib.contains_fn(hash_script, pub_only) => { + // Get scripted function + let func = lib.get_fn(hash_script, pub_only).unwrap().get_fn_def(); + + let scope = &mut Scope::new(); + let mods = &mut Imports::new(); + + // Add captured variables into scope + #[cfg(not(feature = "no_capture"))] + if let Some(captured) = &capture { + add_captured_variables_into_scope(&func.externals, captured, scope); + } + + let result = if is_method { + // Method call of script function - map first argument to `this` + let (first, rest) = args.split_at_mut(1); + self.call_script_fn( + scope, + mods, + state, + lib, + &mut Some(first[0]), + fn_name, + func, + rest, + level, + )? + } else { + // Normal call of script function - map first argument to `this` + let mut first_copy: Dynamic = Default::default(); + let mut old_first: Option<&mut Dynamic> = None; + + // The first argument is a reference? + normalize_first_arg(is_ref, &mut first_copy, &mut old_first, args); + + let result = self.call_script_fn( + scope, mods, state, lib, &mut None, fn_name, func, args, level, + )?; + + // Restore the original reference + restore_first_arg(old_first, args); + + result + }; + + Ok((result, false)) + } + // Normal native function call _ => { let mut scope = Scope::new(); let mut mods = Imports::new(); self.call_fn_raw( - &mut scope, &mut mods, state, lib, fn_name, hashes, args, is_ref, is_method, + &mut scope, &mut mods, state, lib, fn_name, hash_fn, args, is_ref, is_method, pub_only, def_val, level, ) } @@ -522,7 +552,7 @@ impl Engine { state: &mut State, lib: &Module, name: &str, - hash: u64, + hash_script: u64, target: &mut Target, idx_val: Dynamic, def_val: Option, @@ -545,7 +575,11 @@ impl Engine { // Redirect function name let fn_name = fn_ptr.fn_name(); // Recalculate hash - let hash = calc_fn_hash(empty(), fn_name, curry.len() + idx.len(), empty()); + let hash = if native { + 0 + } else { + calc_fn_hash(empty(), fn_name, curry.len() + idx.len(), empty()) + }; // Arguments are passed as-is, adding the curried arguments let mut arg_values = curry .iter_mut() @@ -555,7 +589,7 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - state, lib, fn_name, native, hash, args, false, false, pub_only, def_val, level, + state, lib, fn_name, hash, args, false, false, pub_only, None, def_val, level, ) } else if _fn_name == KEYWORD_FN_PTR_CALL && idx.len() > 0 && idx[0].is::() { // FnPtr call on object @@ -564,7 +598,11 @@ impl Engine { // Redirect function name let fn_name = fn_ptr.get_fn_name().clone(); // Recalculate hash - let hash = calc_fn_hash(empty(), &fn_name, curry.len() + idx.len(), empty()); + let hash = if native { + 0 + } else { + calc_fn_hash(empty(), &fn_name, curry.len() + idx.len(), empty()) + }; // Replace the first argument with the object pointer, adding the curried arguments let mut arg_values = once(obj) .chain(curry.iter_mut()) @@ -574,7 +612,7 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - state, lib, &fn_name, native, hash, args, is_ref, true, pub_only, def_val, level, + state, lib, &fn_name, hash, args, is_ref, true, pub_only, None, def_val, level, ) } else if _fn_name == KEYWORD_FN_PTR_CURRY && obj.is::() { // Curry call @@ -595,7 +633,7 @@ impl Engine { } else { #[cfg(not(feature = "no_object"))] let redirected; - let mut _hash = hash; + let mut _hash = hash_script; // Check if it is a map method call in OOP style #[cfg(not(feature = "no_object"))] @@ -611,12 +649,16 @@ impl Engine { } }; + if native { + _hash = 0; + } + // Attached object pointer in front of the arguments let mut arg_values = once(obj).chain(idx.iter_mut()).collect::>(); let args = arg_values.as_mut(); self.exec_fn_call( - state, lib, _fn_name, native, _hash, args, is_ref, true, pub_only, def_val, level, + state, lib, _fn_name, _hash, args, is_ref, true, pub_only, None, def_val, level, ) }?; @@ -641,16 +683,17 @@ impl Engine { name: &str, args_expr: &[Expr], def_val: Option, - mut hash: u64, + mut hash_script: u64, native: bool, pub_only: bool, + capture: bool, level: usize, ) -> Result> { // Handle Fn() if name == KEYWORD_FN_PTR && args_expr.len() == 1 { let hash_fn = calc_fn_hash(empty(), name, 1, once(TypeId::of::())); - if !self.has_override(lib, hash_fn, hash, pub_only) { + if !self.has_override(lib, hash_fn, hash_script, pub_only) { // Fn - only in function call style let expr = args_expr.get(0).unwrap(); let arg_value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; @@ -698,11 +741,43 @@ impl Engine { .into()); } + // Handle call() - Redirect function call + let redirected; + let mut args_expr = args_expr.as_ref(); + let mut curry: StaticVec<_> = Default::default(); + let mut name = name; + + if name == KEYWORD_FN_PTR_CALL + && args_expr.len() >= 1 + && !self.has_override(lib, 0, hash_script, pub_only) + { + let expr = args_expr.get(0).unwrap(); + let fn_name = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + + if fn_name.is::() { + let fn_ptr = fn_name.cast::(); + curry = fn_ptr.curry().iter().cloned().collect(); + // Redirect function name + redirected = fn_ptr.take_data().0; + name = &redirected; + // Skip the first argument + args_expr = &args_expr.as_ref()[1..]; + // Recalculate hash + hash_script = calc_fn_hash(empty(), name, curry.len() + args_expr.len(), empty()); + } else { + return Err(Box::new(EvalAltResult::ErrorMismatchOutputType( + self.map_type_name(type_name::()).into(), + fn_name.type_name().into(), + expr.position(), + ))); + } + } + // Handle eval() if name == KEYWORD_EVAL && args_expr.len() == 1 { let hash_fn = calc_fn_hash(empty(), name, 1, once(TypeId::of::())); - if !self.has_override(lib, hash_fn, hash, pub_only) { + if !self.has_override(lib, hash_fn, hash_script, pub_only) { // eval - only in function call style let prev_len = scope.len(); let expr = args_expr.get(0).unwrap(); @@ -721,42 +796,15 @@ impl Engine { } } - // Handle call() - Redirect function call - let redirected; - let mut args_expr = args_expr.as_ref(); - let mut curry: StaticVec<_> = Default::default(); - let mut name = name; - - if name == KEYWORD_FN_PTR_CALL - && args_expr.len() >= 1 - && !self.has_override(lib, 0, hash, pub_only) - { - let expr = args_expr.get(0).unwrap(); - let fn_name = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; - - if fn_name.is::() { - let fn_ptr = fn_name.cast::(); - curry = fn_ptr.curry().iter().cloned().collect(); - // Redirect function name - redirected = fn_ptr.take_data().0; - name = &redirected; - // Skip the first argument - args_expr = &args_expr.as_ref()[1..]; - // Recalculate hash - hash = calc_fn_hash(empty(), name, curry.len() + args_expr.len(), empty()); - } else { - return Err(Box::new(EvalAltResult::ErrorMismatchOutputType( - self.map_type_name(type_name::()).into(), - fn_name.type_name().into(), - expr.position(), - ))); - } - } - - // Normal function call - except for Fn and eval (handled above) + // Normal function call - except for Fn, curry, call and eval (handled above) let mut arg_values: StaticVec<_>; let mut args: StaticVec<_>; let mut is_ref = false; + let capture = if capture && !scope.is_empty() { + Some(scope.clone()) + } else { + None + }; if args_expr.is_empty() && curry.is_empty() { // No arguments @@ -797,9 +845,11 @@ impl Engine { } } + let hash = if native { 0 } else { hash_script }; let args = args.as_mut(); + self.exec_fn_call( - state, lib, name, native, hash, args, is_ref, false, pub_only, def_val, level, + state, lib, name, hash, args, is_ref, false, pub_only, capture, def_val, level, ) .map(|(v, _)| v) } @@ -818,10 +868,18 @@ impl Engine { args_expr: &[Expr], def_val: Option, hash_script: u64, + capture: bool, level: usize, ) -> Result> { let modules = modules.as_ref().unwrap(); + #[cfg(not(feature = "no_capture"))] + let capture = if capture && !scope.is_empty() { + Some(scope.clone()) + } else { + None + }; + let mut arg_values: StaticVec<_>; let mut args: StaticVec<_>; @@ -887,12 +945,18 @@ impl Engine { #[cfg(not(feature = "no_function"))] Some(f) if f.is_script() => { let args = args.as_mut(); - let fn_def = f.get_fn_def(); - let mut scope = Scope::new(); - let mut mods = Imports::new(); - self.call_script_fn( - &mut scope, &mut mods, state, lib, &mut None, name, fn_def, args, level, - ) + let func = f.get_fn_def(); + + let scope = &mut Scope::new(); + let mods = &mut Imports::new(); + + // Add captured variables into scope + #[cfg(not(feature = "no_capture"))] + if let Some(captured) = &capture { + add_captured_variables_into_scope(&func.externals, captured, scope); + } + + self.call_script_fn(scope, mods, state, lib, &mut None, name, func, args, level) } Some(f) => f.get_native_fn()(self, lib, args.as_mut()), None if def_val.is_some() => Ok(def_val.unwrap().into()), diff --git a/src/fn_native.rs b/src/fn_native.rs index 5751351c..f6bb962e 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -130,13 +130,13 @@ impl FnPtr { &mut Default::default(), lib.as_ref(), fn_name, - false, hash_script, args.as_mut(), has_this, has_this, true, None, + None, 0, ) .map(|(v, _)| v) diff --git a/src/optimize.rs b/src/optimize.rs index e26d1908..7c71aced 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -138,7 +138,7 @@ fn call_fn_with_constant_arguments( &mut Default::default(), state.lib, fn_name, - (hash_fn, 0), + hash_fn, arg_values.iter_mut().collect::>().as_mut(), false, false, @@ -576,7 +576,7 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { && state.optimization_level == OptimizationLevel::Full // full optimizations && x.3.iter().all(|expr| expr.is_constant()) // all arguments are constants => { - let ((name, _, pos), _, _, args, def_value) = x.as_mut(); + let ((name, _, _, pos), _, _, args, def_value) = x.as_mut(); // First search in functions lib (can override built-in) // Cater for both normal function call style and method call style (one additional arguments) diff --git a/src/parser.rs b/src/parser.rs index abe3c7d8..c8f4e314 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -748,12 +748,12 @@ pub enum Expr { Stmt(Box<(Stmt, Position)>), /// Wrapped expression - should not be optimized away. Expr(Box), - /// func(expr, ... ) - ((function name, native_only, position), optional modules, hash, arguments, optional default value) + /// func(expr, ... ) - ((function name, native_only, capture, position), optional modules, hash, arguments, optional default value) /// Use `Cow<'static, str>` because a lot of operators (e.g. `==`, `>=`) are implemented as function calls /// and the function names are predictable, so no need to allocate a new `String`. FnCall( Box<( - (Cow<'static, str>, bool, Position), + (Cow<'static, str>, bool, bool, Position), Option>, u64, StaticVec, @@ -871,7 +871,7 @@ impl Expr { Self::Property(x) => x.1, Self::Stmt(x) => x.1, Self::Variable(x) => (x.0).1, - Self::FnCall(x) => (x.0).2, + Self::FnCall(x) => (x.0).3, Self::Assignment(x) => x.0.position(), Self::And(x) | Self::Or(x) | Self::In(x) => x.2, @@ -903,7 +903,7 @@ impl Expr { Self::Variable(x) => (x.0).1 = new_pos, Self::Property(x) => x.1 = new_pos, Self::Stmt(x) => x.1 = new_pos, - Self::FnCall(x) => (x.0).2 = new_pos, + Self::FnCall(x) => (x.0).3 = new_pos, Self::And(x) => x.2 = new_pos, Self::Or(x) => x.2 = new_pos, Self::In(x) => x.2 = new_pos, @@ -1009,6 +1009,7 @@ impl Expr { #[cfg(not(feature = "no_index"))] Token::LeftBracket => true, Token::LeftParen => true, + Token::Bang => true, Token::DoubleColon => true, _ => false, }, @@ -1101,6 +1102,7 @@ fn parse_fn_call( state: &mut ParseState, lib: &mut FunctionsLib, id: String, + capture: bool, mut modules: Option>, settings: ParseSettings, ) -> Result { @@ -1143,7 +1145,7 @@ fn parse_fn_call( }; return Ok(Expr::FnCall(Box::new(( - (id.into(), false, settings.pos), + (id.into(), false, capture, settings.pos), modules, hash_script, args, @@ -1185,7 +1187,7 @@ fn parse_fn_call( }; return Ok(Expr::FnCall(Box::new(( - (id.into(), false, settings.pos), + (id.into(), false, capture, settings.pos), modules, hash_script, args, @@ -1594,6 +1596,8 @@ fn parse_primary( _ => input.next().unwrap(), }; + let (next_token, _) = input.peek().unwrap(); + let mut root_expr = match token { Token::IntegerConstant(x) => Expr::IntegerConstant(Box::new((x, settings.pos))), #[cfg(not(feature = "no_float"))] @@ -1605,7 +1609,7 @@ fn parse_primary( Expr::Variable(Box::new(((s, settings.pos), None, 0, index))) } // Function call is allowed to have reserved keyword - Token::Reserved(s) if input.peek().unwrap().0 == Token::LeftParen => { + Token::Reserved(s) if *next_token == Token::LeftParen || *next_token == Token::Bang => { if is_keyword_function(&s) { Expr::Variable(Box::new(((s, settings.pos), None, 0, None))) } else { @@ -1613,7 +1617,7 @@ fn parse_primary( } } // Access to `this` as a variable is OK - Token::Reserved(s) if s == KEYWORD_THIS && input.peek().unwrap().0 != Token::LeftParen => { + Token::Reserved(s) if s == KEYWORD_THIS && *next_token != Token::LeftParen => { if !settings.is_function_scope { return Err( PERR::BadInput(format!("'{}' can only be used in functions", s)) @@ -1653,11 +1657,26 @@ fn parse_primary( settings.pos = token_pos; root_expr = match (root_expr, token) { + // Function call + #[cfg(not(feature = "no_capture"))] + (Expr::Variable(x), Token::Bang) => { + if !match_token(input, Token::LeftParen)? { + return Err(PERR::MissingToken( + Token::LeftParen.syntax().into(), + "to start arguments list of function call".into(), + ) + .into_err(input.peek().unwrap().1)); + } + + let ((name, pos), modules, _, _) = *x; + settings.pos = pos; + parse_fn_call(input, state, lib, name, true, modules, settings.level_up())? + } // Function call (Expr::Variable(x), Token::LeftParen) => { let ((name, pos), modules, _, _) = *x; settings.pos = pos; - parse_fn_call(input, state, lib, name, modules, settings.level_up())? + parse_fn_call(input, state, lib, name, false, modules, settings.level_up())? } (Expr::Property(_), _) => unreachable!(), // module access @@ -1767,7 +1786,7 @@ fn parse_unary( args.push(expr); Ok(Expr::FnCall(Box::new(( - (op.into(), true, pos), + (op.into(), true, false, pos), None, hash, args, @@ -1792,7 +1811,7 @@ fn parse_unary( let hash = calc_fn_hash(empty(), op, 2, empty()); Ok(Expr::FnCall(Box::new(( - (op.into(), true, pos), + (op.into(), true, false, pos), None, hash, args, @@ -1987,7 +2006,14 @@ fn make_dot_expr(lhs: Expr, rhs: Expr, op_pos: Position) -> Result { + return Err(PERR::MalformedCapture( + "method-call style does not support capturing".into(), + ) + .into_err((x.0).3)) + } + // lhs.func(...) (lhs, func @ Expr::FnCall(_)) => Expr::Dot(Box::new((lhs, func, op_pos))), // lhs.rhs (_, rhs) => return Err(PERR::PropertyExpected.into_err(rhs.position())), @@ -2196,7 +2222,7 @@ fn parse_binary_op( let cmp_def = Some(false); let op = op_token.syntax(); let hash = calc_fn_hash(empty(), &op, 2, empty()); - let op = (op, true, pos); + let op = (op, true, false, pos); let mut args = StaticVec::new(); args.push(root); @@ -2257,7 +2283,7 @@ fn parse_binary_op( .unwrap_or(false) => { // Accept non-native functions for custom operators - let op = (op.0, false, op.2); + let op = (op.0, false, op.2, op.3); Expr::FnCall(Box::new((op, None, hash, args, None))) } @@ -2975,10 +3001,12 @@ fn parse_fn( let (token, pos) = input.next().unwrap(); - let name = token.into_function_name().map_err(|t| match t { - Token::Reserved(s) => PERR::Reserved(s).into_err(pos), - _ => PERR::FnMissingName.into_err(pos), - })?; + let name = token + .into_function_name_for_override() + .map_err(|t| match t { + Token::Reserved(s) => PERR::Reserved(s).into_err(pos), + _ => PERR::FnMissingName.into_err(pos), + })?; match input.peek().unwrap() { (Token::LeftParen, _) => eat_token(input, Token::LeftParen), @@ -3085,7 +3113,7 @@ fn make_curry_from_externals( let hash = calc_fn_hash(empty(), KEYWORD_FN_PTR_CURRY, num_externals, empty()); let fn_call = Expr::FnCall(Box::new(( - (KEYWORD_FN_PTR_CURRY.into(), false, pos), + (KEYWORD_FN_PTR_CURRY.into(), false, false, pos), None, hash, args, diff --git a/src/scope.rs b/src/scope.rs index 5693554c..3e43b850 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -316,6 +316,14 @@ impl<'a> Scope<'a> { }) } + /// Get an entry in the Scope, starting from the last. + pub(crate) fn get_entry(&self, name: &str) -> Option<&Entry> { + self.0 + .iter() + .rev() + .find(|Entry { name: key, .. }| name == key) + } + /// Get the value of an entry in the Scope, starting from the last. /// /// # Examples @@ -329,10 +337,7 @@ impl<'a> Scope<'a> { /// assert_eq!(my_scope.get_value::("x").unwrap(), 42); /// ``` pub fn get_value(&self, name: &str) -> Option { - self.0 - .iter() - .rev() - .find(|Entry { name: key, .. }| name == key) + self.get_entry(name) .and_then(|Entry { value, .. }| value.downcast_ref::().cloned()) } diff --git a/src/token.rs b/src/token.rs index 7d1a07ba..a35ff4f6 100644 --- a/src/token.rs +++ b/src/token.rs @@ -678,9 +678,9 @@ impl Token { } /// Convert a token into a function name, if possible. - pub(crate) fn into_function_name(self) -> Result { + pub(crate) fn into_function_name_for_override(self) -> Result { match self { - Self::Reserved(s) if is_keyword_function(&s) => Ok(s), + Self::Reserved(s) if can_override_keyword(&s) => Ok(s), Self::Custom(s) | Self::Identifier(s) if is_valid_identifier(s.chars()) => Ok(s), _ => Err(self), } @@ -1439,6 +1439,16 @@ pub fn is_keyword_function(name: &str) -> bool { || name == KEYWORD_FN_PTR_CURRY } +/// Can this keyword be overridden as a function? +#[inline(always)] +pub fn can_override_keyword(name: &str) -> bool { + name == KEYWORD_PRINT + || name == KEYWORD_DEBUG + || name == KEYWORD_TYPE_OF + || name == KEYWORD_EVAL + || name == KEYWORD_FN_PTR +} + pub fn is_valid_identifier(name: impl Iterator) -> bool { let mut first_alphabetic = false; diff --git a/tests/functions.rs b/tests/functions.rs index 78d20fa3..9336cb6d 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -1,5 +1,5 @@ #![cfg(not(feature = "no_function"))] -use rhai::{Engine, EvalAltResult, INT}; +use rhai::{Engine, EvalAltResult, ParseError, ParseErrorType, INT}; #[test] fn test_functions() -> Result<(), Box> { @@ -120,3 +120,53 @@ fn test_function_pointers() -> Result<(), Box> { Ok(()) } + +#[test] +#[cfg(not(feature = "no_capture"))] +fn test_function_captures() -> Result<(), Box> { + let engine = Engine::new(); + + assert_eq!( + engine.eval::( + r#" + fn foo(y) { x += y; x } + + let x = 41; + let y = 999; + + foo!(1) + x + "# + )?, + 83 + ); + + assert!(engine + .eval::( + r#" + fn foo(y) { x += y; x } + + let x = 41; + let y = 999; + + foo(1) + x + "# + ) + .is_err()); + + #[cfg(not(feature = "no_object"))] + assert!(matches!( + engine.compile( + r#" + fn foo() { this += x; } + + let x = 41; + let y = 999; + + y.foo!(); + "# + ).expect_err("should error"), + ParseError(err, _) if matches!(*err, ParseErrorType::MalformedCapture(_)) + )); + + Ok(()) +} From 7d4620d0d9cf4f8ff1349dc08bd5b476121f80c8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 30 Jul 2020 23:29:11 +0800 Subject: [PATCH 4/7] Unbounded -> unbound. --- doc/src/language/functions.md | 2 +- src/engine.rs | 4 ++-- src/result.rs | 12 ++++++------ tests/fn_ptr.rs | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/doc/src/language/functions.md b/doc/src/language/functions.md index d8f67502..3bd15cd0 100644 --- a/doc/src/language/functions.md +++ b/doc/src/language/functions.md @@ -127,5 +127,5 @@ x.change(); // call 'change' in method-call style, 'this' binds to 'x' x == 42; // 'x' is changed! -change(); // <- error: `this` is unbounded +change(); // <- error: `this` is unbound ``` diff --git a/src/engine.rs b/src/engine.rs index ac264af4..6aa1f9c2 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -556,7 +556,7 @@ pub fn search_scope_only<'s, 'a>( if let Some(val) = this_ptr { return Ok(((*val).into(), KEYWORD_THIS, ScopeEntryType::Normal, *pos)); } else { - return Err(Box::new(EvalAltResult::ErrorUnboundedThis(*pos))); + return Err(Box::new(EvalAltResult::ErrorUnboundThis(*pos))); } } @@ -1247,7 +1247,7 @@ impl Engine { if let Some(val) = this_ptr { Ok(val.clone()) } else { - Err(Box::new(EvalAltResult::ErrorUnboundedThis((x.0).1))) + Err(Box::new(EvalAltResult::ErrorUnboundThis((x.0).1))) } } Expr::Variable(_) => { diff --git a/src/result.rs b/src/result.rs index 550dcc37..c0cc620d 100644 --- a/src/result.rs +++ b/src/result.rs @@ -39,8 +39,8 @@ pub enum EvalAltResult { /// An error has occurred inside a called function. /// Wrapped values are the name of the function and the interior error. ErrorInFunctionCall(String, Box, Position), - /// Access to `this` that is not bounded. - ErrorUnboundedThis(Position), + /// Access to `this` that is not bound. + ErrorUnboundThis(Position), /// Non-boolean operand encountered for boolean operator. Wrapped value is the operator. ErrorBooleanArgMismatch(String, Position), /// Non-character value encountered where a character is required. @@ -112,7 +112,7 @@ impl EvalAltResult { Self::ErrorParsing(p, _) => p.desc(), Self::ErrorInFunctionCall(_, _, _) => "Error in called function", Self::ErrorFunctionNotFound(_, _) => "Function not found", - Self::ErrorUnboundedThis(_) => "'this' is not bounded", + Self::ErrorUnboundThis(_) => "'this' is not bound", Self::ErrorBooleanArgMismatch(_, _) => "Boolean operator expects boolean operands", Self::ErrorCharMismatch(_) => "Character expected", Self::ErrorNumericIndexExpr(_) => { @@ -187,7 +187,7 @@ impl fmt::Display for EvalAltResult { Self::ErrorIndexingType(_, _) | Self::ErrorNumericIndexExpr(_) | Self::ErrorStringIndexExpr(_) - | Self::ErrorUnboundedThis(_) + | Self::ErrorUnboundThis(_) | Self::ErrorImportExpr(_) | Self::ErrorLogicGuard(_) | Self::ErrorFor(_) @@ -276,7 +276,7 @@ impl EvalAltResult { Self::ErrorParsing(_, pos) | Self::ErrorFunctionNotFound(_, pos) | Self::ErrorInFunctionCall(_, _, pos) - | Self::ErrorUnboundedThis(pos) + | Self::ErrorUnboundThis(pos) | Self::ErrorBooleanArgMismatch(_, pos) | Self::ErrorCharMismatch(pos) | Self::ErrorArrayBounds(_, _, pos) @@ -316,7 +316,7 @@ impl EvalAltResult { Self::ErrorParsing(_, pos) | Self::ErrorFunctionNotFound(_, pos) | Self::ErrorInFunctionCall(_, _, pos) - | Self::ErrorUnboundedThis(pos) + | Self::ErrorUnboundThis(pos) | Self::ErrorBooleanArgMismatch(_, pos) | Self::ErrorCharMismatch(pos) | Self::ErrorArrayBounds(_, _, pos) diff --git a/tests/fn_ptr.rs b/tests/fn_ptr.rs index 833bb20c..c6658f17 100644 --- a/tests/fn_ptr.rs +++ b/tests/fn_ptr.rs @@ -73,7 +73,7 @@ fn test_fn_ptr() -> Result<(), Box> { "# ) .expect_err("should error"), - EvalAltResult::ErrorInFunctionCall(fn_name, err, _) if fn_name == "foo" && matches!(*err, EvalAltResult::ErrorUnboundedThis(_)) + EvalAltResult::ErrorInFunctionCall(fn_name, err, _) if fn_name == "foo" && matches!(*err, EvalAltResult::ErrorUnboundThis(_)) )); Ok(()) From a7ff20763659e055568e54d70a8a9ca4440c936d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 30 Jul 2020 23:29:30 +0800 Subject: [PATCH 5/7] Use Scope::flatten_clone for capturing. --- src/fn_call.rs | 25 ++++++++++++------------- src/parser.rs | 6 +++--- src/scope.rs | 21 +++++++++++++++++++-- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index ca1aef51..6672d746 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -36,6 +36,7 @@ use crate::engine::{Map, Target, FN_GET, FN_SET}; use crate::stdlib::{ any::{type_name, TypeId}, boxed::Box, + collections::HashSet, convert::TryFrom, format, iter::{empty, once}, @@ -109,22 +110,20 @@ fn restore_first_arg<'a>(old_this_ptr: Option<&'a mut Dynamic>, args: &mut FnCal // Add captured variables into scope #[cfg(not(feature = "no_capture"))] fn add_captured_variables_into_scope<'s>( - externals: &[String], - captured: &'s Scope<'s>, + externals: &HashSet, + captured: Scope<'s>, scope: &mut Scope<'s>, ) { - externals - .iter() - .map(|var_name| captured.get_entry(var_name)) - .filter(Option::is_some) - .map(Option::unwrap) + captured + .into_iter() + .filter(|ScopeEntry { name, .. }| externals.contains(name.as_ref())) .for_each( |ScopeEntry { name, typ, value, .. }| { match typ { - ScopeEntryType::Normal => scope.push(name.clone(), value.clone()), - ScopeEntryType::Constant => scope.push_constant(name.clone(), value.clone()), + ScopeEntryType::Normal => scope.push(name, value), + ScopeEntryType::Constant => scope.push_constant(name, value), }; }, ); @@ -451,7 +450,7 @@ impl Engine { // Add captured variables into scope #[cfg(not(feature = "no_capture"))] - if let Some(captured) = &capture { + if let Some(captured) = capture { add_captured_variables_into_scope(&func.externals, captured, scope); } @@ -801,7 +800,7 @@ impl Engine { let mut args: StaticVec<_>; let mut is_ref = false; let capture = if capture && !scope.is_empty() { - Some(scope.clone()) + Some(scope.flatten_clone()) } else { None }; @@ -875,7 +874,7 @@ impl Engine { #[cfg(not(feature = "no_capture"))] let capture = if capture && !scope.is_empty() { - Some(scope.clone()) + Some(scope.flatten_clone()) } else { None }; @@ -952,7 +951,7 @@ impl Engine { // Add captured variables into scope #[cfg(not(feature = "no_capture"))] - if let Some(captured) = &capture { + if let Some(captured) = capture { add_captured_variables_into_scope(&func.externals, captured, scope); } diff --git a/src/parser.rs b/src/parser.rs index c8f4e314..3fd6365b 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -25,7 +25,7 @@ use crate::stdlib::{ borrow::Cow, boxed::Box, char, - collections::HashMap, + collections::{HashMap, HashSet}, fmt, format, hash::{Hash, Hasher}, iter::empty, @@ -355,7 +355,7 @@ impl fmt::Display for FnAccess { /// ## WARNING /// /// This type is volatile and may change. -#[derive(Debug, Clone, Hash)] +#[derive(Debug, Clone)] pub struct ScriptFnDef { /// Function name. pub name: ImmutableString, @@ -365,7 +365,7 @@ pub struct ScriptFnDef { pub params: StaticVec, /// Access to external variables. #[cfg(not(feature = "no_capture"))] - pub externals: StaticVec, + pub externals: HashSet, /// Function body. pub body: Stmt, /// Position of the function definition. diff --git a/src/scope.rs b/src/scope.rs index 3e43b850..b2024564 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -4,7 +4,9 @@ use crate::any::{Dynamic, Variant}; use crate::parser::{map_dynamic_to_expr, Expr}; use crate::token::Position; -use crate::stdlib::{borrow::Cow, boxed::Box, iter, string::String, vec::Vec}; +use crate::stdlib::{ + borrow::Cow, boxed::Box, collections::HashMap, iter, string::String, vec::Vec, +}; /// Type of an entry in the Scope. #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -389,13 +391,28 @@ impl<'a> Scope<'a> { self } + /// Clone the Scope, keeping only the last instances of each variable name. + /// Shadowed variables are omitted in the copy. + #[cfg(not(feature = "no_capture"))] + pub(crate) fn flatten_clone(&self) -> Self { + let mut entries: HashMap<&str, Entry> = Default::default(); + + self.0.iter().rev().for_each(|entry| { + entries + .entry(entry.name.as_ref()) + .or_insert_with(|| entry.clone()); + }); + + Self(entries.into_iter().map(|(_, v)| v).collect()) + } + /// Get an iterator to entries in the Scope. #[cfg(not(feature = "no_module"))] pub(crate) fn into_iter(self) -> impl Iterator> { self.0.into_iter() } - /// Get an iterator to entries in the Scope. + /// Get an iterator to entries in the Scope in reverse order. pub(crate) fn to_iter(&self) -> impl Iterator { self.0.iter().rev() // Always search a Scope in reverse order } From cb005506e2f3984dc3e8f5c0f225f3d0ec25ab0b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 31 Jul 2020 12:11:16 +0800 Subject: [PATCH 6/7] Simplify function calling. --- src/engine.rs | 15 ++-- src/fn_call.rs | 192 +++++++++++++++++++++++++++-------------------- src/fn_native.rs | 10 +++ src/optimize.rs | 12 +-- 4 files changed, 130 insertions(+), 99 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 6aa1f9c2..dc916876 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1179,7 +1179,6 @@ impl Engine { #[cfg(not(feature = "no_index"))] Dynamic(Union::Array(mut rhs_value)) => { let op = "=="; - let mut scope = Scope::new(); // Call the `==` operator to compare each value for value in rhs_value.iter_mut() { @@ -1190,13 +1189,13 @@ impl Engine { let hash = calc_fn_hash(empty(), op, args.len(), args.iter().map(|a| a.type_id())); - let (r, _) = self - .call_fn_raw( - &mut scope, mods, state, lib, op, hash, args, false, false, false, - def_value, level, - ) - .map_err(|err| err.new_position(rhs.position()))?; - if r.as_bool().unwrap_or(false) { + if self + .call_native_fn(state, lib, op, hash, args, false, false, def_value) + .map_err(|err| err.new_position(rhs.position()))? + .0 + .as_bool() + .unwrap_or(false) + { return Ok(true.into()); } } diff --git a/src/fn_call.rs b/src/fn_call.rs index 6672d746..91c8e67b 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -41,7 +41,7 @@ use crate::stdlib::{ format, iter::{empty, once}, mem, - string::ToString, + string::{String, ToString}, vec::Vec, }; @@ -67,43 +67,69 @@ fn extract_prop_from_setter(_fn_name: &str) -> Option<&str> { None } -/// This function replaces the first argument of a method call with a clone copy. -/// This is to prevent a pure function unintentionally consuming the first argument. -fn normalize_first_arg<'a>( - normalize: bool, - this_copy: &mut Dynamic, - old_this_ptr: &mut Option<&'a mut Dynamic>, - args: &mut FnCallArgs<'a>, -) { - // Only do it for method calls with arguments. - if !normalize || args.is_empty() { - return; - } - - // Clone the original value. - *this_copy = args[0].clone(); - - // Replace the first reference with a reference to the clone, force-casting the lifetime. - // Keep the original reference. Must remember to restore it later with `restore_first_arg_of_method_call`. - // - // # Safety - // - // Blindly casting a a reference to another lifetime saves on allocations and string cloning, - // but must be used with the utmost care. - // - // We can do this here because, at the end of this scope, we'd restore the original reference - // with `restore_first_arg_of_method_call`. Therefore this shorter lifetime does not get "out". - let this_pointer = mem::replace(args.get_mut(0).unwrap(), unsafe { - mem::transmute(this_copy) - }); - - *old_this_ptr = Some(this_pointer); +/// A type that temporarily stores a mutable reference to a `Dynamic`, +/// replacing it with a cloned copy. +#[derive(Debug, Default)] +struct ArgBackup<'a> { + orig_mut: Option<&'a mut Dynamic>, + value_copy: Dynamic, } -/// This function restores the first argument that was replaced by `normalize_first_arg_of_method_call`. -fn restore_first_arg<'a>(old_this_ptr: Option<&'a mut Dynamic>, args: &mut FnCallArgs<'a>) { - if let Some(this_pointer) = old_this_ptr { - args[0] = this_pointer; +impl<'a> ArgBackup<'a> { + /// This function replaces the first argument of a method call with a clone copy. + /// This is to prevent a pure function unintentionally consuming the first argument. + /// + /// `restore_first_arg` must be called before the end of the scope to prevent the shorter lifetime from leaking. + /// + /// # Safety + /// + /// This method blindly casts a reference to another lifetime, which saves allocation and string cloning. + /// + /// If `restore_first_arg` is called before the end of the scope, the shorter lifetime will not leak. + fn change_first_arg_to_copy(&mut self, normalize: bool, args: &mut FnCallArgs<'a>) { + // Only do it for method calls with arguments. + if !normalize || args.is_empty() { + return; + } + + // Clone the original value. + self.value_copy = args[0].clone(); + + // Replace the first reference with a reference to the clone, force-casting the lifetime. + // Must remember to restore it later with `restore_first_arg`. + // + // # Safety + // + // Blindly casting a reference to another lifetime saves allocation and string cloning, + // but must be used with the utmost care. + // + // We can do this here because, before the end of this scope, we'd restore the original reference + // via `restore_first_arg`. Therefore this shorter lifetime does not leak. + self.orig_mut = Some(mem::replace(args.get_mut(0).unwrap(), unsafe { + mem::transmute(&mut self.value_copy) + })); + } + + /// This function restores the first argument that was replaced by `change_first_arg_to_copy`. + /// + /// # Safety + /// + /// If `change_first_arg_to_copy` has been called, this function **MUST** be called _BEFORE_ exiting + /// the current scope. Otherwise it is undefined behavior as the shorter lifetime will leak. + fn restore_first_arg(&mut self, args: &mut FnCallArgs<'a>) { + if let Some(this_pointer) = self.orig_mut.take() { + args[0] = this_pointer; + } + } +} + +impl Drop for ArgBackup<'_> { + fn drop(&mut self) { + // Panic if the shorter lifetime leaks. + assert!( + self.orig_mut.is_none(), + "MutBackup::restore has not been called prior to existing this scope" + ); } } @@ -138,34 +164,21 @@ impl Engine { /// Function call arguments be _consumed_ when the function requires them to be passed by value. /// All function arguments not in the first position are always passed by value and thus consumed. /// **DO NOT** reuse the argument values unless for the first `&mut` argument - all others are silently replaced by `()`! - pub(crate) fn call_fn_raw( + pub(crate) fn call_native_fn( &self, - _scope: &mut Scope, - _mods: &mut Imports, state: &mut State, lib: &Module, fn_name: &str, hash_fn: u64, args: &mut FnCallArgs, is_ref: bool, - _is_method: bool, pub_only: bool, def_val: Option, - _level: usize, ) -> Result<(Dynamic, bool), Box> { self.inc_operations(state)?; - // Check for stack overflow - #[cfg(not(feature = "no_function"))] - #[cfg(not(feature = "unchecked"))] - if _level > self.limits.max_call_stack_depth { - return Err(Box::new( - EvalAltResult::ErrorStackOverflow(Position::none()), - )); - } - - // Search for the function - // First search registered native functions (can override packages) + // Search for the native function + // First search registered functions (can override packages) // Then search packages let func = self .global_module @@ -173,22 +186,19 @@ impl Engine { .or_else(|| self.packages.get_fn(hash_fn, pub_only)); if let Some(func) = func { - #[cfg(not(feature = "no_function"))] - let need_normalize = is_ref && (func.is_pure() || (func.is_script() && !_is_method)); - #[cfg(feature = "no_function")] - let need_normalize = is_ref && func.is_pure(); - - let mut this_copy: Dynamic = Default::default(); - let mut old_this_ptr: Option<&mut Dynamic> = None; + assert!(func.is_native()); // Calling pure function but the first argument is a reference? - normalize_first_arg(need_normalize, &mut this_copy, &mut old_this_ptr, args); + let mut backup: ArgBackup = Default::default(); + backup.change_first_arg_to_copy(is_ref && func.is_pure(), args); // Run external function - let result = func.get_native_fn()(self, lib, args)?; + let result = func.get_native_fn()(self, lib, args); // Restore the original reference - restore_first_arg(old_this_ptr, args); + backup.restore_first_arg(args); + + let result = result?; // See if the function match print/debug (which requires special processing) return Ok(match fn_name { @@ -320,6 +330,17 @@ impl Engine { args: &mut FnCallArgs, level: usize, ) -> Result> { + self.inc_operations(state)?; + + // Check for stack overflow + #[cfg(not(feature = "no_function"))] + #[cfg(not(feature = "unchecked"))] + if level > self.limits.max_call_stack_depth { + return Err(Box::new( + EvalAltResult::ErrorStackOverflow(Position::none()), + )); + } + let orig_scope_level = state.scope_level; state.scope_level += 1; @@ -382,7 +403,7 @@ impl Engine { || self.packages.contains_fn(hash_fn, pub_only) } - /// Perform an actual function call, taking care of special functions + /// Perform an actual function call, native Rust or scripted, taking care of special functions. /// Position in `EvalAltResult` is `None` and must be set afterwards. /// /// ## WARNING @@ -470,33 +491,26 @@ impl Engine { )? } else { // Normal call of script function - map first argument to `this` - let mut first_copy: Dynamic = Default::default(); - let mut old_first: Option<&mut Dynamic> = None; - // The first argument is a reference? - normalize_first_arg(is_ref, &mut first_copy, &mut old_first, args); + let mut backup: ArgBackup = Default::default(); + backup.change_first_arg_to_copy(is_ref, args); let result = self.call_script_fn( scope, mods, state, lib, &mut None, fn_name, func, args, level, - )?; + ); // Restore the original reference - restore_first_arg(old_first, args); + backup.restore_first_arg(args); - result + result? }; Ok((result, false)) } // Normal native function call - _ => { - let mut scope = Scope::new(); - let mut mods = Imports::new(); - self.call_fn_raw( - &mut scope, &mut mods, state, lib, fn_name, hash_fn, args, is_ref, is_method, - pub_only, def_val, level, - ) - } + _ => self.call_native_fn( + state, lib, fn_name, hash_fn, args, is_ref, pub_only, def_val, + ), } } @@ -508,9 +522,21 @@ impl Engine { mods: &mut Imports, state: &mut State, lib: &Module, - script: &Dynamic, + script_expr: &Dynamic, + level: usize, ) -> Result> { - let script = script.as_str().map_err(|typ| { + self.inc_operations(state)?; + + // Check for stack overflow + #[cfg(not(feature = "no_function"))] + #[cfg(not(feature = "unchecked"))] + if level > self.limits.max_call_stack_depth { + return Err(Box::new( + EvalAltResult::ErrorStackOverflow(Position::none()), + )); + } + + let script = script_expr.as_str().map_err(|typ| { EvalAltResult::ErrorMismatchOutputType( self.map_type_name(type_name::()).into(), typ.into(), @@ -782,12 +808,12 @@ impl Engine { let expr = args_expr.get(0).unwrap(); let script = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; let result = self - .eval_script_expr(scope, mods, state, lib, &script) + .eval_script_expr(scope, mods, state, lib, &script, level + 1) .map_err(|err| err.new_position(expr.position())); + // IMPORTANT! If the eval defines new variables in the current scope, + // all variable offsets from this point on will be mis-aligned. if scope.len() != prev_len { - // IMPORTANT! If the eval defines new variables in the current scope, - // all variable offsets from this point on will be mis-aligned. state.always_search = true; } diff --git a/src/fn_native.rs b/src/fn_native.rs index f6bb962e..064b82fa 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -280,6 +280,16 @@ impl CallableFunction { Self::Pure(_) | Self::Method(_) | Self::Iterator(_) => false, } } + /// Is this a native Rust function? + pub fn is_native(&self) -> bool { + match self { + Self::Pure(_) | Self::Method(_) => true, + Self::Iterator(_) => true, + + #[cfg(not(feature = "no_function"))] + Self::Script(_) => false, + } + } /// Get the access mode. pub fn access(&self) -> FnAccess { match self { diff --git a/src/optimize.rs b/src/optimize.rs index 7c71aced..3244540b 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -3,7 +3,7 @@ use crate::any::Dynamic; use crate::calc_fn_hash; use crate::engine::{ - Engine, Imports, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT, KEYWORD_TYPE_OF, + Engine, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT, KEYWORD_TYPE_OF, }; use crate::fn_native::FnPtr; use crate::module::Module; @@ -132,22 +132,18 @@ fn call_fn_with_constant_arguments( state .engine - .call_fn_raw( - &mut Scope::new(), - &mut Imports::new(), + .call_native_fn( &mut Default::default(), state.lib, fn_name, hash_fn, arg_values.iter_mut().collect::>().as_mut(), false, - false, true, None, - 0, ) - .map(|(v, _)| Some(v)) - .unwrap_or_else(|_| None) + .ok() + .map(|(v, _)| v) } /// Optimize a statement. From 49392d57d7d3dfe1cbb105c0c8ac0f2b8eee71c0 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 31 Jul 2020 12:40:16 +0800 Subject: [PATCH 7/7] Fix no_std feature. --- src/fn_call.rs | 4 ++-- src/scope.rs | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index 91c8e67b..da029c78 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -523,14 +523,14 @@ impl Engine { state: &mut State, lib: &Module, script_expr: &Dynamic, - level: usize, + _level: usize, ) -> Result> { self.inc_operations(state)?; // Check for stack overflow #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "unchecked"))] - if level > self.limits.max_call_stack_depth { + if _level > self.limits.max_call_stack_depth { return Err(Box::new( EvalAltResult::ErrorStackOverflow(Position::none()), )); diff --git a/src/scope.rs b/src/scope.rs index b2024564..a167a216 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -407,7 +407,6 @@ impl<'a> Scope<'a> { } /// Get an iterator to entries in the Scope. - #[cfg(not(feature = "no_module"))] pub(crate) fn into_iter(self) -> impl Iterator> { self.0.into_iter() }