From c55fc5a9a5d0a4d529ac523f02ace10902ba078a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 1 Nov 2020 22:46:46 +0800 Subject: [PATCH 1/8] Optimize Scope. --- src/engine.rs | 2 +- src/fn_call.rs | 28 ++----- src/module/mod.rs | 16 ++-- src/optimize.rs | 27 +++---- src/scope.rs | 182 +++++++++++++++++++++------------------------- 5 files changed, 109 insertions(+), 146 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index ee635ed8..e7544d56 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -2070,7 +2070,7 @@ impl Engine { } else { unsafe_cast_var_name_to_lifetime(&var_def.name).into() }; - scope.push_dynamic_value(var_name, entry_type, val, false); + scope.push_dynamic_value(var_name, entry_type, val); Ok(Default::default()) } diff --git a/src/fn_call.rs b/src/fn_call.rs index 0ae48900..f5b0a190 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -36,10 +36,6 @@ use crate::engine::{Map, Target, FN_GET, FN_SET}; #[cfg(not(feature = "no_closure"))] use crate::engine::KEYWORD_IS_SHARED; -#[cfg(not(feature = "no_closure"))] -#[cfg(not(feature = "no_function"))] -use crate::scope::Entry as ScopeEntry; - use crate::stdlib::{ any::{type_name, TypeId}, boxed::Box, @@ -553,22 +549,14 @@ impl Engine { if let Some(captured) = _capture_scope { captured .into_iter() - .filter(|ScopeEntry { name, .. }| { - func.externals.contains(name.as_ref()) - }) - .for_each( - |ScopeEntry { - name, typ, value, .. - }| { - // Consume the scope values. - match typ { - ScopeEntryType::Normal => scope.push(name, value), - ScopeEntryType::Constant => { - scope.push_constant(name, value) - } - }; - }, - ); + .filter(|(name, _, _, _)| func.externals.contains(name.as_ref())) + .for_each(|(name, typ, value, _)| { + // Consume the scope values. + match typ { + ScopeEntryType::Normal => scope.push(name, value), + ScopeEntryType::Constant => scope.push_constant(name, value), + }; + }); } let result = if _is_method { diff --git a/src/module/mod.rs b/src/module/mod.rs index 79dbe625..4766ebce 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -16,7 +16,7 @@ use crate::{ast::ScriptFnDef, fn_native::Shared}; use crate::{ ast::AST, engine::{Engine, Imports}, - scope::{Entry as ScopeEntry, Scope}, + scope::Scope, }; #[cfg(not(feature = "no_index"))] @@ -1348,14 +1348,12 @@ impl Module { // Create new module let mut module = Module::new(); - scope - .into_iter() - .for_each(|ScopeEntry { value, alias, .. }| { - // Variables with an alias left in the scope become module variables - if let Some(alias) = alias { - module.variables.insert(*alias, value); - } - }); + scope.into_iter().for_each(|(_, _, value, alias)| { + // Variables with an alias left in the scope become module variables + if let Some(alias) = alias { + module.variables.insert(alias, value); + } + }); // Modules left in the scope become sub-modules mods.into_iter().for_each(|(alias, m)| { diff --git a/src/optimize.rs b/src/optimize.rs index e482c805..1e6d1eaf 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -9,7 +9,7 @@ use crate::engine::{ use crate::fn_call::run_builtin_binary_op; use crate::module::Module; use crate::parser::map_dynamic_to_expr; -use crate::scope::{Entry as ScopeEntry, Scope}; +use crate::scope::Scope; use crate::token::{is_valid_identifier, Position}; use crate::{calc_native_fn_hash, StaticVec}; @@ -721,24 +721,15 @@ fn optimize( // Set up the state let mut state = State::new(engine, lib, level); - // Add constants from the scope into the state + // Add constants from the scope that can be made into a literal into the state scope - .to_iter() - // Get all the constants that can be made into a constant literal. - .filter(|ScopeEntry { typ, .. }| typ.is_constant()) - .for_each( - |ScopeEntry { - name, expr, value, .. - }| { - if let Some(val) = expr - .as_ref() - .map(|expr| expr.as_ref().clone()) - .or_else(|| map_dynamic_to_expr(value.clone(), Position::none())) - { - state.push_constant(name.as_ref(), val); - } - }, - ); + .iter() + .filter(|(_, typ, _)| *typ) + .for_each(|(name, _, value)| { + if let Some(val) = map_dynamic_to_expr(value.clone(), Position::none()) { + state.push_constant(name, val); + } + }); let orig_constants_len = state.constants.len(); diff --git a/src/scope.rs b/src/scope.rs index 13442a3b..77e45a87 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -1,11 +1,8 @@ //! Module that defines the `Scope` type representing a function call-stack scope. -use crate::ast::Expr; use crate::dynamic::{Dynamic, Variant}; -use crate::parser::map_dynamic_to_expr; -use crate::token::Position; -use crate::stdlib::{borrow::Cow, boxed::Box, iter, string::String, vec::Vec}; +use crate::stdlib::{borrow::Cow, iter, string::String, vec::Vec}; /// Type of an entry in the Scope. #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -27,21 +24,6 @@ impl EntryType { } } -/// An entry in the Scope. -#[derive(Debug, Clone)] -pub struct Entry<'a> { - /// Name of the entry. - pub name: Cow<'a, str>, - /// Type of the entry. - pub typ: EntryType, - /// Current value of the entry. - pub value: Dynamic, - /// Alias of the entry. - pub alias: Option>, - /// A constant expression if the initial value matches one of the recognized types. - pub expr: Option>, -} - /// Type containing information about the current scope. /// Useful for keeping state between `Engine` evaluation runs. /// @@ -72,8 +54,25 @@ pub struct Entry<'a> { /// /// When searching for entries, newly-added entries are found before similarly-named but older entries, /// allowing for automatic _shadowing_. +// +// # Implementation Notes +// +// `Scope` is implemented as three `Vec`'s of exactly the same length. Variables data (name, type, etc.) +// is manually split into three equal-length arrays. That's because variable names take up the most space, +// with `Cow` being four words long, but in the vast majority of cases the name is NOT used to look up +// a variable's value. Variable lookup is usually via direct index, by-passing the name altogether. +// +// Since `Dynamic` is reasonably small, packing it tightly improves cache locality when variables are accessed. +// The variable type is packed separately into another array because it is even smaller. #[derive(Debug, Clone, Default)] -pub struct Scope<'a>(Vec>); +pub struct Scope<'a> { + /// (Name, alias) of the entry. + names: Vec<(Cow<'a, str>, Option)>, + /// Type of the entry. + types: Vec, + /// Current value of the entry. + values: Vec, +} impl<'a> Scope<'a> { /// Create a new Scope. @@ -114,7 +113,9 @@ impl<'a> Scope<'a> { /// ``` #[inline(always)] pub fn clear(&mut self) -> &mut Self { - self.0.clear(); + self.names.clear(); + self.types.clear(); + self.values.clear(); self } @@ -133,7 +134,7 @@ impl<'a> Scope<'a> { /// ``` #[inline(always)] pub fn len(&self) -> usize { - self.0.len() + self.values.len() } /// Is the Scope empty? @@ -151,7 +152,7 @@ impl<'a> Scope<'a> { /// ``` #[inline(always)] pub fn is_empty(&self) -> bool { - self.0.len() == 0 + self.values.len() == 0 } /// Add (push) a new entry to the Scope. @@ -172,7 +173,7 @@ impl<'a> Scope<'a> { name: K, value: T, ) -> &mut Self { - self.push_dynamic_value(name, EntryType::Normal, Dynamic::from(value), false) + self.push_dynamic_value(name, EntryType::Normal, Dynamic::from(value)) } /// Add (push) a new `Dynamic` entry to the Scope. @@ -189,7 +190,7 @@ impl<'a> Scope<'a> { /// ``` #[inline(always)] pub fn push_dynamic>>(&mut self, name: K, value: Dynamic) -> &mut Self { - self.push_dynamic_value(name, EntryType::Normal, value, false) + self.push_dynamic_value(name, EntryType::Normal, value) } /// Add (push) a new constant to the Scope. @@ -216,7 +217,7 @@ impl<'a> Scope<'a> { name: K, value: T, ) -> &mut Self { - self.push_dynamic_value(name, EntryType::Constant, Dynamic::from(value), true) + self.push_dynamic_value(name, EntryType::Constant, Dynamic::from(value)) } /// Add (push) a new constant with a `Dynamic` value to the Scope. @@ -244,7 +245,7 @@ impl<'a> Scope<'a> { name: K, value: Dynamic, ) -> &mut Self { - self.push_dynamic_value(name, EntryType::Constant, value, true) + self.push_dynamic_value(name, EntryType::Constant, value) } /// Add (push) a new entry with a `Dynamic` value to the Scope. @@ -254,22 +255,10 @@ impl<'a> Scope<'a> { name: K, entry_type: EntryType, value: Dynamic, - map_expr: bool, ) -> &mut Self { - let expr = if map_expr { - map_dynamic_to_expr(value.clone(), Position::none()).map(Box::new) - } else { - None - }; - - self.0.push(Entry { - name: name.into(), - typ: entry_type, - alias: None, - value: value.into(), - expr, - }); - + self.names.push((name.into(), None)); + self.types.push(entry_type); + self.values.push(value.into()); self } @@ -301,7 +290,9 @@ impl<'a> Scope<'a> { /// ``` #[inline(always)] pub fn rewind(&mut self, size: usize) -> &mut Self { - self.0.truncate(size); + self.names.truncate(size); + self.types.truncate(size); + self.values.truncate(size); self } @@ -320,37 +311,28 @@ impl<'a> Scope<'a> { /// ``` #[inline(always)] pub fn contains(&self, name: &str) -> bool { - self.0 + self.names .iter() .rev() // Always search a Scope in reverse order - .any(|Entry { name: key, .. }| name == key) + .any(|(key, _)| name == key.as_ref()) } /// Find an entry in the Scope, starting from the last. #[inline(always)] pub(crate) fn get_index(&self, name: &str) -> Option<(usize, EntryType)> { - self.0 + self.names .iter() .enumerate() .rev() // Always search a Scope in reverse order - .find_map(|(index, Entry { name: key, typ, .. })| { - if name == key { - Some((index, *typ)) + .find_map(|(index, (key, _))| { + if name == key.as_ref() { + Some((index, self.types[index])) } else { None } }) } - /// Get an entry in the Scope, starting from the last. - #[inline(always)] - pub(crate) fn get_entry(&self, name: &str) -> Option<&Entry> { - self.0 - .iter() - .rev() - .find(|Entry { name: key, .. }| name == key) - } - /// Get the value of an entry in the Scope, starting from the last. /// /// # Example @@ -365,8 +347,12 @@ impl<'a> Scope<'a> { /// ``` #[inline(always)] pub fn get_value(&self, name: &str) -> Option { - self.get_entry(name) - .and_then(|Entry { value, .. }| value.flatten_clone().try_cast()) + self.names + .iter() + .enumerate() + .rev() + .find(|(_, (key, _))| name == key.as_ref()) + .and_then(|(index, _)| self.values[index].flatten_clone().try_cast()) } /// Update the value of the named entry. @@ -398,7 +384,7 @@ impl<'a> Scope<'a> { } Some((_, EntryType::Constant)) => panic!("variable {} is constant", name), Some((index, EntryType::Normal)) => { - self.0.get_mut(index).unwrap().value = Dynamic::from(value); + *self.values.get_mut(index).unwrap() = Dynamic::from(value); } } self @@ -407,16 +393,18 @@ impl<'a> Scope<'a> { /// Get a mutable reference to an entry in the Scope. #[inline(always)] pub(crate) fn get_mut(&mut self, index: usize) -> (&mut Dynamic, EntryType) { - let entry = self.0.get_mut(index).expect("invalid index in Scope"); - (&mut entry.value, entry.typ) + ( + self.values.get_mut(index).expect("invalid index in Scope"), + self.types[index], + ) } /// Update the access type of an entry in the Scope. #[cfg(not(feature = "no_module"))] #[inline(always)] pub(crate) fn set_entry_alias(&mut self, index: usize, alias: String) -> &mut Self { - let entry = self.0.get_mut(index).expect("invalid index in Scope"); - entry.alias = Some(Box::new(alias)); + let entry = self.names.get_mut(index).expect("invalid index in Scope"); + entry.1 = Some(alias); self } @@ -424,34 +412,36 @@ impl<'a> Scope<'a> { /// Shadowed variables are omitted in the copy. #[inline] pub(crate) fn clone_visible(&self) -> Self { - let mut entries: Vec = Default::default(); + let mut entries: Self = Default::default(); - self.0.iter().rev().for_each(|entry| { - if entries - .iter() - .find(|Entry { name, .. }| &entry.name == name) - .is_none() - { - entries.push(entry.clone()); - } - }); + self.names + .iter() + .enumerate() + .rev() + .for_each(|(index, (name, alias))| { + if !entries.names.iter().any(|(key, _)| key == name) { + entries.names.push((name.clone(), alias.clone())); + entries.types.push(self.types[index]); + entries.values.push(self.values[index].clone()); + } + }); - Self(entries) + entries } /// Get an iterator to entries in the Scope. #[inline(always)] - pub(crate) fn into_iter(self) -> impl Iterator> { - self.0.into_iter() - } - - /// Get an iterator to entries in the Scope in reverse order. - #[inline(always)] - pub(crate) fn to_iter(&self) -> impl Iterator { - self.0.iter().rev() // Always search a Scope in reverse order + pub(crate) fn into_iter( + self, + ) -> impl Iterator, EntryType, Dynamic, Option)> { + self.names + .into_iter() + .zip(self.types.into_iter().zip(self.values.into_iter())) + .map(|((name, alias), (typ, value))| (name, typ, value, alias)) } /// Get an iterator to entries in the Scope. + /// Shared values are flatten-cloned. /// /// # Example /// @@ -484,25 +474,21 @@ impl<'a> Scope<'a> { /// Get an iterator to entries in the Scope. /// Shared values are not expanded. #[inline(always)] - pub fn iter_raw(&self) -> impl Iterator { - self.0.iter().map( - |Entry { - name, typ, value, .. - }| { (name.as_ref(), typ.is_constant(), value) }, - ) + pub fn iter_raw<'x: 'a>(&'x self) -> impl Iterator + 'x { + self.names + .iter() + .zip(self.types.iter().zip(self.values.iter())) + .map(|((name, _), (typ, value))| (name.as_ref(), typ.is_constant(), value)) } } impl<'a, K: Into>> iter::Extend<(K, EntryType, Dynamic)> for Scope<'a> { #[inline(always)] fn extend>(&mut self, iter: T) { - self.0 - .extend(iter.into_iter().map(|(name, typ, value)| Entry { - name: name.into(), - typ, - alias: None, - value: value.into(), - expr: None, - })); + iter.into_iter().for_each(|(name, typ, value)| { + self.names.push((name.into(), None)); + self.types.push(typ); + self.values.push(value); + }); } } From 717e8e7eee22c85ac0d14d4bc5f779cf908bfcd5 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 1 Nov 2020 22:55:19 +0800 Subject: [PATCH 2/8] Remove unnecessary clone. --- src/optimize.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/optimize.rs b/src/optimize.rs index 1e6d1eaf..def61077 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -726,7 +726,7 @@ fn optimize( .iter() .filter(|(_, typ, _)| *typ) .for_each(|(name, _, value)| { - if let Some(val) = map_dynamic_to_expr(value.clone(), Position::none()) { + if let Some(val) = map_dynamic_to_expr(value, Position::none()) { state.push_constant(name, val); } }); From b07a2aa79c5e58da61aa74a0ec293ae359275e59 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 1 Nov 2020 23:42:00 +0800 Subject: [PATCH 3/8] Pack Imports. --- doc/src/language/modules/import.md | 8 +++ src/engine.rs | 79 ++++++++++++++++++++++++------ src/module/mod.rs | 8 +-- 3 files changed, 73 insertions(+), 22 deletions(-) diff --git a/doc/src/language/modules/import.md b/doc/src/language/modules/import.md index 44ee5412..ed8918da 100644 --- a/doc/src/language/modules/import.md +++ b/doc/src/language/modules/import.md @@ -25,6 +25,14 @@ import "crypto_init"; // run the script file 'crypto_init.rhai' withou import "crypto" as lock; // run the script file 'crypto.rhai' and import it as a module named 'lock' +const SECRET_NUMBER = 42; + +let mod_file = "crypto_" + SECRET_NUMBER; + +import mod_file as my_mod; // load the script file "crypto_42.rhai" and import it as a module named 'my_mod' + // notice that module path names can be dynamically constructed! + // any expression that evaluates to a string is acceptable after the 'import' keyword + lock::encrypt(secret); // use functions defined under the module via '::' lock::hash::sha256(key); // sub-modules are also supported diff --git a/src/engine.rs b/src/engine.rs index e7544d56..8be258f6 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -69,10 +69,58 @@ pub type Map = HashMap; /// /// This type is volatile and may change. // -// Note - We cannot use &str or Cow here because `eval` may load a module -// and the module name will live beyond the AST of the eval script text. -// The best we can do is a shared reference. -pub type Imports = Vec<(ImmutableString, Module)>; +// # Implementation Notes +// +// We cannot use &str or Cow here because `eval` may load a module and the module name will live beyond +// the AST of the eval script text. The best we can do is a shared reference. +// +// `Imports` is implemented as two `Vec`'s of exactly the same length. That's because a `Module` is large, +// so packing the import names together improves cache locality. +#[derive(Debug, Clone, Default)] +pub struct Imports(StaticVec, StaticVec); + +impl Imports { + /// Get the length of this stack of imported modules. + pub fn len(&self) -> usize { + self.0.len() + } + /// Get the imported module at a particular index. + pub fn get(&self, index: usize) -> Option<&Module> { + self.1.get(index) + } + /// Get a mutable reference to the imported module at a particular index. + pub fn get_mut(&mut self, index: usize) -> Option<&mut Module> { + self.1.get_mut(index) + } + /// Get the index of an imported module by name. + pub fn find(&self, name: &str) -> Option { + self.0 + .iter() + .enumerate() + .rev() + .find(|(_, key)| key.as_str() == name) + .map(|(index, _)| index) + } + /// Push an imported module onto the stack. + pub fn push(&mut self, name: impl Into, module: Module) { + self.0.push(name.into()); + self.1.push(module); + } + /// Truncate the stack of imported modules to a particular length. + pub fn truncate(&mut self, size: usize) { + self.0.truncate(size); + self.1.truncate(size); + } + /// Get an iterator to this stack of imported modules. + #[allow(dead_code)] + pub fn iter(&self) -> impl Iterator { + self.0.iter().map(|name| name.as_str()).zip(self.1.iter()) + } + /// Get a consuming iterator to this stack of imported modules. + pub fn into_iter(self) -> impl Iterator { + self.0.into_iter().zip(self.1.into_iter()) + } +} #[cfg(not(feature = "unchecked"))] #[cfg(debug_assertions)] @@ -602,12 +650,10 @@ pub fn search_imports<'s>( Ok(if index > 0 { let offset = mods.len() - index; - &mods.get(offset).unwrap().1 + mods.get(offset).expect("invalid index in Imports") } else { - mods.iter() - .rev() - .find(|(n, _)| n == root) - .map(|(_, m)| m) + mods.find(root) + .map(|n| mods.get(n).expect("invalid index in Imports")) .ok_or_else(|| EvalAltResult::ErrorModuleNotFound(root.to_string(), *pos))? }) } @@ -630,13 +676,14 @@ pub fn search_imports_mut<'s>( Ok(if index > 0 { let offset = mods.len() - index; - &mut mods.get_mut(offset).unwrap().1 + mods.get_mut(offset).expect("invalid index in Imports") } else { - mods.iter_mut() - .rev() - .find(|(n, _)| n == root) - .map(|(_, m)| m) - .ok_or_else(|| EvalAltResult::ErrorModuleNotFound(root.to_string(), *pos))? + if let Some(n) = mods.find(root) { + mods.get_mut(n) + } else { + None + } + .ok_or_else(|| EvalAltResult::ErrorModuleNotFound(root.to_string(), *pos))? }) } @@ -2092,7 +2139,7 @@ impl Engine { if let Some(name_def) = alias { module.index_all_sub_modules(); - mods.push((name_def.name.clone(), module)); + mods.push(name_def.name.clone(), module); } state.modules += 1; diff --git a/src/module/mod.rs b/src/module/mod.rs index 4766ebce..9b6f135e 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -13,11 +13,7 @@ use crate::{calc_native_fn_hash, calc_script_fn_hash, StaticVec}; use crate::{ast::ScriptFnDef, fn_native::Shared}; #[cfg(not(feature = "no_module"))] -use crate::{ - ast::AST, - engine::{Engine, Imports}, - scope::Scope, -}; +use crate::{ast::AST, engine::Engine, scope::Scope}; #[cfg(not(feature = "no_index"))] use crate::engine::{Array, FN_IDX_GET, FN_IDX_SET}; @@ -1340,7 +1336,7 @@ impl Module { ast: &AST, engine: &Engine, ) -> Result> { - let mut mods = Imports::new(); + let mut mods = Default::default(); // Run the script engine.eval_ast_with_scope_raw(&mut scope, &mut mods, &ast)?; From 6f3ce96d9d0eea8d37f9e988102460a5a0f4d4ca Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 2 Nov 2020 11:04:45 +0800 Subject: [PATCH 4/8] Enable termination token. --- RELEASES.md | 2 + doc/src/safety/progress.md | 7 +-- src/engine.rs | 23 +++------- src/engine_api.rs | 8 ++-- src/fn_native.rs | 7 ++- src/module/mod.rs | 5 +-- src/packages/array_basic.rs | 90 +++++++++++++++---------------------- src/packages/string_more.rs | 26 +++-------- src/result.rs | 36 +++++++-------- 9 files changed, 82 insertions(+), 122 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index d70fcdfd..06d3f9f8 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -20,6 +20,8 @@ Breaking changes * Custom syntax can no longer start with a keyword (even a _reserved_ one), even if it has been disabled. That is to avoid breaking scripts later when the keyword is no longer disabled. * `EvalAltResult::ErrorAssignmentToUnknownLHS` is moved to `ParseError::AssignmentToInvalidLHS`. `ParseError::AssignmentToCopy` is removed. +* `EvalAltResult::ErrorDataTooLarge` is simplified. +* `Engine::on_progress` closure signature now returns `Option` with the termination value passed on to `EvalAltResult::ErrorTerminated`. New features ------------ diff --git a/doc/src/safety/progress.md b/doc/src/safety/progress.md index c34eed7c..99158470 100644 --- a/doc/src/safety/progress.md +++ b/doc/src/safety/progress.md @@ -17,13 +17,14 @@ engine.on_progress(|&count| { // parameter is '&u64' - number of operations al if count % 1000 == 0 { println!("{}", count); // print out a progress log every 1,000 operations } - true // return 'true' to continue running the script - // return 'false' to immediately terminate the script + None // return 'None' to continue running the script + // return 'Some(token)' to immediately terminate the script }); ``` The closure passed to `Engine::on_progress` will be called once for every operation. -Return `false` to terminate the script immediately. +Return `Some(token)` to terminate the script immediately, with the provided value +(any [`Dynamic`] value) passed to `EvalAltResult::ErrorTerminated` as a termination token. Operations Count vs. Progress Percentage diff --git a/src/engine.rs b/src/engine.rs index 8be258f6..494d2e3a 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -576,7 +576,7 @@ pub struct Engine { /// Callback closure for implementing the `debug` command. pub(crate) debug: Callback, /// Callback closure for progress reporting. - pub(crate) progress: Option>, + pub(crate) progress: Option>>, /// Optimize the AST after compilation. pub(crate) optimization_level: OptimizationLevel, @@ -2300,8 +2300,6 @@ impl Engine { if s > self.max_string_size() { return EvalAltResult::ErrorDataTooLarge( "Length of string".to_string(), - self.max_string_size(), - s, Position::none(), ) .into(); @@ -2309,21 +2307,14 @@ impl Engine { #[cfg(not(feature = "no_index"))] if _arr > self.max_array_size() { - return EvalAltResult::ErrorDataTooLarge( - "Size of array".to_string(), - self.max_array_size(), - _arr, - Position::none(), - ) - .into(); + return EvalAltResult::ErrorDataTooLarge("Size of array".to_string(), Position::none()) + .into(); } #[cfg(not(feature = "no_object"))] if _map > self.max_map_size() { return EvalAltResult::ErrorDataTooLarge( - "Number of properties in object map".to_string(), - self.max_map_size(), - _map, + "Size of object map".to_string(), Position::none(), ) .into(); @@ -2345,9 +2336,9 @@ impl Engine { // Report progress - only in steps if let Some(progress) = &self.progress { - if !progress(&state.operations) { - // Terminate script if progress returns false - return EvalAltResult::ErrorTerminated(Position::none()).into(); + if let Some(token) = progress(&state.operations) { + // Terminate script if progress returns a termination token + return EvalAltResult::ErrorTerminated(token, Position::none()).into(); } } diff --git a/src/engine_api.rs b/src/engine_api.rs index 36e664dc..26fd57ee 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1732,12 +1732,12 @@ impl Engine { /// /// engine.on_progress(move |&ops| { /// if ops > 10000 { - /// false + /// Some("Over 10,000 operations!".into()) /// } else if ops % 800 == 0 { /// *logger.write().unwrap() = ops; - /// true + /// None /// } else { - /// true + /// None /// } /// }); /// @@ -1752,7 +1752,7 @@ impl Engine { #[inline(always)] pub fn on_progress( &mut self, - callback: impl Fn(&u64) -> bool + SendSync + 'static, + callback: impl Fn(&u64) -> Option + SendSync + 'static, ) -> &mut Self { self.progress = Some(Box::new(callback)); self diff --git a/src/fn_native.rs b/src/fn_native.rs index db95c6b3..312af42b 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -160,7 +160,7 @@ impl FnPtr { /// clone them _before_ calling this function. pub fn call_dynamic( &self, - context: NativeCallContext, + ctx: NativeCallContext, this_ptr: Option<&mut Dynamic>, mut arg_values: impl AsMut<[Dynamic]>, ) -> Result> { @@ -182,11 +182,10 @@ impl FnPtr { args.insert(0, obj); } - context - .engine() + ctx.engine() .exec_fn_call( &mut Default::default(), - context.lib, + ctx.lib, fn_name, hash_script, args.as_mut(), diff --git a/src/module/mod.rs b/src/module/mod.rs index 9b6f135e..a8714a4d 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -529,9 +529,8 @@ impl Module { + SendSync + 'static, ) -> u64 { - let f = move |context: NativeCallContext, args: &mut FnCallArgs| { - func(context, args).map(Dynamic::from) - }; + let f = + move |ctx: NativeCallContext, args: &mut FnCallArgs| func(ctx, args).map(Dynamic::from); self.set_fn( name, FnAccess::Public, diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index 9f3179a5..fa831e6f 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -41,12 +41,12 @@ macro_rules! gen_array_functions { } #[rhai_fn(return_raw)] - pub fn pad(_context: NativeCallContext, list: &mut Array, len: INT, item: $arg_type) -> Result> { + pub fn pad(_ctx: NativeCallContext, list: &mut Array, len: INT, item: $arg_type) -> Result> { // Check if array will be over max size limit #[cfg(not(feature = "unchecked"))] - if _context.engine().max_array_size() > 0 && len > 0 && (len as usize) > _context.engine().max_array_size() { + if _ctx.engine().max_array_size() > 0 && len > 0 && (len as usize) > _ctx.engine().max_array_size() { return EvalAltResult::ErrorDataTooLarge( - "Size of array".to_string(), _context.engine().max_array_size(), len as usize, Position::none(), + "Size of array".to_string(), Position::none() ).into(); } @@ -197,7 +197,7 @@ mod array_functions { } #[rhai_fn(return_raw)] pub fn map( - context: NativeCallContext, + ctx: NativeCallContext, list: &mut Array, mapper: FnPtr, ) -> Result> { @@ -206,12 +206,12 @@ mod array_functions { for (i, item) in list.iter().enumerate() { array.push( mapper - .call_dynamic(context, None, [item.clone()]) + .call_dynamic(ctx, None, [item.clone()]) .or_else(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.starts_with(mapper.fn_name()) => { - mapper.call_dynamic(context, None, [item.clone(), (i as INT).into()]) + mapper.call_dynamic(ctx, None, [item.clone(), (i as INT).into()]) } _ => Err(err), }) @@ -229,7 +229,7 @@ mod array_functions { } #[rhai_fn(return_raw)] pub fn filter( - context: NativeCallContext, + ctx: NativeCallContext, list: &mut Array, filter: FnPtr, ) -> Result> { @@ -237,12 +237,12 @@ mod array_functions { for (i, item) in list.iter().enumerate() { if filter - .call_dynamic(context, None, [item.clone()]) + .call_dynamic(ctx, None, [item.clone()]) .or_else(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.starts_with(filter.fn_name()) => { - filter.call_dynamic(context, None, [item.clone(), (i as INT).into()]) + filter.call_dynamic(ctx, None, [item.clone(), (i as INT).into()]) } _ => Err(err), }) @@ -264,18 +264,18 @@ mod array_functions { } #[rhai_fn(return_raw)] pub fn some( - context: NativeCallContext, + ctx: NativeCallContext, list: &mut Array, filter: FnPtr, ) -> Result> { for (i, item) in list.iter().enumerate() { if filter - .call_dynamic(context, None, [item.clone()]) + .call_dynamic(ctx, None, [item.clone()]) .or_else(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.starts_with(filter.fn_name()) => { - filter.call_dynamic(context, None, [item.clone(), (i as INT).into()]) + filter.call_dynamic(ctx, None, [item.clone(), (i as INT).into()]) } _ => Err(err), }) @@ -297,18 +297,18 @@ mod array_functions { } #[rhai_fn(return_raw)] pub fn all( - context: NativeCallContext, + ctx: NativeCallContext, list: &mut Array, filter: FnPtr, ) -> Result> { for (i, item) in list.iter().enumerate() { if !filter - .call_dynamic(context, None, [item.clone()]) + .call_dynamic(ctx, None, [item.clone()]) .or_else(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.starts_with(filter.fn_name()) => { - filter.call_dynamic(context, None, [item.clone(), (i as INT).into()]) + filter.call_dynamic(ctx, None, [item.clone(), (i as INT).into()]) } _ => Err(err), }) @@ -330,7 +330,7 @@ mod array_functions { } #[rhai_fn(return_raw)] pub fn reduce( - context: NativeCallContext, + ctx: NativeCallContext, list: &mut Array, reducer: FnPtr, ) -> Result> { @@ -338,16 +338,12 @@ mod array_functions { for (i, item) in list.iter().enumerate() { result = reducer - .call_dynamic(context, None, [result.clone(), item.clone()]) + .call_dynamic(ctx, None, [result.clone(), item.clone()]) .or_else(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.starts_with(reducer.fn_name()) => { - reducer.call_dynamic( - context, - None, - [result, item.clone(), (i as INT).into()], - ) + reducer.call_dynamic(ctx, None, [result, item.clone(), (i as INT).into()]) } _ => Err(err), }) @@ -364,12 +360,12 @@ mod array_functions { } #[rhai_fn(name = "reduce", return_raw)] pub fn reduce_with_initial( - context: NativeCallContext, + ctx: NativeCallContext, list: &mut Array, reducer: FnPtr, initial: FnPtr, ) -> Result> { - let mut result = initial.call_dynamic(context, None, []).map_err(|err| { + let mut result = initial.call_dynamic(ctx, None, []).map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce".to_string(), err, @@ -379,16 +375,12 @@ mod array_functions { for (i, item) in list.iter().enumerate() { result = reducer - .call_dynamic(context, None, [result.clone(), item.clone()]) + .call_dynamic(ctx, None, [result.clone(), item.clone()]) .or_else(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.starts_with(reducer.fn_name()) => { - reducer.call_dynamic( - context, - None, - [result, item.clone(), (i as INT).into()], - ) + reducer.call_dynamic(ctx, None, [result, item.clone(), (i as INT).into()]) } _ => Err(err), }) @@ -405,7 +397,7 @@ mod array_functions { } #[rhai_fn(return_raw)] pub fn reduce_rev( - context: NativeCallContext, + ctx: NativeCallContext, list: &mut Array, reducer: FnPtr, ) -> Result> { @@ -413,16 +405,12 @@ mod array_functions { for (i, item) in list.iter().enumerate().rev() { result = reducer - .call_dynamic(context, None, [result.clone(), item.clone()]) + .call_dynamic(ctx, None, [result.clone(), item.clone()]) .or_else(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.starts_with(reducer.fn_name()) => { - reducer.call_dynamic( - context, - None, - [result, item.clone(), (i as INT).into()], - ) + reducer.call_dynamic(ctx, None, [result, item.clone(), (i as INT).into()]) } _ => Err(err), }) @@ -439,12 +427,12 @@ mod array_functions { } #[rhai_fn(name = "reduce_rev", return_raw)] pub fn reduce_rev_with_initial( - context: NativeCallContext, + ctx: NativeCallContext, list: &mut Array, reducer: FnPtr, initial: FnPtr, ) -> Result> { - let mut result = initial.call_dynamic(context, None, []).map_err(|err| { + let mut result = initial.call_dynamic(ctx, None, []).map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce".to_string(), err, @@ -454,16 +442,12 @@ mod array_functions { for (i, item) in list.iter().enumerate().rev() { result = reducer - .call_dynamic(context, None, [result.clone(), item.clone()]) + .call_dynamic(ctx, None, [result.clone(), item.clone()]) .or_else(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.starts_with(reducer.fn_name()) => { - reducer.call_dynamic( - context, - None, - [result, item.clone(), (i as INT).into()], - ) + reducer.call_dynamic(ctx, None, [result, item.clone(), (i as INT).into()]) } _ => Err(err), }) @@ -480,13 +464,13 @@ mod array_functions { } #[rhai_fn(return_raw)] pub fn sort( - context: NativeCallContext, + ctx: NativeCallContext, list: &mut Array, comparer: FnPtr, ) -> Result> { list.sort_by(|x, y| { comparer - .call_dynamic(context, None, [x.clone(), y.clone()]) + .call_dynamic(ctx, None, [x.clone(), y.clone()]) .ok() .and_then(|v| v.as_int().ok()) .map(|v| { @@ -516,7 +500,7 @@ mod array_functions { } #[rhai_fn(return_raw)] pub fn drain( - context: NativeCallContext, + ctx: NativeCallContext, list: &mut Array, filter: FnPtr, ) -> Result> { @@ -528,12 +512,12 @@ mod array_functions { i -= 1; if filter - .call_dynamic(context, None, [list[i].clone()]) + .call_dynamic(ctx, None, [list[i].clone()]) .or_else(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.starts_with(filter.fn_name()) => { - filter.call_dynamic(context, None, [list[i].clone(), (i as INT).into()]) + filter.call_dynamic(ctx, None, [list[i].clone(), (i as INT).into()]) } _ => Err(err), }) @@ -575,7 +559,7 @@ mod array_functions { } #[rhai_fn(return_raw)] pub fn retain( - context: NativeCallContext, + ctx: NativeCallContext, list: &mut Array, filter: FnPtr, ) -> Result> { @@ -587,12 +571,12 @@ mod array_functions { i -= 1; if !filter - .call_dynamic(context, None, [list[i].clone()]) + .call_dynamic(ctx, None, [list[i].clone()]) .or_else(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.starts_with(filter.fn_name()) => { - filter.call_dynamic(context, None, [list[i].clone(), (i as INT).into()]) + filter.call_dynamic(ctx, None, [list[i].clone(), (i as INT).into()]) } _ => Err(err), }) diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index 7a76d91e..7ce4fb06 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -251,20 +251,16 @@ mod string_functions { #[rhai_fn(return_raw)] pub fn pad( - _context: NativeCallContext, + _ctx: NativeCallContext, s: &mut ImmutableString, len: INT, ch: char, ) -> Result> { // Check if string will be over max size limit #[cfg(not(feature = "unchecked"))] - if _context.engine().max_string_size() > 0 - && len as usize > _context.engine().max_string_size() - { + if _ctx.engine().max_string_size() > 0 && len as usize > _ctx.engine().max_string_size() { return EvalAltResult::ErrorDataTooLarge( "Length of string".to_string(), - _context.engine().max_string_size(), - len as usize, Position::none(), ) .into(); @@ -281,13 +277,10 @@ mod string_functions { } #[cfg(not(feature = "unchecked"))] - if _context.engine().max_string_size() > 0 - && s.len() > _context.engine().max_string_size() + if _ctx.engine().max_string_size() > 0 && s.len() > _ctx.engine().max_string_size() { return EvalAltResult::ErrorDataTooLarge( "Length of string".to_string(), - _context.engine().max_string_size(), - s.len(), Position::none(), ) .into(); @@ -299,20 +292,16 @@ mod string_functions { } #[rhai_fn(name = "pad", return_raw)] pub fn pad_with_string( - _context: NativeCallContext, + _ctx: NativeCallContext, s: &mut ImmutableString, len: INT, padding: &str, ) -> Result> { // Check if string will be over max size limit #[cfg(not(feature = "unchecked"))] - if _context.engine().max_string_size() > 0 - && len as usize > _context.engine().max_string_size() - { + if _ctx.engine().max_string_size() > 0 && len as usize > _ctx.engine().max_string_size() { return EvalAltResult::ErrorDataTooLarge( "Length of string".to_string(), - _context.engine().max_string_size(), - len as usize, Position::none(), ) .into(); @@ -336,13 +325,10 @@ mod string_functions { } #[cfg(not(feature = "unchecked"))] - if _context.engine().max_string_size() > 0 - && s.len() > _context.engine().max_string_size() + if _ctx.engine().max_string_size() > 0 && s.len() > _ctx.engine().max_string_size() { return EvalAltResult::ErrorDataTooLarge( "Length of string".to_string(), - _context.engine().max_string_size(), - s.len(), Position::none(), ) .into(); diff --git a/src/result.rs b/src/result.rs index 1a42c118..5370850e 100644 --- a/src/result.rs +++ b/src/result.rs @@ -83,11 +83,11 @@ pub enum EvalAltResult { ErrorTooManyModules(Position), /// Call stack over maximum limit. ErrorStackOverflow(Position), - /// Data value over maximum size limit. Wrapped values are the type name, maximum size and current size. - ErrorDataTooLarge(String, usize, usize, Position), - /// The script is prematurely terminated. - ErrorTerminated(Position), - /// Run-time error encountered. Wrapped value is the error. + /// Data value over maximum size limit. Wrapped value is the type name. + ErrorDataTooLarge(String, Position), + /// The script is prematurely terminated. Wrapped value is the termination token. + ErrorTerminated(Dynamic, Position), + /// Run-time error encountered. Wrapped value is the error token. ErrorRuntime(Dynamic, Position), /// Breaking out of loops - not an error if within a loop. @@ -135,8 +135,8 @@ impl EvalAltResult { Self::ErrorTooManyOperations(_) => "Too many operations", Self::ErrorTooManyModules(_) => "Too many modules imported", Self::ErrorStackOverflow(_) => "Stack overflow", - Self::ErrorDataTooLarge(_, _, _, _) => "Data size exceeds maximum limit", - Self::ErrorTerminated(_) => "Script terminated.", + Self::ErrorDataTooLarge(_, _) => "Data size exceeds maximum limit", + Self::ErrorTerminated(_,_) => "Script terminated.", Self::ErrorRuntime(_, _) => "Runtime error", Self::LoopBreak(true, _) => "Break statement not inside a loop", Self::LoopBreak(false, _) => "Continue statement not inside a loop", @@ -185,7 +185,7 @@ impl fmt::Display for EvalAltResult { | Self::ErrorTooManyOperations(_) | Self::ErrorTooManyModules(_) | Self::ErrorStackOverflow(_) - | Self::ErrorTerminated(_) => f.write_str(desc)?, + | Self::ErrorTerminated(_, _) => f.write_str(desc)?, Self::ErrorRuntime(d, _) if d.is::() => { let s = d.as_str().unwrap(); @@ -237,9 +237,7 @@ impl fmt::Display for EvalAltResult { "String index {} is out of bounds: only {} characters in the string", index, max )?, - Self::ErrorDataTooLarge(typ, max, size, _) => { - write!(f, "{} ({}) exceeds the maximum limit ({})", typ, size, max)? - } + Self::ErrorDataTooLarge(typ, _) => write!(f, "{} exceeds maximum limit", typ)?, } // Do not write any position if None @@ -297,8 +295,8 @@ impl EvalAltResult { Self::ErrorTooManyOperations(_) | Self::ErrorTooManyModules(_) | Self::ErrorStackOverflow(_) - | Self::ErrorDataTooLarge(_, _, _, _) - | Self::ErrorTerminated(_) => false, + | Self::ErrorDataTooLarge(_, _) + | Self::ErrorTerminated(_, _) => false, Self::LoopBreak(_, _) | Self::Return(_, _) => unreachable!(), } @@ -313,9 +311,9 @@ impl EvalAltResult { Self::ErrorTooManyOperations(_) | Self::ErrorTooManyModules(_) | Self::ErrorStackOverflow(_) - | Self::ErrorDataTooLarge(_, _, _, _) => true, + | Self::ErrorDataTooLarge(_, _) => true, - Self::ErrorTerminated(_) => true, + Self::ErrorTerminated(_, _) => true, Self::LoopBreak(_, _) | Self::Return(_, _) => unreachable!(), @@ -349,8 +347,8 @@ impl EvalAltResult { | Self::ErrorTooManyOperations(pos) | Self::ErrorTooManyModules(pos) | Self::ErrorStackOverflow(pos) - | Self::ErrorDataTooLarge(_, _, _, pos) - | Self::ErrorTerminated(pos) + | Self::ErrorDataTooLarge(_, pos) + | Self::ErrorTerminated(_, pos) | Self::ErrorRuntime(_, pos) | Self::LoopBreak(_, pos) | Self::Return(_, pos) => *pos, @@ -383,8 +381,8 @@ impl EvalAltResult { | Self::ErrorTooManyOperations(pos) | Self::ErrorTooManyModules(pos) | Self::ErrorStackOverflow(pos) - | Self::ErrorDataTooLarge(_, _, _, pos) - | Self::ErrorTerminated(pos) + | Self::ErrorDataTooLarge(_, pos) + | Self::ErrorTerminated(_, pos) | Self::ErrorRuntime(_, pos) | Self::LoopBreak(_, pos) | Self::Return(_, pos) => *pos = new_position, From d7d6f74dfd0f6a81f80af0acb84aaca52884d024 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 2 Nov 2020 12:50:27 +0800 Subject: [PATCH 5/8] Add constant NO_POS. --- RELEASES.md | 1 + doc/src/engine/custom-syntax.md | 22 +++++----- doc/src/engine/var.md | 14 +++---- doc/src/plugins/function.md | 4 +- doc/src/plugins/module.md | 4 +- src/engine.rs | 42 +++++++------------ src/engine_api.rs | 8 ++-- src/fn_call.rs | 72 +++++++++++++++------------------ src/fn_native.rs | 4 +- src/lib.rs | 4 +- src/module/mod.rs | 10 ++--- src/optimize.rs | 4 +- src/packages/arithmetic.rs | 4 +- src/packages/array_basic.rs | 28 ++++++------- src/packages/math_basic.rs | 27 +++++-------- src/packages/string_more.rs | 18 +++------ src/parse_error.rs | 56 ++++++++++++++----------- src/parser.rs | 51 +++++++++++++---------- src/result.rs | 8 ++-- src/serde_impl/de.rs | 17 ++++---- src/serde_impl/ser.rs | 55 ++++++------------------- src/serde_impl/str.rs | 10 ++--- src/syntax.rs | 6 +-- src/token.rs | 5 ++- tests/data_size.rs | 20 ++++----- tests/operations.rs | 18 ++++++--- tests/syntax.rs | 14 ++++--- tests/tokens.rs | 13 +++--- tests/var_scope.rs | 6 +-- 29 files changed, 253 insertions(+), 292 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 06d3f9f8..af4262ba 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -22,6 +22,7 @@ Breaking changes * `EvalAltResult::ErrorAssignmentToUnknownLHS` is moved to `ParseError::AssignmentToInvalidLHS`. `ParseError::AssignmentToCopy` is removed. * `EvalAltResult::ErrorDataTooLarge` is simplified. * `Engine::on_progress` closure signature now returns `Option` with the termination value passed on to `EvalAltResult::ErrorTerminated`. +* `ParseErrorType::BadInput` now wraps a `LexError` instead of a text string. New features ------------ diff --git a/doc/src/engine/custom-syntax.md b/doc/src/engine/custom-syntax.md index 61cfae84..ebac08f0 100644 --- a/doc/src/engine/custom-syntax.md +++ b/doc/src/engine/custom-syntax.md @@ -294,9 +294,8 @@ engine.register_custom_syntax_raw( "update" | "check" | "add" | "remove" => Ok(Some("$ident$".to_string())), "cleanup" => Ok(None), cmd => Err(ParseError(Box::new(ParseErrorType::BadInput( - format!("Improper command: {}", cmd))), - Position::none(), - )), + LexError::ImproperSymbol(format!("Improper command: {}", cmd)) + )), NO_POS)), }, // perform command arg ... 3 => match (stream[1].as_str(), stream[2].as_str()) { @@ -308,9 +307,10 @@ engine.register_custom_syntax_raw( ("add", arg) => Ok(None), ("remove", arg) => Ok(None), (cmd, arg) => Err(ParseError(Box::new(ParseErrorType::BadInput( - format!("Invalid argument for command {}: {}", cmd, arg))), - Position::none(), - )), + LexError::ImproperSymbol( + format!("Invalid argument for command {}: {}", cmd, arg) + ) + )), NO_POS)), }, _ => unreachable!(), }, @@ -335,8 +335,8 @@ where: The return value is `Result, ParseError>` where: -| Value | Description | -| ------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `Ok(None)` | parsing complete and there are no more symbols to match | -| `Ok(Some(symbol))` | next symbol to match, which can also be `"$expr$"`, `"$ident$"` or `"$block$"` | -| `Err(ParseError)` | error that is reflected back to the [`Engine`].
Normally this is `ParseError(ParseErrorType::BadInput(message), Position::none())` to indicate that there is a syntax error, but it can be any `ParseError`. | +| Value | Description | +| ------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Ok(None)` | parsing complete and there are no more symbols to match | +| `Ok(Some(symbol))` | next symbol to match, which can also be `"$expr$"`, `"$ident$"` or `"$block$"` | +| `Err(ParseError)` | error that is reflected back to the [`Engine`].
Normally this is `ParseError(ParseErrorType::BadInput(LexError::ImproperSymbol(message)), NO_POS)` to indicate that there is a syntax error, but it can be any `ParseError`. | diff --git a/doc/src/engine/var.md b/doc/src/engine/var.md index dd661f8b..5af45a41 100644 --- a/doc/src/engine/var.md +++ b/doc/src/engine/var.md @@ -21,11 +21,11 @@ engine.on_var(|name, index, context| { "MYSTIC_NUMBER" => Ok(Some((42 as INT).into())), // Override a variable - make it not found even if it exists! "DO_NOT_USE" => Err(Box::new( - EvalAltResult::ErrorVariableNotFound(name.to_string(), Position::none()) + EvalAltResult::ErrorVariableNotFound(name.to_string(), NO_POS) )), // Silently maps 'chameleon' into 'innocent'. "chameleon" => context.scope.get_value("innocent").map(Some).ok_or_else(|| Box::new( - EvalAltResult::ErrorVariableNotFound(name.to_string(), Position::none()) + EvalAltResult::ErrorVariableNotFound(name.to_string(), NO_POS) )), // Return Ok(None) to continue with the normal variable resolution process. _ => Ok(None) @@ -82,8 +82,8 @@ where: The return value is `Result, Box>` where: -| Value | Description | -| ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `Ok(None)` | normal variable resolution process should continue, i.e. continue searching through the [`Scope`] | -| `Ok(Some(Dynamic))` | value of the variable, treated as a constant | -| `Err(Box)` | error that is reflected back to the [`Engine`].
Normally this is `EvalAltResult::ErrorVariableNotFound(var_name, Position::none())` to indicate that the variable does not exist, but it can be any `EvalAltResult`. | +| Value | Description | +| ------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Ok(None)` | normal variable resolution process should continue, i.e. continue searching through the [`Scope`] | +| `Ok(Some(Dynamic))` | value of the variable, treated as a constant | +| `Err(Box)` | error that is reflected back to the [`Engine`].
Normally this is `EvalAltResult::ErrorVariableNotFound(var_name, NO_POS)` to indicate that the variable does not exist, but it can be any `EvalAltResult`. | diff --git a/doc/src/plugins/function.md b/doc/src/plugins/function.md index bd31de0c..e63448d4 100644 --- a/doc/src/plugins/function.md +++ b/doc/src/plugins/function.md @@ -117,7 +117,7 @@ pub fn greet(context: NativeCallContext, callback: FnPtr) The native call context is also useful in another scenario: protecting a function from malicious scripts. ```rust -use rhai::{Dynamic, Array, NativeCallContext, EvalAltResult, Position}; +use rhai::{Dynamic, Array, NativeCallContext, EvalAltResult, NO_POS}; use rhai::plugin::*; // a "prelude" import for macros // This function builds an array of arbitrary size, but is protected @@ -137,7 +137,7 @@ pub fn grow(context: NativeCallContext, size: i64) "Size to grow".to_string(), context.engine().max_array_size(), size as usize, - Position::none(), + NO_POS, ).into(); } diff --git a/doc/src/plugins/module.md b/doc/src/plugins/module.md index 6d07abdc..a60d6fed 100644 --- a/doc/src/plugins/module.md +++ b/doc/src/plugins/module.md @@ -376,7 +376,7 @@ mod my_module { The native call context is also useful in another scenario: protecting a function from malicious scripts. ```rust -use rhai::{Dynamic, Array, NativeCallContext, EvalAltResult, Position}; +use rhai::{Dynamic, Array, NativeCallContext, EvalAltResult, NO_POS}; use rhai::plugin::*; // a "prelude" import for macros #[export_module] @@ -397,7 +397,7 @@ mod my_module { "Size to grow".to_string(), context.engine().max_array_size(), size as usize, - Position::none(), + NO_POS, ).into(); } diff --git a/src/engine.rs b/src/engine.rs index 494d2e3a..a9a2a636 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -11,7 +11,7 @@ use crate::r#unsafe::unsafe_cast_var_name_to_lifetime; use crate::result::EvalAltResult; use crate::scope::{EntryType as ScopeEntryType, Scope}; use crate::syntax::CustomSyntax; -use crate::token::Position; +use crate::token::{Position, NO_POS}; use crate::{calc_native_fn_hash, StaticVec}; #[cfg(not(feature = "no_index"))] @@ -351,7 +351,7 @@ impl<'a> Target<'a> { #[cfg(not(feature = "no_index"))] Self::StringChar(_, _, ch) => { let char_value = ch.clone(); - self.set_value((char_value, Position::none())).unwrap(); + self.set_value((char_value, NO_POS)).unwrap(); } } } @@ -993,7 +993,7 @@ impl Engine { { EvalAltResult::ErrorIndexingType( self.map_type_name(val_type_name).into(), - Position::none(), + NO_POS, ) } err => err, @@ -1463,20 +1463,16 @@ impl Engine { .map(|(v, _)| v.into()) .map_err(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.ends_with(']') => { - Box::new(EvalAltResult::ErrorIndexingType( - type_name.into(), - Position::none(), - )) + Box::new(EvalAltResult::ErrorIndexingType(type_name.into(), NO_POS)) } _ => err, }) } - _ => EvalAltResult::ErrorIndexingType( - self.map_type_name(val.type_name()).into(), - Position::none(), - ) - .into(), + _ => { + EvalAltResult::ErrorIndexingType(self.map_type_name(val.type_name()).into(), NO_POS) + .into() + } } } @@ -2036,7 +2032,7 @@ impl Engine { let value = if let EvalAltResult::ErrorRuntime(ref x, _) = err { x.clone() } else { - err.set_position(Position::none()); + err.set_position(NO_POS); err.to_string().into() }; @@ -2298,26 +2294,18 @@ impl Engine { let (_arr, _map, s) = calc_size(result.as_ref().unwrap()); if s > self.max_string_size() { - return EvalAltResult::ErrorDataTooLarge( - "Length of string".to_string(), - Position::none(), - ) - .into(); + return EvalAltResult::ErrorDataTooLarge("Length of string".to_string(), NO_POS).into(); } #[cfg(not(feature = "no_index"))] if _arr > self.max_array_size() { - return EvalAltResult::ErrorDataTooLarge("Size of array".to_string(), Position::none()) - .into(); + return EvalAltResult::ErrorDataTooLarge("Size of array".to_string(), NO_POS).into(); } #[cfg(not(feature = "no_object"))] if _map > self.max_map_size() { - return EvalAltResult::ErrorDataTooLarge( - "Size of object map".to_string(), - Position::none(), - ) - .into(); + return EvalAltResult::ErrorDataTooLarge("Size of object map".to_string(), NO_POS) + .into(); } result @@ -2331,14 +2319,14 @@ impl Engine { #[cfg(not(feature = "unchecked"))] // Guard against too many operations if self.max_operations() > 0 && state.operations > self.max_operations() { - return EvalAltResult::ErrorTooManyOperations(Position::none()).into(); + return EvalAltResult::ErrorTooManyOperations(NO_POS).into(); } // Report progress - only in steps if let Some(progress) = &self.progress { if let Some(token) = progress(&state.operations) { // Terminate script if progress returns a termination token - return EvalAltResult::ErrorTerminated(token, Position::none()).into(); + return EvalAltResult::ErrorTerminated(token, NO_POS).into(); } } diff --git a/src/engine_api.rs b/src/engine_api.rs index 26fd57ee..619e5f54 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -8,7 +8,7 @@ use crate::optimize::OptimizationLevel; use crate::parse_error::ParseError; use crate::result::EvalAltResult; use crate::scope::Scope; -use crate::token::Position; +use crate::token::{Position, NO_POS}; #[cfg(not(feature = "no_index"))] use crate::{ @@ -1391,7 +1391,7 @@ impl Engine { EvalAltResult::ErrorMismatchOutputType( self.map_type_name(type_name::()).into(), typ.into(), - Position::none(), + NO_POS, ) .into() }); @@ -1527,7 +1527,7 @@ impl Engine { EvalAltResult::ErrorMismatchOutputType( self.map_type_name(type_name::()).into(), typ.into(), - Position::none(), + NO_POS, ) .into() }); @@ -1617,7 +1617,7 @@ impl Engine { ) -> Result> { let fn_def = lib .get_script_fn(name, args.len(), true) - .ok_or_else(|| EvalAltResult::ErrorFunctionNotFound(name.into(), Position::none()))?; + .ok_or_else(|| EvalAltResult::ErrorFunctionNotFound(name.into(), NO_POS))?; let mut state = Default::default(); let mut mods = Default::default(); diff --git a/src/fn_call.rs b/src/fn_call.rs index f5b0a190..313b4b15 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -14,7 +14,7 @@ use crate::parse_error::ParseErrorType; use crate::result::EvalAltResult; use crate::scope::Scope; use crate::stdlib::ops::Deref; -use crate::token::Position; +use crate::token::NO_POS; use crate::utils::ImmutableString; use crate::{calc_native_fn_hash, calc_script_fn_hash, StaticVec, INT}; @@ -159,7 +159,7 @@ pub fn ensure_no_data_race( { return EvalAltResult::ErrorDataRace( format!("argument #{} of function '{}'", n + 1 + skip, fn_name), - Position::none(), + NO_POS, ) .into(); } @@ -223,7 +223,7 @@ impl Engine { EvalAltResult::ErrorMismatchOutputType( self.map_type_name(type_name::()).into(), typ.into(), - Position::none(), + NO_POS, ) })?) .into(), @@ -234,7 +234,7 @@ impl Engine { EvalAltResult::ErrorMismatchOutputType( self.map_type_name(type_name::()).into(), typ.into(), - Position::none(), + NO_POS, ) })?) .into(), @@ -265,7 +265,7 @@ impl Engine { prop, self.map_type_name(args[0].type_name()) ), - Position::none(), + NO_POS, ) .into(); } @@ -279,7 +279,7 @@ impl Engine { self.map_type_name(args[0].type_name()), self.map_type_name(args[1].type_name()), ), - Position::none(), + NO_POS, ) .into(); } @@ -293,7 +293,7 @@ impl Engine { self.map_type_name(args[0].type_name()), self.map_type_name(args[1].type_name()), ), - Position::none(), + NO_POS, ) .into(); } @@ -307,7 +307,7 @@ impl Engine { self.map_type_name(args[0].type_name()), self.map_type_name(args[1].type_name()), ), - Position::none(), + NO_POS, ) .into(); } @@ -326,7 +326,7 @@ impl Engine { .collect::>() .join(", ") ), - Position::none(), + NO_POS, ) .into() } @@ -357,9 +357,7 @@ impl Engine { #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "unchecked"))] if level > self.max_call_levels() { - return Err(Box::new( - EvalAltResult::ErrorStackOverflow(Position::none()), - )); + return Err(Box::new(EvalAltResult::ErrorStackOverflow(NO_POS))); } let orig_scope_level = state.scope_level; @@ -396,29 +394,25 @@ impl Engine { // Evaluate the function at one higher level of call depth let stmt = &fn_def.body; - let result = self - .eval_stmt(scope, mods, state, unified_lib, this_ptr, stmt, level + 1) - .or_else(|err| match *err { - // Convert return statement to return value - EvalAltResult::Return(x, _) => Ok(x), - EvalAltResult::ErrorInFunctionCall(name, err, _) => { - EvalAltResult::ErrorInFunctionCall( - format!("{} > {}", fn_def.name, name), - err, - Position::none(), - ) - .into() - } - // System errors are passed straight-through - err if err.is_system_exception() => Err(Box::new(err)), - // Other errors are wrapped in `ErrorInFunctionCall` - _ => EvalAltResult::ErrorInFunctionCall( - fn_def.name.to_string(), - err, - Position::none(), - ) - .into(), - }); + let result = + self.eval_stmt(scope, mods, state, unified_lib, this_ptr, stmt, level + 1) + .or_else(|err| match *err { + // Convert return statement to return value + EvalAltResult::Return(x, _) => Ok(x), + EvalAltResult::ErrorInFunctionCall(name, err, _) => { + EvalAltResult::ErrorInFunctionCall( + format!("{} > {}", fn_def.name, name), + err, + NO_POS, + ) + .into() + } + // System errors are passed straight-through + err if err.is_system_exception() => Err(Box::new(err)), + // Other errors are wrapped in `ErrorInFunctionCall` + _ => EvalAltResult::ErrorInFunctionCall(fn_def.name.to_string(), err, NO_POS) + .into(), + }); // Remove all local variables scope.rewind(prev_scope_len); @@ -519,7 +513,7 @@ impl Engine { fn_name, fn_name ) .into(), - Position::none(), + NO_POS, ) .into() } @@ -656,9 +650,7 @@ impl Engine { #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "unchecked"))] if _level > self.max_call_levels() { - return Err(Box::new( - EvalAltResult::ErrorStackOverflow(Position::none()), - )); + return Err(Box::new(EvalAltResult::ErrorStackOverflow(NO_POS))); } // Compile the script text @@ -1218,7 +1210,7 @@ impl Engine { .collect::>() .join(", ") ), - Position::none(), + NO_POS, ) .into(), } diff --git a/src/fn_native.rs b/src/fn_native.rs index 312af42b..f41490d0 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -6,7 +6,7 @@ use crate::engine::{Engine, EvalContext}; use crate::module::Module; use crate::plugin::PluginFunction; use crate::result::EvalAltResult; -use crate::token::{is_valid_identifier, Position}; +use crate::token::{is_valid_identifier, NO_POS}; use crate::utils::ImmutableString; use crate::{calc_script_fn_hash, StaticVec}; @@ -215,7 +215,7 @@ impl TryFrom for FnPtr { if is_valid_identifier(value.chars()) { Ok(Self(value, Default::default())) } else { - EvalAltResult::ErrorFunctionNotFound(value.into(), Position::none()).into() + EvalAltResult::ErrorFunctionNotFound(value.into(), NO_POS).into() } } } diff --git a/src/lib.rs b/src/lib.rs index 621aea86..71fbf3cf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -115,11 +115,11 @@ pub use engine::{Engine, EvalContext}; pub use fn_native::{FnPtr, NativeCallContext}; pub use fn_register::{RegisterFn, RegisterResultFn}; pub use module::Module; -pub use parse_error::{ParseError, ParseErrorType}; +pub use parse_error::{LexError, ParseError, ParseErrorType}; pub use result::EvalAltResult; pub use scope::Scope; pub use syntax::Expression; -pub use token::Position; +pub use token::{Position, NO_POS}; pub use utils::ImmutableString; #[cfg(feature = "internals")] diff --git a/src/module/mod.rs b/src/module/mod.rs index a8714a4d..8c9a17a3 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -5,7 +5,7 @@ use crate::dynamic::{Dynamic, Variant}; use crate::fn_native::{CallableFunction, FnCallArgs, IteratorFn, NativeCallContext, SendSync}; use crate::fn_register::by_value as cast_arg; use crate::result::EvalAltResult; -use crate::token::{Position, Token}; +use crate::token::{Token, NO_POS}; use crate::utils::{ImmutableString, StraightHasherBuilder}; use crate::{calc_native_fn_hash, calc_script_fn_hash, StaticVec}; @@ -271,11 +271,11 @@ impl Module { hash_var: u64, ) -> Result<&mut Dynamic, Box> { if hash_var == 0 { - Err(EvalAltResult::ErrorVariableNotFound(String::new(), Position::none()).into()) + Err(EvalAltResult::ErrorVariableNotFound(String::new(), NO_POS).into()) } else { - self.all_variables.get_mut(&hash_var).ok_or_else(|| { - EvalAltResult::ErrorVariableNotFound(String::new(), Position::none()).into() - }) + self.all_variables + .get_mut(&hash_var) + .ok_or_else(|| EvalAltResult::ErrorVariableNotFound(String::new(), NO_POS).into()) } } diff --git a/src/optimize.rs b/src/optimize.rs index def61077..1ef43a5a 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -10,7 +10,7 @@ use crate::fn_call::run_builtin_binary_op; use crate::module::Module; use crate::parser::map_dynamic_to_expr; use crate::scope::Scope; -use crate::token::{is_valid_identifier, Position}; +use crate::token::{is_valid_identifier, NO_POS}; use crate::{calc_native_fn_hash, StaticVec}; #[cfg(not(feature = "no_function"))] @@ -726,7 +726,7 @@ fn optimize( .iter() .filter(|(_, typ, _)| *typ) .for_each(|(name, _, value)| { - if let Some(val) = map_dynamic_to_expr(value, Position::none()) { + if let Some(val) = map_dynamic_to_expr(value, NO_POS) { state.push_constant(name, val); } }); diff --git a/src/packages/arithmetic.rs b/src/packages/arithmetic.rs index d061ceeb..c8398cea 100644 --- a/src/packages/arithmetic.rs +++ b/src/packages/arithmetic.rs @@ -4,7 +4,7 @@ use crate::def_package; use crate::plugin::*; use crate::INT; -use crate::{result::EvalAltResult, token::Position}; +use crate::{result::EvalAltResult, token::NO_POS}; #[cfg(not(feature = "no_float"))] use crate::FLOAT; @@ -17,7 +17,7 @@ use crate::stdlib::{format, string::String}; #[inline(always)] pub fn make_err(msg: impl Into) -> Box { - EvalAltResult::ErrorArithmetic(msg.into(), Position::none()).into() + EvalAltResult::ErrorArithmetic(msg.into(), NO_POS).into() } macro_rules! gen_arithmetic_functions { diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index fa831e6f..89609714 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -7,7 +7,7 @@ use crate::engine::Array; use crate::fn_native::{FnPtr, NativeCallContext}; use crate::plugin::*; use crate::result::EvalAltResult; -use crate::token::Position; +use crate::token::NO_POS; use crate::utils::ImmutableString; use crate::INT; @@ -46,7 +46,7 @@ macro_rules! gen_array_functions { #[cfg(not(feature = "unchecked"))] if _ctx.engine().max_array_size() > 0 && len > 0 && (len as usize) > _ctx.engine().max_array_size() { return EvalAltResult::ErrorDataTooLarge( - "Size of array".to_string(), Position::none() + "Size of array".to_string(), NO_POS ).into(); } @@ -219,7 +219,7 @@ mod array_functions { Box::new(EvalAltResult::ErrorInFunctionCall( "map".to_string(), err, - Position::none(), + NO_POS, )) })?, ); @@ -250,7 +250,7 @@ mod array_functions { Box::new(EvalAltResult::ErrorInFunctionCall( "filter".to_string(), err, - Position::none(), + NO_POS, )) })? .as_bool() @@ -283,7 +283,7 @@ mod array_functions { Box::new(EvalAltResult::ErrorInFunctionCall( "filter".to_string(), err, - Position::none(), + NO_POS, )) })? .as_bool() @@ -316,7 +316,7 @@ mod array_functions { Box::new(EvalAltResult::ErrorInFunctionCall( "filter".to_string(), err, - Position::none(), + NO_POS, )) })? .as_bool() @@ -351,7 +351,7 @@ mod array_functions { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce".to_string(), err, - Position::none(), + NO_POS, )) })?; } @@ -369,7 +369,7 @@ mod array_functions { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce".to_string(), err, - Position::none(), + NO_POS, )) })?; @@ -388,7 +388,7 @@ mod array_functions { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce".to_string(), err, - Position::none(), + NO_POS, )) })?; } @@ -418,7 +418,7 @@ mod array_functions { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce".to_string(), err, - Position::none(), + NO_POS, )) })?; } @@ -436,7 +436,7 @@ mod array_functions { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce".to_string(), err, - Position::none(), + NO_POS, )) })?; @@ -455,7 +455,7 @@ mod array_functions { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce".to_string(), err, - Position::none(), + NO_POS, )) })?; } @@ -525,7 +525,7 @@ mod array_functions { Box::new(EvalAltResult::ErrorInFunctionCall( "filter".to_string(), err, - Position::none(), + NO_POS, )) })? .as_bool() @@ -584,7 +584,7 @@ mod array_functions { Box::new(EvalAltResult::ErrorInFunctionCall( "filter".to_string(), err, - Position::none(), + NO_POS, )) })? .as_bool() diff --git a/src/packages/math_basic.rs b/src/packages/math_basic.rs index ef55765f..1dd00419 100644 --- a/src/packages/math_basic.rs +++ b/src/packages/math_basic.rs @@ -2,7 +2,7 @@ use crate::def_package; use crate::plugin::*; -use crate::token::Position; +use crate::token::NO_POS; use crate::INT; #[cfg(not(feature = "no_float"))] @@ -85,11 +85,8 @@ mod int_functions { #[rhai_fn(name = "parse_int", return_raw)] pub fn parse_int_radix(s: &str, radix: INT) -> Result> { if radix < 2 || radix > 36 { - return EvalAltResult::ErrorArithmetic( - format!("Invalid radix: '{}'", radix), - Position::none(), - ) - .into(); + return EvalAltResult::ErrorArithmetic(format!("Invalid radix: '{}'", radix), NO_POS) + .into(); } INT::from_str_radix(s.trim(), radix as u32) @@ -97,7 +94,7 @@ mod int_functions { .map_err(|err| { EvalAltResult::ErrorArithmetic( format!("Error parsing integer number '{}': {}", s, err), - Position::none(), + NO_POS, ) .into() }) @@ -206,11 +203,8 @@ mod float_functions { #[rhai_fn(name = "to_int", return_raw)] pub fn f32_to_int(x: f32) -> Result> { if cfg!(not(feature = "unchecked")) && x > (MAX_INT as f32) { - EvalAltResult::ErrorArithmetic( - format!("Integer overflow: to_int({})", x), - Position::none(), - ) - .into() + EvalAltResult::ErrorArithmetic(format!("Integer overflow: to_int({})", x), NO_POS) + .into() } else { Ok((x.trunc() as INT).into()) } @@ -218,11 +212,8 @@ mod float_functions { #[rhai_fn(name = "to_int", return_raw)] pub fn f64_to_int(x: f64) -> Result> { if cfg!(not(feature = "unchecked")) && x > (MAX_INT as f64) { - EvalAltResult::ErrorArithmetic( - format!("Integer overflow: to_int({})", x), - Position::none(), - ) - .into() + EvalAltResult::ErrorArithmetic(format!("Integer overflow: to_int({})", x), NO_POS) + .into() } else { Ok((x.trunc() as INT).into()) } @@ -235,7 +226,7 @@ mod float_functions { .map_err(|err| { EvalAltResult::ErrorArithmetic( format!("Error parsing floating-point number '{}': {}", s, err), - Position::none(), + NO_POS, ) .into() }) diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index 7ce4fb06..6025ad3f 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -9,7 +9,7 @@ use crate::StaticVec; use crate::INT; #[cfg(not(feature = "unchecked"))] -use crate::{result::EvalAltResult, token::Position}; +use crate::{result::EvalAltResult, token::NO_POS}; use crate::stdlib::{ any::TypeId, boxed::Box, format, mem, string::String, string::ToString, vec::Vec, @@ -259,11 +259,7 @@ mod string_functions { // Check if string will be over max size limit #[cfg(not(feature = "unchecked"))] if _ctx.engine().max_string_size() > 0 && len as usize > _ctx.engine().max_string_size() { - return EvalAltResult::ErrorDataTooLarge( - "Length of string".to_string(), - Position::none(), - ) - .into(); + return EvalAltResult::ErrorDataTooLarge("Length of string".to_string(), NO_POS).into(); } if len > 0 { @@ -281,7 +277,7 @@ mod string_functions { { return EvalAltResult::ErrorDataTooLarge( "Length of string".to_string(), - Position::none(), + NO_POS, ) .into(); } @@ -300,11 +296,7 @@ mod string_functions { // Check if string will be over max size limit #[cfg(not(feature = "unchecked"))] if _ctx.engine().max_string_size() > 0 && len as usize > _ctx.engine().max_string_size() { - return EvalAltResult::ErrorDataTooLarge( - "Length of string".to_string(), - Position::none(), - ) - .into(); + return EvalAltResult::ErrorDataTooLarge("Length of string".to_string(), NO_POS).into(); } if len > 0 { @@ -329,7 +321,7 @@ mod string_functions { { return EvalAltResult::ErrorDataTooLarge( "Length of string".to_string(), - Position::none(), + NO_POS, ) .into(); } diff --git a/src/parse_error.rs b/src/parse_error.rs index ca0f9d71..23613a24 100644 --- a/src/parse_error.rs +++ b/src/parse_error.rs @@ -1,7 +1,7 @@ //! Module containing error definitions for the parsing process. use crate::result::EvalAltResult; -use crate::token::Position; +use crate::token::{Position, NO_POS}; use crate::stdlib::{ boxed::Box, @@ -43,26 +43,34 @@ impl fmt::Display for LexError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::UnexpectedInput(s) => write!(f, "Unexpected '{}'", s), - Self::MalformedEscapeSequence(s) => write!(f, "Invalid escape sequence: '{}'", s), - Self::MalformedNumber(s) => write!(f, "Invalid number: '{}'", s), - Self::MalformedChar(s) => write!(f, "Invalid character: '{}'", s), - Self::MalformedIdentifier(s) => write!(f, "Variable name is not proper: '{}'", s), - Self::UnterminatedString => write!(f, "Open string is not terminated"), - Self::StringTooLong(max) => write!( - f, - "Length of string literal exceeds the maximum limit ({})", - max - ), + Self::MalformedEscapeSequence(s) => write!(f, "{}: '{}'", self.desc(), s), + Self::MalformedNumber(s) => write!(f, "{}: '{}'", self.desc(), s), + Self::MalformedChar(s) => write!(f, "{}: '{}'", self.desc(), s), + Self::MalformedIdentifier(s) => write!(f, "{}: '{}'", self.desc(), s), + Self::UnterminatedString => f.write_str(self.desc()), + Self::StringTooLong(max) => write!(f, "{} ({})", self.desc(), max), Self::ImproperSymbol(s) => f.write_str(s), } } } impl LexError { - /// Convert a `LexError` into a `ParseError`. + pub(crate) fn desc(&self) -> &str { + match self { + Self::UnexpectedInput(_) => "Unexpected character encountered", + Self::UnterminatedString => "Open string is not terminated", + Self::StringTooLong(_) => "Length of string literal exceeds the maximum limit", + Self::MalformedEscapeSequence(_) => "Invalid escape sequence", + Self::MalformedNumber(_) => "Invalid number", + Self::MalformedChar(_) => "Invalid character", + Self::MalformedIdentifier(_) => "Variable name is not proper", + Self::ImproperSymbol(_) => "Invalid symbol encountered", + } + } + /// Convert a `&LexError` into a `ParseError`. #[inline(always)] pub fn into_err(&self, pos: Position) -> ParseError { - ParseError(Box::new(self.into()), pos) + ParseError(Box::new(self.clone().into()), pos) } } @@ -74,10 +82,10 @@ impl LexError { #[derive(Debug, Eq, PartialEq, Clone, Hash)] #[non_exhaustive] pub enum ParseErrorType { - /// Error in the script text. Wrapped value is the error message. - BadInput(String), /// The script ends prematurely. UnexpectedEOF, + /// Error in the script text. Wrapped value is the lex error. + BadInput(LexError), /// An unknown operator is encountered. Wrapped value is the operator. UnknownOperator(String), /// Expecting a particular token but not finding one. Wrapped values are the token and description. @@ -164,8 +172,8 @@ impl ParseErrorType { pub(crate) fn desc(&self) -> &str { match self { - Self::BadInput(p) => p, Self::UnexpectedEOF => "Script is incomplete", + Self::BadInput(p) => p.desc(), Self::UnknownOperator(_) => "Unknown operator", Self::MissingToken(_, _) => "Expecting a certain token that is missing", Self::MalformedCallExpr(_) => "Invalid expression in function call arguments", @@ -196,9 +204,9 @@ impl ParseErrorType { impl fmt::Display for ParseErrorType { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::BadInput(s) | ParseErrorType::MalformedCallExpr(s) => { - f.write_str(if s.is_empty() { self.desc() } else { s }) - } + Self::BadInput(err) => write!(f, "{}", err), + + Self::MalformedCallExpr(s) => f.write_str(if s.is_empty() { self.desc() } else { s }), Self::UnknownOperator(s) => write!(f, "{}: '{}'", self.desc(), s), Self::MalformedIndexExpr(s) | Self::MalformedInExpr(s) | Self::MalformedCapture(s) => { @@ -247,14 +255,14 @@ impl fmt::Display for ParseErrorType { } } -impl From<&LexError> for ParseErrorType { +impl From for ParseErrorType { #[inline(always)] - fn from(err: &LexError) -> Self { + fn from(err: LexError) -> Self { match err { LexError::StringTooLong(max) => { - Self::LiteralTooLarge("Length of string literal".to_string(), *max) + Self::LiteralTooLarge("Length of string literal".to_string(), max) } - _ => Self::BadInput(err.to_string()), + _ => Self::BadInput(err), } } } @@ -282,7 +290,7 @@ impl fmt::Display for ParseError { impl From for Box { #[inline(always)] fn from(err: ParseErrorType) -> Self { - Box::new(EvalAltResult::ErrorParsing(err, Position::none())) + Box::new(EvalAltResult::ErrorParsing(err, NO_POS)) } } diff --git a/src/parser.rs b/src/parser.rs index 7acef0d1..61fc858b 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -10,7 +10,9 @@ use crate::optimize::{optimize_into_ast, OptimizationLevel}; use crate::parse_error::{LexError, ParseError, ParseErrorType}; use crate::scope::{EntryType as ScopeEntryType, Scope}; use crate::syntax::CustomSyntax; -use crate::token::{is_keyword_function, is_valid_identifier, Position, Token, TokenStream}; +use crate::token::{ + is_keyword_function, is_valid_identifier, Position, Token, TokenStream, NO_POS, +}; use crate::utils::StraightHasherBuilder; use crate::{calc_script_fn_hash, StaticVec}; @@ -816,10 +818,11 @@ fn parse_primary( // Access to `this` as a variable is OK Token::Reserved(s) if s == KEYWORD_THIS && *next_token != Token::LeftParen => { if !settings.is_function_scope { - return Err( - PERR::BadInput(format!("'{}' can only be used in functions", s)) - .into_err(settings.pos), - ); + return Err(PERR::BadInput(LexError::ImproperSymbol(format!( + "'{}' can only be used in functions", + s + ))) + .into_err(settings.pos)); } else { Expr::Variable(Box::new((Ident::new(s, settings.pos), None, 0, None))) } @@ -840,7 +843,8 @@ fn parse_primary( _ => { return Err( - PERR::BadInput(format!("Unexpected '{}'", token.syntax())).into_err(settings.pos) + PERR::BadInput(LexError::UnexpectedInput(token.syntax().to_string())) + .into_err(settings.pos), ); } }; @@ -862,8 +866,10 @@ fn parse_primary( return Err(if !match_token(input, Token::LeftParen).0 { LexError::UnexpectedInput(Token::Bang.syntax().to_string()).into_err(token_pos) } else { - PERR::BadInput("'!' cannot be used to call module functions".to_string()) - .into_err(token_pos) + PERR::BadInput(LexError::ImproperSymbol( + "'!' cannot be used to call module functions".to_string(), + )) + .into_err(token_pos) }); } // Function call with ! @@ -1140,9 +1146,10 @@ fn make_assignment_stmt<'a>( Err(PERR::AssignmentToConstant("".into()).into_err(lhs.position())) } // ??? && ??? = rhs, ??? || ??? = rhs - Expr::And(_, _) | Expr::Or(_, _) => { - Err(PERR::BadInput("Possibly a typo of '=='?".to_string()).into_err(pos)) - } + Expr::And(_, _) | Expr::Or(_, _) => Err(PERR::BadInput(LexError::ImproperSymbol( + "Possibly a typo of '=='?".to_string(), + )) + .into_err(pos)), // expr = rhs _ => Err(PERR::AssignmentToInvalidLHS("".to_string()).into_err(lhs.position())), } @@ -1238,10 +1245,10 @@ fn make_dot_expr(lhs: Expr, rhs: Expr, op_pos: Position) -> Result { - return Err(PERR::BadInput(format!( + return Err(PERR::BadInput(LexError::ImproperSymbol(format!( "'{}' should not be called in method style. Try {}(...);", x.name, x.name - )) + ))) .into_err(pos)); } // lhs.func!(...) @@ -1734,9 +1741,10 @@ fn ensure_not_statement_expr(input: &mut TokenStream, type_name: &str) -> Result /// Make sure that the expression is not a mis-typed assignment (i.e. `a = b` instead of `a == b`). fn ensure_not_assignment(input: &mut TokenStream) -> Result<(), ParseError> { match input.peek().unwrap() { - (Token::Equals, pos) => { - Err(PERR::BadInput("Possibly a typo of '=='?".to_string()).into_err(*pos)) - } + (Token::Equals, pos) => Err(PERR::BadInput(LexError::ImproperSymbol( + "Possibly a typo of '=='?".to_string(), + )) + .into_err(*pos)), (Token::PlusAssign, pos) | (Token::MinusAssign, pos) | (Token::MultiplyAssign, pos) @@ -1747,9 +1755,9 @@ fn ensure_not_assignment(input: &mut TokenStream) -> Result<(), ParseError> { | (Token::PowerOfAssign, pos) | (Token::AndAssign, pos) | (Token::OrAssign, pos) - | (Token::XOrAssign, pos) => Err(PERR::BadInput( + | (Token::XOrAssign, pos) => Err(PERR::BadInput(LexError::ImproperSymbol( "Expecting a boolean expression, not an assignment".to_string(), - ) + )) .into_err(*pos)), _ => Ok(()), @@ -2683,7 +2691,7 @@ impl Engine { is_function_scope: false, is_breakable: false, level: 0, - pos: Position::none(), + pos: NO_POS, }; let expr = parse_expr(input, &mut state, &mut functions, settings)?; @@ -2694,7 +2702,8 @@ impl Engine { // Return error if the expression doesn't end (token, pos) => { return Err( - PERR::BadInput(format!("Unexpected '{}'", token.syntax())).into_err(*pos) + PERR::BadInput(LexError::UnexpectedInput(token.syntax().to_string())) + .into_err(*pos), ) } } @@ -2732,7 +2741,7 @@ impl Engine { is_function_scope: false, is_breakable: false, level: 0, - pos: Position::none(), + pos: NO_POS, }; let stmt = match parse_stmt(input, &mut state, &mut functions, settings)? { diff --git a/src/result.rs b/src/result.rs index 5370850e..79fb519b 100644 --- a/src/result.rs +++ b/src/result.rs @@ -2,7 +2,7 @@ use crate::dynamic::Dynamic; use crate::parse_error::ParseErrorType; -use crate::token::Position; +use crate::token::{Position, NO_POS}; use crate::utils::ImmutableString; use crate::INT; @@ -252,7 +252,7 @@ impl fmt::Display for EvalAltResult { impl> From for EvalAltResult { #[inline(always)] fn from(err: T) -> Self { - Self::ErrorRuntime(err.as_ref().to_string().into(), Position::none()) + Self::ErrorRuntime(err.as_ref().to_string().into(), NO_POS) } } @@ -261,7 +261,7 @@ impl> From for Box { fn from(err: T) -> Self { Box::new(EvalAltResult::ErrorRuntime( err.as_ref().to_string().into(), - Position::none(), + NO_POS, )) } } @@ -324,7 +324,7 @@ impl EvalAltResult { /// Get the `Position` of this error. pub fn position(&self) -> Position { match self { - Self::ErrorSystem(_, _) => Position::none(), + Self::ErrorSystem(_, _) => NO_POS, Self::ErrorParsing(_, pos) | Self::ErrorFunctionNotFound(_, pos) diff --git a/src/serde_impl/de.rs b/src/serde_impl/de.rs index 3d14f1ad..a607c589 100644 --- a/src/serde_impl/de.rs +++ b/src/serde_impl/de.rs @@ -4,7 +4,7 @@ use super::str::ImmutableStringDeserializer; use crate::dynamic::{Dynamic, Union}; use crate::parse_error::ParseErrorType; use crate::result::EvalAltResult; -use crate::token::Position; +use crate::token::NO_POS; use crate::utils::ImmutableString; use serde::de::{ @@ -45,12 +45,8 @@ impl<'de> DynamicDeserializer<'de> { } /// Shortcut for a type conversion error. fn type_error_str(&self, error: &str) -> Result> { - EvalAltResult::ErrorMismatchOutputType( - error.into(), - self.value.type_name().into(), - Position::none(), - ) - .into() + EvalAltResult::ErrorMismatchOutputType(error.into(), self.value.type_name().into(), NO_POS) + .into() } fn deserialize_int>( &mut self, @@ -127,8 +123,11 @@ pub fn from_dynamic<'de, T: Deserialize<'de>>( impl Error for Box { fn custom(err: T) -> Self { - EvalAltResult::ErrorParsing(ParseErrorType::BadInput(err.to_string()), Position::none()) - .into() + EvalAltResult::ErrorParsing(ParseErrorType::BadInput( + LexError::ImproperSymbol(err.to_string()), + NO_POS, + )) + .into() } } diff --git a/src/serde_impl/ser.rs b/src/serde_impl/ser.rs index c7a77251..4074577c 100644 --- a/src/serde_impl/ser.rs +++ b/src/serde_impl/ser.rs @@ -2,7 +2,7 @@ use crate::dynamic::Dynamic; use crate::result::EvalAltResult; -use crate::token::Position; +use crate::token::NO_POS; #[cfg(not(feature = "no_index"))] use crate::engine::Array; @@ -99,7 +99,7 @@ pub fn to_dynamic(value: T) -> Result> impl Error for Box { fn custom(err: T) -> Self { - EvalAltResult::ErrorRuntime(err.to_string().into(), Position::none()).into() + EvalAltResult::ErrorRuntime(err.to_string().into(), NO_POS).into() } } @@ -295,24 +295,16 @@ impl Serializer for &mut DynamicSerializer { make_variant(_variant, content) } #[cfg(feature = "no_object")] - return EvalAltResult::ErrorMismatchOutputType( - "Dynamic".into(), - "map".into(), - Position::none(), - ) - .into(); + return EvalAltResult::ErrorMismatchOutputType("Dynamic".into(), "map".into(), NO_POS) + .into(); } fn serialize_seq(self, _len: Option) -> Result> { #[cfg(not(feature = "no_index"))] return Ok(DynamicSerializer::new(Array::new().into())); #[cfg(feature = "no_index")] - return EvalAltResult::ErrorMismatchOutputType( - "Dynamic".into(), - "array".into(), - Position::none(), - ) - .into(); + return EvalAltResult::ErrorMismatchOutputType("Dynamic".into(), "array".into(), NO_POS) + .into(); } fn serialize_tuple(self, len: usize) -> Result> { @@ -345,12 +337,7 @@ impl Serializer for &mut DynamicSerializer { let err_type = "map"; #[cfg(not(feature = "no_object"))] let err_type = "array"; - EvalAltResult::ErrorMismatchOutputType( - "Dynamic".into(), - err_type.into(), - Position::none(), - ) - .into() + EvalAltResult::ErrorMismatchOutputType("Dynamic".into(), err_type.into(), NO_POS).into() } } @@ -358,12 +345,8 @@ impl Serializer for &mut DynamicSerializer { #[cfg(not(feature = "no_object"))] return Ok(DynamicSerializer::new(Map::new().into())); #[cfg(feature = "no_object")] - return EvalAltResult::ErrorMismatchOutputType( - "Dynamic".into(), - "map".into(), - Position::none(), - ) - .into(); + return EvalAltResult::ErrorMismatchOutputType("Dynamic".into(), "map".into(), NO_POS) + .into(); } fn serialize_struct( @@ -387,12 +370,8 @@ impl Serializer for &mut DynamicSerializer { map: Map::with_capacity(_len), }); #[cfg(feature = "no_object")] - return EvalAltResult::ErrorMismatchOutputType( - "Dynamic".into(), - "map".into(), - Position::none(), - ) - .into(); + return EvalAltResult::ErrorMismatchOutputType("Dynamic".into(), "map".into(), NO_POS) + .into(); } } @@ -501,11 +480,7 @@ impl SerializeMap for DynamicSerializer { let key = mem::take(&mut self._key) .take_immutable_string() .map_err(|typ| { - EvalAltResult::ErrorMismatchOutputType( - "string".into(), - typ.into(), - Position::none(), - ) + EvalAltResult::ErrorMismatchOutputType("string".into(), typ.into(), NO_POS) })?; let _value = _value.serialize(&mut *self)?; let map = self._value.downcast_mut::().unwrap(); @@ -525,11 +500,7 @@ impl SerializeMap for DynamicSerializer { { let _key: Dynamic = _key.serialize(&mut *self)?; let _key = _key.take_immutable_string().map_err(|typ| { - EvalAltResult::ErrorMismatchOutputType( - "string".into(), - typ.into(), - Position::none(), - ) + EvalAltResult::ErrorMismatchOutputType("string".into(), typ.into(), NO_POS) })?; let _value = _value.serialize(&mut *self)?; let map = self._value.downcast_mut::().unwrap(); diff --git a/src/serde_impl/str.rs b/src/serde_impl/str.rs index 8283cc79..c499ad49 100644 --- a/src/serde_impl/str.rs +++ b/src/serde_impl/str.rs @@ -1,7 +1,7 @@ //! Implement deserialization support of `ImmutableString` for [`serde`](https://crates.io/crates/serde). use crate::result::EvalAltResult; -use crate::token::Position; +use crate::token::NO_POS; use crate::utils::ImmutableString; use serde::de::{Deserializer, Visitor}; @@ -20,12 +20,8 @@ impl<'a> ImmutableStringDeserializer<'a> { } /// Shortcut for a type conversion error. fn type_error(&self) -> Result> { - EvalAltResult::ErrorMismatchOutputType( - type_name::().into(), - "string".into(), - Position::none(), - ) - .into() + EvalAltResult::ErrorMismatchOutputType(type_name::().into(), "string".into(), NO_POS) + .into() } } diff --git a/src/syntax.rs b/src/syntax.rs index 4d0a6519..e03c6a21 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -6,7 +6,7 @@ use crate::engine::{Engine, EvalContext, MARKER_BLOCK, MARKER_EXPR, MARKER_IDENT use crate::fn_native::{SendSync, Shared}; use crate::parse_error::{LexError, ParseError}; use crate::result::EvalAltResult; -use crate::token::{is_valid_identifier, Position, Token}; +use crate::token::{is_valid_identifier, Position, Token, NO_POS}; use crate::utils::ImmutableString; use crate::StaticVec; @@ -139,7 +139,7 @@ impl Engine { segments.len() + 1, s )) - .into_err(Position::none()) + .into_err(NO_POS) .into()); } // Identifier in first position @@ -156,7 +156,7 @@ impl Engine { segments.len() + 1, s )) - .into_err(Position::none()) + .into_err(NO_POS) .into()); } }; diff --git a/src/token.rs b/src/token.rs index 3e4baba9..944b2f48 100644 --- a/src/token.rs +++ b/src/token.rs @@ -44,6 +44,9 @@ pub struct Position { pos: u16, } +/// No `Position`. +pub const NO_POS: Position = Position { line: 0, pos: 0 }; + impl Position { /// Create a new `Position`. /// @@ -121,7 +124,7 @@ impl Position { /// Create a `Position` representing no position. #[inline(always)] pub fn none() -> Self { - Self { line: 0, pos: 0 } + NO_POS } /// Is there no `Position`? diff --git a/tests/data_size.rs b/tests/data_size.rs index ca91fb5c..3c67b4bc 100644 --- a/tests/data_size.rs +++ b/tests/data_size.rs @@ -38,7 +38,7 @@ fn test_max_string_size() -> Result<(), Box> { "# ) .expect_err("should error"), - EvalAltResult::ErrorDataTooLarge(_, 10, 13, _) + EvalAltResult::ErrorDataTooLarge(_, _) )); #[cfg(not(feature = "no_object"))] @@ -52,7 +52,7 @@ fn test_max_string_size() -> Result<(), Box> { "# ) .expect_err("should error"), - EvalAltResult::ErrorDataTooLarge(_, 10, 100, _) + EvalAltResult::ErrorDataTooLarge(_, _) )); engine.set_max_string_size(0); @@ -98,7 +98,7 @@ fn test_max_array_size() -> Result<(), Box> { " ) .expect_err("should error"), - EvalAltResult::ErrorDataTooLarge(_, 10, 12, _) + EvalAltResult::ErrorDataTooLarge(_, _) )); #[cfg(not(feature = "no_object"))] @@ -112,7 +112,7 @@ fn test_max_array_size() -> Result<(), Box> { " ) .expect_err("should error"), - EvalAltResult::ErrorDataTooLarge(_, 10, 100, _) + EvalAltResult::ErrorDataTooLarge(_, _) )); assert!(matches!( @@ -124,7 +124,7 @@ fn test_max_array_size() -> Result<(), Box> { " ) .expect_err("should error"), - EvalAltResult::ErrorDataTooLarge(_, 10, 12, _) + EvalAltResult::ErrorDataTooLarge(_, _) )); #[cfg(not(feature = "no_object"))] @@ -137,7 +137,7 @@ fn test_max_array_size() -> Result<(), Box> { " ) .expect_err("should error"), - EvalAltResult::ErrorDataTooLarge(_, 10, 12, _) + EvalAltResult::ErrorDataTooLarge(_, _) )); assert!(matches!( @@ -151,7 +151,7 @@ fn test_max_array_size() -> Result<(), Box> { " ) .expect_err("should error"), - EvalAltResult::ErrorDataTooLarge(_, 10, 12, _) + EvalAltResult::ErrorDataTooLarge(_, _) )); engine.set_max_array_size(0); @@ -216,7 +216,7 @@ fn test_max_map_size() -> Result<(), Box> { " ) .expect_err("should error"), - EvalAltResult::ErrorDataTooLarge(_, 10, 12, _) + EvalAltResult::ErrorDataTooLarge(_, _) )); assert!(matches!( @@ -228,7 +228,7 @@ fn test_max_map_size() -> Result<(), Box> { " ) .expect_err("should error"), - EvalAltResult::ErrorDataTooLarge(_, 10, 12, _) + EvalAltResult::ErrorDataTooLarge(_, _) )); #[cfg(not(feature = "no_index"))] @@ -241,7 +241,7 @@ fn test_max_map_size() -> Result<(), Box> { " ) .expect_err("should error"), - EvalAltResult::ErrorDataTooLarge(_, 10, 12, _) + EvalAltResult::ErrorDataTooLarge(_, _) )); engine.set_max_map_size(0); diff --git a/tests/operations.rs b/tests/operations.rs index dae539f0..a0f52550 100644 --- a/tests/operations.rs +++ b/tests/operations.rs @@ -1,5 +1,5 @@ #![cfg(not(feature = "unchecked"))] -use rhai::{Engine, EvalAltResult}; +use rhai::{Engine, EvalAltResult, INT}; #[test] fn test_max_operations() -> Result<(), Box> { @@ -10,7 +10,7 @@ fn test_max_operations() -> Result<(), Box> { if count % 100 == 0 { println!("{}", count); } - true + None }); engine.eval::<()>("let x = 0; while x < 20 { x += 1; }")?; @@ -38,7 +38,7 @@ fn test_max_operations_functions() -> Result<(), Box> { if count % 100 == 0 { println!("{}", count); } - true + None }); engine.eval::<()>( @@ -94,7 +94,7 @@ fn test_max_operations_eval() -> Result<(), Box> { if count % 100 == 0 { println!("{}", count); } - true + None }); assert!(matches!( @@ -117,13 +117,19 @@ fn test_max_operations_progress() -> Result<(), Box> { let mut engine = Engine::new(); engine.set_max_operations(500); - engine.on_progress(|&count| count < 100); + engine.on_progress(|&count| { + if count < 100 { + None + } else { + Some((42 as INT).into()) + } + }); assert!(matches!( *engine .eval::<()>("for x in range(0, 500) {}") .expect_err("should error"), - EvalAltResult::ErrorTerminated(_) + EvalAltResult::ErrorTerminated(x, _) if x.as_int()? == 42 )); Ok(()) diff --git a/tests/syntax.rs b/tests/syntax.rs index 9cda2bf6..d6a875cc 100644 --- a/tests/syntax.rs +++ b/tests/syntax.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, ParseError, ParseErrorType, Position, INT}; +use rhai::{Engine, EvalAltResult, LexError, ParseError, ParseErrorType, INT, NO_POS}; #[test] fn test_custom_syntax() -> Result<(), Box> { @@ -68,9 +68,9 @@ fn test_custom_syntax() -> Result<(), Box> { .register_custom_syntax(&["!"], 0, |_, _| Ok(().into())) .expect_err("should error") .0, - ParseErrorType::BadInput( + ParseErrorType::BadInput(LexError::ImproperSymbol( "Improper symbol for custom syntax at position #1: '!'".to_string() - ) + )) ); Ok(()) @@ -88,8 +88,10 @@ fn test_custom_syntax_raw() -> Result<(), Box> { 2 => match stream[1].as_str() { "world" | "kitty" => Ok(None), s => Err(ParseError( - Box::new(ParseErrorType::BadInput(s.to_string())), - Position::none(), + Box::new(ParseErrorType::BadInput(LexError::ImproperSymbol( + s.to_string(), + ))), + NO_POS, )), }, _ => unreachable!(), @@ -109,7 +111,7 @@ fn test_custom_syntax_raw() -> Result<(), Box> { assert_eq!(engine.eval::("hello kitty")?, 42); assert_eq!( *engine.compile("hello hey").expect_err("should error").0, - ParseErrorType::BadInput("hey".to_string()) + ParseErrorType::BadInput(LexError::ImproperSymbol("hey".to_string())) ); Ok(()) diff --git a/tests/tokens.rs b/tests/tokens.rs index e393663d..0070c23d 100644 --- a/tests/tokens.rs +++ b/tests/tokens.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, ParseErrorType, RegisterFn, INT}; +use rhai::{Engine, EvalAltResult, LexError, ParseErrorType, RegisterFn, INT}; #[test] fn test_tokens_disabled() { @@ -16,10 +16,13 @@ fn test_tokens_disabled() { engine.disable_symbol("+="); // disable the '+=' operator - assert!(matches!( - *engine.compile("let x = 40 + 2; x += 1;").expect_err("should error").0, - ParseErrorType::BadInput(ref s) if s == "Unexpected '+='" - )); + assert_eq!( + *engine + .compile("let x = 40 + 2; x += 1;") + .expect_err("should error") + .0, + ParseErrorType::BadInput(LexError::UnexpectedInput("+=".to_string())) + ); } #[test] diff --git a/tests/var_scope.rs b/tests/var_scope.rs index 4faf37b5..e6dc825a 100644 --- a/tests/var_scope.rs +++ b/tests/var_scope.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, Position, Scope, INT}; +use rhai::{Engine, EvalAltResult, Scope, INT, NO_POS}; #[test] fn test_var_scope() -> Result<(), Box> { @@ -67,7 +67,7 @@ fn test_var_resolver() -> Result<(), Box> { "MYSTIC_NUMBER" => Ok(Some((42 as INT).into())), // Override a variable - make it not found even if it exists! "DO_NOT_USE" => { - Err(EvalAltResult::ErrorVariableNotFound(name.to_string(), Position::none()).into()) + Err(EvalAltResult::ErrorVariableNotFound(name.to_string(), NO_POS).into()) } // Silently maps 'chameleon' into 'innocent'. "chameleon" => context @@ -75,7 +75,7 @@ fn test_var_resolver() -> Result<(), Box> { .get_value("innocent") .map(Some) .ok_or_else(|| { - EvalAltResult::ErrorVariableNotFound(name.to_string(), Position::none()).into() + EvalAltResult::ErrorVariableNotFound(name.to_string(), NO_POS).into() }), // Return Ok(None) to continue with the normal variable resolution process. _ => Ok(None), From cc304ba5134c87d9cd3fa46cd156a418e6c6a79a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 2 Nov 2020 13:18:37 +0800 Subject: [PATCH 6/8] Fix serde build. --- src/optimize.rs | 2 +- src/serde_impl/de.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/optimize.rs b/src/optimize.rs index 1ef43a5a..55c5d988 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -10,7 +10,7 @@ use crate::fn_call::run_builtin_binary_op; use crate::module::Module; use crate::parser::map_dynamic_to_expr; use crate::scope::Scope; -use crate::token::{is_valid_identifier, NO_POS}; +use crate::token::{is_valid_identifier, NO_POS}; use crate::{calc_native_fn_hash, StaticVec}; #[cfg(not(feature = "no_function"))] diff --git a/src/serde_impl/de.rs b/src/serde_impl/de.rs index a607c589..be0a5633 100644 --- a/src/serde_impl/de.rs +++ b/src/serde_impl/de.rs @@ -2,7 +2,7 @@ use super::str::ImmutableStringDeserializer; use crate::dynamic::{Dynamic, Union}; -use crate::parse_error::ParseErrorType; +use crate::parse_error::{LexError, ParseErrorType}; use crate::result::EvalAltResult; use crate::token::NO_POS; use crate::utils::ImmutableString; @@ -123,10 +123,10 @@ pub fn from_dynamic<'de, T: Deserialize<'de>>( impl Error for Box { fn custom(err: T) -> Self { - EvalAltResult::ErrorParsing(ParseErrorType::BadInput( - LexError::ImproperSymbol(err.to_string()), + EvalAltResult::ErrorParsing( + ParseErrorType::BadInput(LexError::ImproperSymbol(err.to_string())), NO_POS, - )) + ) .into() } } From b9de8eaa7f2bc8a1184fde09d798bad2519620b1 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 2 Nov 2020 23:54:19 +0800 Subject: [PATCH 7/8] Minor code refactor. --- src/ast.rs | 42 ++++++++++---------- src/engine.rs | 71 +++++++++++++++++---------------- src/fn_call.rs | 12 +++--- src/fn_native.rs | 2 +- src/optimize.rs | 10 ++--- src/parser.rs | 52 ++++++++++++------------ src/scope.rs | 8 ++-- src/token.rs | 101 +++++++++++++++++++++-------------------------- 8 files changed, 145 insertions(+), 153 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index bb99d2f3..c13c2b8b 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -861,24 +861,24 @@ pub struct BinaryExpr { /// A function call. #[derive(Debug, Clone, Hash, Default)] pub struct FnCallInfo { - /// Function name. - /// Use `Cow<'static, str>` because a lot of operators (e.g. `==`, `>=`) are implemented as function calls - /// and the function names are predictable, so no need to allocate a new `String`. - pub name: Cow<'static, str>, - /// Namespace of the function, if any. - pub namespace: Option, + /// Pre-calculated hash for a script-defined function of the same name and number of parameters. + pub hash: u64, /// Call native functions only? Set to `true` to skip searching for script-defined function overrides /// when it is certain that the function must be native (e.g. an operator). pub native_only: bool, /// Does this function call capture the parent scope? pub capture: bool, - /// Pre-calculated hash for a script-defined function of the same name and number of parameters. - pub hash: u64, - /// List of function call arguments. - pub args: StaticVec, /// Default value when the function is not found, mostly used to provide a default for comparison functions. /// Type is `bool` in order for `FnCallInfo` to be `Hash` pub def_value: Option, + /// Namespace of the function, if any. + pub namespace: Option>, + /// Function name. + /// Use `Cow<'static, str>` because a lot of operators (e.g. `==`, `>=`) are implemented as function calls + /// and the function names are predictable, so no need to allocate a new `String`. + pub name: Cow<'static, str>, + /// List of function call arguments. + pub args: StaticVec, } /// _[INTERNALS]_ An expression sub-tree. @@ -903,10 +903,10 @@ pub enum Expr { StringConstant(Box), /// FnPtr constant. FnPointer(Box), - /// Variable access - (variable name, optional modules, hash, optional index) - Variable(Box<(Ident, Option, u64, Option)>), - /// Property access. - Property(Box<(IdentX, (String, String))>), + /// Variable access - (optional index, optional modules, hash, variable name) + Variable(Box<(Option, Option>, u64, Ident)>), + /// Property access - (getter, setter), prop + Property(Box<((String, String), IdentX)>), /// { stmt } Stmt(Box, Position), /// Wrapped expression - should not be optimized away. @@ -1014,7 +1014,7 @@ impl Expr { /// Is the expression a simple variable access? pub(crate) fn get_variable_access(&self, non_qualified: bool) -> Option<&str> { match self { - Self::Variable(x) if !non_qualified || x.1.is_none() => Some((x.0).name.as_str()), + Self::Variable(x) if !non_qualified || x.1.is_none() => Some((x.3).name.as_str()), _ => None, } } @@ -1033,9 +1033,9 @@ impl Expr { Self::FnPointer(x) => x.pos, Self::Array(_, pos) => *pos, Self::Map(_, pos) => *pos, - Self::Property(x) => (x.0).pos, + Self::Property(x) => (x.1).pos, Self::Stmt(_, pos) => *pos, - Self::Variable(x) => (x.0).pos, + Self::Variable(x) => (x.3).pos, Self::FnCall(_, pos) => *pos, Self::And(x, _) | Self::Or(x, _) | Self::In(x, _) => x.lhs.position(), @@ -1064,8 +1064,8 @@ impl Expr { Self::FnPointer(x) => x.pos = new_pos, Self::Array(_, pos) => *pos = new_pos, Self::Map(_, pos) => *pos = new_pos, - Self::Variable(x) => (x.0).pos = new_pos, - Self::Property(x) => (x.0).pos = new_pos, + Self::Variable(x) => (x.3).pos = new_pos, + Self::Property(x) => (x.1).pos = new_pos, Self::Stmt(_, pos) => *pos = new_pos, Self::FnCall(_, pos) => *pos = new_pos, Self::And(_, pos) | Self::Or(_, pos) | Self::In(_, pos) => *pos = new_pos, @@ -1231,10 +1231,10 @@ impl Expr { pub(crate) fn into_property(self) -> Self { match self { Self::Variable(x) if x.1.is_none() => { - let ident = x.0; + let ident = x.3; let getter = make_getter(&ident.name); let setter = make_setter(&ident.name); - Self::Property(Box::new((ident.into(), (getter, setter)))) + Self::Property(Box::new(((getter, setter), ident.into()))) } _ => self, } diff --git a/src/engine.rs b/src/engine.rs index a9a2a636..9792c766 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -77,7 +77,7 @@ pub type Map = HashMap; // `Imports` is implemented as two `Vec`'s of exactly the same length. That's because a `Module` is large, // so packing the import names together improves cache locality. #[derive(Debug, Clone, Default)] -pub struct Imports(StaticVec, StaticVec); +pub struct Imports(StaticVec, StaticVec); impl Imports { /// Get the length of this stack of imported modules. @@ -86,15 +86,15 @@ impl Imports { } /// Get the imported module at a particular index. pub fn get(&self, index: usize) -> Option<&Module> { - self.1.get(index) + self.0.get(index) } /// Get a mutable reference to the imported module at a particular index. pub fn get_mut(&mut self, index: usize) -> Option<&mut Module> { - self.1.get_mut(index) + self.0.get_mut(index) } /// Get the index of an imported module by name. pub fn find(&self, name: &str) -> Option { - self.0 + self.1 .iter() .enumerate() .rev() @@ -103,8 +103,8 @@ impl Imports { } /// Push an imported module onto the stack. pub fn push(&mut self, name: impl Into, module: Module) { - self.0.push(name.into()); - self.1.push(module); + self.0.push(module); + self.1.push(name.into()); } /// Truncate the stack of imported modules to a particular length. pub fn truncate(&mut self, size: usize) { @@ -114,11 +114,11 @@ impl Imports { /// Get an iterator to this stack of imported modules. #[allow(dead_code)] pub fn iter(&self) -> impl Iterator { - self.0.iter().map(|name| name.as_str()).zip(self.1.iter()) + self.1.iter().map(|name| name.as_str()).zip(self.0.iter()) } /// Get a consuming iterator to this stack of imported modules. pub fn into_iter(self) -> impl Iterator { - self.0.into_iter().zip(self.1.into_iter()) + self.1.into_iter().zip(self.0.into_iter()) } } @@ -812,7 +812,7 @@ impl Engine { match expr { Expr::Variable(v) => match v.as_ref() { // Qualified variable - (Ident { name, pos }, Some(modules), hash_var, _) => { + (_, Some(modules), hash_var, Ident { name, pos }) => { let module = search_imports_mut(mods, state, modules)?; let target = module.get_qualified_var_mut(*hash_var).map_err(|mut err| { match *err { @@ -844,7 +844,7 @@ impl Engine { this_ptr: &'s mut Option<&mut Dynamic>, expr: &'a Expr, ) -> Result<(Target<'s>, &'a str, ScopeEntryType, Position), Box> { - let (Ident { name, pos }, _, _, index) = match expr { + let (index, _, _, Ident { name, pos }) = match expr { Expr::Variable(v) => v.as_ref(), _ => unreachable!(), }; @@ -984,7 +984,7 @@ impl Engine { let args = &mut [val, &mut idx_val2, &mut new_val.0]; self.exec_fn_call( - state, lib, FN_IDX_SET, 0, args, is_ref, true, false, None, &None, + state, lib, FN_IDX_SET, 0, args, is_ref, true, false, None, None, level, ) .map_err(|err| match *err { @@ -1026,8 +1026,7 @@ impl Engine { let def_value = def_value.map(Into::::into); let args = idx_val.as_fn_call_args(); self.make_method_call( - state, lib, name, *hash, target, args, &def_value, *native, false, - level, + state, lib, name, *hash, target, args, def_value, *native, false, level, ) .map_err(|err| err.fill_position(*pos)) } @@ -1035,7 +1034,7 @@ impl Engine { Expr::FnCall(_, _) => unreachable!(), // {xxx:map}.id = ??? Expr::Property(x) if target.is::() && new_val.is_some() => { - let IdentX { name, pos } = &x.0; + let IdentX { name, pos } = &x.1; let index = name.clone().into(); let mut val = self .get_indexed_mut(state, lib, target, index, *pos, true, false, level)?; @@ -1045,7 +1044,7 @@ impl Engine { } // {xxx:map}.id Expr::Property(x) if target.is::() => { - let IdentX { name, pos } = &x.0; + let IdentX { name, pos } = &x.1; let index = name.clone().into(); let val = self.get_indexed_mut( state, lib, target, index, *pos, false, false, level, @@ -1055,11 +1054,11 @@ impl Engine { } // xxx.id = ??? Expr::Property(x) if new_val.is_some() => { - let (IdentX { pos, .. }, (_, setter)) = x.as_ref(); + let ((_, setter), IdentX { pos, .. }) = x.as_ref(); let mut new_val = new_val; let mut args = [target.as_mut(), &mut new_val.as_mut().unwrap().0]; self.exec_fn_call( - state, lib, setter, 0, &mut args, is_ref, true, false, None, &None, + state, lib, setter, 0, &mut args, is_ref, true, false, None, None, level, ) .map(|(v, _)| (v, true)) @@ -1067,10 +1066,10 @@ impl Engine { } // xxx.id Expr::Property(x) => { - let (IdentX { pos, .. }, (getter, _)) = x.as_ref(); + let ((getter, _), IdentX { pos, .. }) = x.as_ref(); let mut args = [target.as_mut()]; self.exec_fn_call( - state, lib, getter, 0, &mut args, is_ref, true, false, None, &None, + state, lib, getter, 0, &mut args, is_ref, true, false, None, None, level, ) .map(|(v, _)| (v, false)) @@ -1080,7 +1079,7 @@ impl Engine { Expr::Index(x, x_pos) | Expr::Dot(x, x_pos) if target.is::() => { let mut val = match &x.lhs { Expr::Property(p) => { - let IdentX { name, pos } = &p.0; + let IdentX { name, pos } = &p.1; let index = name.clone().into(); self.get_indexed_mut( state, lib, target, index, *pos, false, true, level, @@ -1099,7 +1098,7 @@ impl Engine { let args = idx_val.as_fn_call_args(); let (val, _) = self .make_method_call( - state, lib, name, *hash, target, args, &def_value, *native, + state, lib, name, *hash, target, args, def_value, *native, false, level, ) .map_err(|err| err.fill_position(*pos))?; @@ -1122,13 +1121,13 @@ impl Engine { match &x.lhs { // xxx.prop[expr] | xxx.prop.expr Expr::Property(p) => { - let (IdentX { pos, .. }, (getter, setter)) = p.as_ref(); + let ((getter, setter), IdentX { 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 .exec_fn_call( state, lib, getter, 0, args, is_ref, true, false, None, - &None, level, + None, level, ) .map_err(|err| err.fill_position(*pos))?; @@ -1154,7 +1153,7 @@ impl Engine { arg_values[1] = val; self.exec_fn_call( state, lib, setter, 0, arg_values, is_ref, true, false, - None, &None, level, + None, None, level, ) .or_else( |err| match *err { @@ -1182,7 +1181,7 @@ impl Engine { let args = idx_val.as_fn_call_args(); let (mut val, _) = self .make_method_call( - state, lib, name, *hash, target, args, &def_value, *native, + state, lib, name, *hash, target, args, def_value, *native, false, level, ) .map_err(|err| err.fill_position(*pos))?; @@ -1257,7 +1256,7 @@ impl Engine { let Ident { name: var_name, pos: var_pos, - } = &x.0; + } = &x.3; self.inc_operations(state) .map_err(|err| err.fill_position(*var_pos))?; @@ -1458,7 +1457,7 @@ impl Engine { let mut idx = idx; let args = &mut [val, &mut idx]; self.exec_fn_call( - state, _lib, FN_IDX_GET, 0, args, is_ref, true, false, None, &None, _level, + state, _lib, FN_IDX_GET, 0, args, is_ref, true, false, None, None, _level, ) .map(|(v, _)| v.into()) .map_err(|err| match *err { @@ -1504,13 +1503,14 @@ impl Engine { for value in rhs_value.iter_mut() { let args = &mut [&mut lhs_value.clone(), value]; + let def_value = def_value.clone(); // Qualifiers (none) + function name + number of arguments + argument `TypeId`'s. let hash = calc_native_fn_hash(empty(), OP_FUNC, args.iter().map(|a| a.type_id())); if self - .call_native_fn(state, lib, OP_FUNC, hash, args, false, false, &def_value) + .call_native_fn(state, lib, OP_FUNC, hash, args, false, false, def_value) .map_err(|err| err.fill_position(rhs.position()))? .0 .as_bool() @@ -1520,7 +1520,7 @@ impl Engine { } } - Ok(def_value.unwrap()) + Ok(false.into()) } #[cfg(not(feature = "no_object"))] Dynamic(Union::Map(rhs_value)) => match lhs_value { @@ -1564,11 +1564,11 @@ impl Engine { Expr::FnPointer(x) => { Ok(FnPtr::new_unchecked(x.name.clone(), Default::default()).into()) } - Expr::Variable(x) if (x.0).name == KEYWORD_THIS => { + Expr::Variable(x) if (x.3).name == KEYWORD_THIS => { if let Some(val) = this_ptr { Ok(val.clone()) } else { - EvalAltResult::ErrorUnboundThis((x.0).pos).into() + EvalAltResult::ErrorUnboundThis((x.3).pos).into() } } Expr::Variable(_) => { @@ -1623,7 +1623,7 @@ impl Engine { } = x.as_ref(); let def_value = def_value.map(Into::::into); self.make_function_call( - scope, mods, state, lib, this_ptr, name, args, &def_value, *hash, *native, + scope, mods, state, lib, this_ptr, name, args, def_value, *hash, *native, false, *cap_scope, level, ) .map_err(|err| err.fill_position(*pos)) @@ -1639,8 +1639,9 @@ impl Engine { def_value, .. } = x.as_ref(); + let modules = namespace.as_ref().map(|v| v.as_ref()); self.make_qualified_function_call( - scope, mods, state, lib, this_ptr, namespace, name, args, *def_value, *hash, + scope, mods, state, lib, this_ptr, modules, name, args, *def_value, *hash, level, ) .map_err(|err| err.fill_position(*pos)) @@ -1812,7 +1813,7 @@ impl Engine { // Run function let (value, _) = self .exec_fn_call( - state, lib, op, 0, args, false, false, false, None, &None, + state, lib, op, 0, args, false, false, false, None, None, level, ) .map_err(|err| err.fill_position(*op_pos))?; @@ -1850,7 +1851,7 @@ impl Engine { let result = self .exec_fn_call( - state, lib, op, 0, args, false, false, false, None, &None, level, + state, lib, op, 0, args, false, false, false, None, None, level, ) .map(|(v, _)| v) .map_err(|err| err.fill_position(*op_pos))?; diff --git a/src/fn_call.rs b/src/fn_call.rs index 313b4b15..1a645870 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -186,7 +186,7 @@ impl Engine { args: &mut FnCallArgs, is_ref: bool, pub_only: bool, - def_val: &Option, + def_val: Option, ) -> Result<(Dynamic, bool), Box> { self.inc_operations(state)?; @@ -254,7 +254,7 @@ impl Engine { // Return default value (if any) if let Some(val) = def_val { - return Ok((val.clone(), false)); + return Ok((val, false)); } // Getter function not found? @@ -479,7 +479,7 @@ impl Engine { _is_method: bool, pub_only: bool, _capture_scope: Option, - def_val: &Option, + def_val: Option, _level: usize, ) -> Result<(Dynamic, bool), Box> { // Check for data race. @@ -686,7 +686,7 @@ impl Engine { hash_script: u64, target: &mut Target, mut call_args: StaticVec, - def_val: &Option, + def_val: Option, native: bool, pub_only: bool, level: usize, @@ -837,7 +837,7 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, name: &str, args_expr: impl AsRef<[Expr]>, - def_val: &Option, + def_val: Option, mut hash_script: u64, native: bool, pub_only: bool, @@ -1074,7 +1074,7 @@ impl Engine { state: &mut State, lib: &[&Module], this_ptr: &mut Option<&mut Dynamic>, - modules: &Option, + modules: Option<&ModuleRef>, name: &str, args_expr: impl AsRef<[Expr]>, def_val: Option, diff --git a/src/fn_native.rs b/src/fn_native.rs index f41490d0..56488ac2 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -193,7 +193,7 @@ impl FnPtr { has_this, true, None, - &None, + None, 0, ) .map(|(v, _)| v) diff --git a/src/optimize.rs b/src/optimize.rs index 55c5d988..3895fd52 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -147,7 +147,7 @@ fn call_fn_with_constant_arguments( arg_values.iter_mut().collect::>().as_mut(), false, true, - &None, + None, ) .ok() .map(|(v, _)| v) @@ -454,7 +454,7 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { Expr::Dot(x, dot_pos) => match (x.lhs, x.rhs) { // map.string (Expr::Map(m, pos), Expr::Property(p)) if m.iter().all(|(_, x)| x.is_pure()) => { - let prop = &p.0.name; + let prop = &p.1.name; // Map literal where everything is pure - promote the indexed item. // All other items can be thrown away. state.set_dirty(); @@ -686,12 +686,12 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { } // constant-name - Expr::Variable(x) if x.1.is_none() && state.contains_constant(&x.0.name) => { + Expr::Variable(x) if x.1.is_none() && state.contains_constant(&x.3.name) => { state.set_dirty(); // Replace constant with value - let mut expr = state.find_constant(&x.0.name).unwrap().clone(); - expr.set_position(x.0.pos); + let mut expr = state.find_constant(&x.3.name).unwrap().clone(); + expr.set_position(x.3.pos); expr } diff --git a/src/parser.rs b/src/parser.rs index 61fc858b..df7a28ad 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -267,7 +267,7 @@ fn parse_fn_call( lib: &mut FunctionsLib, id: String, capture: bool, - mut namespace: Option, + mut namespace: Option>, settings: ParseSettings, ) -> Result { let (token, token_pos) = input.peek().unwrap(); @@ -788,7 +788,7 @@ fn parse_primary( { state.allow_capture = true; } - Expr::Variable(Box::new((Ident::new(s, settings.pos), None, 0, None))) + Expr::Variable(Box::new((None, None, 0, Ident::new(s, settings.pos)))) } // Module qualification #[cfg(not(feature = "no_module"))] @@ -798,18 +798,18 @@ fn parse_primary( { state.allow_capture = true; } - Expr::Variable(Box::new((Ident::new(s, settings.pos), None, 0, None))) + Expr::Variable(Box::new((None, None, 0, Ident::new(s, settings.pos)))) } // Normal variable access Token::Identifier(s) => { let index = state.access_var(&s, settings.pos); - Expr::Variable(Box::new((Ident::new(s, settings.pos), None, 0, index))) + Expr::Variable(Box::new((index, None, 0, Ident::new(s, settings.pos)))) } // Function call is allowed to have reserved keyword Token::Reserved(s) if *next_token == Token::LeftParen || *next_token == Token::Bang => { if is_keyword_function(&s) { - Expr::Variable(Box::new((Ident::new(s, settings.pos), None, 0, None))) + Expr::Variable(Box::new((None, None, 0, Ident::new(s, settings.pos)))) } else { return Err(PERR::Reserved(s).into_err(settings.pos)); } @@ -824,7 +824,7 @@ fn parse_primary( ))) .into_err(settings.pos)); } else { - Expr::Variable(Box::new((Ident::new(s, settings.pos), None, 0, None))) + Expr::Variable(Box::new((None, None, 0, Ident::new(s, settings.pos)))) } } @@ -883,13 +883,13 @@ fn parse_primary( .into_err(pos)); } - let (Ident { name, pos }, modules, _, _) = *x; + let (_, modules, _, Ident { name, pos }) = *x; settings.pos = pos; parse_fn_call(input, state, lib, name, true, modules, settings.level_up())? } // Function call (Expr::Variable(x), Token::LeftParen) => { - let (Ident { name, pos }, modules, _, _) = *x; + let (_, modules, _, Ident { name, pos }) = *x; settings.pos = pos; parse_fn_call(input, state, lib, name, false, modules, settings.level_up())? } @@ -897,17 +897,17 @@ fn parse_primary( // module access (Expr::Variable(x), Token::DoubleColon) => match input.next().unwrap() { (Token::Identifier(id2), pos2) => { - let (var_name_def, mut modules, _, index) = *x; + let (index, mut modules, _, var_name_def) = *x; if let Some(ref mut modules) = modules { modules.push(var_name_def); } else { let mut m: ModuleRef = Default::default(); m.push(var_name_def); - modules = Some(m); + modules = Some(Box::new(m)); } - Expr::Variable(Box::new((Ident::new(id2, pos2), modules, 0, index))) + Expr::Variable(Box::new((index, modules, 0, Ident::new(id2, pos2)))) } (Token::Reserved(id2), pos2) if is_valid_identifier(id2.chars()) => { return Err(PERR::Reserved(id2).into_err(pos2)); @@ -931,7 +931,7 @@ fn parse_primary( match &mut root_expr { // Cache the hash key for module-qualified variables Expr::Variable(x) if x.1.is_some() => { - let (Ident { name, .. }, modules, hash, _) = x.as_mut(); + let (_, modules, hash, Ident { name, .. }) = x.as_mut(); let modules = modules.as_mut().unwrap(); // Qualifiers + variable name @@ -1087,19 +1087,19 @@ fn make_assignment_stmt<'a>( ) -> Result { match &lhs { // var (non-indexed) = rhs - Expr::Variable(x) if x.3.is_none() => { + Expr::Variable(x) if x.1.is_none() => { Ok(Stmt::Assignment(Box::new((lhs, fn_name.into(), rhs)), pos)) } // var (indexed) = rhs Expr::Variable(x) => { let ( + index, + _, + _, Ident { name, pos: name_pos, }, - _, - _, - index, ) = x.as_ref(); match state.stack[(state.stack.len() - index.unwrap().get())].1 { ScopeEntryType::Normal => { @@ -1114,19 +1114,19 @@ fn make_assignment_stmt<'a>( // xxx[???] = rhs, xxx.??? = rhs Expr::Index(x, _) | Expr::Dot(x, _) => match &x.lhs { // var[???] (non-indexed) = rhs, var.??? (non-indexed) = rhs - Expr::Variable(x) if x.3.is_none() => { + Expr::Variable(x) if x.1.is_none() => { Ok(Stmt::Assignment(Box::new((lhs, fn_name.into(), rhs)), pos)) } // var[???] (indexed) = rhs, var.??? (indexed) = rhs Expr::Variable(x) => { let ( + index, + _, + _, Ident { name, pos: name_pos, }, - _, - _, - index, ) = x.as_ref(); match state.stack[(state.stack.len() - index.unwrap().get())].1 { ScopeEntryType::Normal => { @@ -1204,10 +1204,10 @@ fn make_dot_expr(lhs: Expr, rhs: Expr, op_pos: Position) -> Result { - let ident = x.0; + let ident = x.3; let getter = make_getter(&ident.name); let setter = make_setter(&ident.name); - let rhs = Expr::Property(Box::new((ident.into(), (getter, setter)))); + let rhs = Expr::Property(Box::new(((getter, setter), ident.into()))); Expr::Dot(Box::new(BinaryExpr { lhs, rhs }), op_pos) } @@ -1642,10 +1642,10 @@ fn parse_custom_syntax( (Token::Identifier(s), pos) => { segments.push(s.clone()); exprs.push(Expr::Variable(Box::new(( - Ident::new(s, pos), + None, None, 0, - None, + Ident::new(s, pos), )))); } (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { @@ -2500,12 +2500,12 @@ fn make_curry_from_externals(fn_expr: Expr, externals: StaticVec, pos: Po #[cfg(not(feature = "no_closure"))] externals.iter().for_each(|x| { - args.push(Expr::Variable(Box::new((x.clone(), None, 0, None)))); + args.push(Expr::Variable(Box::new((None, None, 0, x.clone())))); }); #[cfg(feature = "no_closure")] externals.into_iter().for_each(|x| { - args.push(Expr::Variable(Box::new((x.clone(), None, 0, None)))); + args.push(Expr::Variable(Box::new((None, None, 0, x.clone())))); }); let hash = calc_script_fn_hash(empty(), KEYWORD_FN_PTR_CURRY, num_externals + 1); diff --git a/src/scope.rs b/src/scope.rs index 77e45a87..970ecba0 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -66,12 +66,12 @@ impl EntryType { // The variable type is packed separately into another array because it is even smaller. #[derive(Debug, Clone, Default)] pub struct Scope<'a> { - /// (Name, alias) of the entry. - names: Vec<(Cow<'a, str>, Option)>, - /// Type of the entry. - types: Vec, /// Current value of the entry. values: Vec, + /// Type of the entry. + types: Vec, + /// (Name, alias) of the entry. + names: Vec<(Cow<'a, str>, Option)>, } impl<'a> Scope<'a> { diff --git a/src/token.rs b/src/token.rs index 944b2f48..a27dcd82 100644 --- a/src/token.rs +++ b/src/token.rs @@ -45,9 +45,14 @@ pub struct Position { } /// No `Position`. -pub const NO_POS: Position = Position { line: 0, pos: 0 }; +pub const NO_POS: Position = Position::NONE; impl Position { + /// A `Position` representing no position. + pub const NONE: Self = Self { line: 0, pos: 0 }; + /// A `Position` representing the first position. + pub const START: Self = Self { line: 1, pos: 0 }; + /// Create a new `Position`. /// /// `line` must not be zero. @@ -65,27 +70,24 @@ impl Position { pos: position, } } - /// Get the line number (1-based), or `None` if there is no position. #[inline(always)] - pub fn line(&self) -> Option { + pub fn line(self) -> Option { if self.is_none() { None } else { Some(self.line as usize) } } - /// Get the character position (1-based), or `None` if at beginning of a line. #[inline(always)] - pub fn position(&self) -> Option { + pub fn position(self) -> Option { if self.is_none() || self.pos == 0 { None } else { Some(self.pos as usize) } } - /// Advance by one character position. #[inline(always)] pub(crate) fn advance(&mut self) { @@ -96,7 +98,6 @@ impl Position { self.pos += 1; } } - /// Go backwards by one character position. /// /// # Panics @@ -108,7 +109,6 @@ impl Position { assert!(self.pos > 0, "cannot rewind at position 0"); self.pos -= 1; } - /// Advance to the next line. #[inline(always)] pub(crate) fn new_line(&mut self) { @@ -120,24 +120,22 @@ impl Position { self.pos = 0; } } - - /// Create a `Position` representing no position. + /// Is this `Position` at the beginning of a line? #[inline(always)] - pub fn none() -> Self { - NO_POS + pub fn is_beginning_of_line(self) -> bool { + self.line == 0 && !self.is_none() } - /// Is there no `Position`? #[inline(always)] - pub fn is_none(&self) -> bool { - self.line == 0 && self.pos == 0 + pub fn is_none(self) -> bool { + self == Self::NONE } } impl Default for Position { #[inline(always)] fn default() -> Self { - Self::new(1, 0) + Self::START } } @@ -330,7 +328,7 @@ pub enum Token { #[cfg(not(feature = "no_module"))] As, /// A lexer error. - LexError(Box), + LexError(LexError), /// A comment block. Comment(String), /// A reserved symbol. @@ -1120,9 +1118,7 @@ fn get_next_token_inner( INT::from_str_radix(&out, radix) .map(Token::IntegerConstant) .unwrap_or_else(|_| { - Token::LexError(Box::new(LERR::MalformedNumber( - result.into_iter().collect(), - ))) + Token::LexError(LERR::MalformedNumber(result.into_iter().collect())) }), start_pos, )); @@ -1136,9 +1132,7 @@ fn get_next_token_inner( return Some(( num.unwrap_or_else(|_| { - Token::LexError(Box::new(LERR::MalformedNumber( - result.into_iter().collect(), - ))) + Token::LexError(LERR::MalformedNumber(result.into_iter().collect())) }), start_pos, )); @@ -1153,7 +1147,7 @@ fn get_next_token_inner( // " - string literal ('"', _) => { return parse_string_literal(stream, state, pos, '"').map_or_else( - |err| Some((Token::LexError(Box::new(err.0)), err.1)), + |err| Some((Token::LexError(err.0), err.1)), |out| Some((Token::StringConstant(out), start_pos)), ) } @@ -1161,22 +1155,19 @@ fn get_next_token_inner( // ' - character literal ('\'', '\'') => { return Some(( - Token::LexError(Box::new(LERR::MalformedChar("".to_string()))), + Token::LexError(LERR::MalformedChar("".to_string())), start_pos, )) } ('\'', _) => { return Some(parse_string_literal(stream, state, pos, '\'').map_or_else( - |err| (Token::LexError(Box::new(err.0)), err.1), + |err| (Token::LexError(err.0), err.1), |result| { let mut chars = result.chars(); let first = chars.next().unwrap(); if chars.next().is_some() { - ( - Token::LexError(Box::new(LERR::MalformedChar(result))), - start_pos, - ) + (Token::LexError(LERR::MalformedChar(result)), start_pos) } else { (Token::CharConstant(first), start_pos) } @@ -1452,7 +1443,7 @@ fn get_next_token_inner( } (ch, _) => { return Some(( - Token::LexError(Box::new(LERR::UnexpectedInput(ch.to_string()))), + Token::LexError(LERR::UnexpectedInput(ch.to_string())), start_pos, )) } @@ -1494,7 +1485,7 @@ fn get_identifier( if !is_valid_identifier { return Some(( - Token::LexError(Box::new(LERR::MalformedIdentifier(identifier))), + Token::LexError(LERR::MalformedIdentifier(identifier)), start_pos, )); } @@ -1652,42 +1643,42 @@ impl<'a> Iterator for TokenIterator<'a, '_> { Some((Token::Reserved(s), pos)) => Some((match (s.as_str(), self.engine.custom_keywords.contains_key(&s)) { - ("===", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + ("===", false) => Token::LexError(LERR::ImproperSymbol( "'===' is not a valid operator. This is not JavaScript! Should it be '=='?".to_string(), - ))), - ("!==", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + )), + ("!==", false) => Token::LexError(LERR::ImproperSymbol( "'!==' is not a valid operator. This is not JavaScript! Should it be '!='?".to_string(), - ))), - ("->", false) => Token::LexError(Box::new(LERR::ImproperSymbol( - "'->' is not a valid symbol. This is not C or C++!".to_string()))), - ("<-", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + )), + ("->", false) => Token::LexError(LERR::ImproperSymbol( + "'->' is not a valid symbol. This is not C or C++!".to_string())), + ("<-", false) => Token::LexError(LERR::ImproperSymbol( "'<-' is not a valid symbol. This is not Go! Should it be '<='?".to_string(), - ))), - ("=>", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + )), + ("=>", false) => Token::LexError(LERR::ImproperSymbol( "'=>' is not a valid symbol. This is not Rust! Should it be '>='?".to_string(), - ))), - (":=", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + )), + (":=", false) => Token::LexError(LERR::ImproperSymbol( "':=' is not a valid assignment operator. This is not Go! Should it be simply '='?".to_string(), - ))), - ("::<", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + )), + ("::<", false) => Token::LexError(LERR::ImproperSymbol( "'::<>' is not a valid symbol. This is not Rust! Should it be '::'?".to_string(), - ))), - ("(*", false) | ("*)", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + )), + ("(*", false) | ("*)", false) => Token::LexError(LERR::ImproperSymbol( "'(* .. *)' is not a valid comment format. This is not Pascal! Should it be '/* .. */'?".to_string(), - ))), - ("#", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + )), + ("#", false) => Token::LexError(LERR::ImproperSymbol( "'#' is not a valid symbol. Should it be '#{'?".to_string(), - ))), + )), // Reserved keyword/operator that is custom. (_, true) => Token::Custom(s), // Reserved operator that is not custom. - (token, false) if !is_valid_identifier(token.chars()) => Token::LexError(Box::new(LERR::ImproperSymbol( + (token, false) if !is_valid_identifier(token.chars()) => Token::LexError(LERR::ImproperSymbol( format!("'{}' is a reserved symbol", token) - ))), + )), // Reserved keyword that is not custom and disabled. - (token, false) if self.engine.disabled_symbols.contains(token) => Token::LexError(Box::new(LERR::ImproperSymbol( + (token, false) if self.engine.disabled_symbols.contains(token) => Token::LexError(LERR::ImproperSymbol( format!("reserved symbol '{}' is disabled", token) - ))), + )), // Reserved keyword/operator that is not custom. (_, false) => Token::Reserved(s), }, pos)), @@ -1708,7 +1699,7 @@ impl<'a> Iterator for TokenIterator<'a, '_> { // Disabled operator Some((token, pos)) if token.is_operator() && self.engine.disabled_symbols.contains(token.syntax().as_ref()) => { Some(( - Token::LexError(Box::new(LexError::UnexpectedInput(token.syntax().into()))), + Token::LexError(LexError::UnexpectedInput(token.syntax().into())), pos, )) } From f74d947c6b850e377a0ee55b6f79f3c171beaa6d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 3 Nov 2020 13:08:19 +0800 Subject: [PATCH 8/8] Fix constant assignment. --- src/optimize.rs | 25 ++++++++++++++++++++----- src/parser.rs | 4 ++-- src/result.rs | 6 ++++-- tests/constants.rs | 19 +++++++++++++++++-- 4 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/optimize.rs b/src/optimize.rs index 3895fd52..6d9bc23d 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -156,11 +156,16 @@ fn call_fn_with_constant_arguments( /// Optimize a statement. fn optimize_stmt(stmt: Stmt, state: &mut State, preserve_result: bool) -> Stmt { match stmt { - // id op= expr - Stmt::Assignment(x, pos) => Stmt::Assignment( - Box::new((optimize_expr(x.0, state), x.1, optimize_expr(x.2, state))), - pos, - ), + // expr op= expr + Stmt::Assignment(x, pos) => match x.0 { + Expr::Variable(_) => { + Stmt::Assignment(Box::new((x.0, x.1, optimize_expr(x.2, state))), pos) + } + _ => Stmt::Assignment( + Box::new((optimize_expr(x.0, state), x.1, optimize_expr(x.2, state))), + pos, + ), + }, // if false { if_block } -> Noop Stmt::IfThenElse(Expr::False(pos), x, _) if x.1.is_none() => { state.set_dirty(); @@ -462,6 +467,11 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { .map(|(_, mut expr)| { expr.set_position(pos); expr }) .unwrap_or_else(|| Expr::Unit(pos)) } + // var.rhs + (lhs @ Expr::Variable(_), rhs) => Expr::Dot(Box::new(BinaryExpr { + lhs, + rhs: optimize_expr(rhs, state), + }), dot_pos), // lhs.rhs (lhs, rhs) => Expr::Dot(Box::new(BinaryExpr { lhs: optimize_expr(lhs, state), @@ -498,6 +508,11 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { state.set_dirty(); Expr::CharConstant(s.name.chars().nth(i as usize).unwrap(), s.pos) } + // var[rhs] + (lhs @ Expr::Variable(_), rhs) => Expr::Index(Box::new(BinaryExpr { + lhs, + rhs: optimize_expr(rhs, state), + }), idx_pos), // lhs[rhs] (lhs, rhs) => Expr::Index(Box::new(BinaryExpr { lhs: optimize_expr(lhs, state), diff --git a/src/parser.rs b/src/parser.rs index df7a28ad..bd6d613c 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1087,7 +1087,7 @@ fn make_assignment_stmt<'a>( ) -> Result { match &lhs { // var (non-indexed) = rhs - Expr::Variable(x) if x.1.is_none() => { + Expr::Variable(x) if x.0.is_none() => { Ok(Stmt::Assignment(Box::new((lhs, fn_name.into(), rhs)), pos)) } // var (indexed) = rhs @@ -1114,7 +1114,7 @@ fn make_assignment_stmt<'a>( // xxx[???] = rhs, xxx.??? = rhs Expr::Index(x, _) | Expr::Dot(x, _) => match &x.lhs { // var[???] (non-indexed) = rhs, var.??? (non-indexed) = rhs - Expr::Variable(x) if x.1.is_none() => { + Expr::Variable(x) if x.0.is_none() => { Ok(Stmt::Assignment(Box::new((lhs, fn_name.into(), rhs)), pos)) } // var[???] (indexed) = rhs, var.??? (indexed) = rhs diff --git a/src/result.rs b/src/result.rs index 79fb519b..8153a2c5 100644 --- a/src/result.rs +++ b/src/result.rs @@ -127,7 +127,7 @@ impl EvalAltResult { Self::ErrorVariableNotFound(_, _) => "Variable not found", Self::ErrorModuleNotFound(_, _) => "Module not found", Self::ErrorDataRace(_, _) => "Data race detected when accessing variable", - Self::ErrorAssignmentToConstant(_, _) => "Assignment to a constant variable", + Self::ErrorAssignmentToConstant(_, _) => "Cannot assign to a constant", Self::ErrorMismatchOutputType(_, _, _) => "Output type is incorrect", Self::ErrorInExpr(_) => "Malformed 'in' expression", Self::ErrorDotExpr(_, _) => "Malformed dot expression", @@ -194,7 +194,9 @@ impl fmt::Display for EvalAltResult { Self::ErrorRuntime(d, _) if d.is::<()>() => f.write_str(desc)?, Self::ErrorRuntime(d, _) => write!(f, "{}: {}", desc, d)?, - Self::ErrorAssignmentToConstant(s, _) => write!(f, "{}: '{}'", desc, s)?, + Self::ErrorAssignmentToConstant(s, _) => { + write!(f, "Cannot assign to constant '{}'", s)? + } Self::ErrorMismatchOutputType(r, s, _) => { write!(f, "Output type is incorrect: {} (expecting {})", r, s)? } diff --git a/tests/constants.rs b/tests/constants.rs index 4569cda0..2f516a1b 100644 --- a/tests/constants.rs +++ b/tests/constants.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, ParseErrorType, INT}; +use rhai::{Engine, EvalAltResult, ParseErrorType, Scope, INT}; #[test] fn test_constant() -> Result<(), Box> { @@ -15,13 +15,28 @@ fn test_constant() -> Result<(), Box> { #[cfg(not(feature = "no_index"))] assert!(matches!( - *engine.eval::("const x = [1, 2, 3, 4, 5]; x[2] = 42;").expect_err("expects error"), + *engine.consume("const x = [1, 2, 3, 4, 5]; x[2] = 42;").expect_err("expects error"), EvalAltResult::ErrorParsing(ParseErrorType::AssignmentToConstant(x), _) if x == "x" )); Ok(()) } +#[test] +fn test_constant_scope() -> Result<(), Box> { + let engine = Engine::new(); + + let mut scope = Scope::new(); + scope.push_constant("x", 42 as INT); + + assert!(matches!( + *engine.consume_with_scope(&mut scope, "x = 1").expect_err("expects error"), + EvalAltResult::ErrorAssignmentToConstant(x, _) if x == "x" + )); + + Ok(()) +} + #[test] fn test_var_is_def() -> Result<(), Box> { let engine = Engine::new();