From 9fa4d60336ffe80ac72b40f999008a571455f2ff Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 28 Nov 2021 22:57:28 +0800 Subject: [PATCH] Minor code and docs refactor. --- CHANGELOG.md | 3 +- src/api/call_fn.rs | 6 +-- src/api/deprecated.rs | 26 ++++++++++ src/api/eval.rs | 2 +- src/api/limits.rs | 74 +++++++++++++++++++++++++++++ src/api/mod.rs | 2 +- src/api/run.rs | 2 +- src/ast.rs | 32 +++++-------- src/engine.rs | 95 +++++++++---------------------------- src/func/call.rs | 2 +- src/module/mod.rs | 24 ++++++---- src/packages/array_basic.rs | 9 +++- src/types/scope.rs | 25 +++++----- 13 files changed, 175 insertions(+), 127 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cd61973..83d899cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ Version 1.3.0 Compiler requirement -------------------- -* Minimum compiler version is 1.51. +* Minimum compiler version is bumped to 1.51. Enhancements ------------ @@ -20,6 +20,7 @@ Deprecated API's * The internal `no_smartstring` feature is removed since `SmartString` now supports `no-std`. * `NativeCallContext::new` is deprecated because it is simpler to call a function pointer via `FnPtr::call`. +* `AST::shared_lib` is changed to return `&Shared` while `AST::lib` is deprecated. Version 1.2.1 diff --git a/src/api/call_fn.rs b/src/api/call_fn.rs index b344c1ae..33549155 100644 --- a/src/api/call_fn.rs +++ b/src/api/call_fn.rs @@ -156,7 +156,7 @@ impl Engine { if eval_ast && !statements.is_empty() { // Make sure new variables introduced at global level do not _spill_ into the function call - self.eval_global_statements(scope, mods, state, statements, &[ast.lib()], 0)?; + self.eval_global_statements(scope, mods, state, statements, &[ast.as_ref()], 0)?; if rewind_scope { scope.rewind(orig_scope_len); @@ -169,7 +169,7 @@ impl Engine { let mut args: StaticVec<_> = arg_values.as_mut().iter_mut().collect(); let fn_def = ast - .lib() + .shared_lib() .get_script_fn(name, args.len()) .ok_or_else(|| EvalAltResult::ErrorFunctionNotFound(name.into(), Position::NONE))?; @@ -181,7 +181,7 @@ impl Engine { scope, mods, state, - &[ast.lib()], + &[ast.as_ref()], &mut this_ptr, fn_def, &mut args, diff --git a/src/api/deprecated.rs b/src/api/deprecated.rs index e84579c4..773e18aa 100644 --- a/src/api/deprecated.rs +++ b/src/api/deprecated.rs @@ -230,6 +230,12 @@ impl NativeCallContext<'_> { /// /// If `is_method` is [`true`], the first argument is assumed to be passed /// by reference and is not consumed. + /// + /// # Deprecated + /// + /// This method is deprecated. Use [`call_fn_raw`][NativeCallContext::call_fn_raw] instead. + /// + /// This method will be removed in the next major version. #[deprecated(since = "1.2.0", note = "use `call_fn_raw` instead")] #[inline(always)] pub fn call_fn_dynamic_raw( @@ -250,3 +256,23 @@ impl From for Result> { Err(err.into()) } } + +impl AST { + /// _(internals)_ Get the internal [`Module`] containing all script-defined functions. + /// Exported under the `internals` feature only. + /// + /// Not available under `no_function`. + /// + /// # Deprecated + /// + /// This method is deprecated. Use [`shared_lib`][AST::shared_lib] instead. + /// + /// This method will be removed in the next major version. + #[deprecated(since = "1.3.0", note = "use `shared_lib` instead")] + #[cfg(feature = "internals")] + #[inline(always)] + #[must_use] + pub fn lib(&self) -> &Module { + &self.functions + } +} diff --git a/src/api/eval.rs b/src/api/eval.rs index 42e4dea1..8cd86735 100644 --- a/src/api/eval.rs +++ b/src/api/eval.rs @@ -226,7 +226,7 @@ impl Engine { return Ok(Dynamic::UNIT); } - let lib = &[ast.lib()]; + let lib = &[ast.as_ref()]; self.eval_global_statements(scope, mods, &mut state, statements, lib, level) } } diff --git a/src/api/limits.rs b/src/api/limits.rs index 0e9990e6..6b099824 100644 --- a/src/api/limits.rs +++ b/src/api/limits.rs @@ -7,6 +7,80 @@ use std::prelude::v1::*; use std::num::{NonZeroU64, NonZeroUsize}; +/// _(internals)_ A type containing all the limits imposed by the [`Engine`]. +/// Exported under the `internals` feature only. +/// +/// Not available under `unchecked`. +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub struct Limits { + /// Maximum levels of call-stack to prevent infinite recursion. + /// + /// Set to zero to effectively disable function calls. + /// + /// Not available under `no_function`. + #[cfg(not(feature = "no_function"))] + pub max_call_stack_depth: usize, + /// Maximum depth of statements/expressions at global level. + pub max_expr_depth: Option, + /// Maximum depth of statements/expressions in functions. + /// + /// Not available under `no_function`. + #[cfg(not(feature = "no_function"))] + pub max_function_expr_depth: Option, + /// Maximum number of operations allowed to run. + pub max_operations: Option, + /// Maximum number of [modules][Module] allowed to load. + /// + /// Set to zero to effectively disable loading any [module][Module]. + /// + /// Not available under `no_module`. + #[cfg(not(feature = "no_module"))] + pub max_modules: usize, + /// Maximum length of a [string][ImmutableString]. + pub max_string_size: Option, + /// Maximum length of an [array][Array]. + /// + /// Not available under `no_index`. + #[cfg(not(feature = "no_index"))] + pub max_array_size: Option, + /// Maximum number of properties in an [object map][Map]. + /// + /// Not available under `no_object`. + #[cfg(not(feature = "no_object"))] + pub max_map_size: Option, +} + +impl Limits { + /// Create a new [`Limits`] with default values. + /// + /// Not available under `unchecked`. + #[inline] + pub const fn new() -> Self { + Self { + #[cfg(not(feature = "no_function"))] + max_call_stack_depth: crate::engine::MAX_CALL_STACK_DEPTH, + max_expr_depth: NonZeroUsize::new(crate::engine::MAX_EXPR_DEPTH), + #[cfg(not(feature = "no_function"))] + max_function_expr_depth: NonZeroUsize::new(crate::engine::MAX_FUNCTION_EXPR_DEPTH), + max_operations: None, + #[cfg(not(feature = "no_module"))] + max_modules: usize::MAX, + max_string_size: None, + #[cfg(not(feature = "no_index"))] + max_array_size: None, + #[cfg(not(feature = "no_object"))] + max_map_size: None, + } + } +} + +impl Default for Limits { + #[inline(always)] + fn default() -> Self { + Self::new() + } +} + impl Engine { /// Set the maximum levels of function calls allowed for a script in order to avoid /// infinite recursion and stack overflows. diff --git a/src/api/mod.rs b/src/api/mod.rs index 14248fe6..875db590 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -75,7 +75,7 @@ impl Engine { #[cfg(not(feature = "no_function"))] let lib = ast - .lib() + .shared_lib() .iter_fn() .filter(|f| f.func.is_script()) .map(|f| { diff --git a/src/api/run.rs b/src/api/run.rs index 07d6b6cc..844ad074 100644 --- a/src/api/run.rs +++ b/src/api/run.rs @@ -64,7 +64,7 @@ impl Engine { let statements = ast.statements(); if !statements.is_empty() { - let lib = &[ast.lib()]; + let lib = &[ast.as_ref()]; self.eval_global_statements(scope, mods, &mut state, statements, lib, 0)?; } Ok(()) diff --git a/src/ast.rs b/src/ast.rs index a5e1891f..f3fff209 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -302,8 +302,8 @@ impl AST { #[cfg(not(feature = "no_function"))] #[inline(always)] #[must_use] - pub(crate) fn shared_lib(&self) -> Shared { - self.functions.clone() + pub(crate) fn shared_lib(&self) -> &Shared { + &self.functions } /// _(internals)_ Get the internal shared [`Module`] containing all script-defined functions. /// Exported under the `internals` feature only. @@ -314,24 +314,7 @@ impl AST { #[cfg(not(feature = "no_function"))] #[inline(always)] #[must_use] - pub fn shared_lib(&self) -> Shared { - self.functions.clone() - } - /// Get the internal [`Module`] containing all script-defined functions. - #[cfg(not(feature = "internals"))] - #[inline(always)] - #[must_use] - pub(crate) fn lib(&self) -> &Module { - &self.functions - } - /// _(internals)_ Get the internal [`Module`] containing all script-defined functions. - /// Exported under the `internals` feature only. - /// - /// Not available under `no_function`. - #[cfg(feature = "internals")] - #[inline(always)] - #[must_use] - pub fn lib(&self) -> &Module { + pub fn shared_lib(&self) -> &Shared { &self.functions } /// Get the embedded [module resolver][`ModuleResolver`]. @@ -895,7 +878,14 @@ impl AsRef<[Stmt]> for AST { impl AsRef for AST { #[inline(always)] fn as_ref(&self) -> &Module { - self.lib() + self.shared_lib().as_ref() + } +} + +impl AsRef> for AST { + #[inline(always)] + fn as_ref(&self) -> &Shared { + self.shared_lib() } } diff --git a/src/engine.rs b/src/engine.rs index 0de9549c..ac5956bc 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -65,7 +65,9 @@ pub struct Imports { pub num_operations: u64, /// Number of modules loaded. pub num_modules: usize, - /// Function call hashes to FN_IDX_GET and FN_IDX_SET. + /// Function call hashes to index getters and setters. + /// + /// Not available under `no_index` and `no_object`. #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] fn_hash_indexing: (u64, u64), /// Embedded module resolver. @@ -74,11 +76,20 @@ pub struct Imports { #[cfg(not(feature = "no_module"))] pub embedded_module_resolver: Option>, /// Cache of globally-defined constants. + /// + /// Not available under `no_module` and `no_function`. #[cfg(not(feature = "no_module"))] #[cfg(not(feature = "no_function"))] global_constants: Option>>>, } +impl Default for Imports { + #[inline(always)] + fn default() -> Self { + Self::new() + } +} + impl Imports { /// Create a new stack of imported [modules][Module]. #[inline(always)] @@ -129,12 +140,15 @@ impl Imports { #[must_use] pub fn find(&self, name: impl AsRef) -> Option { let name = name.as_ref(); + let len = self.keys.len(); - self.keys - .iter() - .enumerate() - .rev() - .find_map(|(i, key)| if key == name { Some(i) } else { None }) + self.keys.iter().rev().enumerate().find_map(|(i, key)| { + if key == name { + Some(len - 1 - i) + } else { + None + } + }) } /// Push an imported [module][Module] onto the stack. #[inline(always)] @@ -154,8 +168,8 @@ impl Imports { pub fn iter(&self) -> impl Iterator { self.keys .iter() - .zip(self.modules.iter()) .rev() + .zip(self.modules.iter().rev()) .map(|(name, module)| (name.as_str(), module.as_ref())) } /// Get an iterator to this stack of imported [modules][Module] in reverse order. @@ -812,69 +826,6 @@ impl EvalState { } } -/// _(internals)_ A type containing all the limits imposed by the [`Engine`]. -/// Exported under the `internals` feature only. -#[cfg(not(feature = "unchecked"))] -#[derive(Debug, Clone, Eq, PartialEq, Hash)] -pub struct Limits { - /// Maximum levels of call-stack to prevent infinite recursion. - /// - /// Set to zero to effectively disable function calls. - /// - /// Not available under `no_function`. - #[cfg(not(feature = "no_function"))] - pub max_call_stack_depth: usize, - /// Maximum depth of statements/expressions at global level. - pub max_expr_depth: Option, - /// Maximum depth of statements/expressions in functions. - /// - /// Not available under `no_function`. - #[cfg(not(feature = "no_function"))] - pub max_function_expr_depth: Option, - /// Maximum number of operations allowed to run. - pub max_operations: Option, - /// Maximum number of [modules][Module] allowed to load. - /// - /// Set to zero to effectively disable loading any [module][Module]. - /// - /// Not available under `no_module`. - #[cfg(not(feature = "no_module"))] - pub max_modules: usize, - /// Maximum length of a [string][ImmutableString]. - pub max_string_size: Option, - /// Maximum length of an [array][Array]. - /// - /// Not available under `no_index`. - #[cfg(not(feature = "no_index"))] - pub max_array_size: Option, - /// Maximum number of properties in an [object map][Map]. - /// - /// Not available under `no_object`. - #[cfg(not(feature = "no_object"))] - pub max_map_size: Option, -} - -#[cfg(not(feature = "unchecked"))] -impl Limits { - pub const fn new() -> Self { - Self { - #[cfg(not(feature = "no_function"))] - max_call_stack_depth: MAX_CALL_STACK_DEPTH, - max_expr_depth: NonZeroUsize::new(MAX_EXPR_DEPTH), - #[cfg(not(feature = "no_function"))] - max_function_expr_depth: NonZeroUsize::new(MAX_FUNCTION_EXPR_DEPTH), - max_operations: None, - #[cfg(not(feature = "no_module"))] - max_modules: usize::MAX, - max_string_size: None, - #[cfg(not(feature = "no_index"))] - max_array_size: None, - #[cfg(not(feature = "no_object"))] - max_map_size: None, - } - } -} - /// Context of a script evaluation process. #[derive(Debug)] pub struct EvalContext<'a, 'x, 'px, 'm, 's, 'b, 't, 'pt> { @@ -1024,7 +975,7 @@ pub struct Engine { /// Max limits. #[cfg(not(feature = "unchecked"))] - pub(crate) limits: Limits, + pub(crate) limits: crate::api::limits::Limits, } impl fmt::Debug for Engine { @@ -1146,7 +1097,7 @@ impl Engine { optimization_level: crate::OptimizationLevel::default(), #[cfg(not(feature = "unchecked"))] - limits: Limits::new(), + limits: crate::api::limits::Limits::new(), }; // Add the global namespace module diff --git a/src/func/call.rs b/src/func/call.rs index 3866497d..8d768f79 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -886,7 +886,7 @@ impl Engine { )?; // If new functions are defined within the eval string, it is an error - if !ast.lib().is_empty() { + if !ast.shared_lib().is_empty() { return Err(ParseErrorType::WrongFnDefinition.into()); } diff --git a/src/module/mod.rs b/src/module/mod.rs index 1b11150e..62f2ed51 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -190,13 +190,6 @@ impl fmt::Debug for Module { } } -impl AsRef for Module { - #[inline(always)] - fn as_ref(&self) -> &Module { - self - } -} - impl> Add for &Module { type Output = Module; @@ -1467,8 +1460,8 @@ impl Module { // Non-private functions defined become module functions #[cfg(not(feature = "no_function"))] - if !ast.lib().functions.is_empty() { - ast.lib() + if !ast.shared_lib().functions.is_empty() { + ast.shared_lib() .functions .values() .filter(|f| match f.access { @@ -1484,7 +1477,7 @@ impl Module { .expect("scripted function") .as_ref() .clone(); - func.lib = Some(ast.shared_lib()); + func.lib = Some(ast.shared_lib().clone()); func.mods = func_mods.clone(); module.set_script_fn(func); }); @@ -1728,6 +1721,17 @@ impl DerefMut for NamespaceRef { } } +impl From> for NamespaceRef { + #[inline(always)] + fn from(mut path: Vec) -> Self { + path.shrink_to_fit(); + Self { + index: None, + path: path.into(), + } + } +} + impl From> for NamespaceRef { #[inline(always)] fn from(mut path: StaticVec) -> Self { diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index daa95728..fd847129 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -728,8 +728,9 @@ mod array_functions { } let mut result = initial; + let len = array.len(); - for (i, item) in array.iter().enumerate().rev() { + for (i, item) in array.iter().rev().enumerate() { let item = item.clone(); result = reducer @@ -738,7 +739,11 @@ mod array_functions { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.starts_with(reducer.fn_name()) => { - reducer.call_dynamic(&ctx, None, [result, item, (i as INT).into()]) + reducer.call_dynamic( + &ctx, + None, + [result, item, ((len - 1 - i) as INT).into()], + ) } _ => Err(err), }) diff --git a/src/types/scope.rs b/src/types/scope.rs index 09fbdc6b..50bf3c69 100644 --- a/src/types/scope.rs +++ b/src/types/scope.rs @@ -301,22 +301,21 @@ impl<'a> Scope<'a> { #[inline] #[must_use] pub fn contains(&self, name: &str) -> bool { - self.names - .iter() - .rev() // Always search a Scope in reverse order - .any(|(key, _)| name == key.as_ref()) + self.names.iter().any(|(key, _)| name == key.as_ref()) } /// Find an entry in the [`Scope`], starting from the last. #[inline] #[must_use] pub(crate) fn get_index(&self, name: &str) -> Option<(usize, AccessMode)> { + let len = self.len(); + self.names .iter() .rev() // Always search a Scope in reverse order .enumerate() - .find_map(|(index, (key, _))| { + .find_map(|(i, (key, _))| { if name == key.as_ref() { - let index = self.len() - 1 - index; + let index = len - 1 - i; Some((index, self.values[index].access_mode())) } else { None @@ -338,16 +337,14 @@ impl<'a> Scope<'a> { #[inline] #[must_use] pub fn get_value(&self, name: &str) -> Option { + let len = self.len(); + self.names .iter() .rev() .enumerate() .find(|(_, (key, _))| name == key.as_ref()) - .and_then(|(index, _)| { - self.values[self.len() - 1 - index] - .flatten_clone() - .try_cast() - }) + .and_then(|(index, _)| self.values[len - 1 - index].flatten_clone().try_cast()) } /// Check if the named entry in the [`Scope`] is constant. /// @@ -522,14 +519,14 @@ impl<'a> Scope<'a> { #[inline] #[must_use] pub fn clone_visible(&self) -> Self { + let len = self.len(); + self.names.iter().rev().enumerate().fold( Self::new(), |mut entries, (index, (name, alias))| { if !entries.names.iter().any(|(key, _)| key == name) { entries.names.push((name.clone(), alias.clone())); - entries - .values - .push(self.values[self.len() - 1 - index].clone()); + entries.values.push(self.values[len - 1 - index].clone()); } entries },