From a3a167424b0e6e386da3d691a0177d021de1fef4 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 27 Jul 2020 12:52:32 +0800 Subject: [PATCH 1/5] Allow Rust functions in FnPtr::call_dynamic. --- doc/src/rust/register-raw.md | 3 +- examples/repl.rs | 8 +- src/engine.rs | 66 ++++++++----- src/fn_call.rs | 174 ++++++++++++++++++++++------------- src/fn_native.rs | 48 ++++++++-- src/module.rs | 12 +-- src/optimize.rs | 1 + src/parser.rs | 14 ++- tests/call_fn.rs | 57 +++++++++--- 9 files changed, 248 insertions(+), 135 deletions(-) diff --git a/doc/src/rust/register-raw.md b/doc/src/rust/register-raw.md index 76866249..cf997286 100644 --- a/doc/src/rust/register-raw.md +++ b/doc/src/rust/register-raw.md @@ -125,8 +125,7 @@ engine.register_raw_fn( let this_ptr = args.get_mut(0).unwrap(); // 1st argument - this pointer // Use 'FnPtr::call_dynamic' to call the function pointer. - // Beware, only script-defined functions are supported by 'FnPtr::call_dynamic'. - // If it is a native Rust function, directly call it here in Rust instead! + // Beware, private script-defined functions will not be found. fp.call_dynamic(engine, lib, Some(this_ptr), [value]) }, ); diff --git a/examples/repl.rs b/examples/repl.rs index cf255986..b3877dd0 100644 --- a/examples/repl.rs +++ b/examples/repl.rs @@ -72,15 +72,17 @@ fn main() { println!("=============="); print_help(); - loop { + 'main_loop: loop { print!("rhai> "); stdout().flush().expect("couldn't flush stdout"); input.clear(); loop { - if let Err(err) = stdin().read_line(&mut input) { - panic!("input error: {}", err); + match stdin().read_line(&mut input) { + Ok(0) => break 'main_loop, + Ok(_) => (), + Err(err) => panic!("input error: {}", err), } let line = input.as_str().trim_end(); diff --git a/src/engine.rs b/src/engine.rs index f4435698..aea01500 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -695,8 +695,8 @@ 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, None, - level, + state, lib, FN_IDX_SET, true, 0, args, is_ref, true, false, + None, level, ) .or_else(|err| match *err { // If there is no index setter, no need to set it back because the indexer is read-only @@ -719,8 +719,8 @@ 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, None, - level, + state, lib, FN_IDX_SET, true, 0, args, is_ref, true, false, + None, level, )?; } // Error @@ -741,7 +741,12 @@ impl Engine { match rhs { // xxx.fn_name(arg_expr_list) Expr::FnCall(x) if x.1.is_none() => { - self.make_method_call(state, lib, target, rhs, idx_val, level) + 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, + ) + .map_err(|err| err.new_position(*pos)) } // xxx.module::fn_name(...) - syntax error Expr::FnCall(_) => unreachable!(), @@ -770,7 +775,8 @@ 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, None, level, + state, lib, setter, true, 0, &mut args, is_ref, true, false, None, + level, ) .map(|(v, _)| (v, true)) .map_err(|err| err.new_position(*pos)) @@ -780,7 +786,8 @@ 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, None, level, + state, lib, getter, true, 0, &mut args, is_ref, true, false, None, + level, ) .map(|(v, _)| (v, false)) .map_err(|err| err.new_position(*pos)) @@ -797,9 +804,13 @@ 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 (val, _) = self.make_method_call( - state, lib, target, sub_lhs, idx_val, level, - )?; + 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, + *native, false, level, + ) + .map_err(|err| err.new_position(*pos))?; val.into() } // {xxx:map}.module::fn_name(...) - syntax error @@ -826,8 +837,8 @@ 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, None, - level, + state, lib, getter, true, 0, args, is_ref, true, false, + None, level, ) .map_err(|err| err.new_position(*pos))?; @@ -847,7 +858,7 @@ impl Engine { arg_values[1] = val; self.exec_fn_call( state, lib, setter, true, 0, arg_values, is_ref, true, - None, level, + false, None, level, ) .or_else( |err| match *err { @@ -864,9 +875,13 @@ 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 (mut val, _) = self.make_method_call( - state, lib, target, sub_lhs, idx_val, level, - )?; + 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, + *native, false, level, + ) + .map_err(|err| err.new_position(*pos))?; let val = &mut val; let target = &mut val.into(); @@ -1132,7 +1147,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, None, _level, + state, _lib, FN_IDX_GET, true, 0, args, is_ref, true, false, None, _level, ) .map(|(v, _)| v.into()) .map_err(|err| match *err { @@ -1188,7 +1203,7 @@ impl Engine { let (r, _) = self .call_fn_raw( - &mut scope, mods, state, lib, op, hashes, args, false, false, + &mut scope, mods, state, lib, op, hashes, args, false, false, false, def_value, level, ) .map_err(|err| err.new_position(rhs.position()))?; @@ -1303,7 +1318,8 @@ impl Engine { // Run function let (value, _) = self .exec_fn_call( - state, lib, op, true, hash, args, false, false, None, level, + state, lib, op, true, hash, args, false, false, false, None, + level, ) .map_err(|err| err.new_position(*op_pos))?; // Set value to LHS @@ -1331,9 +1347,11 @@ impl Engine { &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, None, level) - .map(|(v, _)| v) - .map_err(|err| err.new_position(*op_pos))? + self.exec_fn_call( + state, lib, op, true, hash, args, false, false, false, None, level, + ) + .map(|(v, _)| v) + .map_err(|err| err.new_position(*op_pos))? }); match lhs_expr { @@ -1403,7 +1421,7 @@ impl Engine { let ((name, native, 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, - level, + false, level, ) .map_err(|err| err.new_position(*pos)) } @@ -1413,7 +1431,7 @@ impl Engine { let ((name, _, 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, + true, level, ) .map_err(|err| err.new_position(*pos)) } diff --git a/src/fn_call.rs b/src/fn_call.rs index c55124e2..d581268a 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -8,10 +8,10 @@ use crate::engine::{ KEYWORD_TYPE_OF, }; use crate::error::ParseErrorType; -use crate::fn_native::{FnCallArgs, FnPtr}; +use crate::fn_native::{CallableFunction, FnCallArgs, FnPtr}; use crate::module::{Module, ModuleRef}; use crate::optimize::OptimizationLevel; -use crate::parser::{Expr, ImmutableString, AST, INT}; +use crate::parser::{Expr, FnAccess, ImmutableString, AST, INT}; use crate::result::EvalAltResult; use crate::scope::Scope; use crate::token::Position; @@ -105,6 +105,19 @@ fn restore_first_arg<'a>(old_this_ptr: Option<&'a mut Dynamic>, args: &mut FnCal } } +#[inline] +fn check_public_access(func: &CallableFunction) -> Option<&CallableFunction> { + if func.access() == FnAccess::Private { + None + } else { + Some(func) + } +} +#[inline(always)] +fn no_check_access(func: &CallableFunction) -> Option<&CallableFunction> { + Some(func) +} + impl Engine { /// Universal method for calling functions either registered with the `Engine` or written in Rhai. /// Position in `EvalAltResult` is `None` and must be set afterwards. @@ -125,6 +138,7 @@ impl Engine { args: &mut FnCallArgs, is_ref: bool, _is_method: bool, + pub_only: bool, def_val: Option, _level: usize, ) -> Result<(Dynamic, bool), Box> { @@ -132,6 +146,12 @@ impl Engine { let native_only = hash_script == 0; + let check = if pub_only { + check_public_access + } else { + no_check_access + }; + // Check for stack overflow #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "unchecked"))] @@ -150,14 +170,16 @@ impl Engine { // 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) //.or_else(|| lib.get_fn(hash_fn)) + lib.get_fn(hash_script).and_then(check) //.or_else(|| lib.get_fn(hash_fn)).and_then(check) } else { None } - //.or_else(|| self.global_module.get_fn(hash_script)) + //.or_else(|| self.global_module.get_fn(hash_script)).and_then(check) .or_else(|| self.global_module.get_fn(hash_fn)) - //.or_else(|| self.packages.get_fn(hash_script)) - .or_else(|| self.packages.get_fn(hash_fn)); + .and_then(check) + //.or_else(|| self.packages.get_fn(hash_script)).and_then(check) + .or_else(|| self.packages.get_fn(hash_fn)) + .and_then(check); if let Some(func) = func { #[cfg(not(feature = "no_function"))] @@ -378,18 +400,28 @@ impl Engine { } // Has a system function an override? - fn has_override(&self, lib: &Module, hash_fn: u64, hash_script: u64) -> bool { + fn has_override(&self, lib: &Module, hash_fn: u64, hash_script: u64, pub_only: bool) -> bool { + let check = if pub_only { + check_public_access + } else { + no_check_access + }; + // NOTE: We skip script functions for global_module and packages, and native functions for lib // First check script-defined functions - lib.contains_fn(hash_script) - //|| lib.contains_fn(hash_fn) - // Then check registered functions - //|| self.global_module.contains_fn(hash_script) - || self.global_module.contains_fn(hash_fn) - // Then check packages - //|| self.packages.contains_fn(hash_script) - || self.packages.contains_fn(hash_fn) + lib.get_fn(hash_script) + .and_then(check) + //.or_else(|| lib.get_fn(hash_fn)).and_then(check) + // Then check registered functions + //.or_else(|| self.global_module.get_fn(hash_script)).and_then(check) + .or_else(|| self.global_module.get_fn(hash_fn)) + .and_then(check) + // Then check packages + //.or_else(|| self.packages.get_fn(hash_script)).and_then(check) + .or_else(|| self.packages.get_fn(hash_fn)) + .and_then(check) + .is_some() } /// Perform an actual function call, taking care of special functions @@ -410,6 +442,7 @@ impl Engine { args: &mut FnCallArgs, is_ref: bool, is_method: bool, + pub_only: bool, def_val: Option, level: usize, ) -> Result<(Dynamic, bool), Box> { @@ -420,7 +453,9 @@ impl Engine { match fn_name { // type_of - KEYWORD_TYPE_OF if args.len() == 1 && !self.has_override(lib, hashes.0, hashes.1) => { + KEYWORD_TYPE_OF + if args.len() == 1 && !self.has_override(lib, hashes.0, hashes.1, pub_only) => + { Ok(( self.map_type_name(args[0].type_name()).to_string().into(), false, @@ -428,7 +463,9 @@ impl Engine { } // Fn - KEYWORD_FN_PTR if args.len() == 1 && !self.has_override(lib, hashes.0, hashes.1) => { + KEYWORD_FN_PTR + if args.len() == 1 && !self.has_override(lib, hashes.0, hashes.1, pub_only) => + { Err(Box::new(EvalAltResult::ErrorRuntime( "'Fn' should not be called in method style. Try Fn(...);".into(), Position::none(), @@ -436,7 +473,9 @@ 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) => { + KEYWORD_EVAL + if args.len() == 1 && !self.has_override(lib, hashes.0, hashes.1, pub_only) => + { Err(Box::new(EvalAltResult::ErrorRuntime( "'eval' should not be called in method style. Try eval(...);".into(), Position::none(), @@ -449,7 +488,7 @@ impl Engine { let mut mods = Imports::new(); self.call_fn_raw( &mut scope, &mut mods, state, lib, fn_name, hashes, args, is_ref, is_method, - def_val, level, + pub_only, def_val, level, ) } } @@ -499,28 +538,28 @@ impl Engine { } /// Call a dot method. + /// Position in `EvalAltResult` is `None` and must be set afterwards. #[cfg(not(feature = "no_object"))] pub(crate) fn make_method_call( &self, state: &mut State, lib: &Module, + name: &str, + hash: u64, target: &mut Target, - expr: &Expr, idx_val: Dynamic, + def_val: Option, + native: bool, + pub_only: bool, level: usize, ) -> Result<(Dynamic, bool), Box> { - let ((name, native, pos), _, hash, _, def_val) = match expr { - Expr::FnCall(x) => x.as_ref(), - _ => unreachable!(), - }; - let is_ref = target.is_ref(); let is_value = target.is_value(); // Get a reference to the mutation target Dynamic let obj = target.as_mut(); let mut idx = idx_val.cast::>(); - let mut _fn_name = name.as_ref(); + let mut _fn_name = name; let (result, updated) = if _fn_name == KEYWORD_FN_PTR_CALL && obj.is::() { // FnPtr call @@ -539,7 +578,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, *def_val, level, + state, lib, fn_name, native, hash, args, false, false, pub_only, def_val, level, ) } else if _fn_name == KEYWORD_FN_PTR_CALL && idx.len() > 0 && idx[0].is::() { // FnPtr call on object @@ -558,7 +597,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, *def_val, level, + state, lib, &fn_name, native, hash, args, is_ref, true, pub_only, def_val, level, ) } else if _fn_name == KEYWORD_FN_PTR_CURRY && obj.is::() { // Curry call @@ -579,7 +618,7 @@ impl Engine { } else { #[cfg(not(feature = "no_object"))] let redirected; - let mut _hash = *hash; + let mut _hash = hash; // Check if it is a map method call in OOP style #[cfg(not(feature = "no_object"))] @@ -600,10 +639,9 @@ impl Engine { let args = arg_values.as_mut(); self.exec_fn_call( - state, lib, _fn_name, *native, _hash, args, is_ref, true, *def_val, level, + state, lib, _fn_name, native, _hash, args, is_ref, true, pub_only, def_val, level, ) - } - .map_err(|err| err.new_position(*pos))?; + }?; // Feed the changed temp value back if updated && !is_ref && !is_value { @@ -628,13 +666,14 @@ impl Engine { def_val: Option, mut hash: u64, native: bool, + pub_only: 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) { + if !self.has_override(lib, hash_fn, hash, 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)?; @@ -685,7 +724,7 @@ impl Engine { 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) { + if !self.has_override(lib, hash_fn, hash, pub_only) { // eval - only in function call style let prev_len = scope.len(); let expr = args_expr.get(0).unwrap(); @@ -710,7 +749,10 @@ impl Engine { 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) { + 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)?; @@ -779,7 +821,7 @@ impl Engine { let args = args.as_mut(); self.exec_fn_call( - state, lib, name, native, hash, args, is_ref, false, def_val, level, + state, lib, name, native, hash, args, is_ref, false, pub_only, def_val, level, ) .map(|(v, _)| v) } @@ -798,6 +840,7 @@ impl Engine { args_expr: &[Expr], def_val: Option, hash_script: u64, + pub_only: bool, level: usize, ) -> Result> { let modules = modules.as_ref().unwrap(); @@ -844,9 +887,15 @@ impl Engine { let module = search_imports(mods, state, modules)?; // First search in script-defined functions (can override built-in) - let func = match module.get_qualified_fn(hash_script) { - Err(err) if matches!(*err, EvalAltResult::ErrorFunctionNotFound(_, _)) => { - // Then search in Rust functions + let check = if pub_only { + check_public_access + } else { + no_check_access + }; + + let func = match module.get_qualified_fn(hash_script).and_then(check) { + // Then search in Rust functions + None => { self.inc_operations(state)?; // Qualified Rust functions are indexed in two steps: @@ -858,14 +907,14 @@ impl Engine { // 3) The final hash is the XOR of the two hashes. let hash_qualified_fn = hash_script ^ hash_fn_args; - module.get_qualified_fn(hash_qualified_fn) + module.get_qualified_fn(hash_qualified_fn).and_then(check) } r => r, }; match func { #[cfg(not(feature = "no_function"))] - Ok(f) if f.is_script() => { + Some(f) if f.is_script() => { let args = args.as_mut(); let fn_def = f.get_fn_def(); let mut scope = Scope::new(); @@ -874,31 +923,24 @@ impl Engine { &mut scope, &mut mods, state, lib, &mut None, name, fn_def, args, level, ) } - Ok(f) => f.get_native_fn()(self, lib, args.as_mut()), - Err(err) => match *err { - EvalAltResult::ErrorFunctionNotFound(_, _) if def_val.is_some() => { - Ok(def_val.unwrap().into()) - } - EvalAltResult::ErrorFunctionNotFound(_, pos) => { - Err(Box::new(EvalAltResult::ErrorFunctionNotFound( - format!( - "{}{} ({})", - modules, - name, - args.iter() - .map(|a| if a.is::() { - "&str | ImmutableString | String" - } else { - self.map_type_name((*a).type_name()) - }) - .collect::>() - .join(", ") - ), - pos, - ))) - } - _ => Err(err), - }, + Some(f) => f.get_native_fn()(self, lib, args.as_mut()), + None if def_val.is_some() => Ok(def_val.unwrap().into()), + None => Err(Box::new(EvalAltResult::ErrorFunctionNotFound( + format!( + "{}{} ({})", + modules, + name, + args.iter() + .map(|a| if a.is::() { + "&str | ImmutableString | String" + } else { + self.map_type_name((*a).type_name()) + }) + .collect::>() + .join(", ") + ), + Position::none(), + ))), } } } diff --git a/src/fn_native.rs b/src/fn_native.rs index bfb8b956..22919274 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -1,16 +1,18 @@ //! Module defining interfaces to native-Rust functions. use crate::any::Dynamic; +use crate::calc_fn_hash; use crate::engine::Engine; use crate::module::Module; +use crate::parser::FnAccess; use crate::result::EvalAltResult; use crate::token::{is_valid_identifier, Position}; use crate::utils::ImmutableString; #[cfg(not(feature = "no_function"))] -use crate::{module::FuncReturn, parser::ScriptFnDef, scope::Scope, utils::StaticVec}; +use crate::{module::FuncReturn, parser::ScriptFnDef, utils::StaticVec}; -use crate::stdlib::{boxed::Box, convert::TryFrom, fmt, string::String, vec::Vec}; +use crate::stdlib::{boxed::Box, convert::TryFrom, fmt, iter::empty, string::String, vec::Vec}; #[cfg(not(feature = "no_function"))] use crate::stdlib::mem; @@ -89,9 +91,7 @@ impl FnPtr { /// Call the function pointer with curried arguments (if any). /// - /// The function must be a script-defined function. It cannot be a Rust function. - /// - /// To call a Rust function, just call it directly in Rust! + /// If this function is a script-defined function, it must not be marked private. /// /// ## WARNING /// @@ -107,14 +107,39 @@ impl FnPtr { this_ptr: Option<&mut Dynamic>, mut arg_values: impl AsMut<[Dynamic]>, ) -> FuncReturn { - let args = self + let mut args_data = self .1 .iter() .cloned() .chain(arg_values.as_mut().iter_mut().map(|v| mem::take(v))) .collect::>(); - engine.call_fn_dynamic(&mut Scope::new(), lib, self.0.as_str(), this_ptr, args) + let has_this = this_ptr.is_some(); + let args_len = args_data.len(); + let mut args = args_data.iter_mut().collect::>(); + + if let Some(obj) = this_ptr { + args.insert(0, obj); + } + + let fn_name = self.0.as_str(); + let hash_script = calc_fn_hash(empty(), fn_name, args_len, empty()); + + engine + .exec_fn_call( + &mut Default::default(), + lib.as_ref(), + fn_name, + false, + hash_script, + args.as_mut(), + has_this, + has_this, + true, + None, + 0, + ) + .map(|(v, _)| v) } } @@ -255,6 +280,15 @@ impl CallableFunction { Self::Pure(_) | Self::Method(_) | Self::Iterator(_) => false, } } + /// Get the access mode. + pub fn access(&self) -> FnAccess { + match self { + CallableFunction::Pure(_) + | CallableFunction::Method(_) + | CallableFunction::Iterator(_) => FnAccess::Public, + CallableFunction::Script(f) => f.access, + } + } /// Get a reference to a native Rust function. /// /// # Panics diff --git a/src/module.rs b/src/module.rs index 07430f6e..9fad5571 100644 --- a/src/module.rs +++ b/src/module.rs @@ -912,16 +912,8 @@ impl Module { /// /// The `u64` hash is calculated by the function `crate::calc_fn_hash` and must match /// the hash calculated by `index_all_sub_modules`. - pub(crate) fn get_qualified_fn( - &self, - hash_qualified_fn: u64, - ) -> Result<&Func, Box> { - self.all_functions.get(&hash_qualified_fn).ok_or_else(|| { - Box::new(EvalAltResult::ErrorFunctionNotFound( - String::new(), - Position::none(), - )) - }) + pub(crate) fn get_qualified_fn(&self, hash_qualified_fn: u64) -> Option<&Func> { + self.all_functions.get(&hash_qualified_fn) } /// Merge another module into this module. diff --git a/src/optimize.rs b/src/optimize.rs index 0948fbd5..bf5f6cd3 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -141,6 +141,7 @@ fn call_fn_with_constant_arguments( arg_values.iter_mut().collect::>().as_mut(), false, false, + true, None, 0, ) diff --git a/src/parser.rs b/src/parser.rs index 51685a6c..1177c3c2 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2674,10 +2674,9 @@ fn parse_block( // Parse statements inside the block settings.is_global = false; - let stmt = if let Some(s) = parse_stmt(input, state, lib, settings.level_up())? { - s - } else { - continue; + let stmt = match parse_stmt(input, state, lib, settings.level_up())? { + Some(s) => s, + None => continue, }; // See if it needs a terminating semicolon @@ -3137,10 +3136,9 @@ impl Engine { pos: Position::none(), }; - let stmt = if let Some(s) = parse_stmt(input, &mut state, &mut functions, settings)? { - s - } else { - continue; + let stmt = match parse_stmt(input, &mut state, &mut functions, settings)? { + Some(s) => s, + None => continue, }; let need_semicolon = !stmt.is_self_terminated(); diff --git a/tests/call_fn.rs b/tests/call_fn.rs index 3b93d520..e75d93ec 100644 --- a/tests/call_fn.rs +++ b/tests/call_fn.rs @@ -1,6 +1,7 @@ #![cfg(not(feature = "no_function"))] use rhai::{ - Dynamic, Engine, EvalAltResult, FnPtr, Func, Module, ParseError, ParseErrorType, Scope, INT, + Dynamic, Engine, EvalAltResult, FnPtr, Func, Module, ParseError, ParseErrorType, RegisterFn, + Scope, INT, }; use std::any::TypeId; @@ -119,21 +120,23 @@ fn test_anonymous_fn() -> Result<(), Box> { fn test_fn_ptr_raw() -> Result<(), Box> { let mut engine = Engine::new(); - engine.register_raw_fn( - "bar", - &[ - TypeId::of::(), - TypeId::of::(), - TypeId::of::(), - ], - move |engine: &Engine, lib: &Module, args: &mut [&mut Dynamic]| { - let fp = std::mem::take(args[1]).cast::(); - let value = args[2].clone(); - let this_ptr = args.get_mut(0).unwrap(); + engine + .register_fn("mul", |x: &mut INT, y: INT| *x *= y) + .register_raw_fn( + "bar", + &[ + TypeId::of::(), + TypeId::of::(), + TypeId::of::(), + ], + move |engine: &Engine, lib: &Module, args: &mut [&mut Dynamic]| { + let fp = std::mem::take(args[1]).cast::(); + let value = args[2].clone(); + let this_ptr = args.get_mut(0).unwrap(); - fp.call_dynamic(engine, lib, Some(this_ptr), [value]) - }, - ); + fp.call_dynamic(engine, lib, Some(this_ptr), [value]) + }, + ); assert_eq!( engine.eval::( @@ -148,6 +151,30 @@ fn test_fn_ptr_raw() -> Result<(), Box> { 42 ); + assert!(matches!( + *engine.eval::( + r#" + private fn foo(x) { this += x; } + + let x = 41; + x.bar(Fn("foo"), 1); + x + "# + ).expect_err("should error"), + EvalAltResult::ErrorFunctionNotFound(x, _) if x.starts_with("foo (") + )); + + assert_eq!( + engine.eval::( + r#" + let x = 21; + x.bar(Fn("mul"), 2); + x + "# + )?, + 42 + ); + Ok(()) } From 057f6435a45aa1160faf0cd3c62956b7d6505f11 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 27 Jul 2020 18:10:45 +0800 Subject: [PATCH 2/5] Add public_only parameter to module function methods. --- RELEASES.md | 1 + src/engine.rs | 30 ++++++++--------- src/fn_call.rs | 80 ++++++++++++++------------------------------- src/module.rs | 58 ++++++++++++++++++++------------ src/packages/mod.rs | 9 ++--- src/result.rs | 2 +- tests/modules.rs | 2 +- tests/time.rs | 5 ++- 8 files changed, 87 insertions(+), 100 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 520a2bf1..bd4623c0 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -27,6 +27,7 @@ Breaking changes * Language keywords are now _reserved_ (even when disabled) and they can no longer be used as variable names. * Function signature for defining custom syntax is simplified. * `Engine::register_raw_fn_XXX` API shortcuts are removed. +* `PackagesCollection::get_fn`, `PackagesCollection::contains_fn`, `Module::get_fn` and `Module::contains_fn` now take an additional `public_only` parameter indicating whether only public functions are accepted. Housekeeping ------------ diff --git a/src/engine.rs b/src/engine.rs index aea01500..5eb11702 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -19,7 +19,7 @@ use crate::utils::StaticVec; use crate::any::Variant; #[cfg(not(feature = "no_function"))] -use crate::parser::{FnAccess, ScriptFnDef}; +use crate::parser::ScriptFnDef; #[cfg(not(feature = "no_module"))] use crate::module::ModuleResolver; @@ -264,19 +264,15 @@ pub fn get_script_function_by_signature<'a>( module: &'a Module, name: &str, params: usize, - public_only: bool, + pub_only: bool, ) -> Option<&'a ScriptFnDef> { // Qualifiers (none) + function name + number of arguments. let hash_script = calc_fn_hash(empty(), name, params, empty()); - let func = module.get_fn(hash_script)?; - if !func.is_script() { - return None; - } - let fn_def = func.get_fn_def(); - - match fn_def.access { - FnAccess::Private if public_only => None, - FnAccess::Private | FnAccess::Public => Some(&fn_def), + let func = module.get_fn(hash_script, pub_only)?; + if func.is_script() { + Some(func.get_fn_def()) + } else { + None } } @@ -798,7 +794,7 @@ impl Engine { let mut val = match sub_lhs { Expr::Property(p) => { - let ((prop, _, _), _) = p.as_ref(); + let ((prop, _, _), pos) = p.as_ref(); let index = prop.clone().into(); self.get_indexed_mut(state, lib, target, index, *pos, false, level)? } @@ -827,12 +823,12 @@ impl Engine { } // xxx.sub_lhs[expr] | xxx.sub_lhs.expr Expr::Index(x) | Expr::Dot(x) => { - let (sub_lhs, expr, pos) = x.as_ref(); + let (sub_lhs, expr, _) = x.as_ref(); match sub_lhs { // xxx.prop[expr] | xxx.prop.expr Expr::Property(p) => { - let ((_, getter, setter), _) = p.as_ref(); + let ((_, getter, setter), pos) = p.as_ref(); let arg_values = &mut [target.as_mut(), &mut Default::default()]; let args = &mut arg_values[..1]; let (mut val, updated) = self @@ -1304,8 +1300,8 @@ impl Engine { if let Some(CallableFunction::Method(func)) = self .global_module - .get_fn(hash_fn) - .or_else(|| self.packages.get_fn(hash_fn)) + .get_fn(hash_fn, false) + .or_else(|| self.packages.get_fn(hash_fn, false)) { // Overriding exact implementation func(self, lib, &mut [lhs_ptr, &mut rhs_val])?; @@ -1431,7 +1427,7 @@ impl Engine { let ((name, _, 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, - true, level, + level, ) .map_err(|err| err.new_position(*pos)) } diff --git a/src/fn_call.rs b/src/fn_call.rs index d581268a..6bd95393 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -8,10 +8,10 @@ use crate::engine::{ KEYWORD_TYPE_OF, }; use crate::error::ParseErrorType; -use crate::fn_native::{CallableFunction, FnCallArgs, FnPtr}; +use crate::fn_native::{FnCallArgs, FnPtr}; use crate::module::{Module, ModuleRef}; use crate::optimize::OptimizationLevel; -use crate::parser::{Expr, FnAccess, ImmutableString, AST, INT}; +use crate::parser::{Expr, ImmutableString, AST, INT}; use crate::result::EvalAltResult; use crate::scope::Scope; use crate::token::Position; @@ -105,19 +105,6 @@ fn restore_first_arg<'a>(old_this_ptr: Option<&'a mut Dynamic>, args: &mut FnCal } } -#[inline] -fn check_public_access(func: &CallableFunction) -> Option<&CallableFunction> { - if func.access() == FnAccess::Private { - None - } else { - Some(func) - } -} -#[inline(always)] -fn no_check_access(func: &CallableFunction) -> Option<&CallableFunction> { - Some(func) -} - impl Engine { /// Universal method for calling functions either registered with the `Engine` or written in Rhai. /// Position in `EvalAltResult` is `None` and must be set afterwards. @@ -146,12 +133,6 @@ impl Engine { let native_only = hash_script == 0; - let check = if pub_only { - check_public_access - } else { - no_check_access - }; - // Check for stack overflow #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "unchecked"))] @@ -170,16 +151,14 @@ impl Engine { // 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).and_then(check) //.or_else(|| lib.get_fn(hash_fn)).and_then(check) + 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)).and_then(check) - .or_else(|| self.global_module.get_fn(hash_fn)) - .and_then(check) - //.or_else(|| self.packages.get_fn(hash_script)).and_then(check) - .or_else(|| self.packages.get_fn(hash_fn)) - .and_then(check); + //.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)); if let Some(func) = func { #[cfg(not(feature = "no_function"))] @@ -274,7 +253,11 @@ impl Engine { // Getter function not found? if let Some(prop) = extract_prop_from_getter(fn_name) { return Err(Box::new(EvalAltResult::ErrorDotExpr( - format!("- property '{}' unknown or write-only", prop), + format!( + "Unknown property '{}' for {}, or it is write-only", + prop, + self.map_type_name(args[0].type_name()) + ), Position::none(), ))); } @@ -282,7 +265,11 @@ impl Engine { // Setter function not found? if let Some(prop) = extract_prop_from_setter(fn_name) { return Err(Box::new(EvalAltResult::ErrorDotExpr( - format!("- property '{}' unknown or read-only", prop), + format!( + "Unknown property '{}' for {}, or it is read-only", + prop, + self.map_type_name(args[0].type_name()) + ), Position::none(), ))); } @@ -401,27 +388,17 @@ impl Engine { // Has a system function an override? fn has_override(&self, lib: &Module, hash_fn: u64, hash_script: u64, pub_only: bool) -> bool { - let check = if pub_only { - check_public_access - } else { - no_check_access - }; - // NOTE: We skip script functions for global_module and packages, and native functions for lib // First check script-defined functions - lib.get_fn(hash_script) - .and_then(check) - //.or_else(|| lib.get_fn(hash_fn)).and_then(check) + lib.contains_fn(hash_script, pub_only) + //|| lib.contains_fn(hash_fn, pub_only) // Then check registered functions - //.or_else(|| self.global_module.get_fn(hash_script)).and_then(check) - .or_else(|| self.global_module.get_fn(hash_fn)) - .and_then(check) + //|| self.global_module.contains_fn(hash_script, pub_only) + || self.global_module.contains_fn(hash_fn, pub_only) // Then check packages - //.or_else(|| self.packages.get_fn(hash_script)).and_then(check) - .or_else(|| self.packages.get_fn(hash_fn)) - .and_then(check) - .is_some() + //|| self.packages.contains_fn(hash_script, pub_only) + || self.packages.contains_fn(hash_fn, pub_only) } /// Perform an actual function call, taking care of special functions @@ -840,7 +817,6 @@ impl Engine { args_expr: &[Expr], def_val: Option, hash_script: u64, - pub_only: bool, level: usize, ) -> Result> { let modules = modules.as_ref().unwrap(); @@ -887,13 +863,7 @@ impl Engine { let module = search_imports(mods, state, modules)?; // First search in script-defined functions (can override built-in) - let check = if pub_only { - check_public_access - } else { - no_check_access - }; - - let func = match module.get_qualified_fn(hash_script).and_then(check) { + let func = match module.get_qualified_fn(hash_script) { // Then search in Rust functions None => { self.inc_operations(state)?; @@ -907,7 +877,7 @@ impl Engine { // 3) The final hash is the XOR of the two hashes. let hash_qualified_fn = hash_script ^ hash_fn_args; - module.get_qualified_fn(hash_qualified_fn).and_then(check) + module.get_qualified_fn(hash_qualified_fn) } r => r, }; diff --git a/src/module.rs b/src/module.rs index 9fad5571..b88cbc59 100644 --- a/src/module.rs +++ b/src/module.rs @@ -355,10 +355,20 @@ impl Module { /// /// let mut module = Module::new(); /// let hash = module.set_fn_0("calc", || Ok(42_i64)); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` - pub fn contains_fn(&self, hash_fn: u64) -> bool { - self.functions.contains_key(&hash_fn) + pub fn contains_fn(&self, hash_fn: u64, public_only: bool) -> bool { + if public_only { + self.functions + .get(&hash_fn) + .map(|(_, access, _, _)| match access { + FnAccess::Public => true, + FnAccess::Private => false, + }) + .unwrap_or(false) + } else { + self.functions.contains_key(&hash_fn) + } } /// Set a Rust function into the module, returning a hash key. @@ -443,7 +453,7 @@ impl Module { /// Ok(orig) // return Result> /// }); /// - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` pub fn set_raw_fn( &mut self, @@ -468,7 +478,7 @@ impl Module { /// /// let mut module = Module::new(); /// let hash = module.set_fn_0("calc", || Ok(42_i64)); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` pub fn set_fn_0( &mut self, @@ -491,7 +501,7 @@ impl Module { /// /// let mut module = Module::new(); /// let hash = module.set_fn_1("calc", |x: i64| Ok(x + 1)); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` pub fn set_fn_1( &mut self, @@ -516,7 +526,7 @@ impl Module { /// /// let mut module = Module::new(); /// let hash = module.set_fn_1_mut("calc", |x: &mut i64| { *x += 1; Ok(*x) }); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` pub fn set_fn_1_mut( &mut self, @@ -541,7 +551,7 @@ impl Module { /// /// let mut module = Module::new(); /// let hash = module.set_getter_fn("value", |x: &mut i64| { Ok(*x) }); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` #[cfg(not(feature = "no_object"))] pub fn set_getter_fn( @@ -565,7 +575,7 @@ impl Module { /// let hash = module.set_fn_2("calc", |x: i64, y: ImmutableString| { /// Ok(x + y.len() as i64) /// }); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` pub fn set_fn_2( &mut self, @@ -596,7 +606,7 @@ impl Module { /// let hash = module.set_fn_2_mut("calc", |x: &mut i64, y: ImmutableString| { /// *x += y.len() as i64; Ok(*x) /// }); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` pub fn set_fn_2_mut( &mut self, @@ -628,7 +638,7 @@ impl Module { /// *x = y.len() as i64; /// Ok(()) /// }); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` #[cfg(not(feature = "no_object"))] pub fn set_setter_fn( @@ -653,7 +663,7 @@ impl Module { /// let hash = module.set_indexer_get_fn(|x: &mut i64, y: ImmutableString| { /// Ok(*x + y.len() as i64) /// }); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` #[cfg(not(feature = "no_object"))] #[cfg(not(feature = "no_index"))] @@ -677,7 +687,7 @@ impl Module { /// let hash = module.set_fn_3("calc", |x: i64, y: ImmutableString, z: i64| { /// Ok(x + y.len() as i64 + z) /// }); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` pub fn set_fn_3< A: Variant + Clone, @@ -714,7 +724,7 @@ impl Module { /// let hash = module.set_fn_3_mut("calc", |x: &mut i64, y: ImmutableString, z: i64| { /// *x += y.len() as i64 + z; Ok(*x) /// }); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` pub fn set_fn_3_mut< A: Variant + Clone, @@ -752,7 +762,7 @@ impl Module { /// *x = y.len() as i64 + value; /// Ok(()) /// }); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` #[cfg(not(feature = "no_object"))] #[cfg(not(feature = "no_index"))] @@ -796,8 +806,8 @@ impl Module { /// Ok(()) /// } /// ); - /// assert!(module.contains_fn(hash_get)); - /// assert!(module.contains_fn(hash_set)); + /// assert!(module.contains_fn(hash_get, true)); + /// assert!(module.contains_fn(hash_set, true)); /// ``` #[cfg(not(feature = "no_object"))] #[cfg(not(feature = "no_index"))] @@ -825,7 +835,7 @@ impl Module { /// let hash = module.set_fn_4("calc", |x: i64, y: ImmutableString, z: i64, _w: ()| { /// Ok(x + y.len() as i64 + z) /// }); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` pub fn set_fn_4< A: Variant + Clone, @@ -869,7 +879,7 @@ impl Module { /// let hash = module.set_fn_4_mut("calc", |x: &mut i64, y: ImmutableString, z: i64, _w: ()| { /// *x += y.len() as i64 + z; Ok(*x) /// }); - /// assert!(module.contains_fn(hash)); + /// assert!(module.contains_fn(hash, true)); /// ``` pub fn set_fn_4_mut< A: Variant + Clone, @@ -903,8 +913,14 @@ impl Module { /// /// The `u64` hash is calculated by the function `crate::calc_fn_hash`. /// It is also returned by the `set_fn_XXX` calls. - pub(crate) fn get_fn(&self, hash_fn: u64) -> Option<&Func> { - self.functions.get(&hash_fn).map(|(_, _, _, v)| v) + pub(crate) fn get_fn(&self, hash_fn: u64, public_only: bool) -> Option<&Func> { + self.functions + .get(&hash_fn) + .and_then(|(_, access, _, f)| match access { + _ if !public_only => Some(f), + FnAccess::Public => Some(f), + FnAccess::Private => None, + }) } /// Get a modules-qualified function. diff --git a/src/packages/mod.rs b/src/packages/mod.rs index ccd78add..1c6dc3a9 100644 --- a/src/packages/mod.rs +++ b/src/packages/mod.rs @@ -61,14 +61,15 @@ impl PackagesCollection { self.0.insert(0, package); } /// Does the specified function hash key exist in the `PackagesCollection`? - pub fn contains_fn(&self, hash: u64) -> bool { - self.0.iter().any(|p| p.contains_fn(hash)) + #[allow(dead_code)] + pub fn contains_fn(&self, hash: u64, public_only: bool) -> bool { + self.0.iter().any(|p| p.contains_fn(hash, public_only)) } /// Get specified function via its hash key. - pub fn get_fn(&self, hash: u64) -> Option<&CallableFunction> { + pub fn get_fn(&self, hash: u64, public_only: bool) -> Option<&CallableFunction> { self.0 .iter() - .map(|p| p.get_fn(hash)) + .map(|p| p.get_fn(hash, public_only)) .find(|f| f.is_some()) .flatten() } diff --git a/src/result.rs b/src/result.rs index 66fd9cdf..550dcc37 100644 --- a/src/result.rs +++ b/src/result.rs @@ -182,7 +182,7 @@ impl fmt::Display for EvalAltResult { | Self::ErrorVariableNotFound(s, _) | Self::ErrorModuleNotFound(s, _) => write!(f, "{}: '{}'", desc, s)?, - Self::ErrorDotExpr(s, _) if !s.is_empty() => write!(f, "{} {}", desc, s)?, + Self::ErrorDotExpr(s, _) if !s.is_empty() => write!(f, "{}", s)?, Self::ErrorIndexingType(_, _) | Self::ErrorNumericIndexExpr(_) diff --git a/tests/modules.rs b/tests/modules.rs index fd049815..3b00ab17 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -35,7 +35,7 @@ fn test_module_sub_module() -> Result<(), Box> { let m2 = m.get_sub_module("universe").unwrap(); assert!(m2.contains_var("answer")); - assert!(m2.contains_fn(hash_inc)); + assert!(m2.contains_fn(hash_inc, false)); assert_eq!(m2.get_var_value::("answer").unwrap(), 41); diff --git a/tests/time.rs b/tests/time.rs index 9873decf..b3f6668f 100644 --- a/tests/time.rs +++ b/tests/time.rs @@ -1,11 +1,14 @@ #![cfg(not(feature = "no_std"))] #![cfg(not(target_arch = "wasm32"))] -use rhai::{Engine, EvalAltResult, INT}; +use rhai::{Engine, EvalAltResult}; #[cfg(not(feature = "no_float"))] use rhai::FLOAT; +#[cfg(feature = "no_float")] +use rhai::INT; + #[test] fn test_timestamp() -> Result<(), Box> { let engine = Engine::new(); From f05cd1fdf36e0f67541223d7530b8ab4f9ca7ff0 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 28 Jul 2020 10:25:26 +0800 Subject: [PATCH 3/5] Add shared and sync to reserved keywords. --- doc/src/appendix/keywords.md | 2 ++ doc/src/language/keywords.md | 30 +++++++++++++++--------------- src/token.rs | 4 ++-- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/doc/src/appendix/keywords.md b/doc/src/appendix/keywords.md index f7c1be73..5877f1a0 100644 --- a/doc/src/appendix/keywords.md +++ b/doc/src/appendix/keywords.md @@ -59,8 +59,10 @@ Reserved Keywords | `package` | Package | | `spawn` | Threading | | `go` | Threading | +| `shared` | Threading | | `await` | Async | | `async` | Async | +| `sync` | Async | | `yield` | Async | | `default` | Special value | | `void` | Special value | diff --git a/doc/src/language/keywords.md b/doc/src/language/keywords.md index 851b7907..509823db 100644 --- a/doc/src/language/keywords.md +++ b/doc/src/language/keywords.md @@ -5,21 +5,21 @@ Keywords The following are reserved keywords in Rhai: -| Active keywords | Reserved keywords | Usage | Inactive under feature | -| ------------------------------------------------- | ---------------------------------------- | --------------------- | :--------------------: | -| `true`, `false` | | Boolean constants | | -| `let`, `const` | `var`, `static` | Variable declarations | | -| `if`, `else` | `then`, `goto`, `exit` | Control flow | | -| | `switch`, `match`, `case` | Matching | | -| `while`, `loop`, `for`, `in`, `continue`, `break` | `do`, `each` | Looping | | -| `fn`, `private` | `public`, `new` | Functions | [`no_function`] | -| `return` | | Return values | | -| `throw` | `try`, `catch` | Throw exceptions | | -| `import`, `export`, `as` | `use`, `with`, `module`, `package` | Modules/packages | [`no_module`] | -| `Fn`, `call`, `curry` | | Function pointers | | -| | `spawn`, `go`, `async`, `await`, `yield` | Threading/async | | -| `type_of`, `print`, `debug`, `eval` | | Special functions | | -| | `default`, `void`, `null`, `nil` | Special values | | +| Active keywords | Reserved keywords | Usage | Inactive under feature | +| ------------------------------------------------- | ---------------------------------------------------------- | --------------------- | :--------------------: | +| `true`, `false` | | Boolean constants | | +| `let`, `const` | `var`, `static` | Variable declarations | | +| `if`, `else` | `then`, `goto`, `exit` | Control flow | | +| | `switch`, `match`, `case` | Matching | | +| `while`, `loop`, `for`, `in`, `continue`, `break` | `do`, `each` | Looping | | +| `fn`, `private` | `public`, `new` | Functions | [`no_function`] | +| `return` | | Return values | | +| `throw` | `try`, `catch` | Throw exceptions | | +| `import`, `export`, `as` | `use`, `with`, `module`, `package` | Modules/packages | [`no_module`] | +| `Fn`, `call`, `curry` | | Function pointers | | +| | `spawn`, `go`, `shared`, `sync`, `async`, `await`, `yield` | Threading/async | | +| `type_of`, `print`, `debug`, `eval` | | Special functions | | +| | `default`, `void`, `null`, `nil` | Special values | | Keywords cannot become the name of a [function] or [variable], even when they are disabled. diff --git a/src/token.rs b/src/token.rs index 0aab8ce5..b8576133 100644 --- a/src/token.rs +++ b/src/token.rs @@ -495,8 +495,8 @@ impl Token { "===" | "!==" | "->" | "<-" | "=>" | ":=" | "::<" | "(*" | "*)" | "#" | "public" | "new" | "use" | "module" | "package" | "var" | "static" | "with" | "do" | "each" | "then" | "goto" | "exit" | "switch" | "match" | "case" | "try" | "catch" - | "default" | "void" | "null" | "nil" | "spawn" | "go" | "async" | "await" - | "yield" => Reserved(syntax.into()), + | "default" | "void" | "null" | "nil" | "spawn" | "go" | "shared" | "sync" + | "async" | "await" | "yield" => Reserved(syntax.into()), KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_TYPE_OF | KEYWORD_EVAL | KEYWORD_FN_PTR | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY | KEYWORD_THIS => Reserved(syntax.into()), From b70fd35f4a89de1285160959e8d15978883628d0 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 28 Jul 2020 10:25:57 +0800 Subject: [PATCH 4/5] Revise docs. --- README.md | 5 +-- doc/src/about/features.md | 13 +++--- doc/src/language/fn-namespaces.md | 71 ++++++++++++++++++------------ doc/src/language/functions.md | 2 +- doc/src/language/method.md | 22 ++++++++- doc/src/language/modules/import.md | 15 +++++-- doc/src/language/overload.md | 10 ++--- doc/src/start/features.md | 2 +- src/module.rs | 2 +- 9 files changed, 91 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index 1516abcc..7aef538c 100644 --- a/README.md +++ b/README.md @@ -27,12 +27,11 @@ Standard features * Easily [call a script-defined function](https://schungx.github.io/rhai/engine/call-fn.html) from Rust. * Fairly low compile-time overhead. * Fairly efficient evaluation (1 million iterations in 0.3 sec on a single core, 2.3 GHz Linux VM). -* Relatively little `unsafe` code (yes there are some for performance reasons, and most `unsafe` code is limited to - one single source file, all with names starting with `"unsafe_"`). +* Relatively little `unsafe` code (yes there are some for performance reasons). * Re-entrant scripting engine can be made `Send + Sync` (via the `sync` feature). * [Function overloading](https://schungx.github.io/rhai/language/overload.html). * [Operator overloading](https://schungx.github.io/rhai/rust/operators.html). -* Dynamic dispatch via [function pointers](https://schungx.github.io/rhai/language/fn-ptr.html). +* Dynamic dispatch via [function pointers](https://schungx.github.io/rhai/language/fn-ptr.html) with additional support for [currying](https://schungx.github.io/rhai/language/fn-curry.html). * Some support for [object-oriented programming (OOP)](https://schungx.github.io/rhai/language/oop.html). * Organize code base with dynamically-loadable [modules](https://schungx.github.io/rhai/language/modules.html). * Serialization/deserialization support via [serde](https://crates.io/crates/serde) (requires the `serde` feature). diff --git a/doc/src/about/features.md b/doc/src/about/features.md index 13b2e4c1..ffa28cdc 100644 --- a/doc/src/about/features.md +++ b/doc/src/about/features.md @@ -35,23 +35,20 @@ Dynamic * Organize code base with dynamically-loadable [modules]. -* Dynamic dispatch via [function pointers]. +* Dynamic dispatch via [function pointers] with additional support for [currying]. * Some support for [object-oriented programming (OOP)][OOP]. -* Serialization/deserialization support via [`serde`]. - Safe ---- -* Relatively little `unsafe` code (yes there are some for performance reasons, and most `unsafe` code is limited to - one single source file, all with names starting with `"unsafe_"`). +* Relatively little `unsafe` code (yes there are some for performance reasons). + +* Sand-boxed - the scripting [`Engine`], if declared immutable, cannot mutate the containing environment unless explicitly permitted (e.g. via a `RefCell`). Rugged ------ -* Sand-boxed - the scripting [`Engine`], if declared immutable, cannot mutate the containing environment unless explicitly permitted (e.g. via a `RefCell`). - * Protected against malicious attacks (such as [stack-overflow][maximum call stack depth], [over-sized data][maximum length of strings], and [runaway scripts][maximum number of operations] etc.) that may come from untrusted third-party user-land scripts. * Track script evaluation [progress] and manually terminate a script run. @@ -61,6 +58,8 @@ Flexible * Re-entrant scripting [`Engine`] can be made `Send + Sync` (via the [`sync`] feature). +* Serialization/deserialization support via [`serde`](https://crates.io/crates/serde). + * Support for [minimal builds] by excluding unneeded language [features]. * Supports [most build targets](targets.md) including `no-std` and [WASM]. diff --git a/doc/src/language/fn-namespaces.md b/doc/src/language/fn-namespaces.md index 5bd8e16f..9516f1e6 100644 --- a/doc/src/language/fn-namespaces.md +++ b/doc/src/language/fn-namespaces.md @@ -69,17 +69,22 @@ The only practical way to ensure that a function is a correct one is to use [mod i.e. define the function in a separate module and then [`import`] it: ```rust -message.rhai: +---------------- +| message.rhai | +---------------- - fn message() { "Hello!" } +fn message() { "Hello!" } -script.rhai: - fn say_hello() { - import "message" as msg; - print(msg::message()); - } - say_hello(); +--------------- +| script.rhai | +--------------- + +fn say_hello() { + import "message" as msg; + print(msg::message()); +} +say_hello(); ``` @@ -94,18 +99,23 @@ defined _within the script_. When called later, those functions will be searche current global namespace and may not be found. ```rust -greeting.rhai: +----------------- +| greeting.rhai | +----------------- - fn message() { "Hello!" }; +fn message() { "Hello!" }; - fn say_hello() { print(message()); } +fn say_hello() { print(message()); } - say_hello(); // 'message' is looked up in the global namespace +say_hello(); // 'message' is looked up in the global namespace -script.rhai: - import "greeting" as g; - g::say_hello(); // <- error: function not found - 'message' +--------------- +| script.rhai | +--------------- + +import "greeting" as g; +g::say_hello(); // <- error: function not found - 'message' ``` In the example above, although the module `greeting.rhai` loads fine (`"Hello!"` is printed), @@ -118,24 +128,29 @@ as possible and avoid cross-calling them from each other. A [function pointer] to call another function within a module-defined function: ```rust -greeting.rhai: +----------------- +| greeting.rhai | +----------------- - fn message() { "Hello!" }; +fn message() { "Hello!" }; - fn say_hello(msg_func) { // 'msg_func' is a function pointer - print(msg_func.call()); // call via the function pointer - } +fn say_hello(msg_func) { // 'msg_func' is a function pointer + print(msg_func.call()); // call via the function pointer +} - say_hello(); // 'message' is looked up in the global namespace +say_hello(); // 'message' is looked up in the global namespace -script.rhai: - import "greeting" as g; +--------------- +| script.rhai | +--------------- - fn my_msg() { - import "greeting" as g; // <- must import again here... - g::message() // <- ... otherwise will not find module 'g' - } +import "greeting" as g; - g::say_hello(Fn("my_msg")); // prints 'Hello!' +fn my_msg() { + import "greeting" as g; // <- must import again here... + g::message() // <- ... otherwise will not find module 'g' +} + +g::say_hello(Fn("my_msg")); // prints 'Hello!' ``` diff --git a/doc/src/language/functions.md b/doc/src/language/functions.md index b6c0a85f..d8f67502 100644 --- a/doc/src/language/functions.md +++ b/doc/src/language/functions.md @@ -16,7 +16,7 @@ fn sub(x, y,) { // trailing comma in parameters list is OK add(2, 3) == 5; -sub(2, 3,) == -1; // trailing comma in arguments list is OK +sub(2, 3,) == -1; // trailing comma in arguments list is OK ``` diff --git a/doc/src/language/method.md b/doc/src/language/method.md index c2e9e99e..290d1a06 100644 --- a/doc/src/language/method.md +++ b/doc/src/language/method.md @@ -37,12 +37,30 @@ array[0].update(); // <- call in method-call style will update 'a' ``` -`&mut` is Efficient ------------------- +Number of Parameters +-------------------- + +Native Rust methods registered with an [`Engine`] take _one additional parameter_ more than +an equivalent method coded in script, where the object is accessed via the `this` pointer instead. + +The following table illustrates the differences: + +| Function type | Parameters | Object reference | Function signature | +| :-----------: | :--------: | :--------------------: | :-----------------------------------------------------: | +| Native Rust | _n_ + 1 | First `&mut` parameter | `fn method`
`(obj: &mut T, x: U, y: V) {}` | +| Rhai script | _n_ | `this` | `fn method(x, y) {}` | + + +`&mut` is Efficient (Except for `ImmutableString`) +------------------------------------------------ Using a `&mut` first parameter is highly encouraged when using types that are expensive to clone, even when the intention is not to mutate that argument, because it avoids cloning that argument value. +For example, the `len` method of an [array] has the signature: `Fn(&mut Array) -> INT`. +The array itself is not modified in any way, but using a `&mut` parameter avoids a cloning that would +otherwise have happened if the signature were `Fn(Array) -> INT`. + For primary types that are cheap to clone (e.g. those that implement `Copy`), including `ImmutableString`, this is not necessary. diff --git a/doc/src/language/modules/import.md b/doc/src/language/modules/import.md index dc94b5a8..5e0a377a 100644 --- a/doc/src/language/modules/import.md +++ b/doc/src/language/modules/import.md @@ -63,7 +63,9 @@ cause a stack overflow in the [`Engine`], unless stopped by setting a limit for For instance, importing itself always causes an infinite recursion: ```rust -// This file is 'hello.rhai' +-------------- +| hello.rhai | +-------------- import "hello" as foo; // import itself - infinite recursion! @@ -73,11 +75,18 @@ foo::do_something(); Modules cross-referencing also cause infinite recursion: ```rust -// This file is 'hello.rhai' - references 'world.rhai' +-------------- +| hello.rhai | +-------------- + import "world" as foo; foo::do_something(); -// This file is 'world.rhai' - references 'hello.rhai' + +-------------- +| world.rhai | +-------------- + import "hello" as bar; bar::do_something_else(); ``` diff --git a/doc/src/language/overload.md b/doc/src/language/overload.md index bb291ca3..0b6b4ea8 100644 --- a/doc/src/language/overload.md +++ b/doc/src/language/overload.md @@ -9,15 +9,15 @@ and _number_ of parameters, but not parameter _types_ since all parameters are t New definitions _overwrite_ previous definitions of the same name and number of parameters. ```rust -fn foo(x,y,z) { print("Three!!! " + x + "," + y + "," + z) } +fn foo(x,y,z) { print("Three!!! " + x + "," + y + "," + z); } -fn foo(x) { print("One! " + x) } +fn foo(x) { print("One! " + x); } -fn foo(x,y) { print("Two! " + x + "," + y) } +fn foo(x,y) { print("Two! " + x + "," + y); } -fn foo() { print("None.") } +fn foo() { print("None."); } -fn foo(x) { print("HA! NEW ONE! " + x) } // overwrites previous definition +fn foo(x) { print("HA! NEW ONE! " + x); } // overwrites previous definition foo(1,2,3); // prints "Three!!! 1,2,3" diff --git a/doc/src/start/features.md b/doc/src/start/features.md index f2286a38..0d31d077 100644 --- a/doc/src/start/features.md +++ b/doc/src/start/features.md @@ -24,7 +24,7 @@ more control over what a script can (or cannot) do. | `no_function` | Disable script-defined [functions]. | | `no_module` | Disable loading external [modules]. | | `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. | +| `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/module.rs b/src/module.rs index b88cbc59..8eacde7b 100644 --- a/src/module.rs +++ b/src/module.rs @@ -432,7 +432,7 @@ impl Module { /// let mut module = Module::new(); /// let hash = module.set_raw_fn("double_or_not", /// // Pass parameter types via a slice with TypeId's - /// &[std::any::TypeId::of::(), std::any::TypeId::of::() ], + /// &[std::any::TypeId::of::(), std::any::TypeId::of::()], /// // Fixed closure signature /// |engine, lib, args| { /// // 'args' is guaranteed to be the right length and of the correct types From b63ff56e096f73eacc28d6cc7eaefda6d25af9fe Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 28 Jul 2020 10:26:20 +0800 Subject: [PATCH 5/5] Make sure we keep the starting position of each statement (for future uses). --- src/engine.rs | 66 +++++++------------- src/optimize.rs | 45 ++++++++----- src/parser.rs | 163 +++++++++++++++++++++++++++++++++--------------- 3 files changed, 162 insertions(+), 112 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 5eb11702..e8cf5c3c 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -228,11 +228,6 @@ impl> From for Target<'_> { /// [INTERNALS] A type that holds all the current states of the Engine. /// Exported under the `internals` feature only. /// -/// # Safety -/// -/// This type uses some unsafe code, mainly for avoiding cloning of local variable names via -/// direct lifetime casting. -/// /// ## WARNING /// /// This type is volatile and may change. @@ -1495,6 +1490,12 @@ impl Engine { } /// Evaluate a statement + /// + /// + /// # Safety + /// + /// This method uses some unsafe code, mainly for avoiding cloning of local variable names via + /// direct lifetime casting. pub(crate) fn eval_stmt( &self, scope: &mut Scope, @@ -1538,7 +1539,7 @@ impl Engine { // If-else statement Stmt::IfThenElse(x) => { - let (expr, if_block, else_block) = x.as_ref(); + let (expr, if_block, else_block, _) = x.as_ref(); self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .as_bool() @@ -1556,7 +1557,7 @@ impl Engine { // While loop Stmt::While(x) => loop { - let (expr, body) = x.as_ref(); + let (expr, body, _) = x.as_ref(); match self .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? @@ -1582,8 +1583,8 @@ impl Engine { }, // Loop statement - Stmt::Loop(body) => loop { - match self.eval_stmt(scope, mods, state, lib, this_ptr, body, level) { + Stmt::Loop(x) => loop { + match self.eval_stmt(scope, mods, state, lib, this_ptr, &x.0, level) { Ok(_) => (), Err(err) => match *err { EvalAltResult::ErrorLoopBreak(false, _) => (), @@ -1595,7 +1596,7 @@ impl Engine { // For loop Stmt::For(x) => { - let (name, expr, stmt) = x.as_ref(); + let (name, expr, stmt, _) = x.as_ref(); let iter_type = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; let tid = iter_type.type_id(); @@ -1641,16 +1642,9 @@ impl Engine { // Return value Stmt::ReturnWithVal(x) if x.1.is_some() && (x.0).0 == ReturnType::Return => { + let expr = x.1.as_ref().unwrap(); Err(Box::new(EvalAltResult::Return( - self.eval_expr( - scope, - mods, - state, - lib, - this_ptr, - x.1.as_ref().unwrap(), - level, - )?, + self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?, (x.0).1, ))) } @@ -1662,15 +1656,8 @@ impl Engine { // Throw value Stmt::ReturnWithVal(x) if x.1.is_some() && (x.0).0 == ReturnType::Exception => { - let val = self.eval_expr( - scope, - mods, - state, - lib, - this_ptr, - x.1.as_ref().unwrap(), - level, - )?; + let expr = x.1.as_ref().unwrap(); + let val = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; Err(Box::new(EvalAltResult::ErrorRuntime( val.take_string().unwrap_or_else(|_| "".into()), (x.0).1, @@ -1686,23 +1673,16 @@ impl Engine { // Let statement Stmt::Let(x) if x.1.is_some() => { - let ((var_name, _), expr) = x.as_ref(); - let val = self.eval_expr( - scope, - mods, - state, - lib, - this_ptr, - expr.as_ref().unwrap(), - level, - )?; + let ((var_name, _), expr, _) = x.as_ref(); + let expr = expr.as_ref().unwrap(); + let val = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; let var_name = unsafe_cast_var_name_to_lifetime(var_name, &state); scope.push_dynamic_value(var_name, ScopeEntryType::Normal, val, false); Ok(Default::default()) } Stmt::Let(x) => { - let ((var_name, _), _) = x.as_ref(); + let ((var_name, _), _, _) = x.as_ref(); let var_name = unsafe_cast_var_name_to_lifetime(var_name, &state); scope.push(var_name, ()); Ok(Default::default()) @@ -1710,7 +1690,7 @@ impl Engine { // Const statement Stmt::Const(x) if x.1.is_constant() => { - let ((var_name, _), expr) = x.as_ref(); + let ((var_name, _), expr, _) = x.as_ref(); let val = self.eval_expr(scope, mods, state, lib, this_ptr, &expr, level)?; let var_name = unsafe_cast_var_name_to_lifetime(var_name, &state); scope.push_dynamic_value(var_name, ScopeEntryType::Constant, val, true); @@ -1723,7 +1703,7 @@ impl Engine { // Import statement #[cfg(not(feature = "no_module"))] Stmt::Import(x) => { - let (expr, (name, _pos)) = x.as_ref(); + let (expr, (name, _pos), _) = x.as_ref(); // Guard against too many modules #[cfg(not(feature = "unchecked"))] @@ -1756,8 +1736,8 @@ impl Engine { // Export statement #[cfg(not(feature = "no_module"))] - Stmt::Export(list) => { - for ((id, id_pos), rename) in list.iter() { + Stmt::Export(x) => { + for ((id, id_pos), rename) in x.0.iter() { // Mark scope variables as public if let Some(index) = scope.get_index(id).map(|(i, _)| i) { let alias = rename.as_ref().map(|(n, _)| n).unwrap_or_else(|| id); diff --git a/src/optimize.rs b/src/optimize.rs index bf5f6cd3..27f6f816 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -185,6 +185,7 @@ fn optimize_stmt(stmt: Stmt, state: &mut State, preserve_result: bool) -> Stmt { optimize_expr(expr, state), optimize_stmt(x.1, state, true), None, + x.3, ))), }, // if expr { if_block } else { else_block } @@ -201,6 +202,7 @@ fn optimize_stmt(stmt: Stmt, state: &mut State, preserve_result: bool) -> Stmt { Stmt::Noop(_) => None, // Noop -> no else block stmt => Some(stmt), }, + x.3, ))), }, // while expr { block } @@ -211,7 +213,7 @@ fn optimize_stmt(stmt: Stmt, state: &mut State, preserve_result: bool) -> Stmt { Stmt::Noop(pos) } // while true { block } -> loop { block } - Expr::True(_) => Stmt::Loop(Box::new(optimize_stmt(x.1, state, false))), + Expr::True(_) => Stmt::Loop(Box::new((optimize_stmt(x.1, state, false), x.2))), // while expr { block } expr => match optimize_stmt(x.1, state, false) { // while expr { break; } -> { expr; } @@ -226,11 +228,11 @@ fn optimize_stmt(stmt: Stmt, state: &mut State, preserve_result: bool) -> Stmt { Stmt::Block(Box::new((statements, pos))) } // while expr { block } - stmt => Stmt::While(Box::new((optimize_expr(expr, state), stmt))), + stmt => Stmt::While(Box::new((optimize_expr(expr, state), stmt, x.2))), }, }, // loop { block } - Stmt::Loop(block) => match optimize_stmt(*block, state, false) { + Stmt::Loop(x) => match optimize_stmt(x.0, state, false) { // loop { break; } -> Noop Stmt::Break(pos) => { // Only a single break statement @@ -238,23 +240,26 @@ fn optimize_stmt(stmt: Stmt, state: &mut State, preserve_result: bool) -> Stmt { Stmt::Noop(pos) } // loop { block } - stmt => Stmt::Loop(Box::new(stmt)), + stmt => Stmt::Loop(Box::new((stmt, x.1))), }, // for id in expr { block } Stmt::For(x) => Stmt::For(Box::new(( x.0, optimize_expr(x.1, state), optimize_stmt(x.2, state, false), + x.3, ))), // let id = expr; - Stmt::Let(x) if x.1.is_some() => { - Stmt::Let(Box::new((x.0, Some(optimize_expr(x.1.unwrap(), state))))) - } + Stmt::Let(x) if x.1.is_some() => Stmt::Let(Box::new(( + x.0, + Some(optimize_expr(x.1.unwrap(), state)), + x.2, + ))), // let id; stmt @ Stmt::Let(_) => stmt, // import expr as id; #[cfg(not(feature = "no_module"))] - Stmt::Import(x) => Stmt::Import(Box::new((optimize_expr(x.0, state), x.1))), + Stmt::Import(x) => Stmt::Import(Box::new((optimize_expr(x.0, state), x.1, x.2))), // { block } Stmt::Block(x) => { let orig_len = x.0.len(); // Original number of statements in the block, for change detection @@ -267,7 +272,7 @@ fn optimize_stmt(stmt: Stmt, state: &mut State, preserve_result: bool) -> Stmt { .map(|stmt| match stmt { // Add constant into the state Stmt::Const(v) => { - let ((name, pos), expr) = *v; + let ((name, pos), expr, _) = *v; state.push_constant(&name, expr); state.set_dirty(); Stmt::Noop(pos) // No need to keep constants @@ -367,9 +372,11 @@ fn optimize_stmt(stmt: Stmt, state: &mut State, preserve_result: bool) -> Stmt { // expr; Stmt::Expr(expr) => Stmt::Expr(Box::new(optimize_expr(*expr, state))), // return expr; - Stmt::ReturnWithVal(x) if x.1.is_some() => { - Stmt::ReturnWithVal(Box::new((x.0, Some(optimize_expr(x.1.unwrap(), state))))) - } + Stmt::ReturnWithVal(x) if x.1.is_some() => Stmt::ReturnWithVal(Box::new(( + x.0, + Some(optimize_expr(x.1.unwrap(), state)), + x.2, + ))), // All other statements - skip stmt => stmt, } @@ -412,7 +419,7 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { state.set_dirty(); let pos = m.1; m.0.into_iter().find(|((name, _), _)| name.as_str() == prop.as_str()) - .map(|(_, expr)| expr.set_position(pos)) + .map(|(_, mut expr)| { expr.set_position(pos); expr }) .unwrap_or_else(|| Expr::Unit(pos)) } // lhs.rhs @@ -429,7 +436,9 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { // Array literal where everything is pure - promote the indexed item. // All other items can be thrown away. state.set_dirty(); - a.0.take(i.0 as usize).set_position(a.1) + let mut expr = a.0.take(i.0 as usize); + expr.set_position(a.1); + expr } // map[string] (Expr::Map(m), Expr::StringConstant(s)) if m.0.iter().all(|(_, x)| x.is_pure()) => { @@ -438,7 +447,7 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { state.set_dirty(); let pos = m.1; m.0.into_iter().find(|((name, _), _)| *name == s.0) - .map(|(_, expr)| expr.set_position(pos)) + .map(|(_, mut expr)| { expr.set_position(pos); expr }) .unwrap_or_else(|| Expr::Unit(pos)) } // string[int] @@ -625,7 +634,9 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { state.set_dirty(); // Replace constant with value - state.find_constant(&name).unwrap().clone().set_position(pos) + let mut expr = state.find_constant(&name).unwrap().clone(); + expr.set_position(pos); + expr } // Custom syntax @@ -687,7 +698,7 @@ fn optimize( match &stmt { Stmt::Const(v) => { // Load constants - let ((name, _), expr) = v.as_ref(); + let ((name, _), expr, _) = v.as_ref(); state.push_constant(&name, expr.clone()); stmt // Keep it in the global scope } diff --git a/src/parser.rs b/src/parser.rs index 1177c3c2..417c35fb 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -26,7 +26,6 @@ use crate::stdlib::{ fmt, format, hash::{Hash, Hasher}, iter::empty, - mem, num::NonZeroUsize, ops::Add, string::{String, ToString}, @@ -511,33 +510,38 @@ pub enum Stmt { /// No-op. Noop(Position), /// if expr { stmt } else { stmt } - IfThenElse(Box<(Expr, Stmt, Option)>), + IfThenElse(Box<(Expr, Stmt, Option, Position)>), /// while expr { stmt } - While(Box<(Expr, Stmt)>), + While(Box<(Expr, Stmt, Position)>), /// loop { stmt } - Loop(Box), + Loop(Box<(Stmt, Position)>), /// for id in expr { stmt } - For(Box<(String, Expr, Stmt)>), + For(Box<(String, Expr, Stmt, Position)>), /// let id = expr - Let(Box<((String, Position), Option)>), + Let(Box<((String, Position), Option, Position)>), /// const id = expr - Const(Box<((String, Position), Expr)>), + Const(Box<((String, Position), Expr, Position)>), /// { stmt; ... } Block(Box<(StaticVec, Position)>), - /// { stmt } + /// expr Expr(Box), /// continue Continue(Position), /// break Break(Position), /// return/throw - ReturnWithVal(Box<((ReturnType, Position), Option)>), + ReturnWithVal(Box<((ReturnType, Position), Option, Position)>), /// import expr as module #[cfg(not(feature = "no_module"))] - Import(Box<(Expr, (String, Position))>), + Import(Box<(Expr, (String, Position), Position)>), /// expr id as name, ... #[cfg(not(feature = "no_module"))] - Export(Box)>>), + Export( + Box<( + StaticVec<((String, Position), Option<(String, Position)>)>, + Position, + )>, + ), } impl Default for Stmt { @@ -555,19 +559,44 @@ impl Stmt { Stmt::Const(x) => (x.0).1, Stmt::ReturnWithVal(x) => (x.0).1, Stmt::Block(x) => x.1, - Stmt::IfThenElse(x) => x.0.position(), + Stmt::IfThenElse(x) => x.3, Stmt::Expr(x) => x.position(), - Stmt::While(x) => x.1.position(), - Stmt::Loop(x) => x.position(), - Stmt::For(x) => x.2.position(), + Stmt::While(x) => x.2, + Stmt::Loop(x) => x.1, + Stmt::For(x) => x.3, #[cfg(not(feature = "no_module"))] - Stmt::Import(x) => (x.1).1, + Stmt::Import(x) => x.2, #[cfg(not(feature = "no_module"))] - Stmt::Export(x) => (x.get(0).0).1, + Stmt::Export(x) => x.1, } } + /// Override the `Position` of this statement. + pub fn set_position(&mut self, new_pos: Position) -> &mut Self { + match self { + Stmt::Noop(pos) | Stmt::Continue(pos) | Stmt::Break(pos) => *pos = new_pos, + Stmt::Let(x) => (x.0).1 = new_pos, + Stmt::Const(x) => (x.0).1 = new_pos, + Stmt::ReturnWithVal(x) => (x.0).1 = new_pos, + Stmt::Block(x) => x.1 = new_pos, + Stmt::IfThenElse(x) => x.3 = new_pos, + Stmt::Expr(x) => { + x.set_position(new_pos); + } + Stmt::While(x) => x.2 = new_pos, + Stmt::Loop(x) => x.1 = new_pos, + Stmt::For(x) => x.3 = new_pos, + + #[cfg(not(feature = "no_module"))] + Stmt::Import(x) => x.2 = new_pos, + #[cfg(not(feature = "no_module"))] + Stmt::Export(x) => x.1 = new_pos, + } + + self + } + /// Is this statement self-terminated (i.e. no need for a semicolon terminator)? pub fn is_self_terminated(&self) -> bool { match self { @@ -602,7 +631,7 @@ impl Stmt { } Stmt::IfThenElse(x) => x.1.is_pure(), Stmt::While(x) => x.0.is_pure() && x.1.is_pure(), - Stmt::Loop(x) => x.is_pure(), + Stmt::Loop(x) => x.0.is_pure(), Stmt::For(x) => x.1.is_pure() && x.2.is_pure(), Stmt::Let(_) | Stmt::Const(_) => false, Stmt::Block(x) => x.0.iter().all(Stmt::is_pure), @@ -832,11 +861,10 @@ impl Expr { } /// Override the `Position` of the expression. - pub(crate) fn set_position(mut self, new_pos: Position) -> Self { - match &mut self { - Self::Expr(ref mut x) => { - let expr = mem::take(x); - *x = Box::new(expr.set_position(new_pos)); + pub fn set_position(&mut self, new_pos: Position) -> &mut Self { + match self { + Self::Expr(x) => { + x.set_position(new_pos); } #[cfg(not(feature = "no_float"))] @@ -2314,7 +2342,7 @@ fn ensure_not_statement_expr(input: &mut TokenStream, type_name: &str) -> Result fn ensure_not_assignment(input: &mut TokenStream) -> Result<(), ParseError> { match input.peek().unwrap() { (Token::Equals, pos) => { - return Err(PERR::BadInput("Possibly a typo of '=='?".to_string()).into_err(*pos)) + Err(PERR::BadInput("Possibly a typo of '=='?".to_string()).into_err(*pos)) } (Token::PlusAssign, pos) | (Token::MinusAssign, pos) @@ -2326,12 +2354,10 @@ fn ensure_not_assignment(input: &mut TokenStream) -> Result<(), ParseError> { | (Token::PowerOfAssign, pos) | (Token::AndAssign, pos) | (Token::OrAssign, pos) - | (Token::XOrAssign, pos) => { - return Err(PERR::BadInput( - "Expecting a boolean expression, not an assignment".to_string(), - ) - .into_err(*pos)) - } + | (Token::XOrAssign, pos) => Err(PERR::BadInput( + "Expecting a boolean expression, not an assignment".to_string(), + ) + .into_err(*pos)), _ => Ok(()), } @@ -2345,7 +2371,8 @@ fn parse_if( mut settings: ParseSettings, ) -> Result { // if ... - settings.pos = eat_token(input, Token::If); + let token_pos = eat_token(input, Token::If); + settings.pos = token_pos; #[cfg(not(feature = "unchecked"))] settings.ensure_level_within_max_limit(state.max_expr_depth)?; @@ -2369,7 +2396,9 @@ fn parse_if( None }; - Ok(Stmt::IfThenElse(Box::new((guard, if_body, else_body)))) + Ok(Stmt::IfThenElse(Box::new(( + guard, if_body, else_body, token_pos, + )))) } /// Parse a while loop. @@ -2380,7 +2409,8 @@ fn parse_while( mut settings: ParseSettings, ) -> Result { // while ... - settings.pos = eat_token(input, Token::While); + let token_pos = eat_token(input, Token::While); + settings.pos = token_pos; #[cfg(not(feature = "unchecked"))] settings.ensure_level_within_max_limit(state.max_expr_depth)?; @@ -2393,7 +2423,7 @@ fn parse_while( settings.is_breakable = true; let body = parse_block(input, state, lib, settings.level_up())?; - Ok(Stmt::While(Box::new((guard, body)))) + Ok(Stmt::While(Box::new((guard, body, token_pos)))) } /// Parse a loop statement. @@ -2404,7 +2434,8 @@ fn parse_loop( mut settings: ParseSettings, ) -> Result { // loop ... - settings.pos = eat_token(input, Token::Loop); + let token_pos = eat_token(input, Token::Loop); + settings.pos = token_pos; #[cfg(not(feature = "unchecked"))] settings.ensure_level_within_max_limit(state.max_expr_depth)?; @@ -2413,7 +2444,7 @@ fn parse_loop( settings.is_breakable = true; let body = parse_block(input, state, lib, settings.level_up())?; - Ok(Stmt::Loop(Box::new(body))) + Ok(Stmt::Loop(Box::new((body, token_pos)))) } /// Parse a for loop. @@ -2424,7 +2455,8 @@ fn parse_for( mut settings: ParseSettings, ) -> Result { // for ... - settings.pos = eat_token(input, Token::For); + let token_pos = eat_token(input, Token::For); + settings.pos = token_pos; #[cfg(not(feature = "unchecked"))] settings.ensure_level_within_max_limit(state.max_expr_depth)?; @@ -2467,7 +2499,7 @@ fn parse_for( state.stack.truncate(prev_stack_len); - Ok(Stmt::For(Box::new((name, expr, body)))) + Ok(Stmt::For(Box::new((name, expr, body, token_pos)))) } /// Parse a variable definition statement. @@ -2479,7 +2511,8 @@ fn parse_let( mut settings: ParseSettings, ) -> Result { // let/const... (specified in `var_type`) - settings.pos = input.next().unwrap().1; + let token_pos = input.next().unwrap().1; + settings.pos = token_pos; #[cfg(not(feature = "unchecked"))] settings.ensure_level_within_max_limit(state.max_expr_depth)?; @@ -2503,12 +2536,16 @@ fn parse_let( // let name = expr ScopeEntryType::Normal => { state.stack.push((name.clone(), ScopeEntryType::Normal)); - Ok(Stmt::Let(Box::new(((name, pos), Some(init_value))))) + Ok(Stmt::Let(Box::new(( + (name, pos), + Some(init_value), + token_pos, + )))) } // const name = { expr:constant } ScopeEntryType::Constant if init_value.is_constant() => { state.stack.push((name.clone(), ScopeEntryType::Constant)); - Ok(Stmt::Const(Box::new(((name, pos), init_value)))) + Ok(Stmt::Const(Box::new(((name, pos), init_value, token_pos)))) } // const name = expr: error ScopeEntryType::Constant => { @@ -2520,11 +2557,15 @@ fn parse_let( match var_type { ScopeEntryType::Normal => { state.stack.push((name.clone(), ScopeEntryType::Normal)); - Ok(Stmt::Let(Box::new(((name, pos), None)))) + Ok(Stmt::Let(Box::new(((name, pos), None, token_pos)))) } ScopeEntryType::Constant => { state.stack.push((name.clone(), ScopeEntryType::Constant)); - Ok(Stmt::Const(Box::new(((name, pos), Expr::Unit(pos))))) + Ok(Stmt::Const(Box::new(( + (name, pos), + Expr::Unit(pos), + token_pos, + )))) } } } @@ -2539,7 +2580,8 @@ fn parse_import( mut settings: ParseSettings, ) -> Result { // import ... - settings.pos = eat_token(input, Token::Import); + let token_pos = eat_token(input, Token::Import); + settings.pos = token_pos; #[cfg(not(feature = "unchecked"))] settings.ensure_level_within_max_limit(state.max_expr_depth)?; @@ -2569,7 +2611,12 @@ fn parse_import( }; state.modules.push(name.clone()); - Ok(Stmt::Import(Box::new((expr, (name, settings.pos))))) + + Ok(Stmt::Import(Box::new(( + expr, + (name, settings.pos), + token_pos, + )))) } /// Parse an export statement. @@ -2580,7 +2627,8 @@ fn parse_export( _lib: &mut FunctionsLib, mut settings: ParseSettings, ) -> Result { - settings.pos = eat_token(input, Token::Export); + let token_pos = eat_token(input, Token::Export); + settings.pos = token_pos; #[cfg(not(feature = "unchecked"))] settings.ensure_level_within_max_limit(_state.max_expr_depth)?; @@ -2640,7 +2688,7 @@ fn parse_export( }) .map_err(|(id2, pos)| PERR::DuplicatedExport(id2.to_string()).into_err(pos))?; - Ok(Stmt::Export(Box::new(exports))) + Ok(Stmt::Export(Box::new((exports, token_pos)))) } /// Parse a statement block. @@ -2827,22 +2875,32 @@ fn parse_stmt( Token::Continue | Token::Break => Err(PERR::LoopBreak.into_err(settings.pos)), Token::Return | Token::Throw => { - let return_type = match input.next().unwrap() { - (Token::Return, _) => ReturnType::Return, - (Token::Throw, _) => ReturnType::Exception, - _ => unreachable!(), - }; + let (return_type, token_pos) = input + .next() + .map(|(token, pos)| { + ( + match token { + Token::Return => ReturnType::Return, + Token::Throw => ReturnType::Exception, + _ => unreachable!(), + }, + pos, + ) + }) + .unwrap(); match input.peek().unwrap() { // `return`/`throw` at (Token::EOF, pos) => Ok(Some(Stmt::ReturnWithVal(Box::new(( (return_type, *pos), None, + token_pos, ))))), // `return;` or `throw;` (Token::SemiColon, _) => Ok(Some(Stmt::ReturnWithVal(Box::new(( (return_type, settings.pos), None, + token_pos, ))))), // `return` or `throw` with expression (_, _) => { @@ -2852,6 +2910,7 @@ fn parse_stmt( Ok(Some(Stmt::ReturnWithVal(Box::new(( (return_type, pos), Some(expr), + token_pos, ))))) } }