From aff207d4f439c93fc18a976597310e6b23a48f89 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 7 Dec 2020 22:21:02 +0800 Subject: [PATCH 01/15] Bump version. --- Cargo.toml | 2 +- doc/src/context.json | 2 +- src/packages/string_basic.rs | 11 ++++++----- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7985f153..81cbe637 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,7 @@ members = [ [package] name = "rhai" -version = "0.19.7" +version = "0.19.8" edition = "2018" authors = ["Jonathan Turner", "Lukáš Hozda", "Stephen Chung", "jhwgh1968"] description = "Embedded scripting for Rust" diff --git a/doc/src/context.json b/doc/src/context.json index 8e389888..5a3f0288 100644 --- a/doc/src/context.json +++ b/doc/src/context.json @@ -1,5 +1,5 @@ { - "version": "0.19.7", + "version": "0.19.8", "repoHome": "https://github.com/jonathandturner/rhai/blob/master", "repoTree": "https://github.com/jonathandturner/rhai/tree/master", "rootUrl": "", diff --git a/src/packages/string_basic.rs b/src/packages/string_basic.rs index 69e53bf1..e7d55023 100644 --- a/src/packages/string_basic.rs +++ b/src/packages/string_basic.rs @@ -172,11 +172,12 @@ mod print_debug_functions { let len = map.len(); map.iter_mut().enumerate().for_each(|(i, (k, v))| { - result.push_str(&format!("{:?}: ", k)); - result.push_str(&print_with_func(FUNC_TO_DEBUG, &ctx, v)); - if i < len - 1 { - result.push_str(", "); - } + result.push_str(&format!( + "{:?}: {}{}", + k, + &print_with_func(FUNC_TO_DEBUG, &ctx, v), + if i < len - 1 { ", " } else { "" } + )); }); result.push_str("}"); From 8e8069f8198c0ef2f94f65a724f704075d844f6c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 8 Dec 2020 22:20:29 +0800 Subject: [PATCH 02/15] Do not propagate constants if shadowed. --- RELEASES.md | 9 +++++ src/optimize.rs | 86 +++++++++++++++++++++++++--------------------- tests/optimizer.rs | 12 ++++--- 3 files changed, 64 insertions(+), 43 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 65f943c1..aec6810f 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,6 +1,15 @@ Rhai Release Notes ================== +Version 0.19.8 +============== + +Bug fixes +--------- + +* Constants are no longer propagated by the optimizer if shadowed by a non-constant variable. + + Version 0.19.7 ============== diff --git a/src/optimize.rs b/src/optimize.rs index 83f2c047..77a60e46 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -1,6 +1,7 @@ //! Module implementing the [`AST`] optimizer. use crate::ast::{Expr, ScriptFnDef, Stmt}; +use crate::dynamic::AccessType; use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_PRINT, KEYWORD_TYPE_OF}; use crate::fn_call::run_builtin_binary_op; use crate::parser::map_dynamic_to_expr; @@ -58,7 +59,7 @@ struct State<'a> { /// Has the [`AST`] been changed during this pass? changed: bool, /// Collection of constants to use for eager function evaluations. - constants: Vec<(String, Expr)>, + variables: Vec<(String, AccessType, Expr)>, /// An [`Engine`] instance for eager function evaluation. engine: &'a Engine, /// [Module] containing script-defined functions. @@ -73,7 +74,7 @@ impl<'a> State<'a> { pub fn new(engine: &'a Engine, lib: &'a [&'a Module], level: OptimizationLevel) -> Self { Self { changed: false, - constants: vec![], + variables: vec![], engine, lib, optimization_level: level, @@ -94,27 +95,26 @@ impl<'a> State<'a> { pub fn is_dirty(&self) -> bool { self.changed } - /// Does a constant exist? - #[inline(always)] - pub fn contains_constant(&self, name: &str) -> bool { - self.constants.iter().any(|(n, _)| n == name) - } /// Prune the list of constants back to a specified size. #[inline(always)] - pub fn restore_constants(&mut self, len: usize) { - self.constants.truncate(len) + pub fn restore_var(&mut self, len: usize) { + self.variables.truncate(len) } /// Add a new constant to the list. #[inline(always)] - pub fn push_constant(&mut self, name: &str, value: Expr) { - self.constants.push((name.into(), value)) + pub fn push_var(&mut self, name: &str, access: AccessType, value: Expr) { + self.variables.push((name.into(), access, value)) } /// Look up a constant from the list. #[inline] pub fn find_constant(&self, name: &str) -> Option<&Expr> { - for (n, expr) in self.constants.iter().rev() { + for (n, access, expr) in self.variables.iter().rev() { if n == name { - return Some(expr); + return if access.is_constant() { + Some(expr) + } else { + None + }; } } @@ -157,20 +157,20 @@ fn optimize_stmt_block( count_promote_as_dirty: bool, ) -> Stmt { let orig_len = statements.len(); // Original number of statements in the block, for change detection - let orig_constants_len = state.constants.len(); // Original number of constants in the state, for restore later + let orig_constants_len = state.variables.len(); // Original number of constants in the state, for restore later // Optimize each statement in the block statements.iter_mut().for_each(|stmt| match stmt { // Add constant literals into the state - Stmt::Const(var_def, Some(expr), _, pos) if expr.is_constant() => { - state.set_dirty(); - state.push_constant(&var_def.name, mem::take(expr)); - *stmt = Stmt::Noop(*pos); // No need to keep constants + Stmt::Const(var_def, Some(expr), _, _) if expr.is_constant() => { + state.push_var(&var_def.name, AccessType::Constant, mem::take(expr)); } - Stmt::Const(var_def, None, _, pos) => { - state.set_dirty(); - state.push_constant(&var_def.name, Expr::Unit(var_def.pos)); - *stmt = Stmt::Noop(*pos); // No need to keep constants + Stmt::Const(var_def, None, _, _) => { + state.push_var(&var_def.name, AccessType::Constant, Expr::Unit(var_def.pos)); + } + // Add variables into the state + Stmt::Let(var_def, _, _, _) => { + state.push_var(&var_def.name, AccessType::Normal, Expr::Unit(var_def.pos)); } // Optimize the statement _ => optimize_stmt(stmt, state, preserve_result), @@ -196,7 +196,9 @@ fn optimize_stmt_block( while let Some(expr) = statements.pop() { match expr { - Stmt::Let(_, expr, _, _) => removed = expr.as_ref().map(Expr::is_pure).unwrap_or(true), + Stmt::Let(_, expr, _, _) | Stmt::Const(_, expr, _, _) => { + removed = expr.as_ref().map(Expr::is_pure).unwrap_or(true) + } #[cfg(not(feature = "no_module"))] Stmt::Import(expr, _, _) => removed = expr.is_pure(), _ => { @@ -241,7 +243,7 @@ fn optimize_stmt_block( } // Pop the stack and remove all the local constants - state.restore_constants(orig_constants_len); + state.restore_var(orig_constants_len); match &statements[..] { // No statements in block - change to No-op @@ -251,6 +253,8 @@ fn optimize_stmt_block( } // Only one let statement - leave it alone [x] if matches!(x, Stmt::Let(_, _, _, _)) => Stmt::Block(statements, pos), + // Only one const statement - leave it alone + [x] if matches!(x, Stmt::Const(_, _, _, _)) => Stmt::Block(statements, pos), // Only one import statement - leave it alone #[cfg(not(feature = "no_module"))] [x] if matches!(x, Stmt::Import(_, _, _)) => Stmt::Block(statements, pos), @@ -708,7 +712,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { Expr::FnCall(x, _) => x.args.iter_mut().for_each(|a| optimize_expr(a, state)), // constant-name - Expr::Variable(x) if x.1.is_none() && state.contains_constant(&x.3.name) => { + Expr::Variable(x) if x.1.is_none() && state.find_constant(&x.3.name).is_some() => { state.set_dirty(); // Replace constant with value @@ -742,22 +746,23 @@ fn optimize_top_level( // Set up the state let mut state = State::new(engine, lib, level); - // Add constants from the scope that can be made into a literal into the state - scope - .iter() - .filter(|(_, typ, _)| *typ) - .for_each(|(name, _, value)| { - if let Some(val) = map_dynamic_to_expr(value, Position::NONE) { - state.push_constant(name, val); - } - }); + // Add constants and variables from the scope + scope.iter().for_each(|(name, constant, value)| { + if !constant { + state.push_var(name, AccessType::Normal, Expr::Unit(Position::NONE)); + } else if let Some(val) = map_dynamic_to_expr(value, Position::NONE) { + state.push_var(name, AccessType::Constant, val); + } else { + state.push_var(name, AccessType::Constant, Expr::Unit(Position::NONE)); + } + }); - let orig_constants_len = state.constants.len(); + let orig_constants_len = state.variables.len(); // Optimization loop loop { state.reset(); - state.restore_constants(orig_constants_len); + state.restore_var(orig_constants_len); let num_statements = statements.len(); @@ -769,7 +774,7 @@ fn optimize_top_level( optimize_expr(value_expr, &mut state); if value_expr.is_constant() { - state.push_constant(&var_def.name, value_expr.clone()); + state.push_var(&var_def.name, AccessType::Constant, value_expr.clone()); } // Keep it in the global scope @@ -779,13 +784,16 @@ fn optimize_top_level( } } Stmt::Const(var_def, None, _, _) => { - state.push_constant(&var_def.name, Expr::Unit(var_def.pos)); + state.push_var(&var_def.name, AccessType::Constant, Expr::Unit(var_def.pos)); + } + Stmt::Let(var_def, _, _, _) => { + state.push_var(&var_def.name, AccessType::Normal, Expr::Unit(var_def.pos)); } _ => { // Keep all variable declarations at this level // and always keep the last return value let keep = match stmt { - Stmt::Let(_, _, _, _) => true, + Stmt::Let(_, _, _, _) | Stmt::Const(_, _, _, _) => true, #[cfg(not(feature = "no_module"))] Stmt::Import(_, _, _) => true, _ => i == num_statements - 1, diff --git a/tests/optimizer.rs b/tests/optimizer.rs index 4e66552c..db8477a5 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -53,15 +53,19 @@ fn test_optimizer_parse() -> Result<(), Box> { let mut engine = Engine::new(); engine.set_optimization_level(OptimizationLevel::Simple); - let ast = engine.compile("{ const DECISION = false; if DECISION { 42 } }")?; + let ast = engine.compile("{ const DECISION = false; if DECISION { 42 } else { 123 } }")?; - assert!(format!("{:?}", ast).starts_with("AST([], Module(")); - - engine.set_optimization_level(OptimizationLevel::Full); + assert!(format!("{:?}", ast).starts_with(r#"AST([Block([Const(Ident { name: "DECISION", pos: 1:9 }, Some(Unit(0:0)), false, 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)]"#)); let ast = engine.compile("if 1 == 2 { 42 }")?; assert!(format!("{:?}", ast).starts_with("AST([], Module(")); + engine.set_optimization_level(OptimizationLevel::Full); + + let ast = engine.compile("abs(-42)")?; + + assert!(format!("{:?}", ast).starts_with(r"AST([Expr(IntegerConstant(42, 1:1))]")); + Ok(()) } From f22a04fc744b7d7ddd25a90948ade5c89503d422 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 8 Dec 2020 22:47:38 +0800 Subject: [PATCH 03/15] Move constancy into Dynamic. --- doc/src/SUMMARY.md | 2 +- doc/src/engine/optimize/index.md | 9 + doc/src/engine/optimize/optimize-levels.md | 8 + .../{other-op.md => assignment-op.md} | 10 +- doc/src/language/constants.md | 32 +- doc/src/links.md | 2 + src/ast.rs | 16 +- src/dynamic.rs | 437 ++++++++++-------- src/engine.rs | 250 +++++----- src/fn_call.rs | 25 +- src/module/mod.rs | 2 +- src/parser.rs | 53 ++- src/scope.rs | 80 +--- tests/arrays.rs | 5 +- 14 files changed, 501 insertions(+), 430 deletions(-) rename doc/src/language/{other-op.md => assignment-op.md} (86%) diff --git a/doc/src/SUMMARY.md b/doc/src/SUMMARY.md index 891b760f..270f6d1b 100644 --- a/doc/src/SUMMARY.md +++ b/doc/src/SUMMARY.md @@ -75,7 +75,7 @@ The Rhai Scripting Language 5. [Variables](language/variables.md) 6. [Constants](language/constants.md) 7. [Logic Operators](language/logic.md) - 8. [Other Operators](language/other-op.md) + 8. [Assignment Operators](language/assignment-op.md) 9. [If Statement](language/if.md) 10. [Switch Expression](language/switch.md) 11. [While Loop](language/while.md) diff --git a/doc/src/engine/optimize/index.md b/doc/src/engine/optimize/index.md index 59ae0f6a..6852604b 100644 --- a/doc/src/engine/optimize/index.md +++ b/doc/src/engine/optimize/index.md @@ -64,6 +64,15 @@ are spliced into the script text in order to turn on/off certain sections. For fixed script texts, the constant values can be provided in a user-defined [`Scope`] object to the [`Engine`] for use in compilation and evaluation. +### Caveat + +If the [constants] are modified later on (yes, it is possible, via Rust functions), +the modified values will not show up in the optimized script. +Only the initialization values of [constants] are ever retained. + +This is almost never a problem because real-world scripts seldom modify a constant, +but the possibility is always there. + Eager Operator Evaluations ------------------------- diff --git a/doc/src/engine/optimize/optimize-levels.md b/doc/src/engine/optimize/optimize-levels.md index 0f43938b..a1f867c1 100644 --- a/doc/src/engine/optimize/optimize-levels.md +++ b/doc/src/engine/optimize/optimize-levels.md @@ -11,6 +11,14 @@ There are three levels of optimization: `None`, `Simple` and `Full`. (i.e. it only relies on static analysis and [built-in operators] for constant [standard types], and will not perform any external function calls). + However, it is important to bear in mind that _constants propagation_ is performed with the + caveat that, if [constants] are modified later on (yes, it is possible, via Rust functions), + the modified values will not show up in the optimized script. Only the initialization values + of [constants] are ever retained. + + Furthermore, overriding a [built-in operator][built-in operators] in the [`Engine`] afterwards + has no effect after the optimizer replaces an expression with its calculated value. + * `Full` is _much_ more aggressive, _including_ calling external functions on constant arguments to determine their result. One benefit to this is that many more optimization opportunities arise, especially with regards to comparison operators. diff --git a/doc/src/language/other-op.md b/doc/src/language/assignment-op.md similarity index 86% rename from doc/src/language/other-op.md rename to doc/src/language/assignment-op.md index bf5017d3..d604dac3 100644 --- a/doc/src/language/other-op.md +++ b/doc/src/language/assignment-op.md @@ -1,12 +1,9 @@ -Other Operators -=============== +Compound Assignment Operators +============================= {{#include ../links.md}} -Compound Assignment Operators ----------------------------- - ```rust let number = 9; @@ -64,3 +61,6 @@ my_obj += #{c:3, d:4, e:5}; my_obj.len() == 5; ``` + +In fact, the `+` and `+=` operators are usually [overloaded][function overloading] when +something is to be _added_ to an existing type. diff --git a/doc/src/language/constants.md b/doc/src/language/constants.md index 52124d21..3e25d5a0 100644 --- a/doc/src/language/constants.md +++ b/doc/src/language/constants.md @@ -61,9 +61,31 @@ r" ``` -Constants Can be Modified, Just Not Reassigned ---------------------------------------------- +Constants Can be Modified via Rust +--------------------------------- -A custom type stored as a constant can be modified via its registered API - -being a constant only prevents it from being re-assigned or operated upon by Rhai; -mutating it via a Rust function is still allowed. +A custom type stored as a constant cannot be modified via script, but _can_ be modified via +a registered Rust function that takes a first `&mut` parameter - because there is no way for +Rhai to know whether the Rust function modifies its argument! + +```rust +const x = 42; // a constant + +x.increment(); // call 'increment' defined in Rust with '&mut' first parameter + +x == 43; // value of 'x' is changed! + +fn double() { + this *= 2; // function squares 'this' +} + +x.double(); // <- error: cannot modify constant 'this' + +x == 43; // value of 'x' is unchanged by script +``` + +This is important to keep in mind because the script [optimizer][script optimization] +by default does _constant propagation_ as a operation. + +If a constant is eventually modified by a Rust function, the optimizer will not see +the updated value and will propagate the original initialization value instead. diff --git a/doc/src/links.md b/doc/src/links.md index 8c2cc211..a63f1b26 100644 --- a/doc/src/links.md +++ b/doc/src/links.md @@ -68,6 +68,8 @@ [variable]: {{rootUrl}}/language/variables.md [variables]: {{rootUrl}}/language/variables.md +[constant]: {{rootUrl}}/language/constants.md +[constants]: {{rootUrl}}/language/constants.md [string]: {{rootUrl}}/language/strings-chars.md [strings]: {{rootUrl}}/language/strings-chars.md diff --git a/src/ast.rs b/src/ast.rs index 0cd0cfaf..14156b88 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1,6 +1,6 @@ //! Module defining the AST (abstract syntax tree). -use crate::dynamic::Union; +use crate::dynamic::{AccessType, Union}; use crate::fn_native::shared_make_mut; use crate::module::NamespaceRef; use crate::stdlib::{ @@ -940,10 +940,10 @@ impl Expr { Self::FloatConstant(x, _) => (*x).into(), Self::CharConstant(x, _) => (*x).into(), Self::StringConstant(x, _) => x.clone().into(), - Self::FnPointer(x, _) => Dynamic(Union::FnPtr(Box::new(FnPtr::new_unchecked( - x.clone(), - Default::default(), - )))), + Self::FnPointer(x, _) => Dynamic(Union::FnPtr( + Box::new(FnPtr::new_unchecked(x.clone(), Default::default())), + AccessType::Constant, + )), Self::BoolConstant(x, _) => (*x).into(), Self::Unit(_) => ().into(), @@ -954,7 +954,7 @@ impl Expr { x.len(), )); arr.extend(x.iter().map(|v| v.get_constant_value().unwrap())); - Dynamic(Union::Array(Box::new(arr))) + Dynamic(Union::Array(Box::new(arr), AccessType::Constant)) } #[cfg(not(feature = "no_object"))] @@ -967,7 +967,7 @@ impl Expr { x.iter() .map(|(k, v)| (k.name.clone(), v.get_constant_value().unwrap())), ); - Dynamic(Union::Map(Box::new(map))) + Dynamic(Union::Map(Box::new(map), AccessType::Constant)) } _ => return None, @@ -1167,7 +1167,7 @@ mod tests { assert_eq!(size_of::>(), 16); assert_eq!(size_of::(), 32); assert_eq!(size_of::>(), 32); - assert_eq!(size_of::(), 72); + assert_eq!(size_of::(), 48); assert_eq!(size_of::(), 56); assert_eq!(size_of::(), 16); assert_eq!(size_of::(), 72); diff --git a/src/dynamic.rs b/src/dynamic.rs index 9360f0a8..10042587 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -116,6 +116,25 @@ impl dyn Variant { } } +/// Type of an entry in the Scope. +#[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] +pub enum AccessType { + /// Normal value. + Normal, + /// Immutable constant value. + Constant, +} + +impl AccessType { + /// Is the access type [`Constant`]? + pub fn is_constant(self) -> bool { + match self { + Self::Normal => false, + Self::Constant => true, + } + } +} + /// Dynamic type containing any value. pub struct Dynamic(pub(crate) Union); @@ -123,25 +142,25 @@ pub struct Dynamic(pub(crate) Union); /// /// Most variants are boxed to reduce the size. pub enum Union { - Unit(()), - Bool(bool), - Str(ImmutableString), - Char(char), - Int(INT), + Unit((), AccessType), + Bool(bool, AccessType), + Str(ImmutableString, AccessType), + Char(char, AccessType), + Int(INT, AccessType), #[cfg(not(feature = "no_float"))] - Float(FLOAT), + Float(FLOAT, AccessType), #[cfg(not(feature = "no_index"))] - Array(Box), + Array(Box, AccessType), #[cfg(not(feature = "no_object"))] - Map(Box), - FnPtr(Box), + Map(Box, AccessType), + FnPtr(Box, AccessType), #[cfg(not(feature = "no_std"))] - TimeStamp(Box), + TimeStamp(Box, AccessType), - Variant(Box>), + Variant(Box>, AccessType), #[cfg(not(feature = "no_closure"))] - Shared(crate::Shared>), + Shared(crate::Shared>, AccessType), } /// Underlying [`Variant`] read guard for [`Dynamic`]. @@ -236,7 +255,7 @@ impl Dynamic { #[inline(always)] pub fn is_variant(&self) -> bool { match self.0 { - Union::Variant(_) => true, + Union::Variant(_, _) => true, _ => false, } } @@ -246,7 +265,7 @@ impl Dynamic { pub fn is_shared(&self) -> bool { match self.0 { #[cfg(not(feature = "no_closure"))] - Union::Shared(_) => true, + Union::Shared(_, _) => true, _ => false, } } @@ -272,29 +291,29 @@ impl Dynamic { /// Otherwise, this call panics if the data is currently borrowed for write. pub fn type_id(&self) -> TypeId { match &self.0 { - Union::Unit(_) => TypeId::of::<()>(), - Union::Bool(_) => TypeId::of::(), - Union::Str(_) => TypeId::of::(), - Union::Char(_) => TypeId::of::(), - Union::Int(_) => TypeId::of::(), + Union::Unit(_, _) => TypeId::of::<()>(), + Union::Bool(_, _) => TypeId::of::(), + Union::Str(_, _) => TypeId::of::(), + Union::Char(_, _) => TypeId::of::(), + Union::Int(_, _) => TypeId::of::(), #[cfg(not(feature = "no_float"))] - Union::Float(_) => TypeId::of::(), + Union::Float(_, _) => TypeId::of::(), #[cfg(not(feature = "no_index"))] - Union::Array(_) => TypeId::of::(), + Union::Array(_, _) => TypeId::of::(), #[cfg(not(feature = "no_object"))] - Union::Map(_) => TypeId::of::(), - Union::FnPtr(_) => TypeId::of::(), + Union::Map(_, _) => TypeId::of::(), + Union::FnPtr(_, _) => TypeId::of::(), #[cfg(not(feature = "no_std"))] - Union::TimeStamp(_) => TypeId::of::(), + Union::TimeStamp(_, _) => TypeId::of::(), - Union::Variant(value) => (***value).type_id(), + Union::Variant(value, _) => (***value).type_id(), #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] - Union::Shared(cell) => (*cell.borrow()).type_id(), + Union::Shared(cell, _) => (*cell.borrow()).type_id(), #[cfg(not(feature = "no_closure"))] #[cfg(feature = "sync")] - Union::Shared(cell) => (*cell.read().unwrap()).type_id(), + Union::Shared(cell, _) => (*cell.read().unwrap()).type_id(), } } /// Get the name of the type of the value held by this [`Dynamic`]. @@ -305,32 +324,32 @@ impl Dynamic { /// Otherwise, this call panics if the data is currently borrowed for write. pub fn type_name(&self) -> &'static str { match &self.0 { - Union::Unit(_) => "()", - Union::Bool(_) => "bool", - Union::Str(_) => "string", - Union::Char(_) => "char", - Union::Int(_) => type_name::(), + Union::Unit(_, _) => "()", + Union::Bool(_, _) => "bool", + Union::Str(_, _) => "string", + Union::Char(_, _) => "char", + Union::Int(_, _) => type_name::(), #[cfg(not(feature = "no_float"))] - Union::Float(_) => type_name::(), + Union::Float(_, _) => type_name::(), #[cfg(not(feature = "no_index"))] - Union::Array(_) => "array", + Union::Array(_, _) => "array", #[cfg(not(feature = "no_object"))] - Union::Map(_) => "map", - Union::FnPtr(_) => "Fn", + Union::Map(_, _) => "map", + Union::FnPtr(_, _) => "Fn", #[cfg(not(feature = "no_std"))] - Union::TimeStamp(_) => "timestamp", + Union::TimeStamp(_, _) => "timestamp", - Union::Variant(value) => (***value).type_name(), + Union::Variant(value, _) => (***value).type_name(), #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] - Union::Shared(cell) => cell + Union::Shared(cell, _) => cell .try_borrow() .map(|v| (*v).type_name()) .unwrap_or(""), #[cfg(not(feature = "no_closure"))] #[cfg(feature = "sync")] - Union::Shared(cell) => (*cell.read().unwrap()).type_name(), + Union::Shared(cell, _) => (*cell.read().unwrap()).type_name(), } } } @@ -340,17 +359,17 @@ impl Hash for Dynamic { mem::discriminant(self).hash(state); match &self.0 { - Union::Unit(_) => ().hash(state), - Union::Bool(value) => value.hash(state), - Union::Str(s) => s.hash(state), - Union::Char(ch) => ch.hash(state), - Union::Int(i) => i.hash(state), + Union::Unit(_, _) => ().hash(state), + Union::Bool(value, _) => value.hash(state), + Union::Str(s, _) => s.hash(state), + Union::Char(ch, _) => ch.hash(state), + Union::Int(i, _) => i.hash(state), #[cfg(not(feature = "no_float"))] - Union::Float(f) => f.to_le_bytes().hash(state), + Union::Float(f, _) => f.to_le_bytes().hash(state), #[cfg(not(feature = "no_index"))] - Union::Array(a) => (**a).hash(state), + Union::Array(a, _) => (**a).hash(state), #[cfg(not(feature = "no_object"))] - Union::Map(m) => { + Union::Map(m, _) => { let mut buf: crate::StaticVec<_> = m.iter().collect(); buf.sort_by(|(a, _), (b, _)| a.cmp(b)); @@ -362,10 +381,10 @@ impl Hash for Dynamic { #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] - Union::Shared(cell) => (*cell.borrow()).hash(state), + Union::Shared(cell, _) => (*cell.borrow()).hash(state), #[cfg(not(feature = "no_closure"))] #[cfg(feature = "sync")] - Union::Shared(cell) => (*cell.read().unwrap()).hash(state), + Union::Shared(cell, _) => (*cell.read().unwrap()).hash(state), _ => unimplemented!(), } @@ -404,29 +423,29 @@ pub(crate) fn map_std_type_name(name: &str) -> &str { impl fmt::Display for Dynamic { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match &self.0 { - Union::Unit(_) => write!(f, ""), - Union::Bool(value) => fmt::Display::fmt(value, f), - Union::Str(value) => fmt::Display::fmt(value, f), - Union::Char(value) => fmt::Display::fmt(value, f), - Union::Int(value) => fmt::Display::fmt(value, f), + Union::Unit(_, _) => write!(f, ""), + Union::Bool(value, _) => fmt::Display::fmt(value, f), + Union::Str(value, _) => fmt::Display::fmt(value, f), + Union::Char(value, _) => fmt::Display::fmt(value, f), + Union::Int(value, _) => fmt::Display::fmt(value, f), #[cfg(not(feature = "no_float"))] - Union::Float(value) => fmt::Display::fmt(value, f), + Union::Float(value, _) => fmt::Display::fmt(value, f), #[cfg(not(feature = "no_index"))] - Union::Array(value) => fmt::Debug::fmt(value, f), + Union::Array(value, _) => fmt::Debug::fmt(value, f), #[cfg(not(feature = "no_object"))] - Union::Map(value) => { + Union::Map(value, _) => { f.write_str("#")?; fmt::Debug::fmt(value, f) } - Union::FnPtr(value) => fmt::Display::fmt(value, f), + Union::FnPtr(value, _) => fmt::Display::fmt(value, f), #[cfg(not(feature = "no_std"))] - Union::TimeStamp(_) => f.write_str(""), + Union::TimeStamp(_, _) => f.write_str(""), - Union::Variant(value) => f.write_str((*value).type_name()), + Union::Variant(value, _) => f.write_str((*value).type_name()), #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] - Union::Shared(cell) => { + Union::Shared(cell, _) => { if let Ok(v) = cell.try_borrow() { fmt::Display::fmt(&*v, f) } else { @@ -435,7 +454,7 @@ impl fmt::Display for Dynamic { } #[cfg(not(feature = "no_closure"))] #[cfg(feature = "sync")] - Union::Shared(cell) => fmt::Display::fmt(&*cell.read().unwrap(), f), + Union::Shared(cell, _) => fmt::Display::fmt(&*cell.read().unwrap(), f), } } } @@ -443,29 +462,29 @@ impl fmt::Display for Dynamic { impl fmt::Debug for Dynamic { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match &self.0 { - Union::Unit(value) => fmt::Debug::fmt(value, f), - Union::Bool(value) => fmt::Debug::fmt(value, f), - Union::Str(value) => fmt::Debug::fmt(value, f), - Union::Char(value) => fmt::Debug::fmt(value, f), - Union::Int(value) => fmt::Debug::fmt(value, f), + Union::Unit(value, _) => fmt::Debug::fmt(value, f), + Union::Bool(value, _) => fmt::Debug::fmt(value, f), + Union::Str(value, _) => fmt::Debug::fmt(value, f), + Union::Char(value, _) => fmt::Debug::fmt(value, f), + Union::Int(value, _) => fmt::Debug::fmt(value, f), #[cfg(not(feature = "no_float"))] - Union::Float(value) => fmt::Debug::fmt(value, f), + Union::Float(value, _) => fmt::Debug::fmt(value, f), #[cfg(not(feature = "no_index"))] - Union::Array(value) => fmt::Debug::fmt(value, f), + Union::Array(value, _) => fmt::Debug::fmt(value, f), #[cfg(not(feature = "no_object"))] - Union::Map(value) => { + Union::Map(value, _) => { f.write_str("#")?; fmt::Debug::fmt(value, f) } - Union::FnPtr(value) => fmt::Debug::fmt(value, f), + Union::FnPtr(value, _) => fmt::Debug::fmt(value, f), #[cfg(not(feature = "no_std"))] - Union::TimeStamp(_) => write!(f, ""), + Union::TimeStamp(_, _) => write!(f, ""), - Union::Variant(value) => write!(f, "{}", (*value).type_name()), + Union::Variant(value, _) => write!(f, "{}", (*value).type_name()), #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] - Union::Shared(cell) => { + Union::Shared(cell, _) => { if let Ok(v) = cell.try_borrow() { write!(f, "{:?} (shared)", *v) } else { @@ -474,33 +493,40 @@ impl fmt::Debug for Dynamic { } #[cfg(not(feature = "no_closure"))] #[cfg(feature = "sync")] - Union::Shared(cell) => fmt::Debug::fmt(&*cell.read().unwrap(), f), + Union::Shared(cell, _) => fmt::Debug::fmt(&*cell.read().unwrap(), f), } } } impl Clone for Dynamic { + /// Clone the [`Dynamic`] value. + /// + /// ## WARNING + /// + /// The cloned copy is marked [`AccessType::Normal`] even if the original is constant. fn clone(&self) -> Self { match self.0 { - Union::Unit(value) => Self(Union::Unit(value)), - Union::Bool(value) => Self(Union::Bool(value)), - Union::Str(ref value) => Self(Union::Str(value.clone())), - Union::Char(value) => Self(Union::Char(value)), - Union::Int(value) => Self(Union::Int(value)), + Union::Unit(value, _) => Self(Union::Unit(value, AccessType::Normal)), + Union::Bool(value, _) => Self(Union::Bool(value, AccessType::Normal)), + Union::Str(ref value, _) => Self(Union::Str(value.clone(), AccessType::Normal)), + Union::Char(value, _) => Self(Union::Char(value, AccessType::Normal)), + Union::Int(value, _) => Self(Union::Int(value, AccessType::Normal)), #[cfg(not(feature = "no_float"))] - Union::Float(value) => Self(Union::Float(value)), + Union::Float(value, _) => Self(Union::Float(value, AccessType::Normal)), #[cfg(not(feature = "no_index"))] - Union::Array(ref value) => Self(Union::Array(value.clone())), + Union::Array(ref value, _) => Self(Union::Array(value.clone(), AccessType::Normal)), #[cfg(not(feature = "no_object"))] - Union::Map(ref value) => Self(Union::Map(value.clone())), - Union::FnPtr(ref value) => Self(Union::FnPtr(value.clone())), + Union::Map(ref value, _) => Self(Union::Map(value.clone(), AccessType::Normal)), + Union::FnPtr(ref value, _) => Self(Union::FnPtr(value.clone(), AccessType::Normal)), #[cfg(not(feature = "no_std"))] - Union::TimeStamp(ref value) => Self(Union::TimeStamp(value.clone())), + Union::TimeStamp(ref value, _) => { + Self(Union::TimeStamp(value.clone(), AccessType::Normal)) + } - Union::Variant(ref value) => (***value).clone_into_dynamic(), + Union::Variant(ref value, _) => (***value).clone_into_dynamic(), #[cfg(not(feature = "no_closure"))] - Union::Shared(ref cell) => Self(Union::Shared(cell.clone())), + Union::Shared(ref cell, _) => Self(Union::Shared(cell.clone(), AccessType::Normal)), } } } @@ -514,27 +540,65 @@ impl Default for Dynamic { impl Dynamic { /// A [`Dynamic`] containing a `()`. - pub const UNIT: Dynamic = Self(Union::Unit(())); + pub const UNIT: Dynamic = Self(Union::Unit((), AccessType::Normal)); /// A [`Dynamic`] containing a `true`. - pub const TRUE: Dynamic = Self(Union::Bool(true)); + pub const TRUE: Dynamic = Self(Union::Bool(true, AccessType::Normal)); /// A [`Dynamic`] containing a [`false`]. - pub const FALSE: Dynamic = Self(Union::Bool(false)); + pub const FALSE: Dynamic = Self(Union::Bool(false, AccessType::Normal)); /// A [`Dynamic`] containing the integer zero. - pub const ZERO: Dynamic = Self(Union::Int(0)); + pub const ZERO: Dynamic = Self(Union::Int(0, AccessType::Normal)); /// A [`Dynamic`] containing the integer one. - pub const ONE: Dynamic = Self(Union::Int(1)); + pub const ONE: Dynamic = Self(Union::Int(1, AccessType::Normal)); /// A [`Dynamic`] containing the integer negative one. - pub const NEGATIVE_ONE: Dynamic = Self(Union::Int(-1)); + pub const NEGATIVE_ONE: Dynamic = Self(Union::Int(-1, AccessType::Normal)); /// A [`Dynamic`] containing the floating-point zero. #[cfg(not(feature = "no_float"))] - pub const FLOAT_ZERO: Dynamic = Self(Union::Float(0.0)); + pub const FLOAT_ZERO: Dynamic = Self(Union::Float(0.0, AccessType::Normal)); /// A [`Dynamic`] containing the floating-point one. #[cfg(not(feature = "no_float"))] - pub const FLOAT_ONE: Dynamic = Self(Union::Float(1.0)); + pub const FLOAT_ONE: Dynamic = Self(Union::Float(1.0, AccessType::Normal)); /// A [`Dynamic`] containing the floating-point negative one. #[cfg(not(feature = "no_float"))] - pub const FLOAT_NEGATIVE_ONE: Dynamic = Self(Union::Float(-1.0)); + pub const FLOAT_NEGATIVE_ONE: Dynamic = Self(Union::Float(-1.0, AccessType::Normal)); + /// Get the [`AccessType`] for this [`Dynamic`]. + pub(crate) fn access_type(&self) -> AccessType { + match self.0 { + Union::Unit(_, access) + | Union::Bool(_, access) + | Union::Str(_, access) + | Union::Char(_, access) + | Union::Int(_, access) + | Union::Float(_, access) + | Union::Array(_, access) + | Union::Map(_, access) + | Union::FnPtr(_, access) + | Union::TimeStamp(_, access) + | Union::Variant(_, access) + | Union::Shared(_, access) => access, + } + } + /// Set the [`AccessType`] for this [`Dynamic`]. + pub(crate) fn set_access_type(&mut self, typ: AccessType) { + match &mut self.0 { + Union::Unit(_, access) + | Union::Bool(_, access) + | Union::Str(_, access) + | Union::Char(_, access) + | Union::Int(_, access) + | Union::Float(_, access) + | Union::Array(_, access) + | Union::Map(_, access) + | Union::FnPtr(_, access) + | Union::TimeStamp(_, access) + | Union::Variant(_, access) + | Union::Shared(_, access) => *access = typ, + } + } + /// Is this [`Dynamic`] constant? + pub(crate) fn is_constant(&self) -> bool { + self.access_type().is_constant() + } /// Create a [`Dynamic`] from any type. A [`Dynamic`] value is simply returned as is. /// /// # Safety @@ -651,7 +715,7 @@ impl Dynamic { } } - Self(Union::Variant(Box::new(boxed))) + Self(Union::Variant(Box::new(boxed), AccessType::Normal)) } /// Turn the [`Dynamic`] value into a shared [`Dynamic`] value backed by an [`Rc`][std::rc::Rc]`<`[`RefCell`][std::cell::RefCell]`<`[`Dynamic`]`>>` /// or [`Arc`][std::sync::Arc]`<`[`RwLock`][std::sync::RwLock]`<`[`Dynamic`]`>>` depending on the `sync` feature. @@ -668,10 +732,12 @@ impl Dynamic { /// Panics under the `no_closure` feature. #[inline(always)] pub fn into_shared(self) -> Self { + let _access = self.access_type(); + #[cfg(not(feature = "no_closure"))] return match self.0 { - Union::Shared(..) => self, - _ => Self(Union::Shared(crate::Locked::new(self).into())), + Union::Shared(_, _) => self, + _ => Self(Union::Shared(crate::Locked::new(self).into(), _access)), }; #[cfg(feature = "no_closure")] @@ -707,11 +773,11 @@ impl Dynamic { match self.0 { #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] - Union::Shared(cell) => return cell.borrow().clone().try_cast(), + Union::Shared(cell, _) => return cell.borrow().clone().try_cast(), #[cfg(not(feature = "no_closure"))] #[cfg(feature = "sync")] - Union::Shared(cell) => return cell.read().unwrap().clone().try_cast(), + Union::Shared(cell, _) => return cell.read().unwrap().clone().try_cast(), _ => (), } @@ -721,7 +787,7 @@ impl Dynamic { if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Int(value) => unsafe_try_cast(value), + Union::Int(value, _) => unsafe_try_cast(value), _ => None, }; } @@ -729,35 +795,35 @@ impl Dynamic { #[cfg(not(feature = "no_float"))] if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Float(value) => unsafe_try_cast(value), + Union::Float(value, _) => unsafe_try_cast(value), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Bool(value) => unsafe_try_cast(value), + Union::Bool(value, _) => unsafe_try_cast(value), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Str(value) => unsafe_try_cast(value), + Union::Str(value, _) => unsafe_try_cast(value), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Str(value) => unsafe_try_cast(value.into_owned()), + Union::Str(value, _) => unsafe_try_cast(value.into_owned()), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Char(value) => unsafe_try_cast(value), + Union::Char(value, _) => unsafe_try_cast(value), _ => None, }; } @@ -765,7 +831,7 @@ impl Dynamic { #[cfg(not(feature = "no_index"))] if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Array(value) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v), + Union::Array(value, _) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v), _ => None, }; } @@ -773,14 +839,14 @@ impl Dynamic { #[cfg(not(feature = "no_object"))] if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Map(value) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v), + Union::Map(value, _) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::FnPtr(value) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v), + Union::FnPtr(value, _) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v), _ => None, }; } @@ -788,22 +854,22 @@ impl Dynamic { #[cfg(not(feature = "no_std"))] if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::TimeStamp(value) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v), + Union::TimeStamp(value, _) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v), _ => None, }; } if TypeId::of::() == TypeId::of::<()>() { return match self.0 { - Union::Unit(value) => unsafe_try_cast(value), + Union::Unit(value, _) => unsafe_try_cast(value), _ => None, }; } match self.0 { - Union::Variant(value) => (*value).as_box_any().downcast().map(|x| *x).ok(), + Union::Variant(value, _) => (*value).as_box_any().downcast().map(|x| *x).ok(), #[cfg(not(feature = "no_closure"))] - Union::Shared(_) => unreachable!(), + Union::Shared(_, _) => unreachable!(), _ => None, } } @@ -859,7 +925,7 @@ impl Dynamic { pub fn flatten_clone(&self) -> Self { match &self.0 { #[cfg(not(feature = "no_closure"))] - Union::Shared(cell) => { + Union::Shared(cell, _) => { #[cfg(not(feature = "sync"))] return cell.borrow().clone(); @@ -879,7 +945,7 @@ impl Dynamic { pub fn flatten(self) -> Self { match self.0 { #[cfg(not(feature = "no_closure"))] - Union::Shared(cell) => crate::fn_native::shared_try_take(cell).map_or_else( + Union::Shared(cell, _) => crate::fn_native::shared_try_take(cell).map_or_else( |cell| { #[cfg(not(feature = "sync"))] return cell.borrow().clone(); @@ -907,7 +973,7 @@ impl Dynamic { pub fn is_locked(&self) -> bool { match self.0 { #[cfg(not(feature = "no_closure"))] - Union::Shared(ref _cell) => { + Union::Shared(ref _cell, _) => { #[cfg(not(feature = "sync"))] return _cell.try_borrow().is_err(); @@ -930,7 +996,7 @@ impl Dynamic { pub fn read_lock(&self) -> Option> { match self.0 { #[cfg(not(feature = "no_closure"))] - Union::Shared(ref cell) => { + Union::Shared(ref cell, _) => { #[cfg(not(feature = "sync"))] let data = cell.borrow(); #[cfg(feature = "sync")] @@ -962,7 +1028,7 @@ impl Dynamic { pub fn write_lock(&mut self) -> Option> { match self.0 { #[cfg(not(feature = "no_closure"))] - Union::Shared(ref cell) => { + Union::Shared(ref cell, _) => { #[cfg(not(feature = "sync"))] let data = cell.borrow_mut(); #[cfg(feature = "sync")] @@ -991,71 +1057,71 @@ impl Dynamic { if TypeId::of::() == TypeId::of::() { return match &self.0 { - Union::Int(value) => ::downcast_ref::(value), + Union::Int(value, _) => ::downcast_ref::(value), _ => None, }; } #[cfg(not(feature = "no_float"))] if TypeId::of::() == TypeId::of::() { return match &self.0 { - Union::Float(value) => ::downcast_ref::(value), + Union::Float(value, _) => ::downcast_ref::(value), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match &self.0 { - Union::Bool(value) => ::downcast_ref::(value), + Union::Bool(value, _) => ::downcast_ref::(value), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match &self.0 { - Union::Str(value) => ::downcast_ref::(value), + Union::Str(value, _) => ::downcast_ref::(value), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match &self.0 { - Union::Str(value) => ::downcast_ref::(value.as_ref()), + Union::Str(value, _) => ::downcast_ref::(value.as_ref()), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match &self.0 { - Union::Char(value) => ::downcast_ref::(value), + Union::Char(value, _) => ::downcast_ref::(value), _ => None, }; } #[cfg(not(feature = "no_index"))] if TypeId::of::() == TypeId::of::() { return match &self.0 { - Union::Array(value) => ::downcast_ref::(value.as_ref()), + Union::Array(value, _) => ::downcast_ref::(value.as_ref()), _ => None, }; } #[cfg(not(feature = "no_object"))] if TypeId::of::() == TypeId::of::() { return match &self.0 { - Union::Map(value) => ::downcast_ref::(value.as_ref()), + Union::Map(value, _) => ::downcast_ref::(value.as_ref()), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match &self.0 { - Union::FnPtr(value) => ::downcast_ref::(value.as_ref()), + Union::FnPtr(value, _) => ::downcast_ref::(value.as_ref()), _ => None, }; } #[cfg(not(feature = "no_std"))] if TypeId::of::() == TypeId::of::() { return match &self.0 { - Union::TimeStamp(value) => ::downcast_ref::(value.as_ref()), + Union::TimeStamp(value, _) => ::downcast_ref::(value.as_ref()), _ => None, }; } if TypeId::of::() == TypeId::of::<()>() { return match &self.0 { - Union::Unit(value) => ::downcast_ref::(value), + Union::Unit(value, _) => ::downcast_ref::(value), _ => None, }; } @@ -1064,9 +1130,9 @@ impl Dynamic { } match &self.0 { - Union::Variant(value) => value.as_ref().as_ref().as_any().downcast_ref::(), + Union::Variant(value, _) => value.as_ref().as_ref().as_any().downcast_ref::(), #[cfg(not(feature = "no_closure"))] - Union::Shared(_) => None, + Union::Shared(_, _) => None, _ => None, } } @@ -1080,65 +1146,65 @@ impl Dynamic { if TypeId::of::() == TypeId::of::() { return match &mut self.0 { - Union::Int(value) => ::downcast_mut::(value), + Union::Int(value, _) => ::downcast_mut::(value), _ => None, }; } #[cfg(not(feature = "no_float"))] if TypeId::of::() == TypeId::of::() { return match &mut self.0 { - Union::Float(value) => ::downcast_mut::(value), + Union::Float(value, _) => ::downcast_mut::(value), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match &mut self.0 { - Union::Bool(value) => ::downcast_mut::(value), + Union::Bool(value, _) => ::downcast_mut::(value), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match &mut self.0 { - Union::Str(value) => ::downcast_mut::(value), + Union::Str(value, _) => ::downcast_mut::(value), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match &mut self.0 { - Union::Char(value) => ::downcast_mut::(value), + Union::Char(value, _) => ::downcast_mut::(value), _ => None, }; } #[cfg(not(feature = "no_index"))] if TypeId::of::() == TypeId::of::() { return match &mut self.0 { - Union::Array(value) => ::downcast_mut::(value.as_mut()), + Union::Array(value, _) => ::downcast_mut::(value.as_mut()), _ => None, }; } #[cfg(not(feature = "no_object"))] if TypeId::of::() == TypeId::of::() { return match &mut self.0 { - Union::Map(value) => ::downcast_mut::(value.as_mut()), + Union::Map(value, _) => ::downcast_mut::(value.as_mut()), _ => None, }; } if TypeId::of::() == TypeId::of::() { return match &mut self.0 { - Union::FnPtr(value) => ::downcast_mut::(value.as_mut()), + Union::FnPtr(value, _) => ::downcast_mut::(value.as_mut()), _ => None, }; } #[cfg(not(feature = "no_std"))] if TypeId::of::() == TypeId::of::() { return match &mut self.0 { - Union::TimeStamp(value) => ::downcast_mut::(value.as_mut()), + Union::TimeStamp(value, _) => ::downcast_mut::(value.as_mut()), _ => None, }; } if TypeId::of::() == TypeId::of::<()>() { return match &mut self.0 { - Union::Unit(value) => ::downcast_mut::(value), + Union::Unit(value, _) => ::downcast_mut::(value), _ => None, }; } @@ -1147,9 +1213,9 @@ impl Dynamic { } match &mut self.0 { - Union::Variant(value) => value.as_mut().as_mut_any().downcast_mut::(), + Union::Variant(value, _) => value.as_mut().as_mut_any().downcast_mut::(), #[cfg(not(feature = "no_closure"))] - Union::Shared(_) => None, + Union::Shared(_, _) => None, _ => None, } } @@ -1158,9 +1224,9 @@ impl Dynamic { #[inline(always)] pub fn as_int(&self) -> Result { match self.0 { - Union::Int(n) => Ok(n), + Union::Int(n, _) => Ok(n), #[cfg(not(feature = "no_closure"))] - Union::Shared(_) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), + Union::Shared(_, _) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), _ => Err(self.type_name()), } } @@ -1170,9 +1236,9 @@ impl Dynamic { #[inline(always)] pub fn as_float(&self) -> Result { match self.0 { - Union::Float(n) => Ok(n), + Union::Float(n, _) => Ok(n), #[cfg(not(feature = "no_closure"))] - Union::Shared(_) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), + Union::Shared(_, _) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), _ => Err(self.type_name()), } } @@ -1181,9 +1247,9 @@ impl Dynamic { #[inline(always)] pub fn as_bool(&self) -> Result { match self.0 { - Union::Bool(b) => Ok(b), + Union::Bool(b, _) => Ok(b), #[cfg(not(feature = "no_closure"))] - Union::Shared(_) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), + Union::Shared(_, _) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), _ => Err(self.type_name()), } } @@ -1192,9 +1258,9 @@ impl Dynamic { #[inline(always)] pub fn as_char(&self) -> Result { match self.0 { - Union::Char(n) => Ok(n), + Union::Char(n, _) => Ok(n), #[cfg(not(feature = "no_closure"))] - Union::Shared(_) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), + Union::Shared(_, _) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), _ => Err(self.type_name()), } } @@ -1205,8 +1271,8 @@ impl Dynamic { #[inline(always)] pub fn as_str(&self) -> Result<&str, &'static str> { match &self.0 { - Union::Str(s) => Ok(s), - Union::FnPtr(f) => Ok(f.fn_name()), + Union::Str(s, _) => Ok(s), + Union::FnPtr(f, _) => Ok(f.fn_name()), _ => Err(self.type_name()), } } @@ -1223,16 +1289,16 @@ impl Dynamic { #[inline] pub fn take_immutable_string(self) -> Result { match self.0 { - Union::Str(s) => Ok(s), - Union::FnPtr(f) => Ok(f.take_data().0), + Union::Str(s, _) => Ok(s), + Union::FnPtr(f, _) => Ok(f.take_data().0), #[cfg(not(feature = "no_closure"))] - Union::Shared(cell) => { + Union::Shared(cell, _) => { #[cfg(not(feature = "sync"))] { let inner = cell.borrow(); match &inner.0 { - Union::Str(s) => Ok(s.clone()), - Union::FnPtr(f) => Ok(f.clone().take_data().0), + Union::Str(s, _) => Ok(s.clone()), + Union::FnPtr(f, _) => Ok(f.clone().take_data().0), _ => Err((*inner).type_name()), } } @@ -1240,8 +1306,8 @@ impl Dynamic { { let inner = cell.read().unwrap(); match &inner.0 { - Union::Str(s) => Ok(s.clone()), - Union::FnPtr(f) => Ok(f.clone().take_data().0), + Union::Str(s, _) => Ok(s.clone()), + Union::FnPtr(f, _) => Ok(f.clone().take_data().0), _ => Err((*inner).type_name()), } } @@ -1254,56 +1320,58 @@ impl Dynamic { impl From<()> for Dynamic { #[inline(always)] fn from(value: ()) -> Self { - Self(Union::Unit(value)) + Self(Union::Unit(value, AccessType::Normal)) } } impl From for Dynamic { #[inline(always)] fn from(value: bool) -> Self { - Self(Union::Bool(value)) + Self(Union::Bool(value, AccessType::Normal)) } } impl From for Dynamic { #[inline(always)] fn from(value: INT) -> Self { - Self(Union::Int(value)) + Self(Union::Int(value, AccessType::Normal)) } } #[cfg(not(feature = "no_float"))] impl From for Dynamic { #[inline(always)] fn from(value: FLOAT) -> Self { - Self(Union::Float(value)) + Self(Union::Float(value, AccessType::Normal)) } } impl From for Dynamic { #[inline(always)] fn from(value: char) -> Self { - Self(Union::Char(value)) + Self(Union::Char(value, AccessType::Normal)) } } impl> From for Dynamic { #[inline(always)] fn from(value: S) -> Self { - Self(Union::Str(value.into())) + Self(Union::Str(value.into(), AccessType::Normal)) } } #[cfg(not(feature = "no_index"))] impl From> for Dynamic { #[inline(always)] fn from(value: crate::stdlib::vec::Vec) -> Self { - Self(Union::Array(Box::new( - value.into_iter().map(Dynamic::from).collect(), - ))) + Self(Union::Array( + Box::new(value.into_iter().map(Dynamic::from).collect()), + AccessType::Normal, + )) } } #[cfg(not(feature = "no_index"))] impl From<&[T]> for Dynamic { #[inline(always)] fn from(value: &[T]) -> Self { - Self(Union::Array(Box::new( - value.iter().cloned().map(Dynamic::from).collect(), - ))) + Self(Union::Array( + Box::new(value.iter().cloned().map(Dynamic::from).collect()), + AccessType::Normal, + )) } } #[cfg(not(feature = "no_object"))] @@ -1312,30 +1380,33 @@ impl, T: Variant + Clone> From) -> Self { - Self(Union::Map(Box::new( - value - .into_iter() - .map(|(k, v)| (k.into(), Dynamic::from(v))) - .collect(), - ))) + Self(Union::Map( + Box::new( + value + .into_iter() + .map(|(k, v)| (k.into(), Dynamic::from(v))) + .collect(), + ), + AccessType::Normal, + )) } } impl From for Dynamic { #[inline(always)] fn from(value: FnPtr) -> Self { - Self(Union::FnPtr(Box::new(value))) + Self(Union::FnPtr(Box::new(value), AccessType::Normal)) } } impl From> for Dynamic { #[inline(always)] fn from(value: Box) -> Self { - Self(Union::FnPtr(value)) + Self(Union::FnPtr(value, AccessType::Normal)) } } #[cfg(not(feature = "no_std"))] impl From for Dynamic { #[inline(always)] fn from(value: Instant) -> Self { - Self(Union::TimeStamp(Box::new(value))) + Self(Union::TimeStamp(Box::new(value), AccessType::Normal)) } } diff --git a/src/engine.rs b/src/engine.rs index 35f50b16..b399c6e7 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1,14 +1,13 @@ //! Main module defining the script evaluation [`Engine`]. use crate::ast::{Expr, FnCallExpr, Ident, IdentX, ReturnType, Stmt}; -use crate::dynamic::{map_std_type_name, Union, Variant}; +use crate::dynamic::{map_std_type_name, AccessType, Union, Variant}; use crate::fn_call::run_builtin_op_assignment; use crate::fn_native::{CallableFunction, Callback, IteratorFn, OnVarCallback}; use crate::module::NamespaceRef; use crate::optimize::OptimizationLevel; use crate::packages::{Package, PackagesCollection, StandardPackage}; use crate::r#unsafe::unsafe_cast_var_name_to_lifetime; -use crate::scope::EntryType as ScopeEntryType; use crate::stdlib::{ any::{type_name, TypeId}, borrow::Cow, @@ -833,7 +832,7 @@ impl Engine { lib: &[&Module], this_ptr: &'s mut Option<&mut Dynamic>, expr: &'a Expr, - ) -> Result<(Target<'s>, &'a str, ScopeEntryType, Position), Box> { + ) -> Result<(Target<'s>, &'a str, Position), Box> { match expr { Expr::Variable(v) => match v.as_ref() { // Qualified variable @@ -850,7 +849,9 @@ impl Engine { })?; // Module variables are constant - Ok((target.clone().into(), name, ScopeEntryType::Constant, *pos)) + let mut target = target.clone(); + target.set_access_type(AccessType::Constant); + Ok((target.into(), name, *pos)) } // Normal variable access _ => self.search_scope_only(scope, mods, state, lib, this_ptr, expr), @@ -868,7 +869,7 @@ impl Engine { lib: &[&Module], this_ptr: &'s mut Option<&mut Dynamic>, expr: &'a Expr, - ) -> Result<(Target<'s>, &'a str, ScopeEntryType, Position), Box> { + ) -> Result<(Target<'s>, &'a str, Position), Box> { let (index, _, _, IdentX { name, pos }) = match expr { Expr::Variable(v) => v.as_ref(), _ => unreachable!(), @@ -877,7 +878,7 @@ impl Engine { // Check if the variable is `this` if name.as_str() == KEYWORD_THIS { if let Some(val) = this_ptr { - return Ok(((*val).into(), KEYWORD_THIS, ScopeEntryType::Normal, *pos)); + return Ok(((*val).into(), KEYWORD_THIS, *pos)); } else { return EvalAltResult::ErrorUnboundThis(*pos).into(); } @@ -901,10 +902,11 @@ impl Engine { this_ptr, level: 0, }; - if let Some(result) = + if let Some(mut result) = resolve_var(name, index, &context).map_err(|err| err.fill_position(*pos))? { - return Ok((result.into(), name, ScopeEntryType::Constant, *pos)); + result.set_access_type(AccessType::Constant); + return Ok((result.into(), name, *pos)); } } @@ -918,7 +920,7 @@ impl Engine { .0 }; - let (val, typ) = scope.get_mut(index); + let val = scope.get_mut(index); // Check for data race - probably not necessary because the only place it should conflict is in a method call // when the object variable is also used as a parameter. @@ -926,7 +928,7 @@ impl Engine { // return EvalAltResult::ErrorDataRace(name.into(), *pos).into(); // } - Ok((val.into(), name, typ, *pos)) + Ok((val.into(), name, *pos)) } /// Chain-evaluate a dot/index chain. @@ -1279,16 +1281,13 @@ impl Engine { self.inc_operations(state) .map_err(|err| err.fill_position(*var_pos))?; - let (target, _, typ, pos) = + let (target, _, pos) = self.search_namespace(scope, mods, state, lib, this_ptr, lhs)?; // Constants cannot be modified - match typ { - ScopeEntryType::Constant if new_val.is_some() => { - return EvalAltResult::ErrorAssignmentToConstant(var_name.to_string(), pos) - .into(); - } - ScopeEntryType::Constant | ScopeEntryType::Normal => (), + if target.as_ref().is_constant() && new_val.is_some() { + return EvalAltResult::ErrorAssignmentToConstant(var_name.to_string(), pos) + .into(); } let obj_ptr = &mut target.into(); @@ -1410,7 +1409,7 @@ impl Engine { match target { #[cfg(not(feature = "no_index"))] - Dynamic(Union::Array(arr)) => { + Dynamic(Union::Array(arr, _)) => { // val_array[idx] let index = idx .as_int() @@ -1430,7 +1429,7 @@ impl Engine { } #[cfg(not(feature = "no_object"))] - Dynamic(Union::Map(map)) => { + Dynamic(Union::Map(map, _)) => { // val_map[idx] Ok(if _create { let index = idx.take_immutable_string().map_err(|err| { @@ -1450,7 +1449,7 @@ impl Engine { } #[cfg(not(feature = "no_index"))] - Dynamic(Union::Str(s)) => { + Dynamic(Union::Str(s, _)) => { // val_string[idx] let chars_len = s.chars().count(); let index = idx @@ -1517,7 +1516,7 @@ impl Engine { match rhs_value { #[cfg(not(feature = "no_index"))] - Dynamic(Union::Array(mut rhs_value)) => { + Dynamic(Union::Array(mut rhs_value, _)) => { // Call the `==` operator to compare each value let def_value = Some(false.into()); let def_value = def_value.as_ref(); @@ -1545,16 +1544,16 @@ impl Engine { Ok(false.into()) } #[cfg(not(feature = "no_object"))] - Dynamic(Union::Map(rhs_value)) => match lhs_value { + Dynamic(Union::Map(rhs_value, _)) => match lhs_value { // Only allows string or char - Dynamic(Union::Str(s)) => Ok(rhs_value.contains_key(&s).into()), - Dynamic(Union::Char(c)) => Ok(rhs_value.contains_key(&c.to_string()).into()), + Dynamic(Union::Str(s, _)) => Ok(rhs_value.contains_key(&s).into()), + Dynamic(Union::Char(c, _)) => Ok(rhs_value.contains_key(&c.to_string()).into()), _ => EvalAltResult::ErrorInExpr(lhs.position()).into(), }, - Dynamic(Union::Str(rhs_value)) => match lhs_value { + Dynamic(Union::Str(rhs_value, _)) => match lhs_value { // Only allows string or char - Dynamic(Union::Str(s)) => Ok(rhs_value.contains(s.as_str()).into()), - Dynamic(Union::Char(c)) => Ok(rhs_value.contains(c).into()), + Dynamic(Union::Str(s, _)) => Ok(rhs_value.contains(s.as_str()).into()), + Dynamic(Union::Char(c, _)) => Ok(rhs_value.contains(c).into()), _ => EvalAltResult::ErrorInExpr(lhs.position()).into(), }, _ => EvalAltResult::ErrorInExpr(rhs.position()).into(), @@ -1576,17 +1575,15 @@ impl Engine { match expr { // var - point directly to the value Expr::Variable(_) => { - let (target, _, typ, pos) = + let (mut target, _, pos) = self.search_namespace(scope, mods, state, lib, this_ptr, expr)?; - Ok(( - match typ { - // If necessary, constants are cloned - ScopeEntryType::Constant if no_const => target.into_owned(), - _ => target, - }, - pos, - )) + // If necessary, constants are cloned + if target.as_ref().is_constant() { + target = target.into_owned(); + } + + Ok((target, pos)) } // var[...] #[cfg(not(feature = "no_index"))] @@ -1706,8 +1703,7 @@ impl Engine { } } Expr::Variable(_) => { - let (val, _, _, _) = - self.search_namespace(scope, mods, state, lib, this_ptr, expr)?; + let (val, _, _) = self.search_namespace(scope, mods, state, lib, this_ptr, expr)?; Ok(val.take_or_clone()) } Expr::Property(_) => unreachable!(), @@ -1736,7 +1732,7 @@ impl Engine { for item in x.as_ref() { arr.push(self.eval_expr(scope, mods, state, lib, this_ptr, item, level)?); } - Ok(Dynamic(Union::Array(Box::new(arr)))) + Ok(Dynamic(Union::Array(Box::new(arr), AccessType::Normal))) } #[cfg(not(feature = "no_object"))] @@ -1749,7 +1745,7 @@ impl Engine { self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?, ); } - Ok(Dynamic(Union::Map(Box::new(map)))) + Ok(Dynamic(Union::Map(Box::new(map), AccessType::Normal))) } // Normal function call @@ -1913,7 +1909,7 @@ impl Engine { let mut rhs_val = self .eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)? .flatten(); - let (mut lhs_ptr, name, typ, pos) = + let (mut lhs_ptr, name, pos) = self.search_namespace(scope, mods, state, lib, this_ptr, lhs_expr)?; if !lhs_ptr.is_ref() { @@ -1923,88 +1919,86 @@ impl Engine { self.inc_operations(state) .map_err(|err| err.fill_position(pos))?; - match typ { + if lhs_ptr.as_ref().is_constant() { // Assignment to constant variable - ScopeEntryType::Constant => Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant(name.to_string(), pos), - )), + Err(Box::new(EvalAltResult::ErrorAssignmentToConstant( + name.to_string(), + pos, + ))) + } else if op.is_empty() { // Normal assignment - ScopeEntryType::Normal if op.is_empty() => { - if cfg!(not(feature = "no_closure")) && lhs_ptr.is_shared() { - *lhs_ptr.as_mut().write_lock::().unwrap() = rhs_val; - } else { - *lhs_ptr.as_mut() = rhs_val; - } - Ok(Dynamic::UNIT) + if cfg!(not(feature = "no_closure")) && lhs_ptr.is_shared() { + *lhs_ptr.as_mut().write_lock::().unwrap() = rhs_val; + } else { + *lhs_ptr.as_mut() = rhs_val; } + Ok(Dynamic::UNIT) + } else { // Op-assignment - in order of precedence: - ScopeEntryType::Normal => { - // 1) Native registered overriding function - // 2) Built-in implementation - // 3) Map to `var = var op rhs` + // 1) Native registered overriding function + // 2) Built-in implementation + // 3) Map to `var = var op rhs` - // Qualifiers (none) + function name + number of arguments + argument `TypeId`'s. - let arg_types = - once(lhs_ptr.as_mut().type_id()).chain(once(rhs_val.type_id())); - let hash_fn = calc_native_fn_hash(empty(), op, arg_types); + // Qualifiers (none) + function name + number of arguments + argument `TypeId`'s. + let arg_types = once(lhs_ptr.as_mut().type_id()).chain(once(rhs_val.type_id())); + let hash_fn = calc_native_fn_hash(empty(), op, arg_types); - match self - .global_namespace - .get_fn(hash_fn, false) - .or_else(|| self.packages.get_fn(hash_fn)) - .or_else(|| mods.get_fn(hash_fn)) - { - // op= function registered as method - Some(func) if func.is_method() => { - let mut lock_guard; - let lhs_ptr_inner; + match self + .global_namespace + .get_fn(hash_fn, false) + .or_else(|| self.packages.get_fn(hash_fn)) + .or_else(|| mods.get_fn(hash_fn)) + { + // op= function registered as method + Some(func) if func.is_method() => { + let mut lock_guard; + let lhs_ptr_inner; - if cfg!(not(feature = "no_closure")) && lhs_ptr.is_shared() { - lock_guard = lhs_ptr.as_mut().write_lock::().unwrap(); - lhs_ptr_inner = lock_guard.deref_mut(); - } else { - lhs_ptr_inner = lhs_ptr.as_mut(); - } - - let args = &mut [lhs_ptr_inner, &mut rhs_val]; - - // Overriding exact implementation - if func.is_plugin_fn() { - func.get_plugin_fn() - .call((self, &*mods, lib).into(), args)?; - } else { - func.get_native_fn()((self, &*mods, lib).into(), args)?; - } + if cfg!(not(feature = "no_closure")) && lhs_ptr.is_shared() { + lock_guard = lhs_ptr.as_mut().write_lock::().unwrap(); + lhs_ptr_inner = lock_guard.deref_mut(); + } else { + lhs_ptr_inner = lhs_ptr.as_mut(); } - // Built-in op-assignment function - _ if run_builtin_op_assignment(op, lhs_ptr.as_mut(), &rhs_val)? - .is_some() => {} - // Not built-in: expand to `var = var op rhs` - _ => { - let op = &op[..op.len() - 1]; // extract operator without = - // Clone the LHS value - let args = &mut [&mut lhs_ptr.as_mut().clone(), &mut rhs_val]; + let args = &mut [lhs_ptr_inner, &mut rhs_val]; - // Run function - let (value, _) = self - .exec_fn_call( - mods, state, lib, op, 0, args, false, false, false, None, - None, level, - ) - .map_err(|err| err.fill_position(*op_pos))?; - - let value = value.flatten(); - - if cfg!(not(feature = "no_closure")) && lhs_ptr.is_shared() { - *lhs_ptr.as_mut().write_lock::().unwrap() = value; - } else { - *lhs_ptr.as_mut() = value; - } + // Overriding exact implementation + if func.is_plugin_fn() { + func.get_plugin_fn() + .call((self, &*mods, lib).into(), args)?; + } else { + func.get_native_fn()((self, &*mods, lib).into(), args)?; + } + } + // Built-in op-assignment function + _ if run_builtin_op_assignment(op, lhs_ptr.as_mut(), &rhs_val)? + .is_some() => {} + // Not built-in: expand to `var = var op rhs` + _ => { + let op = &op[..op.len() - 1]; // extract operator without = + + // Clone the LHS value + let args = &mut [&mut lhs_ptr.as_mut().clone(), &mut rhs_val]; + + // Run function + let (value, _) = self + .exec_fn_call( + mods, state, lib, op, 0, args, false, false, false, None, None, + level, + ) + .map_err(|err| err.fill_position(*op_pos))?; + + let value = value.flatten(); + + if cfg!(not(feature = "no_closure")) && lhs_ptr.is_shared() { + *lhs_ptr.as_mut().write_lock::().unwrap() = value; + } else { + *lhs_ptr.as_mut() = value; } } - Ok(Dynamic::UNIT) } + Ok(Dynamic::UNIT) } } @@ -2175,7 +2169,7 @@ impl Engine { state.scope_level += 1; for iter_value in func(iter_obj) { - let (loop_var, _) = scope.get_mut(index); + let loop_var = scope.get_mut(index); let value = iter_value.flatten(); if cfg!(not(feature = "no_closure")) && loop_var.is_shared() { @@ -2249,8 +2243,10 @@ impl Engine { .map(|_| ().into()); if let Some(result_err) = result.as_ref().err() { - if let EvalAltResult::ErrorRuntime(Dynamic(Union::Unit(_)), pos) = - result_err.as_ref() + if let EvalAltResult::ErrorRuntime( + Dynamic(Union::Unit(_, _)), + pos, + ) = result_err.as_ref() { err.set_position(*pos); result = Err(Box::new(err)); @@ -2293,8 +2289,8 @@ impl Engine { // Let/const statement Stmt::Let(var_def, expr, export, _) | Stmt::Const(var_def, expr, export, _) => { let entry_type = match stmt { - Stmt::Let(_, _, _, _) => ScopeEntryType::Normal, - Stmt::Const(_, _, _, _) => ScopeEntryType::Constant, + Stmt::Let(_, _, _, _) => AccessType::Normal, + Stmt::Const(_, _, _, _) => AccessType::Constant, _ => unreachable!(), }; @@ -2388,8 +2384,8 @@ impl Engine { #[cfg(not(feature = "no_closure"))] Stmt::Share(x) => { match scope.get_index(&x.name) { - Some((index, ScopeEntryType::Normal)) => { - let (val, _) = scope.get_mut(index); + Some((index, AccessType::Normal)) => { + let val = scope.get_mut(index); if !val.is_shared() { // Replace the variable with a shared value. @@ -2445,18 +2441,18 @@ impl Engine { fn calc_size(value: &Dynamic) -> (usize, usize, usize) { match value { #[cfg(not(feature = "no_index"))] - Dynamic(Union::Array(arr)) => { + Dynamic(Union::Array(arr, _)) => { let mut arrays = 0; let mut maps = 0; arr.iter().for_each(|value| match value { - Dynamic(Union::Array(_)) => { + Dynamic(Union::Array(_, _)) => { let (a, m, _) = calc_size(value); arrays += a; maps += m; } #[cfg(not(feature = "no_object"))] - Dynamic(Union::Map(_)) => { + Dynamic(Union::Map(_, _)) => { let (a, m, _) = calc_size(value); arrays += a; maps += m; @@ -2467,18 +2463,18 @@ impl Engine { (arrays, maps, 0) } #[cfg(not(feature = "no_object"))] - Dynamic(Union::Map(map)) => { + Dynamic(Union::Map(map, _)) => { let mut arrays = 0; let mut maps = 0; map.values().for_each(|value| match value { #[cfg(not(feature = "no_index"))] - Dynamic(Union::Array(_)) => { + Dynamic(Union::Array(_, _)) => { let (a, m, _) = calc_size(value); arrays += a; maps += m; } - Dynamic(Union::Map(_)) => { + Dynamic(Union::Map(_, _)) => { let (a, m, _) = calc_size(value); arrays += a; maps += m; @@ -2488,7 +2484,7 @@ impl Engine { (arrays, maps, 0) } - Dynamic(Union::Str(s)) => (0, 0, s.len()), + Dynamic(Union::Str(s, _)) => (0, 0, s.len()), _ => (0, 0, 0), } } @@ -2497,13 +2493,13 @@ impl Engine { // Simply return all errors Err(_) => return result, // String with limit - Ok(Dynamic(Union::Str(_))) if self.max_string_size() > 0 => (), + Ok(Dynamic(Union::Str(_, _))) if self.max_string_size() > 0 => (), // Array with limit #[cfg(not(feature = "no_index"))] - Ok(Dynamic(Union::Array(_))) if self.max_array_size() > 0 => (), + Ok(Dynamic(Union::Array(_, _))) if self.max_array_size() > 0 => (), // Map with limit #[cfg(not(feature = "no_object"))] - Ok(Dynamic(Union::Map(_))) if self.max_map_size() > 0 => (), + Ok(Dynamic(Union::Map(_, _))) if self.max_map_size() > 0 => (), // Everything else is simply returned Ok(_) => return result, }; diff --git a/src/fn_call.rs b/src/fn_call.rs index 13514dcb..bdcd4cb7 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -8,7 +8,6 @@ use crate::engine::{ use crate::fn_native::FnCallArgs; use crate::module::NamespaceRef; use crate::optimize::OptimizationLevel; -use crate::scope::EntryType as ScopeEntryType; use crate::stdlib::{ any::{type_name, TypeId}, boxed::Box, @@ -360,7 +359,7 @@ impl Engine { .map(|(name, value)| { let var_name: crate::stdlib::borrow::Cow<'_, str> = crate::r#unsafe::unsafe_cast_var_name_to_lifetime(name).into(); - (var_name, ScopeEntryType::Normal, value) + (var_name, value) }), ); @@ -540,15 +539,10 @@ impl Engine { if !func.externals.is_empty() { captured .into_iter() - .filter(|(name, _, _, _)| func.externals.contains(name.as_ref())) - .for_each(|(name, typ, value, _)| { + .filter(|(name, _, _)| func.externals.contains(name.as_ref())) + .for_each(|(name, value, _)| { // Consume the scope values. - match typ { - ScopeEntryType::Normal => scope.push(name, value), - ScopeEntryType::Constant => { - scope.push_constant(name, value) - } - }; + scope.push_dynamic(name, value); }); } } @@ -1000,13 +994,12 @@ impl Engine { .map(|expr| self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)) .collect::>()?; - let (target, _, typ, pos) = + let (mut target, _, pos) = self.search_namespace(scope, mods, state, lib, this_ptr, &args_expr[0])?; - let target = match typ { - ScopeEntryType::Normal => target, - ScopeEntryType::Constant => target.into_owned(), - }; + if target.as_ref().is_constant() { + target = target.into_owned(); + } self.inc_operations(state) .map_err(|err| err.fill_position(pos))?; @@ -1087,7 +1080,7 @@ impl Engine { .collect::>()?; // Get target reference to first argument - let (target, _, _, pos) = + let (target, _, pos) = self.search_scope_only(scope, mods, state, lib, this_ptr, &args_expr[0])?; self.inc_operations(state) diff --git a/src/module/mod.rs b/src/module/mod.rs index 1fde1d37..510c0884 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1640,7 +1640,7 @@ impl Module { // Create new module let mut module = Module::new(); - scope.into_iter().for_each(|(_, _, value, mut aliases)| { + scope.into_iter().for_each(|(_, value, mut aliases)| { // Variables with an alias left in the scope become module variables if aliases.len() > 1 { aliases.into_iter().for_each(|alias| { diff --git a/src/parser.rs b/src/parser.rs index 6be6ca03..956e197b 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3,12 +3,11 @@ use crate::ast::{ BinaryExpr, CustomExpr, Expr, FnCallExpr, Ident, IdentX, ReturnType, ScriptFnDef, Stmt, }; -use crate::dynamic::Union; +use crate::dynamic::{AccessType, Union}; use crate::engine::{KEYWORD_THIS, MARKER_BLOCK, MARKER_EXPR, MARKER_IDENT}; use crate::module::NamespaceRef; use crate::optimize::optimize_into_ast; use crate::optimize::OptimizationLevel; -use crate::scope::EntryType as ScopeEntryType; use crate::stdlib::{ borrow::Cow, boxed::Box, @@ -49,7 +48,7 @@ struct ParseState<'e> { /// Interned strings. strings: HashMap, /// Encapsulates a local stack with variable names to simulate an actual runtime scope. - stack: Vec<(ImmutableString, ScopeEntryType)>, + stack: Vec<(ImmutableString, AccessType)>, /// Size of the local variables stack upon entry of the current block scope. entry_stack_len: usize, /// Tracks a list of external variables (variables that are not explicitly declared in the scope). @@ -1291,11 +1290,11 @@ fn make_assignment_stmt<'a>( }, ) = x.as_ref(); match state.stack[(state.stack.len() - index.unwrap().get())].1 { - ScopeEntryType::Normal => { + AccessType::Normal => { Ok(Stmt::Assignment(Box::new((lhs, fn_name.into(), rhs)), pos)) } // Constant values cannot be assigned to - ScopeEntryType::Constant => { + AccessType::Constant => { Err(PERR::AssignmentToConstant(name.to_string()).into_err(*name_pos)) } } @@ -1318,11 +1317,11 @@ fn make_assignment_stmt<'a>( }, ) = x.as_ref(); match state.stack[(state.stack.len() - index.unwrap().get())].1 { - ScopeEntryType::Normal => { + AccessType::Normal => { Ok(Stmt::Assignment(Box::new((lhs, fn_name.into(), rhs)), pos)) } // Constant values cannot be assigned to - ScopeEntryType::Constant => { + AccessType::Constant => { Err(PERR::AssignmentToConstant(name.to_string()).into_err(*name_pos)) } } @@ -1811,7 +1810,7 @@ fn parse_custom_syntax( // Variable searches stop at the first empty variable name. state.stack.resize( state.stack.len() + delta as usize, - ("".into(), ScopeEntryType::Normal), + ("".into(), AccessType::Normal), ); } delta if delta < 0 && state.stack.len() <= delta.abs() as usize => state.stack.clear(), @@ -2110,7 +2109,7 @@ fn parse_for( let loop_var = state.get_interned_string(name.clone()); let prev_stack_len = state.stack.len(); - state.stack.push((loop_var, ScopeEntryType::Normal)); + state.stack.push((loop_var, AccessType::Normal)); settings.is_breakable = true; let body = parse_block(input, state, lib, settings.level_up())?; @@ -2125,7 +2124,7 @@ fn parse_let( input: &mut TokenStream, state: &mut ParseState, lib: &mut FunctionsLib, - var_type: ScopeEntryType, + var_type: AccessType, export: bool, mut settings: ParseSettings, ) -> Result { @@ -2156,16 +2155,16 @@ fn parse_let( match var_type { // let name = expr - ScopeEntryType::Normal => { + AccessType::Normal => { let var_name = state.get_interned_string(name.clone()); - state.stack.push((var_name, ScopeEntryType::Normal)); + state.stack.push((var_name, AccessType::Normal)); let var_def = Ident::new(name, pos); Ok(Stmt::Let(Box::new(var_def), init_expr, export, token_pos)) } // const name = { expr:constant } - ScopeEntryType::Constant => { + AccessType::Constant => { let var_name = state.get_interned_string(name.clone()); - state.stack.push((var_name, ScopeEntryType::Constant)); + state.stack.push((var_name, AccessType::Constant)); let var_def = Ident::new(name, pos); Ok(Stmt::Const(Box::new(var_def), init_expr, export, token_pos)) } @@ -2232,13 +2231,13 @@ fn parse_export( match input.peek().unwrap() { (Token::Let, pos) => { let pos = *pos; - let mut stmt = parse_let(input, state, lib, ScopeEntryType::Normal, true, settings)?; + let mut stmt = parse_let(input, state, lib, AccessType::Normal, true, settings)?; stmt.set_position(pos); return Ok(stmt); } (Token::Const, pos) => { let pos = *pos; - let mut stmt = parse_let(input, state, lib, ScopeEntryType::Constant, true, settings)?; + let mut stmt = parse_let(input, state, lib, AccessType::Constant, true, settings)?; stmt.set_position(pos); return Ok(stmt); } @@ -2393,7 +2392,7 @@ fn parse_stmt( lib: &mut FunctionsLib, mut settings: ParseSettings, ) -> Result, ParseError> { - use ScopeEntryType::{Constant, Normal}; + use AccessType::{Constant, Normal}; let (token, token_pos) = match input.peek().unwrap() { (Token::EOF, pos) => return Ok(Some(Stmt::Noop(*pos))), @@ -2637,7 +2636,7 @@ fn parse_fn( return Err(PERR::FnDuplicatedParam(name, s).into_err(pos)); } let s = state.get_interned_string(s); - state.stack.push((s.clone(), ScopeEntryType::Normal)); + state.stack.push((s.clone(), AccessType::Normal)); params.push((s, pos)) } (Token::LexError(err), pos) => return Err(err.into_err(pos)), @@ -2770,7 +2769,7 @@ fn parse_anon_fn( return Err(PERR::FnDuplicatedParam("".to_string(), s).into_err(pos)); } let s = state.get_interned_string(s); - state.stack.push((s.clone(), ScopeEntryType::Normal)); + state.stack.push((s.clone(), AccessType::Normal)); params.push((s, pos)) } (Token::LexError(err), pos) => return Err(err.into_err(pos)), @@ -3005,15 +3004,15 @@ impl Engine { pub fn map_dynamic_to_expr(value: Dynamic, pos: Position) -> Option { match value.0 { #[cfg(not(feature = "no_float"))] - Union::Float(value) => Some(Expr::FloatConstant(value, pos)), + Union::Float(value, _) => Some(Expr::FloatConstant(value, pos)), - Union::Unit(_) => Some(Expr::Unit(pos)), - Union::Int(value) => Some(Expr::IntegerConstant(value, pos)), - Union::Char(value) => Some(Expr::CharConstant(value, pos)), - Union::Str(value) => Some(Expr::StringConstant(value, pos)), - Union::Bool(value) => Some(Expr::BoolConstant(value, pos)), + Union::Unit(_, _) => Some(Expr::Unit(pos)), + Union::Int(value, _) => Some(Expr::IntegerConstant(value, pos)), + Union::Char(value, _) => Some(Expr::CharConstant(value, pos)), + Union::Str(value, _) => Some(Expr::StringConstant(value, pos)), + Union::Bool(value, _) => Some(Expr::BoolConstant(value, pos)), #[cfg(not(feature = "no_index"))] - Union::Array(array) => { + Union::Array(array, _) => { let items: Vec<_> = array .into_iter() .map(|x| map_dynamic_to_expr(x, pos)) @@ -3029,7 +3028,7 @@ pub fn map_dynamic_to_expr(value: Dynamic, pos: Position) -> Option { } } #[cfg(not(feature = "no_object"))] - Union::Map(map) => { + Union::Map(map, _) => { let items: Vec<_> = map .into_iter() .map(|(k, v)| (IdentX::new(k, pos), map_dynamic_to_expr(v, pos))) diff --git a/src/scope.rs b/src/scope.rs index 700a549f..7cef2ca8 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -1,29 +1,9 @@ //! Module that defines the [`Scope`] type representing a function call-stack scope. -use crate::dynamic::Variant; +use crate::dynamic::{AccessType, Variant}; use crate::stdlib::{borrow::Cow, boxed::Box, iter, string::String, vec::Vec}; use crate::{Dynamic, StaticVec}; -/// Type of an entry in the Scope. -#[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] -pub enum EntryType { - /// Normal value. - Normal, - /// Immutable constant value. - Constant, -} - -impl EntryType { - /// Is this entry constant? - #[inline(always)] - pub fn is_constant(&self) -> bool { - match self { - Self::Normal => false, - Self::Constant => true, - } - } -} - /// Type containing information about the current scope. /// Useful for keeping state between [`Engine`][crate::Engine] evaluation runs. /// @@ -58,19 +38,17 @@ impl EntryType { // // # Implementation Notes // -// [`Scope`] is implemented as three [`Vec`]'s of exactly the same length. Variables data (name, type, etc.) +// [`Scope`] is implemented as two [`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`][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)] +#[derive(Debug)] pub struct Scope<'a> { /// Current value of the entry. values: Vec, - /// Type of the entry. - types: Vec, /// (Name, aliases) of the entry. The list of aliases is Boxed because it occurs rarely. names: Vec<(Cow<'a, str>, Box>)>, } @@ -79,7 +57,6 @@ impl Default for Scope<'_> { fn default() -> Self { Self { values: Vec::with_capacity(16), - types: Vec::with_capacity(16), names: Vec::with_capacity(16), } } @@ -124,7 +101,6 @@ impl<'a> Scope<'a> { #[inline(always)] pub fn clear(&mut self) -> &mut Self { self.names.clear(); - self.types.clear(); self.values.clear(); self } @@ -180,7 +156,7 @@ impl<'a> Scope<'a> { name: impl Into>, value: impl Variant + Clone, ) -> &mut Self { - self.push_dynamic_value(name, EntryType::Normal, Dynamic::from(value)) + self.push_dynamic_value(name, AccessType::Normal, Dynamic::from(value)) } /// Add (push) a new [`Dynamic`] entry to the [`Scope`]. /// @@ -196,7 +172,7 @@ impl<'a> Scope<'a> { /// ``` #[inline(always)] pub fn push_dynamic(&mut self, name: impl Into>, value: Dynamic) -> &mut Self { - self.push_dynamic_value(name, EntryType::Normal, value) + self.push_dynamic_value(name, value.access_type(), value) } /// Add (push) a new constant to the [`Scope`]. /// @@ -219,7 +195,7 @@ impl<'a> Scope<'a> { name: impl Into>, value: impl Variant + Clone, ) -> &mut Self { - self.push_dynamic_value(name, EntryType::Constant, Dynamic::from(value)) + self.push_dynamic_value(name, AccessType::Constant, Dynamic::from(value)) } /// Add (push) a new constant with a [`Dynamic`] value to the Scope. /// @@ -242,18 +218,18 @@ impl<'a> Scope<'a> { name: impl Into>, value: Dynamic, ) -> &mut Self { - self.push_dynamic_value(name, EntryType::Constant, value) + self.push_dynamic_value(name, AccessType::Constant, value) } /// Add (push) a new entry with a [`Dynamic`] value to the [`Scope`]. #[inline] pub(crate) fn push_dynamic_value( &mut self, name: impl Into>, - entry_type: EntryType, - value: Dynamic, + access: AccessType, + mut value: Dynamic, ) -> &mut Self { self.names.push((name.into(), Box::new(Default::default()))); - self.types.push(entry_type); + value.set_access_type(access); self.values.push(value.into()); self } @@ -286,7 +262,6 @@ impl<'a> Scope<'a> { #[inline(always)] pub fn rewind(&mut self, size: usize) -> &mut Self { self.names.truncate(size); - self.types.truncate(size); self.values.truncate(size); self } @@ -312,14 +287,14 @@ impl<'a> Scope<'a> { } /// Find an entry in the [`Scope`], starting from the last. #[inline(always)] - pub(crate) fn get_index(&self, name: &str) -> Option<(usize, EntryType)> { + pub(crate) fn get_index(&self, name: &str) -> Option<(usize, AccessType)> { self.names .iter() .enumerate() .rev() // Always search a Scope in reverse order .find_map(|(index, (key, _))| { if name == key.as_ref() { - Some((index, self.types[index])) + Some((index, self.values[index].access_type())) } else { None } @@ -374,8 +349,8 @@ impl<'a> Scope<'a> { None => { self.push(name, value); } - Some((_, EntryType::Constant)) => panic!("variable {} is constant", name), - Some((index, EntryType::Normal)) => { + Some((_, AccessType::Constant)) => panic!("variable {} is constant", name), + Some((index, AccessType::Normal)) => { *self.values.get_mut(index).unwrap() = Dynamic::from(value); } } @@ -383,11 +358,8 @@ 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) { - ( - self.values.get_mut(index).expect("invalid index in Scope"), - self.types[index], - ) + pub(crate) fn get_mut(&mut self, index: usize) -> &mut Dynamic { + self.values.get_mut(index).expect("invalid index in Scope") } /// Update the access type of an entry in the [`Scope`]. #[cfg(not(feature = "no_module"))] @@ -412,7 +384,6 @@ impl<'a> Scope<'a> { .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()); } }); @@ -422,13 +393,11 @@ impl<'a> Scope<'a> { /// Get an iterator to entries in the [`Scope`]. #[inline(always)] #[allow(dead_code)] - pub(crate) fn into_iter( - self, - ) -> impl Iterator, EntryType, Dynamic, Vec)> { + pub(crate) fn into_iter(self) -> impl Iterator, Dynamic, Vec)> { self.names .into_iter() - .zip(self.types.into_iter().zip(self.values.into_iter())) - .map(|((name, alias), (typ, value))| (name, typ, value, alias.to_vec())) + .zip(self.values.into_iter()) + .map(|((name, alias), value)| (name, value, alias.to_vec())) } /// Get an iterator to entries in the [`Scope`]. /// Shared values are flatten-cloned. @@ -466,17 +435,16 @@ impl<'a> Scope<'a> { 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)) + .zip(self.values.iter()) + .map(|((name, _), value)| (name.as_ref(), value.is_constant(), value)) } } -impl<'a, K: Into>> iter::Extend<(K, EntryType, Dynamic)> for Scope<'a> { +impl<'a, K: Into>> iter::Extend<(K, Dynamic)> for Scope<'a> { #[inline(always)] - fn extend>(&mut self, iter: T) { - iter.into_iter().for_each(|(name, typ, value)| { + fn extend>(&mut self, iter: T) { + iter.into_iter().for_each(|(name, value)| { self.names.push((name.into(), Box::new(Default::default()))); - self.types.push(typ); self.values.push(value); }); } diff --git a/tests/arrays.rs b/tests/arrays.rs index 65811a23..3c1dbff2 100644 --- a/tests/arrays.rs +++ b/tests/arrays.rs @@ -178,7 +178,10 @@ fn test_arrays_map_reduce() -> Result<(), Box> { engine.eval::( r#" let x = [1, 2, 3]; - x.reduce(|sum, v, i| { if i == 0 { sum = 10 } sum + v * v }) + x.reduce(|sum, v, i| { + if i == 0 { sum = 10 } + sum + v * v + }) "# )?, 24 From 7598ec136f97bdfb7f724c1a0b7ec03959aa8722 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 8 Dec 2020 23:09:12 +0800 Subject: [PATCH 04/15] Fix feature builds. --- src/dynamic.rs | 46 ++++++++++++++++++++++++++++++-------------- src/engine.rs | 12 ++++++------ src/fn_call.rs | 2 +- src/scope.rs | 2 +- src/serde_impl/de.rs | 46 ++++++++++++++++++++++---------------------- 5 files changed, 63 insertions(+), 45 deletions(-) diff --git a/src/dynamic.rs b/src/dynamic.rs index 10042587..750c07c6 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -569,13 +569,19 @@ impl Dynamic { | Union::Str(_, access) | Union::Char(_, access) | Union::Int(_, access) - | Union::Float(_, access) - | Union::Array(_, access) - | Union::Map(_, access) | Union::FnPtr(_, access) - | Union::TimeStamp(_, access) - | Union::Variant(_, access) - | Union::Shared(_, access) => access, + | Union::Variant(_, access) => access, + + #[cfg(not(feature = "no_float"))] + Union::Float(_, access) => access, + #[cfg(not(feature = "no_index"))] + Union::Array(_, access) => access, + #[cfg(not(feature = "no_object"))] + Union::Map(_, access) => access, + #[cfg(not(feature = "no_std"))] + Union::TimeStamp(_, access) => access, + #[cfg(not(feature = "no_closure"))] + Union::Shared(_, access) => access, } } /// Set the [`AccessType`] for this [`Dynamic`]. @@ -586,17 +592,29 @@ impl Dynamic { | Union::Str(_, access) | Union::Char(_, access) | Union::Int(_, access) - | Union::Float(_, access) - | Union::Array(_, access) - | Union::Map(_, access) | Union::FnPtr(_, access) - | Union::TimeStamp(_, access) - | Union::Variant(_, access) - | Union::Shared(_, access) => *access = typ, + | Union::Variant(_, access) => *access = typ, + + #[cfg(not(feature = "no_float"))] + Union::Float(_, access) => *access = typ, + #[cfg(not(feature = "no_index"))] + Union::Array(_, access) => *access = typ, + #[cfg(not(feature = "no_object"))] + Union::Map(_, access) => *access = typ, + #[cfg(not(feature = "no_std"))] + Union::TimeStamp(_, access) => *access = typ, + #[cfg(not(feature = "no_closure"))] + Union::Shared(_, access) => *access = typ, } } - /// Is this [`Dynamic`] constant? - pub(crate) fn is_constant(&self) -> bool { + /// Is this [`Dynamic`] read-only? + /// + /// Constant [`Dynamic`] values are read-only. If a [`&mut Dynamic`][Dynamic] to such a constant + /// is passed to a Rust function, the function can use this information to return an error of + /// [`EvalAltResult::ErrorAssignmentToConstant`][crate::EvalAltResult::ErrorAssignmentToConstant] + /// if its value is going to be modified. This safe-guards constant values from being modified + /// from within Rust functions. + pub fn is_read_only(&self) -> bool { self.access_type().is_constant() } /// Create a [`Dynamic`] from any type. A [`Dynamic`] value is simply returned as is. diff --git a/src/engine.rs b/src/engine.rs index b399c6e7..7fcb0dbd 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1285,7 +1285,7 @@ impl Engine { self.search_namespace(scope, mods, state, lib, this_ptr, lhs)?; // Constants cannot be modified - if target.as_ref().is_constant() && new_val.is_some() { + if target.as_ref().is_read_only() && new_val.is_some() { return EvalAltResult::ErrorAssignmentToConstant(var_name.to_string(), pos) .into(); } @@ -1569,7 +1569,7 @@ impl Engine { lib: &[&Module], this_ptr: &'s mut Option<&mut Dynamic>, expr: &Expr, - no_const: bool, + _no_const: bool, level: usize, ) -> Result<(Target<'s>, Position), Box> { match expr { @@ -1579,7 +1579,7 @@ impl Engine { self.search_namespace(scope, mods, state, lib, this_ptr, expr)?; // If necessary, constants are cloned - if target.as_ref().is_constant() { + if target.as_ref().is_read_only() { target = target.into_owned(); } @@ -1598,7 +1598,7 @@ impl Engine { let idx = self.eval_expr(scope, mods, state, lib, this_ptr, &x.rhs, level)?; let idx_pos = x.rhs.position(); let (mut target, pos) = self.eval_expr_as_target( - scope, mods, state, lib, this_ptr, &x.lhs, no_const, level, + scope, mods, state, lib, this_ptr, &x.lhs, _no_const, level, )?; let is_ref = target.is_ref(); @@ -1625,7 +1625,7 @@ impl Engine { // var.prop Expr::Property(ref p) => { let (mut target, _) = self.eval_expr_as_target( - scope, mods, state, lib, this_ptr, &x.lhs, no_const, level, + scope, mods, state, lib, this_ptr, &x.lhs, _no_const, level, )?; let is_ref = target.is_ref(); @@ -1919,7 +1919,7 @@ impl Engine { self.inc_operations(state) .map_err(|err| err.fill_position(pos))?; - if lhs_ptr.as_ref().is_constant() { + if lhs_ptr.as_ref().is_read_only() { // Assignment to constant variable Err(Box::new(EvalAltResult::ErrorAssignmentToConstant( name.to_string(), diff --git a/src/fn_call.rs b/src/fn_call.rs index bdcd4cb7..8e2c88bd 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -997,7 +997,7 @@ impl Engine { let (mut target, _, pos) = self.search_namespace(scope, mods, state, lib, this_ptr, &args_expr[0])?; - if target.as_ref().is_constant() { + if target.as_ref().is_read_only() { target = target.into_owned(); } diff --git a/src/scope.rs b/src/scope.rs index 7cef2ca8..42216538 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -436,7 +436,7 @@ impl<'a> Scope<'a> { self.names .iter() .zip(self.values.iter()) - .map(|((name, _), value)| (name.as_ref(), value.is_constant(), value)) + .map(|((name, _), value)| (name.as_ref(), value.is_read_only(), value)) } } diff --git a/src/serde_impl/de.rs b/src/serde_impl/de.rs index 6d7fcfed..0aa00a6e 100644 --- a/src/serde_impl/de.rs +++ b/src/serde_impl/de.rs @@ -132,39 +132,39 @@ impl<'de> Deserializer<'de> for &mut DynamicDeserializer<'de> { fn deserialize_any>(self, visitor: V) -> Result> { match &self.value.0 { - Union::Unit(_) => self.deserialize_unit(visitor), - Union::Bool(_) => self.deserialize_bool(visitor), - Union::Str(_) => self.deserialize_str(visitor), - Union::Char(_) => self.deserialize_char(visitor), + Union::Unit(_, _) => self.deserialize_unit(visitor), + Union::Bool(_, _) => self.deserialize_bool(visitor), + Union::Str(_, _) => self.deserialize_str(visitor), + Union::Char(_, _) => self.deserialize_char(visitor), #[cfg(not(feature = "only_i32"))] - Union::Int(_) => self.deserialize_i64(visitor), + Union::Int(_, _) => self.deserialize_i64(visitor), #[cfg(feature = "only_i32")] - Union::Int(_) => self.deserialize_i32(visitor), + Union::Int(_, _) => self.deserialize_i32(visitor), #[cfg(not(feature = "no_float"))] - Union::Float(_) => self.deserialize_f64(visitor), + Union::Float(_, _) => self.deserialize_f64(visitor), #[cfg(not(feature = "no_index"))] - Union::Array(_) => self.deserialize_seq(visitor), + Union::Array(_, _) => self.deserialize_seq(visitor), #[cfg(not(feature = "no_object"))] - Union::Map(_) => self.deserialize_map(visitor), - Union::FnPtr(_) => self.type_error(), + Union::Map(_, _) => self.deserialize_map(visitor), + Union::FnPtr(_, _) => self.type_error(), #[cfg(not(feature = "no_std"))] - Union::TimeStamp(_) => self.type_error(), + Union::TimeStamp(_, _) => self.type_error(), - Union::Variant(value) if value.is::() => self.deserialize_i8(visitor), - Union::Variant(value) if value.is::() => self.deserialize_i16(visitor), - Union::Variant(value) if value.is::() => self.deserialize_i32(visitor), - Union::Variant(value) if value.is::() => self.deserialize_i64(visitor), - Union::Variant(value) if value.is::() => self.deserialize_i128(visitor), - Union::Variant(value) if value.is::() => self.deserialize_u8(visitor), - Union::Variant(value) if value.is::() => self.deserialize_u16(visitor), - Union::Variant(value) if value.is::() => self.deserialize_u32(visitor), - Union::Variant(value) if value.is::() => self.deserialize_u64(visitor), - Union::Variant(value) if value.is::() => self.deserialize_u128(visitor), + Union::Variant(value, _) if value.is::() => self.deserialize_i8(visitor), + Union::Variant(value, _) if value.is::() => self.deserialize_i16(visitor), + Union::Variant(value, _) if value.is::() => self.deserialize_i32(visitor), + Union::Variant(value, _) if value.is::() => self.deserialize_i64(visitor), + Union::Variant(value, _) if value.is::() => self.deserialize_i128(visitor), + Union::Variant(value, _) if value.is::() => self.deserialize_u8(visitor), + Union::Variant(value, _) if value.is::() => self.deserialize_u16(visitor), + Union::Variant(value, _) if value.is::() => self.deserialize_u32(visitor), + Union::Variant(value, _) if value.is::() => self.deserialize_u64(visitor), + Union::Variant(value, _) if value.is::() => self.deserialize_u128(visitor), - Union::Variant(_) => self.type_error(), + Union::Variant(_, _) => self.type_error(), #[cfg(not(feature = "no_closure"))] - Union::Shared(_) => self.type_error(), + Union::Shared(_, _) => self.type_error(), } } From dbee0eb0f59f370764d31ea5cd2fa2df82de109c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 9 Dec 2020 18:37:52 +0800 Subject: [PATCH 05/15] Rename AccessType to ReadWrite and ReadOnly. --- RELEASES.md | 1 + src/ast.rs | 8 ++-- src/dynamic.rs | 124 ++++++++++++++++++++++++------------------------ src/engine.rs | 16 +++---- src/optimize.rs | 34 ++++++++----- src/parser.rs | 40 ++++++++-------- src/scope.rs | 22 ++++----- 7 files changed, 127 insertions(+), 118 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index aec6810f..ac4ddd40 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -8,6 +8,7 @@ Bug fixes --------- * Constants are no longer propagated by the optimizer if shadowed by a non-constant variable. +* Constants passed as the `this` parameter to Rhai functions now throws an error if assigned to. Version 0.19.7 diff --git a/src/ast.rs b/src/ast.rs index 14156b88..140a7cd8 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1,6 +1,6 @@ //! Module defining the AST (abstract syntax tree). -use crate::dynamic::{AccessType, Union}; +use crate::dynamic::{AccessMode, Union}; use crate::fn_native::shared_make_mut; use crate::module::NamespaceRef; use crate::stdlib::{ @@ -942,7 +942,7 @@ impl Expr { Self::StringConstant(x, _) => x.clone().into(), Self::FnPointer(x, _) => Dynamic(Union::FnPtr( Box::new(FnPtr::new_unchecked(x.clone(), Default::default())), - AccessType::Constant, + AccessMode::ReadOnly, )), Self::BoolConstant(x, _) => (*x).into(), Self::Unit(_) => ().into(), @@ -954,7 +954,7 @@ impl Expr { x.len(), )); arr.extend(x.iter().map(|v| v.get_constant_value().unwrap())); - Dynamic(Union::Array(Box::new(arr), AccessType::Constant)) + Dynamic(Union::Array(Box::new(arr), AccessMode::ReadOnly)) } #[cfg(not(feature = "no_object"))] @@ -967,7 +967,7 @@ impl Expr { x.iter() .map(|(k, v)| (k.name.clone(), v.get_constant_value().unwrap())), ); - Dynamic(Union::Map(Box::new(map), AccessType::Constant)) + Dynamic(Union::Map(Box::new(map), AccessMode::ReadOnly)) } _ => return None, diff --git a/src/dynamic.rs b/src/dynamic.rs index 750c07c6..e5880ea5 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -116,21 +116,21 @@ impl dyn Variant { } } -/// Type of an entry in the Scope. +/// Modes of access. #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] -pub enum AccessType { - /// Normal value. - Normal, - /// Immutable constant value. - Constant, +pub enum AccessMode { + /// Mutable. + ReadWrite, + /// Immutable. + ReadOnly, } -impl AccessType { - /// Is the access type [`Constant`]? - pub fn is_constant(self) -> bool { +impl AccessMode { + /// Is the access type [`ReadOnly`]? + pub fn is_read_only(self) -> bool { match self { - Self::Normal => false, - Self::Constant => true, + Self::ReadWrite => false, + Self::ReadOnly => true, } } } @@ -142,25 +142,25 @@ pub struct Dynamic(pub(crate) Union); /// /// Most variants are boxed to reduce the size. pub enum Union { - Unit((), AccessType), - Bool(bool, AccessType), - Str(ImmutableString, AccessType), - Char(char, AccessType), - Int(INT, AccessType), + Unit((), AccessMode), + Bool(bool, AccessMode), + Str(ImmutableString, AccessMode), + Char(char, AccessMode), + Int(INT, AccessMode), #[cfg(not(feature = "no_float"))] - Float(FLOAT, AccessType), + Float(FLOAT, AccessMode), #[cfg(not(feature = "no_index"))] - Array(Box, AccessType), + Array(Box, AccessMode), #[cfg(not(feature = "no_object"))] - Map(Box, AccessType), - FnPtr(Box, AccessType), + Map(Box, AccessMode), + FnPtr(Box, AccessMode), #[cfg(not(feature = "no_std"))] - TimeStamp(Box, AccessType), + TimeStamp(Box, AccessMode), - Variant(Box>, AccessType), + Variant(Box>, AccessMode), #[cfg(not(feature = "no_closure"))] - Shared(crate::Shared>, AccessType), + Shared(crate::Shared>, AccessMode), } /// Underlying [`Variant`] read guard for [`Dynamic`]. @@ -506,27 +506,27 @@ impl Clone for Dynamic { /// The cloned copy is marked [`AccessType::Normal`] even if the original is constant. fn clone(&self) -> Self { match self.0 { - Union::Unit(value, _) => Self(Union::Unit(value, AccessType::Normal)), - Union::Bool(value, _) => Self(Union::Bool(value, AccessType::Normal)), - Union::Str(ref value, _) => Self(Union::Str(value.clone(), AccessType::Normal)), - Union::Char(value, _) => Self(Union::Char(value, AccessType::Normal)), - Union::Int(value, _) => Self(Union::Int(value, AccessType::Normal)), + Union::Unit(value, _) => Self(Union::Unit(value, AccessMode::ReadWrite)), + Union::Bool(value, _) => Self(Union::Bool(value, AccessMode::ReadWrite)), + Union::Str(ref value, _) => Self(Union::Str(value.clone(), AccessMode::ReadWrite)), + Union::Char(value, _) => Self(Union::Char(value, AccessMode::ReadWrite)), + Union::Int(value, _) => Self(Union::Int(value, AccessMode::ReadWrite)), #[cfg(not(feature = "no_float"))] - Union::Float(value, _) => Self(Union::Float(value, AccessType::Normal)), + Union::Float(value, _) => Self(Union::Float(value, AccessMode::ReadWrite)), #[cfg(not(feature = "no_index"))] - Union::Array(ref value, _) => Self(Union::Array(value.clone(), AccessType::Normal)), + Union::Array(ref value, _) => Self(Union::Array(value.clone(), AccessMode::ReadWrite)), #[cfg(not(feature = "no_object"))] - Union::Map(ref value, _) => Self(Union::Map(value.clone(), AccessType::Normal)), - Union::FnPtr(ref value, _) => Self(Union::FnPtr(value.clone(), AccessType::Normal)), + Union::Map(ref value, _) => Self(Union::Map(value.clone(), AccessMode::ReadWrite)), + Union::FnPtr(ref value, _) => Self(Union::FnPtr(value.clone(), AccessMode::ReadWrite)), #[cfg(not(feature = "no_std"))] Union::TimeStamp(ref value, _) => { - Self(Union::TimeStamp(value.clone(), AccessType::Normal)) + Self(Union::TimeStamp(value.clone(), AccessMode::ReadWrite)) } Union::Variant(ref value, _) => (***value).clone_into_dynamic(), #[cfg(not(feature = "no_closure"))] - Union::Shared(ref cell, _) => Self(Union::Shared(cell.clone(), AccessType::Normal)), + Union::Shared(ref cell, _) => Self(Union::Shared(cell.clone(), AccessMode::ReadWrite)), } } } @@ -540,29 +540,29 @@ impl Default for Dynamic { impl Dynamic { /// A [`Dynamic`] containing a `()`. - pub const UNIT: Dynamic = Self(Union::Unit((), AccessType::Normal)); + pub const UNIT: Dynamic = Self(Union::Unit((), AccessMode::ReadWrite)); /// A [`Dynamic`] containing a `true`. - pub const TRUE: Dynamic = Self(Union::Bool(true, AccessType::Normal)); + pub const TRUE: Dynamic = Self(Union::Bool(true, AccessMode::ReadWrite)); /// A [`Dynamic`] containing a [`false`]. - pub const FALSE: Dynamic = Self(Union::Bool(false, AccessType::Normal)); + pub const FALSE: Dynamic = Self(Union::Bool(false, AccessMode::ReadWrite)); /// A [`Dynamic`] containing the integer zero. - pub const ZERO: Dynamic = Self(Union::Int(0, AccessType::Normal)); + pub const ZERO: Dynamic = Self(Union::Int(0, AccessMode::ReadWrite)); /// A [`Dynamic`] containing the integer one. - pub const ONE: Dynamic = Self(Union::Int(1, AccessType::Normal)); + pub const ONE: Dynamic = Self(Union::Int(1, AccessMode::ReadWrite)); /// A [`Dynamic`] containing the integer negative one. - pub const NEGATIVE_ONE: Dynamic = Self(Union::Int(-1, AccessType::Normal)); + pub const NEGATIVE_ONE: Dynamic = Self(Union::Int(-1, AccessMode::ReadWrite)); /// A [`Dynamic`] containing the floating-point zero. #[cfg(not(feature = "no_float"))] - pub const FLOAT_ZERO: Dynamic = Self(Union::Float(0.0, AccessType::Normal)); + pub const FLOAT_ZERO: Dynamic = Self(Union::Float(0.0, AccessMode::ReadWrite)); /// A [`Dynamic`] containing the floating-point one. #[cfg(not(feature = "no_float"))] - pub const FLOAT_ONE: Dynamic = Self(Union::Float(1.0, AccessType::Normal)); + pub const FLOAT_ONE: Dynamic = Self(Union::Float(1.0, AccessMode::ReadWrite)); /// A [`Dynamic`] containing the floating-point negative one. #[cfg(not(feature = "no_float"))] - pub const FLOAT_NEGATIVE_ONE: Dynamic = Self(Union::Float(-1.0, AccessType::Normal)); + pub const FLOAT_NEGATIVE_ONE: Dynamic = Self(Union::Float(-1.0, AccessMode::ReadWrite)); - /// Get the [`AccessType`] for this [`Dynamic`]. - pub(crate) fn access_type(&self) -> AccessType { + /// Get the [`AccessMode`] for this [`Dynamic`]. + pub(crate) fn access_mode(&self) -> AccessMode { match self.0 { Union::Unit(_, access) | Union::Bool(_, access) @@ -584,8 +584,8 @@ impl Dynamic { Union::Shared(_, access) => access, } } - /// Set the [`AccessType`] for this [`Dynamic`]. - pub(crate) fn set_access_type(&mut self, typ: AccessType) { + /// Set the [`AccessMode`] for this [`Dynamic`]. + pub(crate) fn set_access_mode(&mut self, typ: AccessMode) { match &mut self.0 { Union::Unit(_, access) | Union::Bool(_, access) @@ -615,7 +615,7 @@ impl Dynamic { /// if its value is going to be modified. This safe-guards constant values from being modified /// from within Rust functions. pub fn is_read_only(&self) -> bool { - self.access_type().is_constant() + self.access_mode().is_read_only() } /// Create a [`Dynamic`] from any type. A [`Dynamic`] value is simply returned as is. /// @@ -733,7 +733,7 @@ impl Dynamic { } } - Self(Union::Variant(Box::new(boxed), AccessType::Normal)) + Self(Union::Variant(Box::new(boxed), AccessMode::ReadWrite)) } /// Turn the [`Dynamic`] value into a shared [`Dynamic`] value backed by an [`Rc`][std::rc::Rc]`<`[`RefCell`][std::cell::RefCell]`<`[`Dynamic`]`>>` /// or [`Arc`][std::sync::Arc]`<`[`RwLock`][std::sync::RwLock]`<`[`Dynamic`]`>>` depending on the `sync` feature. @@ -750,7 +750,7 @@ impl Dynamic { /// Panics under the `no_closure` feature. #[inline(always)] pub fn into_shared(self) -> Self { - let _access = self.access_type(); + let _access = self.access_mode(); #[cfg(not(feature = "no_closure"))] return match self.0 { @@ -1338,38 +1338,38 @@ impl Dynamic { impl From<()> for Dynamic { #[inline(always)] fn from(value: ()) -> Self { - Self(Union::Unit(value, AccessType::Normal)) + Self(Union::Unit(value, AccessMode::ReadWrite)) } } impl From for Dynamic { #[inline(always)] fn from(value: bool) -> Self { - Self(Union::Bool(value, AccessType::Normal)) + Self(Union::Bool(value, AccessMode::ReadWrite)) } } impl From for Dynamic { #[inline(always)] fn from(value: INT) -> Self { - Self(Union::Int(value, AccessType::Normal)) + Self(Union::Int(value, AccessMode::ReadWrite)) } } #[cfg(not(feature = "no_float"))] impl From for Dynamic { #[inline(always)] fn from(value: FLOAT) -> Self { - Self(Union::Float(value, AccessType::Normal)) + Self(Union::Float(value, AccessMode::ReadWrite)) } } impl From for Dynamic { #[inline(always)] fn from(value: char) -> Self { - Self(Union::Char(value, AccessType::Normal)) + Self(Union::Char(value, AccessMode::ReadWrite)) } } impl> From for Dynamic { #[inline(always)] fn from(value: S) -> Self { - Self(Union::Str(value.into(), AccessType::Normal)) + Self(Union::Str(value.into(), AccessMode::ReadWrite)) } } #[cfg(not(feature = "no_index"))] @@ -1378,7 +1378,7 @@ impl From> for Dynamic { fn from(value: crate::stdlib::vec::Vec) -> Self { Self(Union::Array( Box::new(value.into_iter().map(Dynamic::from).collect()), - AccessType::Normal, + AccessMode::ReadWrite, )) } } @@ -1388,7 +1388,7 @@ impl From<&[T]> for Dynamic { fn from(value: &[T]) -> Self { Self(Union::Array( Box::new(value.iter().cloned().map(Dynamic::from).collect()), - AccessType::Normal, + AccessMode::ReadWrite, )) } } @@ -1405,26 +1405,26 @@ impl, T: Variant + Clone> From for Dynamic { #[inline(always)] fn from(value: FnPtr) -> Self { - Self(Union::FnPtr(Box::new(value), AccessType::Normal)) + Self(Union::FnPtr(Box::new(value), AccessMode::ReadWrite)) } } impl From> for Dynamic { #[inline(always)] fn from(value: Box) -> Self { - Self(Union::FnPtr(value, AccessType::Normal)) + Self(Union::FnPtr(value, AccessMode::ReadWrite)) } } #[cfg(not(feature = "no_std"))] impl From for Dynamic { #[inline(always)] fn from(value: Instant) -> Self { - Self(Union::TimeStamp(Box::new(value), AccessType::Normal)) + Self(Union::TimeStamp(Box::new(value), AccessMode::ReadWrite)) } } diff --git a/src/engine.rs b/src/engine.rs index 7fcb0dbd..e42d06ac 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1,7 +1,7 @@ //! Main module defining the script evaluation [`Engine`]. use crate::ast::{Expr, FnCallExpr, Ident, IdentX, ReturnType, Stmt}; -use crate::dynamic::{map_std_type_name, AccessType, Union, Variant}; +use crate::dynamic::{map_std_type_name, AccessMode, Union, Variant}; use crate::fn_call::run_builtin_op_assignment; use crate::fn_native::{CallableFunction, Callback, IteratorFn, OnVarCallback}; use crate::module::NamespaceRef; @@ -850,7 +850,7 @@ impl Engine { // Module variables are constant let mut target = target.clone(); - target.set_access_type(AccessType::Constant); + target.set_access_mode(AccessMode::ReadOnly); Ok((target.into(), name, *pos)) } // Normal variable access @@ -905,7 +905,7 @@ impl Engine { if let Some(mut result) = resolve_var(name, index, &context).map_err(|err| err.fill_position(*pos))? { - result.set_access_type(AccessType::Constant); + result.set_access_mode(AccessMode::ReadOnly); return Ok((result.into(), name, *pos)); } } @@ -1732,7 +1732,7 @@ impl Engine { for item in x.as_ref() { arr.push(self.eval_expr(scope, mods, state, lib, this_ptr, item, level)?); } - Ok(Dynamic(Union::Array(Box::new(arr), AccessType::Normal))) + Ok(Dynamic(Union::Array(Box::new(arr), AccessMode::ReadWrite))) } #[cfg(not(feature = "no_object"))] @@ -1745,7 +1745,7 @@ impl Engine { self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?, ); } - Ok(Dynamic(Union::Map(Box::new(map), AccessType::Normal))) + Ok(Dynamic(Union::Map(Box::new(map), AccessMode::ReadWrite))) } // Normal function call @@ -2289,8 +2289,8 @@ impl Engine { // Let/const statement Stmt::Let(var_def, expr, export, _) | Stmt::Const(var_def, expr, export, _) => { let entry_type = match stmt { - Stmt::Let(_, _, _, _) => AccessType::Normal, - Stmt::Const(_, _, _, _) => AccessType::Constant, + Stmt::Let(_, _, _, _) => AccessMode::ReadWrite, + Stmt::Const(_, _, _, _) => AccessMode::ReadOnly, _ => unreachable!(), }; @@ -2384,7 +2384,7 @@ impl Engine { #[cfg(not(feature = "no_closure"))] Stmt::Share(x) => { match scope.get_index(&x.name) { - Some((index, AccessType::Normal)) => { + Some((index, AccessMode::ReadWrite)) => { let val = scope.get_mut(index); if !val.is_shared() { diff --git a/src/optimize.rs b/src/optimize.rs index 77a60e46..f04dda40 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -1,7 +1,7 @@ //! Module implementing the [`AST`] optimizer. use crate::ast::{Expr, ScriptFnDef, Stmt}; -use crate::dynamic::AccessType; +use crate::dynamic::AccessMode; use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_PRINT, KEYWORD_TYPE_OF}; use crate::fn_call::run_builtin_binary_op; use crate::parser::map_dynamic_to_expr; @@ -59,7 +59,7 @@ struct State<'a> { /// Has the [`AST`] been changed during this pass? changed: bool, /// Collection of constants to use for eager function evaluations. - variables: Vec<(String, AccessType, Expr)>, + variables: Vec<(String, AccessMode, Expr)>, /// An [`Engine`] instance for eager function evaluation. engine: &'a Engine, /// [Module] containing script-defined functions. @@ -102,7 +102,7 @@ impl<'a> State<'a> { } /// Add a new constant to the list. #[inline(always)] - pub fn push_var(&mut self, name: &str, access: AccessType, value: Expr) { + pub fn push_var(&mut self, name: &str, access: AccessMode, value: Expr) { self.variables.push((name.into(), access, value)) } /// Look up a constant from the list. @@ -110,7 +110,7 @@ impl<'a> State<'a> { pub fn find_constant(&self, name: &str) -> Option<&Expr> { for (n, access, expr) in self.variables.iter().rev() { if n == name { - return if access.is_constant() { + return if access.is_read_only() { Some(expr) } else { None @@ -163,14 +163,18 @@ fn optimize_stmt_block( statements.iter_mut().for_each(|stmt| match stmt { // Add constant literals into the state Stmt::Const(var_def, Some(expr), _, _) if expr.is_constant() => { - state.push_var(&var_def.name, AccessType::Constant, mem::take(expr)); + state.push_var(&var_def.name, AccessMode::ReadOnly, mem::take(expr)); } Stmt::Const(var_def, None, _, _) => { - state.push_var(&var_def.name, AccessType::Constant, Expr::Unit(var_def.pos)); + state.push_var(&var_def.name, AccessMode::ReadOnly, Expr::Unit(var_def.pos)); } // Add variables into the state Stmt::Let(var_def, _, _, _) => { - state.push_var(&var_def.name, AccessType::Normal, Expr::Unit(var_def.pos)); + state.push_var( + &var_def.name, + AccessMode::ReadWrite, + Expr::Unit(var_def.pos), + ); } // Optimize the statement _ => optimize_stmt(stmt, state, preserve_result), @@ -749,11 +753,11 @@ fn optimize_top_level( // Add constants and variables from the scope scope.iter().for_each(|(name, constant, value)| { if !constant { - state.push_var(name, AccessType::Normal, Expr::Unit(Position::NONE)); + state.push_var(name, AccessMode::ReadWrite, Expr::Unit(Position::NONE)); } else if let Some(val) = map_dynamic_to_expr(value, Position::NONE) { - state.push_var(name, AccessType::Constant, val); + state.push_var(name, AccessMode::ReadOnly, val); } else { - state.push_var(name, AccessType::Constant, Expr::Unit(Position::NONE)); + state.push_var(name, AccessMode::ReadOnly, Expr::Unit(Position::NONE)); } }); @@ -774,7 +778,7 @@ fn optimize_top_level( optimize_expr(value_expr, &mut state); if value_expr.is_constant() { - state.push_var(&var_def.name, AccessType::Constant, value_expr.clone()); + state.push_var(&var_def.name, AccessMode::ReadOnly, value_expr.clone()); } // Keep it in the global scope @@ -784,10 +788,14 @@ fn optimize_top_level( } } Stmt::Const(var_def, None, _, _) => { - state.push_var(&var_def.name, AccessType::Constant, Expr::Unit(var_def.pos)); + state.push_var(&var_def.name, AccessMode::ReadOnly, Expr::Unit(var_def.pos)); } Stmt::Let(var_def, _, _, _) => { - state.push_var(&var_def.name, AccessType::Normal, Expr::Unit(var_def.pos)); + state.push_var( + &var_def.name, + AccessMode::ReadWrite, + Expr::Unit(var_def.pos), + ); } _ => { // Keep all variable declarations at this level diff --git a/src/parser.rs b/src/parser.rs index 956e197b..34c4f848 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3,7 +3,7 @@ use crate::ast::{ BinaryExpr, CustomExpr, Expr, FnCallExpr, Ident, IdentX, ReturnType, ScriptFnDef, Stmt, }; -use crate::dynamic::{AccessType, Union}; +use crate::dynamic::{AccessMode, Union}; use crate::engine::{KEYWORD_THIS, MARKER_BLOCK, MARKER_EXPR, MARKER_IDENT}; use crate::module::NamespaceRef; use crate::optimize::optimize_into_ast; @@ -48,7 +48,7 @@ struct ParseState<'e> { /// Interned strings. strings: HashMap, /// Encapsulates a local stack with variable names to simulate an actual runtime scope. - stack: Vec<(ImmutableString, AccessType)>, + stack: Vec<(ImmutableString, AccessMode)>, /// Size of the local variables stack upon entry of the current block scope. entry_stack_len: usize, /// Tracks a list of external variables (variables that are not explicitly declared in the scope). @@ -1290,11 +1290,11 @@ fn make_assignment_stmt<'a>( }, ) = x.as_ref(); match state.stack[(state.stack.len() - index.unwrap().get())].1 { - AccessType::Normal => { + AccessMode::ReadWrite => { Ok(Stmt::Assignment(Box::new((lhs, fn_name.into(), rhs)), pos)) } // Constant values cannot be assigned to - AccessType::Constant => { + AccessMode::ReadOnly => { Err(PERR::AssignmentToConstant(name.to_string()).into_err(*name_pos)) } } @@ -1317,11 +1317,11 @@ fn make_assignment_stmt<'a>( }, ) = x.as_ref(); match state.stack[(state.stack.len() - index.unwrap().get())].1 { - AccessType::Normal => { + AccessMode::ReadWrite => { Ok(Stmt::Assignment(Box::new((lhs, fn_name.into(), rhs)), pos)) } // Constant values cannot be assigned to - AccessType::Constant => { + AccessMode::ReadOnly => { Err(PERR::AssignmentToConstant(name.to_string()).into_err(*name_pos)) } } @@ -1810,7 +1810,7 @@ fn parse_custom_syntax( // Variable searches stop at the first empty variable name. state.stack.resize( state.stack.len() + delta as usize, - ("".into(), AccessType::Normal), + ("".into(), AccessMode::ReadWrite), ); } delta if delta < 0 && state.stack.len() <= delta.abs() as usize => state.stack.clear(), @@ -2109,7 +2109,7 @@ fn parse_for( let loop_var = state.get_interned_string(name.clone()); let prev_stack_len = state.stack.len(); - state.stack.push((loop_var, AccessType::Normal)); + state.stack.push((loop_var, AccessMode::ReadWrite)); settings.is_breakable = true; let body = parse_block(input, state, lib, settings.level_up())?; @@ -2124,7 +2124,7 @@ fn parse_let( input: &mut TokenStream, state: &mut ParseState, lib: &mut FunctionsLib, - var_type: AccessType, + var_type: AccessMode, export: bool, mut settings: ParseSettings, ) -> Result { @@ -2155,16 +2155,16 @@ fn parse_let( match var_type { // let name = expr - AccessType::Normal => { + AccessMode::ReadWrite => { let var_name = state.get_interned_string(name.clone()); - state.stack.push((var_name, AccessType::Normal)); + state.stack.push((var_name, AccessMode::ReadWrite)); let var_def = Ident::new(name, pos); Ok(Stmt::Let(Box::new(var_def), init_expr, export, token_pos)) } // const name = { expr:constant } - AccessType::Constant => { + AccessMode::ReadOnly => { let var_name = state.get_interned_string(name.clone()); - state.stack.push((var_name, AccessType::Constant)); + state.stack.push((var_name, AccessMode::ReadOnly)); let var_def = Ident::new(name, pos); Ok(Stmt::Const(Box::new(var_def), init_expr, export, token_pos)) } @@ -2231,13 +2231,13 @@ fn parse_export( match input.peek().unwrap() { (Token::Let, pos) => { let pos = *pos; - let mut stmt = parse_let(input, state, lib, AccessType::Normal, true, settings)?; + let mut stmt = parse_let(input, state, lib, AccessMode::ReadWrite, true, settings)?; stmt.set_position(pos); return Ok(stmt); } (Token::Const, pos) => { let pos = *pos; - let mut stmt = parse_let(input, state, lib, AccessType::Constant, true, settings)?; + let mut stmt = parse_let(input, state, lib, AccessMode::ReadOnly, true, settings)?; stmt.set_position(pos); return Ok(stmt); } @@ -2392,7 +2392,7 @@ fn parse_stmt( lib: &mut FunctionsLib, mut settings: ParseSettings, ) -> Result, ParseError> { - use AccessType::{Constant, Normal}; + use AccessMode::{ReadOnly, ReadWrite}; let (token, token_pos) = match input.peek().unwrap() { (Token::EOF, pos) => return Ok(Some(Stmt::Noop(*pos))), @@ -2520,9 +2520,9 @@ fn parse_stmt( Token::Try => parse_try_catch(input, state, lib, settings.level_up()).map(Some), - Token::Let => parse_let(input, state, lib, Normal, false, settings.level_up()).map(Some), + Token::Let => parse_let(input, state, lib, ReadWrite, false, settings.level_up()).map(Some), Token::Const => { - parse_let(input, state, lib, Constant, false, settings.level_up()).map(Some) + parse_let(input, state, lib, ReadOnly, false, settings.level_up()).map(Some) } #[cfg(not(feature = "no_module"))] @@ -2636,7 +2636,7 @@ fn parse_fn( return Err(PERR::FnDuplicatedParam(name, s).into_err(pos)); } let s = state.get_interned_string(s); - state.stack.push((s.clone(), AccessType::Normal)); + state.stack.push((s.clone(), AccessMode::ReadWrite)); params.push((s, pos)) } (Token::LexError(err), pos) => return Err(err.into_err(pos)), @@ -2769,7 +2769,7 @@ fn parse_anon_fn( return Err(PERR::FnDuplicatedParam("".to_string(), s).into_err(pos)); } let s = state.get_interned_string(s); - state.stack.push((s.clone(), AccessType::Normal)); + state.stack.push((s.clone(), AccessMode::ReadWrite)); params.push((s, pos)) } (Token::LexError(err), pos) => return Err(err.into_err(pos)), diff --git a/src/scope.rs b/src/scope.rs index 42216538..93b4ba7d 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -1,6 +1,6 @@ //! Module that defines the [`Scope`] type representing a function call-stack scope. -use crate::dynamic::{AccessType, Variant}; +use crate::dynamic::{AccessMode, Variant}; use crate::stdlib::{borrow::Cow, boxed::Box, iter, string::String, vec::Vec}; use crate::{Dynamic, StaticVec}; @@ -156,7 +156,7 @@ impl<'a> Scope<'a> { name: impl Into>, value: impl Variant + Clone, ) -> &mut Self { - self.push_dynamic_value(name, AccessType::Normal, Dynamic::from(value)) + self.push_dynamic_value(name, AccessMode::ReadWrite, Dynamic::from(value)) } /// Add (push) a new [`Dynamic`] entry to the [`Scope`]. /// @@ -172,7 +172,7 @@ impl<'a> Scope<'a> { /// ``` #[inline(always)] pub fn push_dynamic(&mut self, name: impl Into>, value: Dynamic) -> &mut Self { - self.push_dynamic_value(name, value.access_type(), value) + self.push_dynamic_value(name, value.access_mode(), value) } /// Add (push) a new constant to the [`Scope`]. /// @@ -195,7 +195,7 @@ impl<'a> Scope<'a> { name: impl Into>, value: impl Variant + Clone, ) -> &mut Self { - self.push_dynamic_value(name, AccessType::Constant, Dynamic::from(value)) + self.push_dynamic_value(name, AccessMode::ReadOnly, Dynamic::from(value)) } /// Add (push) a new constant with a [`Dynamic`] value to the Scope. /// @@ -218,18 +218,18 @@ impl<'a> Scope<'a> { name: impl Into>, value: Dynamic, ) -> &mut Self { - self.push_dynamic_value(name, AccessType::Constant, value) + self.push_dynamic_value(name, AccessMode::ReadOnly, value) } /// Add (push) a new entry with a [`Dynamic`] value to the [`Scope`]. #[inline] pub(crate) fn push_dynamic_value( &mut self, name: impl Into>, - access: AccessType, + access: AccessMode, mut value: Dynamic, ) -> &mut Self { self.names.push((name.into(), Box::new(Default::default()))); - value.set_access_type(access); + value.set_access_mode(access); self.values.push(value.into()); self } @@ -287,14 +287,14 @@ impl<'a> Scope<'a> { } /// Find an entry in the [`Scope`], starting from the last. #[inline(always)] - pub(crate) fn get_index(&self, name: &str) -> Option<(usize, AccessType)> { + pub(crate) fn get_index(&self, name: &str) -> Option<(usize, AccessMode)> { self.names .iter() .enumerate() .rev() // Always search a Scope in reverse order .find_map(|(index, (key, _))| { if name == key.as_ref() { - Some((index, self.values[index].access_type())) + Some((index, self.values[index].access_mode())) } else { None } @@ -349,8 +349,8 @@ impl<'a> Scope<'a> { None => { self.push(name, value); } - Some((_, AccessType::Constant)) => panic!("variable {} is constant", name), - Some((index, AccessType::Normal)) => { + Some((_, AccessMode::ReadOnly)) => panic!("variable {} is constant", name), + Some((index, AccessMode::ReadWrite)) => { *self.values.get_mut(index).unwrap() = Dynamic::from(value); } } From 99dd7a6481f9dbdd0402d561fae78bcc9755b4b1 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 9 Dec 2020 21:06:36 +0800 Subject: [PATCH 06/15] Share constant variables for closures. --- RELEASES.md | 5 +++++ doc/src/language/fn-closure.md | 18 ------------------ src/dynamic.rs | 15 ++++++++++++++- src/engine.rs | 13 +++++-------- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index ac4ddd40..0b296b78 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -10,6 +10,11 @@ Bug fixes * Constants are no longer propagated by the optimizer if shadowed by a non-constant variable. * Constants passed as the `this` parameter to Rhai functions now throws an error if assigned to. +Enhancements +------------ + +* Capturing a constant variable in a closure is now supported, with no cloning. + Version 0.19.7 ============== diff --git a/doc/src/language/fn-closure.md b/doc/src/language/fn-closure.md index 56c79cfd..e4f59fe7 100644 --- a/doc/src/language/fn-closure.md +++ b/doc/src/language/fn-closure.md @@ -57,24 +57,6 @@ f.call(2) == 42; ``` -Constants are Not Captured --------------------------- - -Constants are never shared. Their values are simply cloned. - -```rust -const x = 42; // constant variable 'x' - -let f = |y| x += y; // constant 'x' is cloned and not captured - -x.is_shared() == false; // 'x' is not shared - -f.call(10); // the cloned copy of 'x' is changed - -x == 42; // 'x' is not changed -``` - - Beware: Captured Variables are Truly Shared ------------------------------------------ diff --git a/src/dynamic.rs b/src/dynamic.rs index e5880ea5..41f2756e 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -615,7 +615,20 @@ impl Dynamic { /// if its value is going to be modified. This safe-guards constant values from being modified /// from within Rust functions. pub fn is_read_only(&self) -> bool { - self.access_mode().is_read_only() + match self.0 { + #[cfg(not(feature = "no_closure"))] + Union::Shared(_, access) if access.is_read_only() => true, + + #[cfg(not(feature = "no_closure"))] + #[cfg(not(feature = "sync"))] + Union::Shared(ref cell, _) => cell.borrow().access_mode().is_read_only(), + + #[cfg(not(feature = "no_closure"))] + #[cfg(feature = "sync")] + Union::Shared(ref cell, _) => cell.read().unwrap().access_mode().is_read_only(), + + _ => self.access_mode().is_read_only(), + } } /// Create a [`Dynamic`] from any type. A [`Dynamic`] value is simply returned as is. /// diff --git a/src/engine.rs b/src/engine.rs index e42d06ac..129ff1cf 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -2383,16 +2383,13 @@ impl Engine { // Share statement #[cfg(not(feature = "no_closure"))] Stmt::Share(x) => { - match scope.get_index(&x.name) { - Some((index, AccessMode::ReadWrite)) => { - let val = scope.get_mut(index); + if let Some((index, _)) = scope.get_index(&x.name) { + let val = scope.get_mut(index); - if !val.is_shared() { - // Replace the variable with a shared value. - *val = crate::stdlib::mem::take(val).into_shared(); - } + if !val.is_shared() { + // Replace the variable with a shared value. + *val = crate::stdlib::mem::take(val).into_shared(); } - _ => (), } Ok(Dynamic::UNIT) } From 839da9c7f00086063f27e68a7672bd8c84705d74 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 11 Dec 2020 12:51:28 +0800 Subject: [PATCH 07/15] Increase switch benchmark size. --- benches/eval_expression.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/benches/eval_expression.rs b/benches/eval_expression.rs index e8da7025..ffb0108c 100644 --- a/benches/eval_expression.rs +++ b/benches/eval_expression.rs @@ -164,14 +164,19 @@ fn bench_eval_switch(bench: &mut Bencher) { let rem = 0; for x in range(0, 10) { - rem = x % 5; + rem = x % 10; sum += switch rem { 0 => 10, 1 => 12, 2 => 42, 3 => 1, - _ => 9 + 4 => 12, + 5 => 42, + 6 => 1, + 7 => 12, + 8 => 42, + 9 => 1, } } "#; @@ -191,19 +196,18 @@ fn bench_eval_nested_if(bench: &mut Bencher) { let rem = 0; for x in range(0, 10) { - rem = x % 5; + rem = x % 10; - sum += if rem == 0 { - 10 - } else if rem == 1 { - 12 - } else if rem == 2 { - 42 - } else if rem == 3 { - 1 - } else{ - 9 - }; + sum += if rem == 0 { 10 } + else if rem == 1 { 12 } + else if rem == 2 { 42 } + else if rem == 3 { 1 } + else if rem == 4 { 12 } + else if rem == 5 { 42 } + else if rem == 6 { 1 } + else if rem == 7 { 12 } + else if rem == 8 { 42 } + else if rem == 9 { 1 }; } "#; From bed29da71a0f74803dbf8672123b3adc07c84b2d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 11 Dec 2020 12:57:07 +0800 Subject: [PATCH 08/15] Use ImmutableString. --- src/ast.rs | 4 ++-- src/engine.rs | 6 +++--- src/module/mod.rs | 16 ++++++++++------ src/parser.rs | 11 +++++++---- src/scope.rs | 20 +++++++++++++------- tests/optimizer.rs | 2 +- 6 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 140a7cd8..ecac8042 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -618,9 +618,9 @@ pub enum Stmt { /// `for` id `in` expr `{` stmt `}` For(Expr, Box<(String, Stmt)>, Position), /// \[`export`\] `let` id `=` expr - Let(Box, Option, bool, Position), + Let(Box, Option, bool, Position), /// \[`export`\] `const` id `=` expr - Const(Box, Option, bool, Position), + Const(Box, Option, bool, Position), /// expr op`=` expr Assignment(Box<(Expr, Cow<'static, str>, Expr)>, Position), /// `{` stmt`;` ... `}` diff --git a/src/engine.rs b/src/engine.rs index 129ff1cf..b053af40 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -2302,9 +2302,9 @@ impl Engine { }; let (var_name, _alias): (Cow<'_, str>, _) = if state.is_global() { ( - var_def.name.clone().into(), + var_def.name.to_string().into(), if *export { - Some(var_def.name.to_string()) + Some(var_def.name.clone()) } else { None }, @@ -2371,7 +2371,7 @@ impl Engine { // Mark scope variables as public if let Some(index) = scope.get_index(name).map(|(i, _)| i) { let alias = rename.as_ref().map(|x| &x.name).unwrap_or_else(|| name); - scope.add_entry_alias(index, alias.to_string()); + scope.add_entry_alias(index, alias.clone()); } else { return EvalAltResult::ErrorVariableNotFound(name.to_string(), *id_pos) .into(); diff --git a/src/module/mod.rs b/src/module/mod.rs index 510c0884..6db9889d 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -120,9 +120,9 @@ impl FuncInfo { #[derive(Clone)] pub struct Module { /// Sub-modules. - modules: HashMap>, + modules: HashMap>, /// Module variables. - variables: HashMap, + variables: HashMap, /// Flattened collection of all module variables, including those in sub-modules. all_variables: HashMap, /// External Rust functions. @@ -160,7 +160,7 @@ impl fmt::Debug for Module { "Module(\n modules: {}\n vars: {}\n functions: {}\n)", self.modules .keys() - .map(String::as_str) + .map(|m| m.as_str()) .collect::>() .join(", "), self.variables @@ -331,7 +331,11 @@ impl Module { /// assert_eq!(module.get_var_value::("answer").unwrap(), 42); /// ``` #[inline(always)] - pub fn set_var(&mut self, name: impl Into, value: impl Variant + Clone) -> &mut Self { + pub fn set_var( + &mut self, + name: impl Into, + value: impl Variant + Clone, + ) -> &mut Self { self.variables.insert(name.into(), Dynamic::from(value)); self.indexed = false; self @@ -458,7 +462,7 @@ impl Module { #[inline(always)] pub fn set_sub_module( &mut self, - name: impl Into, + name: impl Into, sub_module: impl Into>, ) -> &mut Self { self.modules.insert(name.into(), sub_module.into()); @@ -1423,7 +1427,7 @@ impl Module { other.modules.iter().for_each(|(k, v)| { let mut m = Self::new(); m.merge_filtered(v, _filter); - self.set_sub_module(k, m); + self.set_sub_module(k.clone(), m); }); #[cfg(feature = "no_function")] self.modules diff --git a/src/parser.rs b/src/parser.rs index 34c4f848..479c2778 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -826,7 +826,7 @@ fn parse_switch( } } - let mut table = HashMap::with_capacity_and_hasher(16, StraightHasherBuilder); + let mut table = HashMap::new(); let mut def_stmt = None; loop { @@ -915,9 +915,12 @@ fn parse_switch( } } + let mut final_table = HashMap::with_capacity_and_hasher(table.len(), StraightHasherBuilder); + final_table.extend(table.into_iter()); + Ok(Stmt::Switch( item, - Box::new((table, def_stmt)), + Box::new((final_table, def_stmt)), settings.pos, )) } @@ -2158,14 +2161,14 @@ fn parse_let( AccessMode::ReadWrite => { let var_name = state.get_interned_string(name.clone()); state.stack.push((var_name, AccessMode::ReadWrite)); - let var_def = Ident::new(name, pos); + let var_def = IdentX::new(name, pos); Ok(Stmt::Let(Box::new(var_def), init_expr, export, token_pos)) } // const name = { expr:constant } AccessMode::ReadOnly => { let var_name = state.get_interned_string(name.clone()); state.stack.push((var_name, AccessMode::ReadOnly)); - let var_def = Ident::new(name, pos); + let var_def = IdentX::new(name, pos); Ok(Stmt::Const(Box::new(var_def), init_expr, export, token_pos)) } } diff --git a/src/scope.rs b/src/scope.rs index 93b4ba7d..4e5cc08a 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -1,8 +1,8 @@ //! Module that defines the [`Scope`] type representing a function call-stack scope. use crate::dynamic::{AccessMode, Variant}; -use crate::stdlib::{borrow::Cow, boxed::Box, iter, string::String, vec::Vec}; -use crate::{Dynamic, StaticVec}; +use crate::stdlib::{borrow::Cow, boxed::Box, iter, vec::Vec}; +use crate::{Dynamic, ImmutableString, StaticVec}; /// Type containing information about the current scope. /// Useful for keeping state between [`Engine`][crate::Engine] evaluation runs. @@ -50,7 +50,7 @@ pub struct Scope<'a> { /// Current value of the entry. values: Vec, /// (Name, aliases) of the entry. The list of aliases is Boxed because it occurs rarely. - names: Vec<(Cow<'a, str>, Box>)>, + names: Vec<(Cow<'a, str>, Box>)>, } impl Default for Scope<'_> { @@ -364,10 +364,14 @@ impl<'a> Scope<'a> { /// Update the access type of an entry in the [`Scope`]. #[cfg(not(feature = "no_module"))] #[inline(always)] - pub(crate) fn add_entry_alias(&mut self, index: usize, alias: String) -> &mut Self { + pub(crate) fn add_entry_alias( + &mut self, + index: usize, + alias: impl Into + PartialEq, + ) -> &mut Self { let entry = self.names.get_mut(index).expect("invalid index in Scope"); - if !entry.1.contains(&alias) { - entry.1.push(alias); + if !entry.1.iter().any(|a| &alias == a) { + entry.1.push(alias.into()); } self } @@ -393,7 +397,9 @@ impl<'a> Scope<'a> { /// Get an iterator to entries in the [`Scope`]. #[inline(always)] #[allow(dead_code)] - pub(crate) fn into_iter(self) -> impl Iterator, Dynamic, Vec)> { + pub(crate) fn into_iter( + self, + ) -> impl Iterator, Dynamic, Vec)> { self.names .into_iter() .zip(self.values.into_iter()) diff --git a/tests/optimizer.rs b/tests/optimizer.rs index db8477a5..fa817bf1 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -55,7 +55,7 @@ fn test_optimizer_parse() -> Result<(), Box> { let ast = engine.compile("{ const DECISION = false; if DECISION { 42 } else { 123 } }")?; - assert!(format!("{:?}", ast).starts_with(r#"AST([Block([Const(Ident { name: "DECISION", pos: 1:9 }, Some(Unit(0:0)), false, 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)]"#)); + assert!(format!("{:?}", ast).starts_with(r#"AST([Block([Const(IdentX { name: "DECISION", pos: 1:9 }, Some(Unit(0:0)), false, 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)]"#)); let ast = engine.compile("if 1 == 2 { 42 }")?; From 4438c358d50efc2c1b496e1a35f6bbda299a8c86 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 12 Dec 2020 10:10:27 +0800 Subject: [PATCH 09/15] on_progress takes u64. --- RELEASES.md | 5 +++++ doc/src/safety/progress.md | 2 +- src/engine.rs | 12 +++++++----- src/engine_api.rs | 4 ++-- src/fn_native.rs | 19 +++++++++++++------ tests/operations.rs | 8 ++++---- 6 files changed, 32 insertions(+), 18 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 0b296b78..62fc4be6 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -10,6 +10,11 @@ Bug fixes * Constants are no longer propagated by the optimizer if shadowed by a non-constant variable. * Constants passed as the `this` parameter to Rhai functions now throws an error if assigned to. +Breaking changes +---------------- + +* `Engine::on_progress` now takes `u64` instead of `&u64`. + Enhancements ------------ diff --git a/doc/src/safety/progress.md b/doc/src/safety/progress.md index 44260b43..ff294976 100644 --- a/doc/src/safety/progress.md +++ b/doc/src/safety/progress.md @@ -13,7 +13,7 @@ the `Engine::on_progress` method: ```rust let mut engine = Engine::new(); -engine.on_progress(|&count| { // parameter is '&u64' - number of operations already performed +engine.on_progress(|count| { // parameter is number of operations already performed if count % 1000 == 0 { println!("{}", count); // print out a progress log every 1,000 operations } diff --git a/src/engine.rs b/src/engine.rs index b053af40..b3314482 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -3,7 +3,9 @@ use crate::ast::{Expr, FnCallExpr, Ident, IdentX, ReturnType, Stmt}; use crate::dynamic::{map_std_type_name, AccessMode, Union, Variant}; use crate::fn_call::run_builtin_op_assignment; -use crate::fn_native::{CallableFunction, Callback, IteratorFn, OnVarCallback}; +use crate::fn_native::{ + CallableFunction, IteratorFn, OnPrintCallback, OnProgressCallback, OnVarCallback, +}; use crate::module::NamespaceRef; use crate::optimize::OptimizationLevel; use crate::packages::{Package, PackagesCollection, StandardPackage}; @@ -622,11 +624,11 @@ pub struct Engine { pub(crate) resolve_var: Option, /// Callback closure for implementing the `print` command. - pub(crate) print: Callback, + pub(crate) print: OnPrintCallback, /// Callback closure for implementing the `debug` command. - pub(crate) debug: Callback, + pub(crate) debug: OnPrintCallback, /// Callback closure for progress reporting. - pub(crate) progress: Option>>, + pub(crate) progress: Option, /// Optimize the AST after compilation. pub(crate) optimization_level: OptimizationLevel, @@ -2542,7 +2544,7 @@ impl Engine { // Report progress - only in steps if let Some(progress) = &self.progress { - if let Some(token) = progress(&state.operations) { + 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 8169d791..6a2a5923 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1726,7 +1726,7 @@ impl Engine { /// /// let mut engine = Engine::new(); /// - /// engine.on_progress(move |&ops| { + /// engine.on_progress(move |ops| { /// if ops > 10000 { /// Some("Over 10,000 operations!".into()) /// } else if ops % 800 == 0 { @@ -1748,7 +1748,7 @@ impl Engine { #[inline(always)] pub fn on_progress( &mut self, - callback: impl Fn(&u64) -> Option + 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 ee87de84..0706983d 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -343,18 +343,25 @@ pub type FnPlugin = dyn PluginFunction; #[cfg(feature = "sync")] pub type FnPlugin = dyn PluginFunction + Send + Sync; -/// A standard callback function. +/// A standard callback function for progress reporting. #[cfg(not(feature = "sync"))] -pub type Callback = Box R + 'static>; -/// A standard callback function. +pub type OnProgressCallback = Box Option + 'static>; +/// A standard callback function for progress reporting. #[cfg(feature = "sync")] -pub type Callback = Box R + Send + Sync + 'static>; +pub type OnProgressCallback = Box Option + Send + Sync + 'static>; -/// A standard callback function. +/// A standard callback function for printing. +#[cfg(not(feature = "sync"))] +pub type OnPrintCallback = Box; +/// A standard callback function for printing. +#[cfg(feature = "sync")] +pub type OnPrintCallback = Box; + +/// A standard callback function for variable access. #[cfg(not(feature = "sync"))] pub type OnVarCallback = Box Result, Box> + 'static>; -/// A standard callback function. +/// A standard callback function for variable access. #[cfg(feature = "sync")] pub type OnVarCallback = Box< dyn Fn(&str, usize, &EvalContext) -> Result, Box> diff --git a/tests/operations.rs b/tests/operations.rs index a0f52550..fc8c25cb 100644 --- a/tests/operations.rs +++ b/tests/operations.rs @@ -6,7 +6,7 @@ fn test_max_operations() -> Result<(), Box> { let mut engine = Engine::new(); engine.set_max_operations(500); - engine.on_progress(|&count| { + engine.on_progress(|count| { if count % 100 == 0 { println!("{}", count); } @@ -34,7 +34,7 @@ fn test_max_operations_functions() -> Result<(), Box> { let mut engine = Engine::new(); engine.set_max_operations(500); - engine.on_progress(|&count| { + engine.on_progress(|count| { if count % 100 == 0 { println!("{}", count); } @@ -90,7 +90,7 @@ fn test_max_operations_eval() -> Result<(), Box> { let mut engine = Engine::new(); engine.set_max_operations(500); - engine.on_progress(|&count| { + engine.on_progress(|count| { if count % 100 == 0 { println!("{}", count); } @@ -117,7 +117,7 @@ fn test_max_operations_progress() -> Result<(), Box> { let mut engine = Engine::new(); engine.set_max_operations(500); - engine.on_progress(|&count| { + engine.on_progress(|count| { if count < 100 { None } else { From 5443368359782eec1ce46d379698ad3949479a1a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 12 Dec 2020 11:15:09 +0800 Subject: [PATCH 10/15] Pass Position into function calls. --- src/engine.rs | 77 +++++++++++++++++++++-------------------------- src/engine_api.rs | 12 +++++++- src/fn_call.rs | 73 +++++++++++++++++++++++--------------------- src/fn_native.rs | 1 + src/optimize.rs | 1 + 5 files changed, 86 insertions(+), 78 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index b3314482..38a62918 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1016,8 +1016,8 @@ impl Engine { let args = &mut [target_val, &mut idx_val2, &mut new_val.0]; self.exec_fn_call( - mods, state, lib, FN_IDX_SET, 0, args, is_ref, true, false, None, - None, level, + mods, state, lib, FN_IDX_SET, 0, args, is_ref, true, false, + new_val.1, None, None, level, ) .map_err(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) @@ -1061,9 +1061,8 @@ impl Engine { let args = idx_val.as_fn_call_args(); self.make_method_call( mods, state, lib, name, *hash, target, args, def_value, *native, false, - level, + *pos, level, ) - .map_err(|err| err.fill_position(*pos)) } // xxx.module::fn_name(...) - syntax error Expr::FnCall(_, _) => unreachable!(), @@ -1094,8 +1093,8 @@ impl Engine { let mut new_val = new_val; let mut args = [target_val, &mut new_val.as_mut().unwrap().0]; self.exec_fn_call( - mods, state, lib, setter, 0, &mut args, is_ref, true, false, None, - None, level, + mods, state, lib, setter, 0, &mut args, is_ref, true, false, *pos, + None, None, level, ) .map(|(v, _)| (v, true)) .map_err(|err| err.fill_position(*pos)) @@ -1105,8 +1104,8 @@ impl Engine { let ((getter, _), IdentX { pos, .. }) = x.as_ref(); let mut args = [target_val]; self.exec_fn_call( - mods, state, lib, getter, 0, &mut args, is_ref, true, false, None, - None, level, + mods, state, lib, getter, 0, &mut args, is_ref, true, false, *pos, + None, None, level, ) .map(|(v, _)| (v, false)) .map_err(|err| err.fill_position(*pos)) @@ -1133,12 +1132,10 @@ impl Engine { } = x.as_ref(); let def_value = def_value.as_ref(); let args = idx_val.as_fn_call_args(); - let (val, _) = self - .make_method_call( - mods, state, lib, name, *hash, target, args, def_value, - *native, false, level, - ) - .map_err(|err| err.fill_position(*pos))?; + let (val, _) = self.make_method_call( + mods, state, lib, name, *hash, target, args, def_value, + *native, false, *pos, level, + )?; val.into() } // {xxx:map}.module::fn_name(...) - syntax error @@ -1164,7 +1161,7 @@ impl Engine { let (mut val, updated) = self .exec_fn_call( mods, state, lib, getter, 0, args, is_ref, true, false, - None, None, level, + *pos, None, None, level, ) .map_err(|err| err.fill_position(*pos))?; @@ -1191,7 +1188,7 @@ impl Engine { arg_values[1] = val; self.exec_fn_call( mods, state, lib, setter, 0, arg_values, is_ref, true, - false, None, None, level, + false, *pos, None, None, level, ) .or_else( |err| match *err { @@ -1217,12 +1214,10 @@ impl Engine { } = f.as_ref(); let def_value = def_value.as_ref(); let args = idx_val.as_fn_call_args(); - let (mut val, _) = self - .make_method_call( - mods, state, lib, name, *hash, target, args, def_value, - *native, false, level, - ) - .map_err(|err| err.fill_position(*pos))?; + let (mut val, _) = self.make_method_call( + mods, state, lib, name, *hash, target, args, def_value, + *native, false, *pos, level, + )?; let val = &mut val; let target = &mut val.into(); @@ -1475,8 +1470,8 @@ impl Engine { let mut idx = idx; let args = &mut [target, &mut idx]; self.exec_fn_call( - _mods, state, _lib, FN_IDX_GET, 0, args, _is_ref, true, false, None, None, - _level, + _mods, state, _lib, FN_IDX_GET, 0, args, _is_ref, true, false, idx_pos, None, + None, _level, ) .map(|(v, _)| v.into()) .map_err(|err| match *err { @@ -1530,11 +1525,12 @@ impl Engine { let hash = calc_native_fn_hash(empty(), OP_EQUALS, args.iter().map(|a| a.type_id())); + let pos = rhs.position(); + if self .call_native_fn( - mods, state, lib, OP_EQUALS, hash, args, false, false, def_value, - ) - .map_err(|err| err.fill_position(rhs.position()))? + mods, state, lib, OP_EQUALS, hash, args, false, false, pos, def_value, + )? .0 .as_bool() .unwrap_or(false) @@ -1654,11 +1650,10 @@ impl Engine { let ((getter, _), IdentX { pos, .. }) = p.as_ref(); let mut args = [target.as_mut()]; self.exec_fn_call( - mods, state, lib, getter, 0, &mut args, is_ref, true, false, None, - None, level, + mods, state, lib, getter, 0, &mut args, is_ref, true, false, *pos, + None, None, level, ) .map(|(v, _)| (v.into(), *pos)) - .map_err(|err| err.fill_position(*pos)) } } // var.??? @@ -1764,9 +1759,8 @@ impl Engine { let def_value = def_value.as_ref(); self.make_function_call( scope, mods, state, lib, this_ptr, name, args, def_value, *hash, *native, - false, *cap_scope, level, + false, *pos, *cap_scope, level, ) - .map_err(|err| err.fill_position(*pos)) } // Namespace-qualified function call @@ -1783,9 +1777,8 @@ impl Engine { let def_value = def_value.as_ref(); self.make_qualified_function_call( scope, mods, state, lib, this_ptr, namespace, name, args, def_value, *hash, - level, + *pos, level, ) - .map_err(|err| err.fill_position(*pos)) } Expr::In(x, _) => { @@ -1984,12 +1977,10 @@ impl Engine { let args = &mut [&mut lhs_ptr.as_mut().clone(), &mut rhs_val]; // Run function - let (value, _) = self - .exec_fn_call( - mods, state, lib, op, 0, args, false, false, false, None, None, - level, - ) - .map_err(|err| err.fill_position(*op_pos))?; + let (value, _) = self.exec_fn_call( + mods, state, lib, op, 0, args, false, false, false, *op_pos, None, + None, level, + )?; let value = value.flatten(); @@ -2023,10 +2014,10 @@ impl Engine { let result = self .exec_fn_call( - mods, state, lib, op, 0, args, false, false, false, None, None, level, + mods, state, lib, op, 0, args, false, false, false, *op_pos, None, + None, level, ) - .map(|(v, _)| v) - .map_err(|err| err.fill_position(*op_pos))?; + .map(|(v, _)| v)?; Some((result, rhs_expr.position())) }; diff --git a/src/engine_api.rs b/src/engine_api.rs index 6a2a5923..5cdaf1e2 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1611,7 +1611,17 @@ impl Engine { crate::fn_call::ensure_no_data_race(name, args, false)?; } - self.call_script_fn(scope, &mut mods, &mut state, lib, this_ptr, fn_def, args, 0) + self.call_script_fn( + scope, + &mut mods, + &mut state, + lib, + this_ptr, + fn_def, + args, + Position::NONE, + 0, + ) } /// Optimize the [`AST`] with constants defined in an external Scope. /// An optimized copy of the [`AST`] is returned while the original [`AST`] is consumed. diff --git a/src/fn_call.rs b/src/fn_call.rs index 8e2c88bd..9c498f29 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -149,7 +149,6 @@ pub fn ensure_no_data_race( impl Engine { /// Call a native Rust function registered with the [`Engine`]. - /// [`Position`] in [`EvalAltResult`] is [`None`][Position::None] and must be set afterwards. /// /// ## WARNING /// @@ -166,6 +165,7 @@ impl Engine { args: &mut FnCallArgs, is_ref: bool, pub_only: bool, + pos: Position, def_val: Option<&Dynamic>, ) -> Result<(Dynamic, bool), Box> { self.inc_operations(state)?; @@ -204,7 +204,7 @@ impl Engine { EvalAltResult::ErrorMismatchOutputType( self.map_type_name(type_name::()).into(), typ.into(), - Position::NONE, + pos, ) })?) .into(), @@ -215,7 +215,7 @@ impl Engine { EvalAltResult::ErrorMismatchOutputType( self.map_type_name(type_name::()).into(), typ.into(), - Position::NONE, + pos, ) })?) .into(), @@ -247,7 +247,7 @@ impl Engine { prop, self.map_type_name(args[0].type_name()) ), - Position::NONE, + pos, ) .into(); } @@ -262,7 +262,7 @@ impl Engine { self.map_type_name(args[0].type_name()), self.map_type_name(args[1].type_name()), ), - Position::NONE, + pos, ) .into(); } @@ -276,7 +276,7 @@ impl Engine { self.map_type_name(args[0].type_name()), self.map_type_name(args[1].type_name()), ), - Position::NONE, + pos, ) .into(); } @@ -290,7 +290,7 @@ impl Engine { self.map_type_name(args[0].type_name()), self.map_type_name(args[1].type_name()), ), - Position::NONE, + pos, ) .into(); } @@ -309,13 +309,12 @@ impl Engine { .collect::>() .join(", ") ), - Position::NONE, + pos, ) .into() } /// Call a script-defined function. - /// [`Position`] in [`EvalAltResult`] is [`None`][Position::None] and must be set afterwards. /// /// ## WARNING /// @@ -332,6 +331,7 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, fn_def: &crate::ast::ScriptFnDef, args: &mut FnCallArgs, + pos: Position, level: usize, ) -> Result> { self.inc_operations(state)?; @@ -340,7 +340,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(pos))); } let orig_scope_level = state.scope_level; @@ -392,17 +392,14 @@ impl Engine { EvalAltResult::ErrorInFunctionCall( format!("{} > {}", fn_def.name, name), err, - Position::NONE, + 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, Position::NONE) - .into() - } + _ => EvalAltResult::ErrorInFunctionCall(fn_def.name.to_string(), err, pos).into(), }); // Remove all local variables @@ -456,7 +453,6 @@ impl Engine { } /// Perform an actual function call, native Rust or scripted, taking care of special functions. - /// [`Position`] in [`EvalAltResult`] is [`None`][Position::None] and must be set afterwards. /// /// ## WARNING /// @@ -474,6 +470,7 @@ impl Engine { is_ref: bool, _is_method: bool, pub_only: bool, + pos: Position, _capture_scope: Option, def_val: Option<&Dynamic>, _level: usize, @@ -511,7 +508,7 @@ impl Engine { fn_name, fn_name ) .into(), - Position::NONE, + pos, ) .into() } @@ -558,6 +555,7 @@ impl Engine { &mut Some(*first), func, rest, + pos, _level, )? } else { @@ -566,8 +564,9 @@ impl Engine { let mut backup: ArgBackup = Default::default(); backup.change_first_arg_to_copy(is_ref, args); - let result = self - .call_script_fn(scope, mods, state, lib, &mut None, func, args, _level); + let result = self.call_script_fn( + scope, mods, state, lib, &mut None, func, args, pos, _level, + ); // Restore the original reference backup.restore_first_arg(args); @@ -587,6 +586,7 @@ impl Engine { args, is_ref, pub_only, + pos, def_val, ) } @@ -594,7 +594,7 @@ impl Engine { // Normal native function call _ => self.call_native_fn( - mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, def_val, + mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, pos, def_val, ), } } @@ -625,7 +625,6 @@ impl Engine { } /// Evaluate a text string as a script - used primarily for 'eval'. - /// [`Position`] in [`EvalAltResult`] is [`None`][Position::None] and must be set afterwards. fn eval_script_expr( &self, scope: &mut Scope, @@ -633,6 +632,7 @@ impl Engine { state: &mut State, lib: &[&Module], script: &str, + pos: Position, _level: usize, ) -> Result> { self.inc_operations(state)?; @@ -646,7 +646,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(pos))); } // Compile the script text @@ -672,7 +672,6 @@ impl Engine { } /// Call a dot method. - /// [`Position`] in [`EvalAltResult`] is [`None`][Position::None] and must be set afterwards. #[cfg(not(feature = "no_object"))] pub(crate) fn make_method_call( &self, @@ -686,6 +685,7 @@ impl Engine { def_val: Option<&Dynamic>, native: bool, pub_only: bool, + pos: Position, level: usize, ) -> Result<(Dynamic, bool), Box> { let is_ref = target.is_ref(); @@ -716,7 +716,8 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - mods, state, lib, fn_name, hash, args, false, false, pub_only, None, def_val, level, + mods, state, lib, fn_name, hash, args, false, false, pub_only, pos, None, def_val, + level, ) } else if fn_name == KEYWORD_FN_PTR_CALL && call_args.len() > 0 @@ -743,7 +744,8 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - mods, state, lib, fn_name, hash, args, is_ref, true, pub_only, None, def_val, level, + mods, state, lib, fn_name, hash, args, is_ref, true, pub_only, pos, None, def_val, + level, ) } else if fn_name == KEYWORD_FN_PTR_CURRY && obj.is::() { // Curry call @@ -811,7 +813,8 @@ impl Engine { let args = arg_values.as_mut(); self.exec_fn_call( - mods, state, lib, fn_name, hash, args, is_ref, true, pub_only, None, def_val, level, + mods, state, lib, fn_name, hash, args, is_ref, true, pub_only, pos, None, def_val, + level, ) }?; @@ -824,7 +827,6 @@ impl Engine { } /// Call a function in normal function-call style. - /// [`Position`] in [`EvalAltResult`] is [`None`][Position::None] and must be set afterwards. pub(crate) fn make_function_call( &self, scope: &mut Scope, @@ -838,6 +840,7 @@ impl Engine { mut hash_script: u64, native: bool, pub_only: bool, + pos: Position, capture_scope: bool, level: usize, ) -> Result> { @@ -956,9 +959,8 @@ impl Engine { let script = script.as_str().map_err(|typ| { self.make_type_mismatch_err::(typ, args_expr[0].position()) })?; - let result = self - .eval_script_expr(scope, mods, state, lib, script, level + 1) - .map_err(|err| err.fill_position(args_expr[0].position())); + let pos = args_expr[0].position(); + let result = self.eval_script_expr(scope, mods, state, lib, script, pos, level + 1); // IMPORTANT! If the eval defines new variables in the current scope, // all variable offsets from this point on will be mis-aligned. @@ -1029,13 +1031,13 @@ impl Engine { let args = args.as_mut(); self.exec_fn_call( - mods, state, lib, name, hash, args, is_ref, false, pub_only, capture, def_val, level, + mods, state, lib, name, hash, args, is_ref, false, pub_only, pos, capture, def_val, + level, ) .map(|(v, _)| v) } /// Call a namespace-qualified function in normal function-call style. - /// [`Position`] in [`EvalAltResult`] is [`None`][Position::None] and must be set afterwards. pub(crate) fn make_qualified_function_call( &self, scope: &mut Scope, @@ -1048,6 +1050,7 @@ impl Engine { args_expr: impl AsRef<[Expr]>, def_val: Option<&Dynamic>, hash_script: u64, + pos: Position, level: usize, ) -> Result> { let args_expr = args_expr.as_ref(); @@ -1143,7 +1146,9 @@ impl Engine { let args = args.as_mut(); let new_scope = &mut Default::default(); let fn_def = f.get_fn_def().clone(); - self.call_script_fn(new_scope, mods, state, lib, &mut None, &fn_def, args, level) + self.call_script_fn( + new_scope, mods, state, lib, &mut None, &fn_def, args, pos, level, + ) } Some(f) if f.is_plugin_fn() => f .get_plugin_fn() @@ -1177,7 +1182,7 @@ impl Engine { .collect::>() .join(", ") ), - Position::NONE, + pos, ) .into(), } diff --git a/src/fn_native.rs b/src/fn_native.rs index 0706983d..df74e163 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -164,6 +164,7 @@ impl<'e, 'a, 'm, 'pm> NativeCallContext<'e, 'a, 'm, 'pm> { is_method, is_method, public_only, + Position::NONE, None, def_value, 0, diff --git a/src/optimize.rs b/src/optimize.rs index f04dda40..4727dff4 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -142,6 +142,7 @@ fn call_fn_with_constant_arguments( arg_values.iter_mut().collect::>().as_mut(), false, true, + Position::NONE, None, ) .ok() From 40b6a014ae1de0e11744a72d0fe8a30ca0f216fa Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 12 Dec 2020 11:47:18 +0800 Subject: [PATCH 11/15] Provide Position to debug. --- RELEASES.md | 2 ++ doc/src/language/print-debug.md | 12 ++++++++---- src/engine.rs | 19 ++++++++++++++----- src/engine_api.rs | 13 +++++++++---- src/fn_call.rs | 17 ++++++++++------- src/fn_native.rs | 9 ++++++++- tests/print.rs | 12 ++++++++---- 7 files changed, 59 insertions(+), 25 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 62fc4be6..7988a792 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -14,11 +14,13 @@ Breaking changes ---------------- * `Engine::on_progress` now takes `u64` instead of `&u64`. +* The closure for `Engine::on_debug` now takes an additional `Position` parameter. Enhancements ------------ * Capturing a constant variable in a closure is now supported, with no cloning. +* Provides position info for `debug` statements. Version 0.19.7 diff --git a/doc/src/language/print-debug.md b/doc/src/language/print-debug.md index 515b098f..606452d6 100644 --- a/doc/src/language/print-debug.md +++ b/doc/src/language/print-debug.md @@ -22,10 +22,12 @@ When embedding Rhai into an application, it is usually necessary to trap `print` (for logging into a tracking log, for example) with the `Engine::on_print` and `Engine::on_debug` methods: ```rust -// Any function or closure that takes an '&str' argument can be used to override -// 'print' and 'debug' +// Any function or closure that takes an '&str' argument can be used to override 'print'. engine.on_print(|x| println!("hello: {}", x)); -engine.on_debug(|x| println!("DEBUG: {}", x)); + +// Any function or closure that takes a '&str' and a 'Position' argument can be used to +// override 'debug'. +engine.on_debug(|x, pos| println!("DEBUG at {:?}: {}", pos, x)); // Example: quick-'n-dirty logging let logbook = Arc::new(RwLock::new(Vec::::new())); @@ -35,7 +37,9 @@ let log = logbook.clone(); engine.on_print(move |s| log.write().unwrap().push(format!("entry: {}", s))); let log = logbook.clone(); -engine.on_debug(move |s| log.write().unwrap().push(format!("DEBUG: {}", s))); +engine.on_debug(move |s, pos| log.write().unwrap().push( + format!("DEBUG at {:?}: {}", pos, s) + )); // Evaluate script engine.eval::<()>(script)?; diff --git a/src/engine.rs b/src/engine.rs index 38a62918..958cf1d9 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -4,7 +4,8 @@ use crate::ast::{Expr, FnCallExpr, Ident, IdentX, ReturnType, Stmt}; use crate::dynamic::{map_std_type_name, AccessMode, Union, Variant}; use crate::fn_call::run_builtin_op_assignment; use crate::fn_native::{ - CallableFunction, IteratorFn, OnPrintCallback, OnProgressCallback, OnVarCallback, + CallableFunction, IteratorFn, OnDebugCallback, OnPrintCallback, OnProgressCallback, + OnVarCallback, }; use crate::module::NamespaceRef; use crate::optimize::OptimizationLevel; @@ -626,7 +627,7 @@ pub struct Engine { /// Callback closure for implementing the `print` command. pub(crate) print: OnPrintCallback, /// Callback closure for implementing the `debug` command. - pub(crate) debug: OnPrintCallback, + pub(crate) debug: OnDebugCallback, /// Callback closure for progress reporting. pub(crate) progress: Option, @@ -677,7 +678,7 @@ pub fn is_anonymous_fn(fn_name: &str) -> bool { fn_name.starts_with(FN_ANONYMOUS) } -/// Print/debug to stdout +/// Print to stdout #[inline(always)] fn default_print(_s: &str) { #[cfg(not(feature = "no_std"))] @@ -685,6 +686,14 @@ fn default_print(_s: &str) { println!("{}", _s); } +/// Debug to stdout +#[inline(always)] +fn default_debug(_s: &str, _pos: Position) { + #[cfg(not(feature = "no_std"))] + #[cfg(not(target_arch = "wasm32"))] + println!("{:?} | {}", _pos, _s); +} + /// Search for a module within an imports stack. /// [`Position`] in [`EvalAltResult`] is [`None`][Position::None] and must be set afterwards. pub fn search_imports( @@ -741,7 +750,7 @@ impl Engine { // default print/debug implementations print: Box::new(default_print), - debug: Box::new(default_print), + debug: Box::new(default_debug), // progress callback progress: None, @@ -797,7 +806,7 @@ impl Engine { resolve_var: None, print: Box::new(|_| {}), - debug: Box::new(|_| {}), + debug: Box::new(|_, _| {}), progress: None, optimization_level: if cfg!(feature = "no_optimize") { diff --git a/src/engine_api.rs b/src/engine_api.rs index 5cdaf1e2..118c4dcc 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1808,16 +1808,21 @@ impl Engine { /// /// // Override action of 'print' function /// let logger = result.clone(); - /// engine.on_debug(move |s| logger.write().unwrap().push_str(s)); + /// engine.on_debug(move |s, pos| logger.write().unwrap().push_str( + /// &format!("{:?} > {}", pos, s) + /// )); /// - /// engine.consume(r#"debug("hello");"#)?; + /// engine.consume(r#"let x = "hello"; debug(x);"#)?; /// - /// assert_eq!(*result.read().unwrap(), r#""hello""#); + /// assert_eq!(*result.read().unwrap(), r#"1:18 > "hello""#); /// # Ok(()) /// # } /// ``` #[inline(always)] - pub fn on_debug(&mut self, callback: impl Fn(&str) + SendSync + 'static) -> &mut Self { + pub fn on_debug( + &mut self, + callback: impl Fn(&str, Position) + SendSync + 'static, + ) -> &mut Self { self.debug = Box::new(callback); self } diff --git a/src/fn_call.rs b/src/fn_call.rs index 9c498f29..85330a81 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -211,13 +211,16 @@ impl Engine { false, ), KEYWORD_DEBUG => ( - (self.debug)(result.as_str().map_err(|typ| { - EvalAltResult::ErrorMismatchOutputType( - self.map_type_name(type_name::()).into(), - typ.into(), - pos, - ) - })?) + (self.debug)( + result.as_str().map_err(|typ| { + EvalAltResult::ErrorMismatchOutputType( + self.map_type_name(type_name::()).into(), + typ.into(), + pos, + ) + })?, + pos, + ) .into(), false, ), diff --git a/src/fn_native.rs b/src/fn_native.rs index df74e163..c9c82300 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -356,7 +356,14 @@ pub type OnProgressCallback = Box Option + Send + Sync + pub type OnPrintCallback = Box; /// A standard callback function for printing. #[cfg(feature = "sync")] -pub type OnPrintCallback = Box; +pub type OnPrintCallback = Box; + +/// A standard callback function for debugging. +#[cfg(not(feature = "sync"))] +pub type OnDebugCallback = Box; +/// A standard callback function for debugging. +#[cfg(feature = "sync")] +pub type OnDebugCallback = Box; /// A standard callback function for variable access. #[cfg(not(feature = "sync"))] diff --git a/tests/print.rs b/tests/print.rs index 3d4c34a7..57471323 100644 --- a/tests/print.rs +++ b/tests/print.rs @@ -2,7 +2,7 @@ use rhai::{Engine, EvalAltResult, RegisterFn, INT}; use std::sync::{Arc, RwLock}; #[test] -fn test_print() -> Result<(), Box> { +fn test_print_debug() -> Result<(), Box> { let logbook = Arc::new(RwLock::new(Vec::::new())); // Redirect print/debug output to 'log' @@ -13,16 +13,20 @@ fn test_print() -> Result<(), Box> { engine .on_print(move |s| log1.write().unwrap().push(format!("entry: {}", s))) - .on_debug(move |s| log2.write().unwrap().push(format!("DEBUG: {}", s))); + .on_debug(move |s, pos| { + log2.write() + .unwrap() + .push(format!("DEBUG at {:?}: {}", pos, s)) + }); // Evaluate script engine.eval::<()>("print(40 + 2)")?; - engine.eval::<()>(r#"debug("hello!")"#)?; + engine.eval::<()>(r#"let x = "hello!"; debug(x)"#)?; // 'logbook' captures all the 'print' and 'debug' output assert_eq!(logbook.read().unwrap().len(), 2); assert_eq!(logbook.read().unwrap()[0], "entry: 42"); - assert_eq!(logbook.read().unwrap()[1], r#"DEBUG: "hello!""#); + assert_eq!(logbook.read().unwrap()[1], r#"DEBUG at 1:19: "hello!""#); for entry in logbook.read().unwrap().iter() { println!("{}", entry); From dbdb8f43b750f1f6d34f064f92b4344d3cae4053 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 12 Dec 2020 15:57:55 +0800 Subject: [PATCH 12/15] Change AST to struct. --- src/ast.rs | 76 ++++++++++++++++++++++++++++------------------ tests/optimizer.rs | 6 ++-- 2 files changed, 50 insertions(+), 32 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index ecac8042..2867b48f 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -111,30 +111,39 @@ impl fmt::Display for ScriptFnDef { /// /// Currently, [`AST`] is neither `Send` nor `Sync`. Turn on the `sync` feature to make it `Send + Sync`. #[derive(Debug, Clone)] -pub struct AST( +pub struct AST { /// Global statements. - Vec, + statements: Vec, /// Script-defined functions. - Shared, -); + functions: Shared, +} impl Default for AST { fn default() -> Self { - Self(Vec::with_capacity(16), Default::default()) + Self { + statements: Vec::with_capacity(16), + functions: Default::default(), + } } } impl AST { /// Create a new [`AST`]. #[inline(always)] - pub fn new(statements: impl IntoIterator, lib: impl Into>) -> Self { - Self(statements.into_iter().collect(), lib.into()) + pub fn new( + statements: impl IntoIterator, + functions: impl Into>, + ) -> Self { + Self { + statements: statements.into_iter().collect(), + functions: functions.into(), + } } /// Get the statements. #[cfg(not(feature = "internals"))] #[inline(always)] pub(crate) fn statements(&self) -> &[Stmt] { - &self.0 + &self.statements } /// _(INTERNALS)_ Get the statements. /// Exported under the `internals` feature only. @@ -142,26 +151,26 @@ impl AST { #[deprecated(note = "this method is volatile and may change")] #[inline(always)] pub fn statements(&self) -> &[Stmt] { - &self.0 + &self.statements } /// Get a mutable reference to the statements. #[cfg(not(feature = "no_optimize"))] #[inline(always)] pub(crate) fn statements_mut(&mut self) -> &mut Vec { - &mut self.0 + &mut self.statements } /// Get the internal shared [`Module`] containing all script-defined functions. #[cfg(not(feature = "no_module"))] #[cfg(not(feature = "no_function"))] #[inline(always)] pub(crate) fn shared_lib(&self) -> Shared { - self.1.clone() + self.functions.clone() } /// Get the internal [`Module`] containing all script-defined functions. #[cfg(not(feature = "internals"))] #[inline(always)] pub(crate) fn lib(&self) -> &Module { - &self.1 + &self.functions } /// _(INTERNALS)_ Get the internal [`Module`] containing all script-defined functions. /// Exported under the `internals` feature only. @@ -169,7 +178,7 @@ impl AST { #[deprecated(note = "this method is volatile and may change")] #[inline(always)] pub fn lib(&self) -> &Module { - &self.1 + &self.functions } /// Clone the [`AST`]'s functions into a new [`AST`]. /// No statements are cloned. @@ -191,14 +200,20 @@ impl AST { mut filter: impl FnMut(FnNamespace, FnAccess, bool, &str, usize) -> bool, ) -> Self { let mut functions: Module = Default::default(); - functions.merge_filtered(&self.1, &mut filter); - Self(Default::default(), functions.into()) + functions.merge_filtered(&self.functions, &mut filter); + Self { + statements: Default::default(), + functions: functions.into(), + } } /// Clone the [`AST`]'s script statements into a new [`AST`]. /// No functions are cloned. #[inline(always)] pub fn clone_statements_only(&self) -> Self { - Self(self.0.clone(), Default::default()) + Self { + statements: self.statements.clone(), + functions: Default::default(), + } } /// Merge two [`AST`] into one. Both [`AST`]'s are untouched and a new, merged, version /// is returned. @@ -363,21 +378,24 @@ impl AST { other: &Self, mut filter: impl FnMut(FnNamespace, FnAccess, bool, &str, usize) -> bool, ) -> Self { - let Self(statements, functions) = self; + let Self { + statements, + functions, + } = self; - let ast = match (statements.is_empty(), other.0.is_empty()) { + let ast = match (statements.is_empty(), other.statements.is_empty()) { (false, false) => { let mut statements = statements.clone(); - statements.extend(other.0.iter().cloned()); + statements.extend(other.statements.iter().cloned()); statements } (false, true) => statements.clone(), - (true, false) => other.0.clone(), + (true, false) => other.statements.clone(), (true, true) => vec![], }; let mut functions = functions.as_ref().clone(); - functions.merge_filtered(&other.1, &mut filter); + functions.merge_filtered(&other.functions, &mut filter); Self::new(ast, functions) } @@ -438,9 +456,9 @@ impl AST { other: Self, mut filter: impl FnMut(FnNamespace, FnAccess, bool, &str, usize) -> bool, ) -> &mut Self { - self.0.extend(other.0.into_iter()); - if !other.1.is_empty() { - shared_make_mut(&mut self.1).merge_filtered(&other.1, &mut filter); + self.statements.extend(other.statements.into_iter()); + if !other.functions.is_empty() { + shared_make_mut(&mut self.functions).merge_filtered(&other.functions, &mut filter); } self } @@ -473,8 +491,8 @@ impl AST { &mut self, filter: impl FnMut(FnNamespace, FnAccess, &str, usize) -> bool, ) -> &mut Self { - if !self.1.is_empty() { - shared_make_mut(&mut self.1).retain_script_functions(filter); + if !self.functions.is_empty() { + shared_make_mut(&mut self.functions).retain_script_functions(filter); } self } @@ -484,18 +502,18 @@ impl AST { pub fn iter_functions<'a>( &'a self, ) -> impl Iterator + 'a { - self.1.iter_script_fn() + self.functions.iter_script_fn() } /// Clear all function definitions in the [`AST`]. #[cfg(not(feature = "no_function"))] #[inline(always)] pub fn clear_functions(&mut self) { - self.1 = Default::default(); + self.functions = Default::default(); } /// Clear all statements in the [`AST`], leaving only function definitions. #[inline(always)] pub fn clear_statements(&mut self) { - self.0 = vec![]; + self.statements = vec![]; } } diff --git a/tests/optimizer.rs b/tests/optimizer.rs index fa817bf1..bffc2633 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -55,17 +55,17 @@ fn test_optimizer_parse() -> Result<(), Box> { let ast = engine.compile("{ const DECISION = false; if DECISION { 42 } else { 123 } }")?; - assert!(format!("{:?}", ast).starts_with(r#"AST([Block([Const(IdentX { name: "DECISION", pos: 1:9 }, Some(Unit(0:0)), false, 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)]"#)); + assert!(format!("{:?}", ast).starts_with(r#"AST { statements: [Block([Const(IdentX { name: "DECISION", pos: 1:9 }, Some(Unit(0:0)), false, 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)]"#)); let ast = engine.compile("if 1 == 2 { 42 }")?; - assert!(format!("{:?}", ast).starts_with("AST([], Module(")); + assert!(format!("{:?}", ast).starts_with("AST { statements: [], functions: Module(")); engine.set_optimization_level(OptimizationLevel::Full); let ast = engine.compile("abs(-42)")?; - assert!(format!("{:?}", ast).starts_with(r"AST([Expr(IntegerConstant(42, 1:1))]")); + assert!(format!("{:?}", ast).starts_with(r"AST { statements: [Expr(IntegerConstant(42, 1:1))]")); Ok(()) } From 1087c338bda1f9440b22ecacda3823cdac963f2d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 12 Dec 2020 16:31:13 +0800 Subject: [PATCH 13/15] Change output of AST::iter_functions. --- RELEASES.md | 1 + examples/repl.rs | 13 ++++++++++++- src/ast.rs | 16 +++++++++++++--- src/fn_call.rs | 2 +- 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 7988a792..7a5b65db 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -15,6 +15,7 @@ Breaking changes * `Engine::on_progress` now takes `u64` instead of `&u64`. * The closure for `Engine::on_debug` now takes an additional `Position` parameter. +* `AST::iter_functions` returns a slice of parameter names instead of the internal `ScriptFnDef`. Enhancements ------------ diff --git a/examples/repl.rs b/examples/repl.rs index fe7c4bc5..a2e48709 100644 --- a/examples/repl.rs +++ b/examples/repl.rs @@ -144,7 +144,18 @@ fn main() { #[cfg(not(feature = "no_function"))] main_ast .iter_functions() - .for_each(|(_, _, _, _, f)| println!("{}", f)); + .for_each(|(_, access, name, _, params)| { + println!( + "{}{}({}) -> Dynamic", + if access.is_private() { "private " } else { "" }, + name, + params + .iter() + .map(|s| s.as_str()) + .collect::>() + .join(", ") + ) + }); println!(); continue; diff --git a/src/ast.rs b/src/ast.rs index 2867b48f..be07e0f7 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -82,7 +82,7 @@ pub struct ScriptFnDef { pub params: StaticVec, /// Access to external variables. #[cfg(not(feature = "no_closure"))] - pub externals: crate::stdlib::collections::HashSet, + pub externals: Vec, } impl fmt::Display for ScriptFnDef { @@ -501,8 +501,18 @@ impl AST { #[inline(always)] pub fn iter_functions<'a>( &'a self, - ) -> impl Iterator + 'a { - self.functions.iter_script_fn() + ) -> impl Iterator + 'a { + self.functions + .iter_script_fn() + .map(|(namespace, access, name, num_params, fn_def)| { + ( + namespace, + access, + name, + num_params, + fn_def.params.as_slice(), + ) + }) } /// Clear all function definitions in the [`AST`]. #[cfg(not(feature = "no_function"))] diff --git a/src/fn_call.rs b/src/fn_call.rs index 85330a81..82673b20 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -539,7 +539,7 @@ impl Engine { if !func.externals.is_empty() { captured .into_iter() - .filter(|(name, _, _)| func.externals.contains(name.as_ref())) + .filter(|(name, _, _)| func.externals.iter().any(|ex| ex == name)) .for_each(|(name, value, _)| { // Consume the scope values. scope.push_dynamic(name, value); From 26449a9f1c2fd0cebf19375b3c30960b662bf6ee Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 12 Dec 2020 18:44:28 +0800 Subject: [PATCH 14/15] Add ScriptFnMetadata. --- RELEASES.md | 2 +- examples/repl.rs | 15 +------------ src/ast.rs | 56 +++++++++++++++++++++++++++++++++++++----------- src/lib.rs | 2 +- src/optimize.rs | 1 + src/parser.rs | 10 ++++++++- 6 files changed, 57 insertions(+), 29 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 7a5b65db..8d2baddb 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -15,7 +15,7 @@ Breaking changes * `Engine::on_progress` now takes `u64` instead of `&u64`. * The closure for `Engine::on_debug` now takes an additional `Position` parameter. -* `AST::iter_functions` returns a slice of parameter names instead of the internal `ScriptFnDef`. +* `AST::iter_functions` now returns `ScriptFnMetadata`. Enhancements ------------ diff --git a/examples/repl.rs b/examples/repl.rs index a2e48709..7d42cfac 100644 --- a/examples/repl.rs +++ b/examples/repl.rs @@ -142,20 +142,7 @@ fn main() { .for_each(|f| println!("{}", f)); #[cfg(not(feature = "no_function"))] - main_ast - .iter_functions() - .for_each(|(_, access, name, _, params)| { - println!( - "{}{}({}) -> Dynamic", - if access.is_private() { "private " } else { "" }, - name, - params - .iter() - .map(|s| s.as_str()) - .collect::>() - .join(", ") - ) - }); + main_ast.iter_functions().for_each(|f| println!("{}", f)); println!(); continue; diff --git a/src/ast.rs b/src/ast.rs index be07e0f7..0ecf5e76 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -83,6 +83,8 @@ pub struct ScriptFnDef { /// Access to external variables. #[cfg(not(feature = "no_closure"))] pub externals: Vec, + /// Comment block for function. + pub fn_comments: Vec, } impl fmt::Display for ScriptFnDef { @@ -105,6 +107,46 @@ impl fmt::Display for ScriptFnDef { } } +/// A type containing a script-defined function's metadata. +#[derive(Debug, Clone, Hash)] +pub struct ScriptFnMetadata { + pub comments: Vec, + pub access: FnAccess, + pub fn_name: ImmutableString, + pub params: Vec, +} + +impl fmt::Display for ScriptFnMetadata { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{}{}({}) -> Dynamic", + if self.access.is_private() { + "private " + } else { + "" + }, + self.fn_name, + self.params + .iter() + .map(|p| p.as_str()) + .collect::>() + .join(", ") + ) + } +} + +impl Into for &ScriptFnDef { + fn into(self) -> ScriptFnMetadata { + ScriptFnMetadata { + comments: self.fn_comments.clone(), + access: self.access, + fn_name: self.name.clone(), + params: self.params.iter().cloned().collect(), + } + } +} + /// Compiled AST (abstract syntax tree) of a Rhai script. /// /// # Thread Safety @@ -499,20 +541,10 @@ impl AST { /// Iterate through all functions #[cfg(not(feature = "no_function"))] #[inline(always)] - pub fn iter_functions<'a>( - &'a self, - ) -> impl Iterator + 'a { + pub fn iter_functions<'a>(&'a self) -> impl Iterator + 'a { self.functions .iter_script_fn() - .map(|(namespace, access, name, num_params, fn_def)| { - ( - namespace, - access, - name, - num_params, - fn_def.params.as_slice(), - ) - }) + .map(|(_, _, _, _, fn_def)| fn_def.into()) } /// Clear all function definitions in the [`AST`]. #[cfg(not(feature = "no_function"))] diff --git a/src/lib.rs b/src/lib.rs index 4e492c30..f19553d9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -115,7 +115,7 @@ pub type FLOAT = f64; #[cfg(feature = "f32_float")] pub type FLOAT = f32; -pub use ast::{FnAccess, AST}; +pub use ast::{FnAccess, ScriptFnMetadata, AST}; pub use dynamic::Dynamic; pub use engine::{Engine, EvalContext}; pub use fn_native::{FnPtr, NativeCallContext, Shared}; diff --git a/src/optimize.rs b/src/optimize.rs index 4727dff4..2ef94938 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -868,6 +868,7 @@ pub fn optimize_into_ast( lib: None, #[cfg(not(feature = "no_module"))] mods: Default::default(), + fn_comments: Default::default(), }) .for_each(|fn_def| { lib2.set_script_fn(fn_def); diff --git a/src/parser.rs b/src/parser.rs index 479c2778..af501e00 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -54,6 +54,8 @@ struct ParseState<'e> { /// Tracks a list of external variables (variables that are not explicitly declared in the scope). #[cfg(not(feature = "no_closure"))] externals: HashMap, + /// Latest global comments block. + comments: Vec, /// An indicator that disables variable capturing into externals one single time /// up until the nearest consumed Identifier token. /// If set to false the next call to `access_var` will not capture the variable. @@ -98,6 +100,7 @@ impl<'e> ParseState<'e> { strings: HashMap::with_capacity(64), stack: Vec::with_capacity(16), entry_stack_len: 0, + comments: Default::default(), #[cfg(not(feature = "no_module"))] modules: Default::default(), } @@ -2449,7 +2452,9 @@ fn parse_stmt( pos: pos, }; - let func = parse_fn(input, &mut new_state, lib, access, settings)?; + let fn_comments = state.comments.clone(); + + let func = parse_fn(input, &mut new_state, lib, access, settings, fn_comments)?; // Qualifiers (none) + function name + number of arguments. let hash = calc_script_fn_hash(empty(), &func.name, func.params.len()); @@ -2608,6 +2613,7 @@ fn parse_fn( lib: &mut FunctionsLib, access: FnAccess, mut settings: ParseSettings, + fn_comments: Vec, ) -> Result { #[cfg(not(feature = "unchecked"))] settings.ensure_level_within_max_limit(state.max_expr_depth)?; @@ -2693,6 +2699,7 @@ fn parse_fn( lib: None, #[cfg(not(feature = "no_module"))] mods: Default::default(), + fn_comments, }) } @@ -2849,6 +2856,7 @@ fn parse_anon_fn( lib: None, #[cfg(not(feature = "no_module"))] mods: Default::default(), + fn_comments: Default::default(), }; let expr = Expr::FnPointer(fn_name, settings.pos); From 87174de051225dac2c00e3161c6640d173c9d2d1 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 12 Dec 2020 20:09:29 +0800 Subject: [PATCH 15/15] Add doc-comment to function metadata. --- src/parse_error.rs | 7 ++++++ src/parser.rs | 38 +++++++++++++++++++++++++++----- src/token.rs | 55 +++++++++++++++++++++++++++------------------- 3 files changed, 72 insertions(+), 28 deletions(-) diff --git a/src/parse_error.rs b/src/parse_error.rs index 966a3649..7419fb98 100644 --- a/src/parse_error.rs +++ b/src/parse_error.rs @@ -121,6 +121,10 @@ pub enum ParseErrorType { Reserved(String), /// Missing an expression. Wrapped value is the expression type. ExprExpected(String), + /// Defining a doc-comment in an appropriate place (e.g. not at global level). + /// + /// Never appears under the `no_function` feature. + WrongDocComment, /// Defining a function `fn` in an appropriate place (e.g. inside another function). /// /// Never appears under the `no_function` feature. @@ -189,6 +193,7 @@ impl ParseErrorType { Self::FnMissingParams(_) => "Expecting parameters in function declaration", Self::FnDuplicatedParam(_,_) => "Duplicated parameters in function declaration", Self::FnMissingBody(_) => "Expecting body statement block for function declaration", + Self::WrongDocComment => "Doc-comment must be followed immediately by a function definition", Self::WrongFnDefinition => "Function definitions must be at global level and cannot be inside a block or another function", Self::WrongExport => "Export statement can only appear at global level", Self::AssignmentToConstant(_) => "Cannot assign to a constant value", @@ -243,7 +248,9 @@ impl fmt::Display for ParseErrorType { Self::LiteralTooLarge(typ, max) => { write!(f, "{} exceeds the maximum limit ({})", typ, max) } + Self::Reserved(s) => write!(f, "'{}' is a reserved keyword", s), + _ => f.write_str(self.desc()), } } diff --git a/src/parser.rs b/src/parser.rs index af501e00..d05e1042 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -21,7 +21,7 @@ use crate::stdlib::{ vec::Vec, }; use crate::syntax::CustomSyntax; -use crate::token::{is_keyword_function, is_valid_identifier, Token, TokenStream}; +use crate::token::{is_doc_comment, is_keyword_function, is_valid_identifier, Token, TokenStream}; use crate::utils::{get_hasher, StraightHasherBuilder}; use crate::{ calc_script_fn_hash, Dynamic, Engine, ImmutableString, LexError, ParseError, ParseErrorType, @@ -54,8 +54,6 @@ struct ParseState<'e> { /// Tracks a list of external variables (variables that are not explicitly declared in the scope). #[cfg(not(feature = "no_closure"))] externals: HashMap, - /// Latest global comments block. - comments: Vec, /// An indicator that disables variable capturing into externals one single time /// up until the nearest consumed Identifier token. /// If set to false the next call to `access_var` will not capture the variable. @@ -100,7 +98,6 @@ impl<'e> ParseState<'e> { strings: HashMap::with_capacity(64), stack: Vec::with_capacity(16), entry_stack_len: 0, - comments: Default::default(), #[cfg(not(feature = "no_module"))] modules: Default::default(), } @@ -2400,6 +2397,37 @@ fn parse_stmt( ) -> Result, ParseError> { use AccessMode::{ReadOnly, ReadWrite}; + let mut fn_comments: Vec = Default::default(); + let mut fn_comments_pos = Position::NONE; + + // Handle doc-comment. + #[cfg(not(feature = "no_function"))] + while let (Token::Comment(ref comment), comment_pos) = input.peek().unwrap() { + if fn_comments_pos.is_none() { + fn_comments_pos = *comment_pos; + } + + if !is_doc_comment(comment) { + unreachable!(); + } + + if !settings.is_global { + return Err(PERR::WrongDocComment.into_err(fn_comments_pos)); + } + + if let Token::Comment(comment) = input.next().unwrap().0 { + fn_comments.push(comment); + + match input.peek().unwrap() { + (Token::Fn, _) | (Token::Private, _) => break, + (Token::Comment(_), _) => (), + _ => return Err(PERR::WrongDocComment.into_err(fn_comments_pos)), + } + } else { + unreachable!(); + } + } + let (token, token_pos) = match input.peek().unwrap() { (Token::EOF, pos) => return Ok(Some(Stmt::Noop(*pos))), x => x, @@ -2452,8 +2480,6 @@ fn parse_stmt( pos: pos, }; - let fn_comments = state.comments.clone(); - let func = parse_fn(input, &mut new_state, lib, access, settings, fn_comments)?; // Qualifiers (none) + function name + number of arguments. diff --git a/src/token.rs b/src/token.rs index 2433a35c..f1c00d14 100644 --- a/src/token.rs +++ b/src/token.rs @@ -355,6 +355,9 @@ impl Token { Custom(s) => s.clone().into(), LexError(err) => err.to_string().into(), + Comment(s) if is_doc_comment(s) => s[..3].to_string().into(), + Comment(s) => s[..2].to_string().into(), + token => match token { LeftBrace => "{", RightBrace => "}", @@ -917,36 +920,36 @@ fn eat_next(stream: &mut impl InputStream, pos: &mut Position) -> Option { /// Scan for a block comment until the end. fn scan_comment( stream: &mut impl InputStream, - state: &mut TokenizeState, + mut level: usize, pos: &mut Position, comment: &mut String, -) { +) -> usize { while let Some(c) = stream.get_next() { pos.advance(); - if state.include_comments { + if !comment.is_empty() { comment.push(c); } match c { '/' => { if let Some(c2) = stream.get_next() { - if state.include_comments { + if !comment.is_empty() { comment.push(c2); } if c2 == '*' { - state.comment_level += 1; + level += 1; } } pos.advance(); } '*' => { if let Some(c2) = stream.get_next() { - if state.include_comments { + if !comment.is_empty() { comment.push(c2); } if c2 == '/' { - state.comment_level -= 1; + level -= 1; } } pos.advance(); @@ -955,10 +958,12 @@ fn scan_comment( _ => (), } - if state.comment_level == 0 { + if level == 0 { break; } } + + level } /// _(INTERNALS)_ Get the next token from the `stream`. @@ -1012,6 +1017,12 @@ fn is_binary_char(c: char) -> bool { } } +/// Test if the comment block is a doc-comment. +#[inline(always)] +pub fn is_doc_comment(comment: &str) -> bool { + comment.starts_with("///") || comment.starts_with("/**") +} + /// Get the next token. fn get_next_token_inner( stream: &mut impl InputStream, @@ -1022,9 +1033,9 @@ fn get_next_token_inner( if state.comment_level > 0 { let start_pos = *pos; let mut comment = String::new(); - scan_comment(stream, state, pos, &mut comment); + state.comment_level = scan_comment(stream, state.comment_level, pos, &mut comment); - if state.include_comments { + if state.include_comments || is_doc_comment(&comment) { return Some((Token::Comment(comment), start_pos)); } } @@ -1271,10 +1282,10 @@ fn get_next_token_inner( ('/', '/') => { eat_next(stream, pos); - let mut comment = if state.include_comments { - "//".to_string() - } else { - String::new() + let mut comment = match stream.peek_next().unwrap() { + '/' => "///".to_string(), + _ if state.include_comments => "//".to_string(), + _ => String::new(), }; while let Some(c) = stream.get_next() { @@ -1283,13 +1294,13 @@ fn get_next_token_inner( break; } - if state.include_comments { + if !comment.is_empty() { comment.push(c); } pos.advance(); } - if state.include_comments { + if state.include_comments || is_doc_comment(&comment) { return Some((Token::Comment(comment), start_pos)); } } @@ -1298,14 +1309,14 @@ fn get_next_token_inner( eat_next(stream, pos); - let mut comment = if state.include_comments { - "/*".to_string() - } else { - String::new() + let mut comment = match stream.peek_next().unwrap() { + '*' => "/**".to_string(), + _ if state.include_comments => "/*".to_string(), + _ => String::new(), }; - scan_comment(stream, state, pos, &mut comment); + state.comment_level = scan_comment(stream, state.comment_level, pos, &mut comment); - if state.include_comments { + if state.include_comments || is_doc_comment(&comment) { return Some((Token::Comment(comment), start_pos)); } }