From e35122ae5db2965d4addd6dac3ce257727acf3cb Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 4 Jun 2021 14:23:30 +0800 Subject: [PATCH 01/24] Disallow registering indexers for integers. --- src/engine_api.rs | 43 ++++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/src/engine_api.rs b/src/engine_api.rs index b9d41ee1..d2a118ac 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -538,8 +538,9 @@ impl Engine { /// /// # Panics /// - /// Panics if the type is [`Array`], [`Map`], [`String`], [`ImmutableString`][crate::ImmutableString] or `&str`. - /// Indexers for arrays, object maps and strings cannot be registered. + /// Panics if the type is [`Array`], [`Map`], [`String`], + /// [`ImmutableString`][crate::ImmutableString], `&str` or [`INT`][crate::INT]. + /// Indexers for arrays, object maps, strings and integers cannot be registered. /// /// # Example /// @@ -595,6 +596,9 @@ impl Engine { { panic!("Cannot register indexer for strings."); } + if TypeId::of::() == TypeId::of::() { + panic!("Cannot register indexer for integers."); + } self.register_fn(crate::engine::FN_IDX_GET, get_fn) } @@ -606,8 +610,9 @@ impl Engine { /// /// # Panics /// - /// Panics if the type is [`Array`], [`Map`], [`String`], [`ImmutableString`][crate::ImmutableString] or `&str`. - /// Indexers for arrays, object maps and strings cannot be registered. + /// Panics if the type is [`Array`], [`Map`], [`String`], + /// [`ImmutableString`][crate::ImmutableString], `&str` or [`INT`][crate::INT]. + /// Indexers for arrays, object maps, strings and integers cannot be registered. /// /// # Example /// @@ -669,6 +674,9 @@ impl Engine { { panic!("Cannot register indexer for strings."); } + if TypeId::of::() == TypeId::of::() { + panic!("Cannot register indexer for integers."); + } self.register_result_fn(crate::engine::FN_IDX_GET, get_fn) } @@ -678,8 +686,9 @@ impl Engine { /// /// # Panics /// - /// Panics if the type is [`Array`], [`Map`], [`String`], [`ImmutableString`][crate::ImmutableString] or `&str`. - /// Indexers for arrays, object maps and strings cannot be registered. + /// Panics if the type is [`Array`], [`Map`], [`String`], + /// [`ImmutableString`][crate::ImmutableString], `&str` or [`INT`][crate::INT]. + /// Indexers for arrays, object maps, strings and integers cannot be registered. /// /// # Example /// @@ -737,6 +746,9 @@ impl Engine { { panic!("Cannot register indexer for strings."); } + if TypeId::of::() == TypeId::of::() { + panic!("Cannot register indexer for integers."); + } self.register_fn(crate::engine::FN_IDX_SET, set_fn) } @@ -746,8 +758,9 @@ impl Engine { /// /// # Panics /// - /// Panics if the type is [`Array`], [`Map`], [`String`], [`ImmutableString`][crate::ImmutableString] or `&str`. - /// Indexers for arrays, object maps and strings cannot be registered. + /// Panics if the type is [`Array`], [`Map`], [`String`], + /// [`ImmutableString`][crate::ImmutableString], `&str` or [`INT`][crate::INT]. + /// Indexers for arrays, object maps, strings and integers cannot be registered. /// /// # Example /// @@ -812,6 +825,9 @@ impl Engine { { panic!("Cannot register indexer for strings."); } + if TypeId::of::() == TypeId::of::() { + panic!("Cannot register indexer for integers."); + } self.register_result_fn(crate::engine::FN_IDX_SET, set_fn) } @@ -821,8 +837,9 @@ impl Engine { /// /// # Panics /// - /// Panics if the type is [`Array`], [`Map`], [`String`], [`ImmutableString`][crate::ImmutableString] or `&str`. - /// Indexers for arrays, object maps and strings cannot be registered. + /// Panics if the type is [`Array`], [`Map`], [`String`], + /// [`ImmutableString`][crate::ImmutableString], `&str` or [`INT`][crate::INT]. + /// Indexers for arrays, object maps, strings and integers cannot be registered. /// /// # Example /// @@ -1876,7 +1893,7 @@ impl Engine { scope: &mut Scope, ast: &AST, name: impl AsRef, - args: impl crate::fn_args::FuncArgs, + args: impl crate::FuncArgs, ) -> Result> { let mut arg_values: crate::StaticVec<_> = Default::default(); args.parse(&mut arg_values); @@ -1886,14 +1903,14 @@ impl Engine { let typ = self.map_type_name(result.type_name()); - return result.try_cast().ok_or_else(|| { + result.try_cast().ok_or_else(|| { EvalAltResult::ErrorMismatchOutputType( self.map_type_name(type_name::()).into(), typ.into(), Position::NONE, ) .into() - }); + }) } /// Call a script function defined in an [`AST`] with multiple [`Dynamic`] arguments /// and optionally a value for binding to the `this` pointer. From 3371eed411cc8d0d5930bd52855a82cec90d0a67 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 4 Jun 2021 14:23:40 +0800 Subject: [PATCH 02/24] Use write_str. --- src/ast.rs | 2 +- src/dynamic.rs | 2 +- src/result.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 7a0dfd73..da2c9fc2 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1776,7 +1776,7 @@ impl fmt::Debug for Expr { Some((_, ref namespace)) => write!(f, "{}", namespace)?, _ => (), } - write!(f, "{}", x.2)?; + f.write_str(&x.2)?; match i.map_or_else(|| x.0, |n| NonZeroUsize::new(n.get() as usize)) { Some(n) => write!(f, ", {}", n)?, _ => (), diff --git a/src/dynamic.rs b/src/dynamic.rs index a9eea0f0..0508dbcb 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -692,7 +692,7 @@ impl fmt::Debug for Dynamic { return fmt::Debug::fmt(_value_any.downcast_ref::().expect(CHECKED), f); } - write!(f, "{}", value.type_name()) + f.write_str(value.type_name()) } #[cfg(not(feature = "no_closure"))] diff --git a/src/result.rs b/src/result.rs index 51d692fc..354f5083 100644 --- a/src/result.rs +++ b/src/result.rs @@ -172,7 +172,7 @@ impl fmt::Display for EvalAltResult { Self::ErrorModuleNotFound(s, _) => write!(f, "{}: '{}'", desc, s)?, - Self::ErrorDotExpr(s, _) if !s.is_empty() => write!(f, "{}", s)?, + Self::ErrorDotExpr(s, _) if !s.is_empty() => f.write_str(s)?, Self::ErrorIndexingType(s, _) => write!(f, "Indexer not registered for type '{}'", s)?, From a530fbf4fffc3e3960c928e5c8b16284c4a51913 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 5 Jun 2021 15:26:43 +0800 Subject: [PATCH 03/24] Remove unnecessary raw stirngs. --- src/ast.rs | 66 ++++++++++++++++++++++---------------------- src/engine_api.rs | 3 +- src/fn_args.rs | 11 ++++---- tests/arrays.rs | 24 ++++++++-------- tests/call_fn.rs | 4 +-- tests/closures.rs | 44 ++++++++++++++--------------- tests/comments.rs | 4 +-- tests/internal_fn.rs | 12 ++++---- 8 files changed, 85 insertions(+), 83 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index da2c9fc2..a2d81e44 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -417,15 +417,15 @@ impl AST { /// /// let engine = Engine::new(); /// - /// let ast1 = engine.compile(r#" - /// fn foo(x) { 42 + x } - /// foo(1) - /// "#)?; + /// let ast1 = engine.compile(" + /// fn foo(x) { 42 + x } + /// foo(1) + /// ")?; /// /// let ast2 = engine.compile(r#" - /// fn foo(n) { `hello${n}` } - /// foo("!") - /// "#)?; + /// fn foo(n) { `hello${n}` } + /// foo("!") + /// "#)?; /// /// let ast = ast1.merge(&ast2); // Merge 'ast2' into 'ast1' /// @@ -469,15 +469,15 @@ impl AST { /// /// let engine = Engine::new(); /// - /// let mut ast1 = engine.compile(r#" - /// fn foo(x) { 42 + x } - /// foo(1) - /// "#)?; + /// let mut ast1 = engine.compile(" + /// fn foo(x) { 42 + x } + /// foo(1) + /// ")?; /// /// let ast2 = engine.compile(r#" - /// fn foo(n) { `hello${n}` } - /// foo("!") - /// "#)?; + /// fn foo(n) { `hello${n}` } + /// foo("!") + /// "#)?; /// /// ast1.combine(ast2); // Combine 'ast2' into 'ast1' /// @@ -523,16 +523,16 @@ impl AST { /// /// let engine = Engine::new(); /// - /// let ast1 = engine.compile(r#" - /// fn foo(x) { 42 + x } - /// foo(1) - /// "#)?; + /// let ast1 = engine.compile(" + /// fn foo(x) { 42 + x } + /// foo(1) + /// ")?; /// /// let ast2 = engine.compile(r#" - /// fn foo(n) { `hello${n}` } - /// fn error() { 0 } - /// foo("!") - /// "#)?; + /// fn foo(n) { `hello${n}` } + /// fn error() { 0 } + /// foo("!") + /// "#)?; /// /// // Merge 'ast2', picking only 'error()' but not 'foo(_)', into 'ast1' /// let ast = ast1.merge_filtered(&ast2, |_, _, script, name, params| @@ -606,16 +606,16 @@ impl AST { /// /// let engine = Engine::new(); /// - /// let mut ast1 = engine.compile(r#" - /// fn foo(x) { 42 + x } - /// foo(1) - /// "#)?; + /// let mut ast1 = engine.compile(" + /// fn foo(x) { 42 + x } + /// foo(1) + /// ")?; /// /// let ast2 = engine.compile(r#" - /// fn foo(n) { `hello${n}` } - /// fn error() { 0 } - /// foo("!") - /// "#)?; + /// fn foo(n) { `hello${n}` } + /// fn error() { 0 } + /// foo("!") + /// "#)?; /// /// // Combine 'ast2', picking only 'error()' but not 'foo(_)', into 'ast1' /// ast1.combine_filtered(ast2, |_, _, script, name, params| @@ -664,9 +664,9 @@ impl AST { /// let engine = Engine::new(); /// /// let mut ast = engine.compile(r#" - /// fn foo(n) { n + 1 } - /// fn bar() { print("hello"); } - /// "#)?; + /// fn foo(n) { n + 1 } + /// fn bar() { print("hello"); } + /// "#)?; /// /// // Remove all functions except 'foo(_)' /// ast.retain_functions(|_, _, name, params| name == "foo" && params == 1); diff --git a/src/engine_api.rs b/src/engine_api.rs index d2a118ac..7d97e884 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1354,7 +1354,8 @@ impl Engine { /// /// let map = engine.parse_json( /// r#"{"a":123, "b":42, "c":{"x":false, "y":true}, "d":null}"# - /// .replace("{", "#{").as_str(), true)?; + /// .replace("{", "#{").as_str(), + /// true)?; /// /// assert_eq!(map.len(), 4); /// assert_eq!(map["a"].as_int().unwrap(), 123); diff --git a/src/fn_args.rs b/src/fn_args.rs index 10a4e022..e575ec19 100644 --- a/src/fn_args.rs +++ b/src/fn_args.rs @@ -40,11 +40,12 @@ pub trait FuncArgs { /// let engine = Engine::new(); /// let mut scope = Scope::new(); /// - /// let ast = engine.compile(r#" - /// fn hello(x, y, z) { - /// if x { `hello ${y}` } else { y + z } - /// } - /// "#)?; + /// let ast = engine.compile( + /// " + /// fn hello(x, y, z) { + /// if x { `hello ${y}` } else { y + z } + /// } + /// ")?; /// /// let result: String = engine.call_fn(&mut scope, &ast, "hello", options)?; /// diff --git a/tests/arrays.rs b/tests/arrays.rs index 74cea10b..eb17d0a9 100644 --- a/tests/arrays.rs +++ b/tests/arrays.rs @@ -208,13 +208,13 @@ fn test_arrays_map_reduce() -> Result<(), Box> { assert_eq!( engine.eval::( - r#" + " let x = [1, 2, 3]; x.reduce(|sum, v, i| { if i == 0 { sum = 10 } sum + v * v }) - "# + " )?, 24 ); @@ -231,40 +231,40 @@ fn test_arrays_map_reduce() -> Result<(), Box> { assert_eq!( engine.eval::( - r#" + " let x = [1, 2, 3]; x.reduce_rev(|sum, v, i| { if i == 2 { sum = 10 } sum + v * v }) - "# + " )?, 24 ); assert!(engine.eval::( - r#" + " let x = [1, 2, 3]; x.some(|v| v > 1) - "# + " )?); assert!(engine.eval::( - r#" + " let x = [1, 2, 3]; x.some(|v, i| v * i == 0) - "# + " )?); assert!(!engine.eval::( - r#" + " let x = [1, 2, 3]; x.all(|v| v > 1) - "# + " )?); assert!(engine.eval::( - r#" + " let x = [1, 2, 3]; x.all(|v, i| v > i) - "# + " )?); Ok(()) diff --git a/tests/call_fn.rs b/tests/call_fn.rs index cb9aa559..a239e4a5 100644 --- a/tests/call_fn.rs +++ b/tests/call_fn.rs @@ -79,11 +79,11 @@ fn test_call_fn_args() -> Result<(), Box> { let mut scope = Scope::new(); let ast = engine.compile( - r#" + " fn hello(x, y, z) { if x { `hello ${y}` } else { y + z } } - "#, + ", )?; let result: String = engine.call_fn(&mut scope, &ast, "hello", options)?; diff --git a/tests/closures.rs b/tests/closures.rs index 67ad11a9..51a728e9 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -25,12 +25,12 @@ fn test_fn_ptr_curry_call() -> Result<(), Box> { #[cfg(not(feature = "no_object"))] assert_eq!( engine.eval::( - r#" + " let addition = |x, y| { x + y }; let curried = addition.curry(2); call_with_arg(curried, 40) - "# + " )?, 42 ); @@ -65,7 +65,7 @@ fn test_closures() -> Result<(), Box> { assert_eq!( engine.eval::( - r#" + " let x = 8; let res = |y, z| { @@ -75,55 +75,55 @@ fn test_closures() -> Result<(), Box> { }.curry(15).call(2); res + (|| x - 3).call() - "# + " )?, 42 ); assert_eq!( engine.eval::( - r#" + " let a = 41; let foo = |x| { a += x }; foo.call(1); a - "# + " )?, 42 ); assert!(engine.eval::( - r#" + " let a = 41; let foo = |x| { a += x }; a.is_shared() - "# + " )?); assert!(engine.eval::( - r#" + " let a = 41; let foo = |x| { a += x }; is_shared(a) - "# + " )?); engine.register_fn("plus_one", |x: INT| x + 1); assert_eq!( engine.eval::( - r#" + " let a = 41; let f = || plus_one(a); f.call() - "# + " )?, 42 ); assert_eq!( engine.eval::( - r#" + " let a = 40; let f = |x| { let f = |x| { @@ -133,19 +133,19 @@ fn test_closures() -> Result<(), Box> { f.call(x) }; f.call(1) - "# + " )?, 42 ); assert_eq!( engine.eval::( - r#" + " let a = 21; let f = |x| a += x; f.call(a); a - "# + " )?, 42 ); @@ -163,13 +163,13 @@ fn test_closures() -> Result<(), Box> { assert_eq!( engine.eval::( - r#" + " let a = 41; let b = 0; let f = || b.custom_call(|| a + 1); f.call() - "# + " )?, 42 ); @@ -231,13 +231,13 @@ fn test_closures_data_race() -> Result<(), Box> { assert_eq!( engine.eval::( - r#" + " let a = 1; let b = 40; let foo = |x| { this += a + x }; b.call(foo, 1); b - "# + " )?, 42 ); @@ -245,12 +245,12 @@ fn test_closures_data_race() -> Result<(), Box> { assert!(matches!( *engine .eval::( - r#" + " let a = 20; let foo = |x| { this += a + x }; a.call(foo, 1); a - "# + " ) .expect_err("should error"), EvalAltResult::ErrorDataRace(_, _) diff --git a/tests/comments.rs b/tests/comments.rs index 695c47a4..e0a2d2a2 100644 --- a/tests/comments.rs +++ b/tests/comments.rs @@ -11,12 +11,12 @@ fn test_comments() -> Result<(), Box> { assert_eq!( engine.eval::( - r#" + " let /* I am a multi-line comment, yay! */ x = 42; x - "# + " )?, 42 ); diff --git a/tests/internal_fn.rs b/tests/internal_fn.rs index eaa3364b..18ee31fd 100644 --- a/tests/internal_fn.rs +++ b/tests/internal_fn.rs @@ -210,28 +210,28 @@ fn test_internal_fn_captures() -> Result<(), Box> { assert_eq!( engine.eval::( - r#" + " fn foo(y) { x += y; x } let x = 41; let y = 999; foo!(1) + x - "# + " )?, 83 ); assert!(engine .eval::( - r#" + " fn foo(y) { x += y; x } let x = 41; let y = 999; foo(1) + x - "# + " ) .is_err()); @@ -239,14 +239,14 @@ fn test_internal_fn_captures() -> Result<(), Box> { assert!(matches!( *engine .compile( - r#" + " fn foo() { this += x; } let x = 41; let y = 999; y.foo!(); - "# + " ) .expect_err("should error") .0, From c02d702081ee2b600ae5ba4904ea211dc445c15b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 6 Jun 2021 12:17:04 +0800 Subject: [PATCH 04/24] Use StaticVec. --- src/ast.rs | 21 ++++++++++----------- src/fn_call.rs | 2 +- src/module/mod.rs | 4 ++-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index a2d81e44..2adba823 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -90,7 +90,7 @@ impl fmt::Display for ScriptFnDef { self.params .iter() .map(|s| s.as_str()) - .collect::>() + .collect::>() .join(", ") ) } @@ -138,7 +138,11 @@ impl fmt::Display for ScriptFnMetadata<'_> { FnAccess::Private => "private ", }, self.name, - self.params.iter().cloned().collect::>().join(", ") + self.params + .iter() + .cloned() + .collect::>() + .join(", ") ) } } @@ -215,10 +219,9 @@ impl AST { statements: impl IntoIterator, functions: impl Into>, ) -> Self { - let statements: StaticVec<_> = statements.into_iter().collect(); Self { source: None, - body: StmtBlock::new(statements, Position::NONE), + body: StmtBlock::new(statements.into_iter().collect(), Position::NONE), functions: functions.into(), #[cfg(not(feature = "no_module"))] resolver: None, @@ -231,10 +234,9 @@ impl AST { functions: impl Into>, source: impl Into, ) -> Self { - let statements: StaticVec<_> = statements.into_iter().collect(); Self { source: Some(source.into()), - body: StmtBlock::new(statements, Position::NONE), + body: StmtBlock::new(statements.into_iter().collect(), Position::NONE), functions: functions.into(), #[cfg(not(feature = "no_module"))] resolver: None, @@ -866,8 +868,7 @@ pub struct StmtBlock(StaticVec, Position); impl StmtBlock { /// Create a new [`StmtBlock`]. - pub fn new(statements: impl Into>, pos: Position) -> Self { - let mut statements = statements.into(); + pub fn new(mut statements: StaticVec, pos: Position) -> Self { statements.shrink_to_fit(); Self(statements, pos) } @@ -1000,9 +1001,7 @@ impl From for StmtBlock { #[inline(always)] fn from(stmt: Stmt) -> Self { match stmt { - Stmt::Block(mut block, pos) => { - Self(block.iter_mut().map(|v| mem::take(v)).collect(), pos) - } + Stmt::Block(mut block, pos) => Self(block.iter_mut().map(mem::take).collect(), pos), Stmt::Noop(pos) => Self(Default::default(), pos), _ => { let pos = stmt.position(); diff --git a/src/fn_call.rs b/src/fn_call.rs index 27cc2116..d1f3d3bc 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -140,7 +140,7 @@ impl Engine { } else { self.map_type_name((*a).type_name()) }) - .collect::>() + .collect::>() .join(", ") ) } diff --git a/src/module/mod.rs b/src/module/mod.rs index c9f9966f..63052d6b 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -71,7 +71,7 @@ impl FuncInfo { let mut sig = format!("{}(", self.name); if !self.param_names.is_empty() { - let mut params: Vec = + let mut params: StaticVec = self.param_names.iter().map(|s| s.as_str().into()).collect(); let return_type = params.pop().unwrap_or_else(|| "()".into()); sig.push_str(¶ms.join(", ")); @@ -1649,7 +1649,7 @@ impl fmt::Debug for NamespaceRef { .path .iter() .map(|Ident { name, .. }| name.as_str()) - .collect::>() + .collect::>() .join("::"), ) } From 859a18c6fdf35b34c03395700b977ec070be1ec1 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 6 Jun 2021 14:47:32 +0800 Subject: [PATCH 05/24] Fix Dynamic hashing. --- src/dynamic.rs | 77 ++++++++++++++++++++++++++++++++++++++---------- src/fn_native.rs | 2 +- 2 files changed, 63 insertions(+), 16 deletions(-) diff --git a/src/dynamic.rs b/src/dynamic.rs index 0508dbcb..66c01668 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -33,6 +33,8 @@ use fmt::Debug; #[cfg(any(target_arch = "wasm32", target_arch = "wasm64"))] use instant::Instant; +const CHECKED: &str = "never fails because the type was checked"; + mod private { use crate::fn_native::SendSync; use std::any::Any; @@ -471,29 +473,27 @@ impl Dynamic { } impl Hash for Dynamic { + /// Hash the [`Dynamic`] value. + /// + /// # Panics + /// + /// Panics if the [`Dynamic`] value contains an unrecognized trait object. fn hash(&self, state: &mut H) { + std::mem::discriminant(&self.0).hash(state); + match &self.0 { Union::Unit(_, _, _) => ().hash(state), - Union::Bool(value, _, _) => value.hash(state), + Union::Bool(b, _, _) => b.hash(state), Union::Str(s, _, _) => s.hash(state), - Union::Char(ch, _, _) => ch.hash(state), + Union::Char(c, _, _) => c.hash(state), Union::Int(i, _, _) => i.hash(state), #[cfg(not(feature = "no_float"))] Union::Float(f, _, _) => f.hash(state), #[cfg(not(feature = "no_index"))] Union::Array(a, _, _) => a.as_ref().hash(state), #[cfg(not(feature = "no_object"))] - Union::Map(m, _, _) => m.iter().for_each(|(key, value)| { - key.hash(state); - value.hash(state); - }), - Union::FnPtr(f, _, _) if f.is_curried() => { - unimplemented!( - "{} with curried arguments cannot be hashed", - self.type_name() - ) - } - Union::FnPtr(f, _, _) => f.fn_name().hash(state), + Union::Map(m, _, _) => m.as_ref().hash(state), + Union::FnPtr(f, _, _) => f.hash(state), #[cfg(not(feature = "no_closure"))] Union::Shared(cell, _, _) => { @@ -505,6 +505,55 @@ impl Hash for Dynamic { (*value).hash(state) } + #[cfg(not(feature = "only_i32"))] + #[cfg(not(feature = "only_i64"))] + Union::Variant(value, _, _) => { + let value = value.as_ref().as_ref(); + let _type_id = value.type_id(); + let _value_any = value.as_any(); + + if _type_id == TypeId::of::() { + TypeId::of::().hash(state); + _value_any.downcast_ref::().expect(CHECKED).hash(state); + } else if _type_id == TypeId::of::() { + TypeId::of::().hash(state); + _value_any.downcast_ref::().expect(CHECKED).hash(state); + } else if _type_id == TypeId::of::() { + TypeId::of::().hash(state); + _value_any.downcast_ref::().expect(CHECKED).hash(state); + } else if _type_id == TypeId::of::() { + TypeId::of::().hash(state); + _value_any.downcast_ref::().expect(CHECKED).hash(state); + } else if _type_id == TypeId::of::() { + TypeId::of::().hash(state); + _value_any.downcast_ref::().expect(CHECKED).hash(state); + } else if _type_id == TypeId::of::() { + TypeId::of::().hash(state); + _value_any.downcast_ref::().expect(CHECKED).hash(state); + } else if _type_id == TypeId::of::() { + TypeId::of::().hash(state); + _value_any.downcast_ref::().expect(CHECKED).hash(state); + } else if _type_id == TypeId::of::() { + TypeId::of::().hash(state); + _value_any.downcast_ref::().expect(CHECKED).hash(state); + } + + #[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))] + if _type_id == TypeId::of::() { + TypeId::of::().hash(state); + _value_any + .downcast_ref::() + .expect(CHECKED) + .hash(state); + } else if _type_id == TypeId::of::() { + TypeId::of::().hash(state); + _value_any + .downcast_ref::() + .expect(CHECKED) + .hash(state); + } + } + _ => unimplemented!("{} cannot be hashed", self.type_name()), } } @@ -573,8 +622,6 @@ impl fmt::Display for Dynamic { let _type_id = value.type_id(); let _value_any = value.as_any(); - const CHECKED: &str = "never fails because the type was checked"; - #[cfg(not(feature = "only_i32"))] #[cfg(not(feature = "only_i64"))] if _type_id == TypeId::of::() { diff --git a/src/fn_native.rs b/src/fn_native.rs index 964488d0..06dccf38 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -263,7 +263,7 @@ pub type FnCallArgs<'a> = [&'a mut Dynamic]; /// A general function pointer, which may carry additional (i.e. curried) argument values /// to be passed onto a function during a call. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Hash)] pub struct FnPtr(Identifier, StaticVec); impl FnPtr { From 989cb702c02ffd919d791075e02040886c272464 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 7 Jun 2021 09:47:49 +0800 Subject: [PATCH 06/24] Use chars() to iterate strings. --- CHANGELOG.md | 6 +++ src/packages/iter_basic.rs | 82 +++++++++++++++++++++++++++++++++++-- src/packages/string_more.rs | 8 +--- src/result.rs | 2 +- tests/for.rs | 2 +- 5 files changed, 87 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 990ceab6..e616dc13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,10 +9,16 @@ Bug fixes * Fixed incorrect optimization regarding chain-indexing with non-numeric index. +Breaking changes +---------------- + +* To keep the API consistent, strings are no longer iterable by default. Use the `chars` method to iterator the characters in a string. + New features ------------ * An integer value can now be indexed to get/set a single bit. +* The `bits` method of an integer can be used to iterate through its bits. Version 0.20.2 diff --git a/src/packages/iter_basic.rs b/src/packages/iter_basic.rs index f5e7cc27..b88f8a10 100644 --- a/src/packages/iter_basic.rs +++ b/src/packages/iter_basic.rs @@ -10,7 +10,7 @@ use num_traits::{CheckedAdd as Add, CheckedSub as Sub}; #[cfg(feature = "unchecked")] use std::ops::{Add, Sub}; -// Register range function with step +// Range iterator with step #[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] struct StepRange(T, T, T) where @@ -126,7 +126,7 @@ where } } -// Register range function with step +// Bit-field iterator with step #[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] struct BitRange(INT, INT, usize); @@ -191,6 +191,64 @@ impl Iterator for BitRange { } } +// String iterator over characters +#[derive(Debug, Clone, Hash, Eq, PartialEq)] +struct CharsStream(Vec, usize); + +impl CharsStream { + pub fn new(string: &str, from: INT, len: INT) -> Self { + if len <= 0 { + return Self(Default::default(), 0); + } + if from >= 0 { + return Self( + string + .chars() + .skip(from as usize) + .take(len as usize) + .collect(), + 0, + ); + } + #[cfg(not(feature = "unchecked"))] + return if let Some(abs_from) = from.checked_abs() { + let num_chars = string.chars().count(); + let offset = if num_chars < (abs_from as usize) { + 0 + } else { + num_chars - (abs_from as usize) + }; + Self(string.chars().skip(offset).take(len as usize).collect(), 0) + } else { + Self(string.chars().skip(0).take(len as usize).collect(), 0) + }; + + #[cfg(feature = "unchecked")] + return Self( + string + .chars() + .skip(from as usize) + .take(len as usize) + .collect, + 0, + ); + } +} + +impl Iterator for CharsStream { + type Item = char; + + fn next(&mut self) -> Option { + if self.1 >= self.0.len() { + None + } else { + let ch = self.0[self.1]; + self.1 += 1; + Some(ch) + } + } +} + macro_rules! reg_range { ($lib:ident | $x:expr => $( $y:ty ),*) => { $( @@ -370,15 +428,31 @@ def_package!(crate:BasicIteratorPackage:"Basic range iterators.", lib, { lib.update_fn_metadata(_hash, &["from: Decimal", "to: Decimal", "step: Decimal", "Iterator"]); } + // Register string iterator + lib.set_iterator::(); + + let _hash = lib.set_native_fn("chars", |string, from,len| Ok(CharsStream::new(string, from, len))); + #[cfg(feature = "metadata")] + lib.update_fn_metadata(_hash, &["string: &str", "from: INT", "len: INT", "Iterator"]); + + let _hash = lib.set_native_fn("chars", |string, from| Ok(CharsStream::new(string, from, INT::MAX))); + #[cfg(feature = "metadata")] + lib.update_fn_metadata(_hash, &["string: &str", "from: INT", "Iterator"]); + + let _hash = lib.set_native_fn("chars", |string| Ok(CharsStream::new(string, 0, INT::MAX))); + #[cfg(feature = "metadata")] + lib.update_fn_metadata(_hash, &["string: &str", "Iterator"]); + + // Register bit-field iterator lib.set_iterator::(); let _hash = lib.set_native_fn("bits", |value, from, len| BitRange::new(value, from, len)); #[cfg(feature = "metadata")] - lib.update_fn_metadata(_hash, &["value: INT", "from: Decimal", "len: Decimal", "Iterator"]); + lib.update_fn_metadata(_hash, &["value: INT", "from: INT", "len: INT", "Iterator"]); let _hash = lib.set_native_fn("bits", |value, from| BitRange::new(value, from, INT::MAX)); #[cfg(feature = "metadata")] - lib.update_fn_metadata(_hash, &["value: INT", "from: Decimal", "Iterator"]); + lib.update_fn_metadata(_hash, &["value: INT", "from: INT", "Iterator"]); let _hash = lib.set_native_fn("bits", |value| BitRange::new(value, 0, INT::MAX)); #[cfg(feature = "metadata")] diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index a88ddf84..86694e84 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -1,7 +1,7 @@ #![allow(non_snake_case)] use crate::plugin::*; -use crate::{def_package, Dynamic, ImmutableString, StaticVec, INT}; +use crate::{def_package, Dynamic, StaticVec, INT}; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{any::TypeId, mem}; @@ -10,12 +10,6 @@ use super::string_basic::{print_with_func, FUNC_TO_STRING}; def_package!(crate:MoreStringPackage:"Additional string utilities, including string building.", lib, { combine_with_exported_module!(lib, "string", string_functions); - - // Register string iterator - lib.set_iter( - TypeId::of::(), - |string| Box::new(string.cast::().chars().collect::>().into_iter().map(Into::into)) - ); }); #[export_module] diff --git a/src/result.rs b/src/result.rs index 354f5083..3b3f08f7 100644 --- a/src/result.rs +++ b/src/result.rs @@ -113,7 +113,7 @@ impl EvalAltResult { Self::ErrorStringBounds(0, _, _) => "Empty string has nothing to index", Self::ErrorStringBounds(_, _, _) => "String index out of bounds", Self::ErrorBitFieldBounds(_, _, _) => "Bit-field index out of bounds", - Self::ErrorFor(_) => "For loop expects an array, object map, or range", + Self::ErrorFor(_) => "For loop expects a type with an iterator defined", Self::ErrorVariableNotFound(_, _) => "Variable not found", Self::ErrorModuleNotFound(_, _) => "Module not found", Self::ErrorDataRace(_, _) => "Data race detected when accessing variable", diff --git a/tests/for.rs b/tests/for.rs index 0a7cb840..59986344 100644 --- a/tests/for.rs +++ b/tests/for.rs @@ -235,7 +235,7 @@ fn test_for_string() -> Result<(), Box> { let s = "hello"; let sum = 0; - for ch in s { + for ch in s.chars() { sum += to_int(ch); } From 1e66f1963aea66513b770f908a3697557005e920 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 7 Jun 2021 11:01:16 +0800 Subject: [PATCH 07/24] Add counter variable to for statement. --- CHANGELOG.md | 1 + src/ast.rs | 10 +++--- src/engine.rs | 43 +++++++++++++++++------ src/optimize.rs | 4 +-- src/parse_error.rs | 4 +++ src/parser.rs | 87 ++++++++++++++++++++++++++++++++++++++-------- tests/for.rs | 16 +++++++++ 7 files changed, 132 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e616dc13..1e4cfb75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Breaking changes New features ------------ +* New syntax for `for` statement to include counter variable. * An integer value can now be indexed to get/set a single bit. * The `bits` method of an integer can be used to iterate through its bits. diff --git a/src/ast.rs b/src/ast.rs index 2adba823..4d6b3b1c 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -944,8 +944,8 @@ pub enum Stmt { While(Expr, Box, Position), /// `do` `{` stmt `}` `while`|`until` expr Do(Box, Expr, bool, Position), - /// `for` id `in` expr `{` stmt `}` - For(Expr, Box<(Ident, StmtBlock)>, Position), + /// `for` `(` id `,` counter `)` `in` expr `{` stmt `}` + For(Expr, Box<(Ident, Option, StmtBlock)>, Position), /// \[`export`\] `let` id `=` expr Let(Expr, Box, bool, Position), /// \[`export`\] `const` id `=` expr @@ -1166,7 +1166,7 @@ impl Stmt { Self::While(condition, block, _) | Self::Do(block, condition, _, _) => { condition.is_pure() && block.0.iter().all(Stmt::is_pure) } - Self::For(iterable, x, _) => iterable.is_pure() && (x.1).0.iter().all(Stmt::is_pure), + Self::For(iterable, x, _) => iterable.is_pure() && (x.2).0.iter().all(Stmt::is_pure), Self::Let(_, _, _, _) | Self::Const(_, _, _, _) | Self::Assignment(_, _) @@ -1286,7 +1286,7 @@ impl Stmt { if !e.walk(path, on_node) { return false; } - for s in &(x.1).0 { + for s in &(x.2).0 { if !s.walk(path, on_node) { return false; } @@ -1777,7 +1777,7 @@ impl fmt::Debug for Expr { } f.write_str(&x.2)?; match i.map_or_else(|| x.0, |n| NonZeroUsize::new(n.get() as usize)) { - Some(n) => write!(f, ", {}", n)?, + Some(n) => write!(f, " #{}", n)?, _ => (), } f.write_str(")") diff --git a/src/engine.rs b/src/engine.rs index f50656b3..51edbe01 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -14,7 +14,7 @@ use crate::token::Token; use crate::utils::get_hasher; use crate::{ Dynamic, EvalAltResult, Identifier, ImmutableString, Module, Position, RhaiResult, Scope, - Shared, StaticVec, + Shared, StaticVec, INT, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -2521,7 +2521,7 @@ impl Engine { // For loop Stmt::For(expr, x, _) => { - let (Ident { name, .. }, statements) = x.as_ref(); + let (Ident { name, .. }, counter, statements) = x.as_ref(); let iter_obj = self .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .flatten(); @@ -2550,17 +2550,40 @@ impl Engine { }); if let Some(func) = func { - // Add the loop variable - let var_name: Cow<'_, str> = if state.is_global() { - name.to_string().into() + // Add the loop variables + let orig_scope_len = scope.len(); + let counter_index = if let Some(Ident { name, .. }) = counter { + scope.push(unsafe_cast_var_name_to_lifetime(name), 0 as INT); + Some(scope.len() - 1) } else { - unsafe_cast_var_name_to_lifetime(name).into() + None }; - scope.push(var_name, ()); + scope.push(unsafe_cast_var_name_to_lifetime(name), ()); let index = scope.len() - 1; state.scope_level += 1; - for iter_value in func(iter_obj) { + for (x, iter_value) in func(iter_obj).enumerate() { + // Increment counter + if let Some(c) = counter_index { + #[cfg(not(feature = "unchecked"))] + if x > INT::MAX as usize { + return EvalAltResult::ErrorArithmetic( + format!("for-loop counter overflow: {}", x), + counter + .as_ref() + .expect("never fails because `counter` is `Some`") + .pos, + ) + .into(); + } + + let mut counter_var = scope + .get_mut_by_index(c) + .write_lock::() + .expect("never fails because the counter always holds an `INT`"); + *counter_var = x as INT; + } + let loop_var = scope.get_mut_by_index(index); let value = iter_value.flatten(); @@ -2600,7 +2623,7 @@ impl Engine { } state.scope_level -= 1; - scope.rewind(scope.len() - 1); + scope.rewind(orig_scope_len); Ok(Dynamic::UNIT) } else { EvalAltResult::ErrorFor(expr.position()).into() @@ -2672,8 +2695,6 @@ impl Engine { } #[cfg(not(feature = "no_object"))] _ => { - use crate::INT; - let mut err_map: Map = Default::default(); let err_pos = err.take_position(); diff --git a/src/optimize.rs b/src/optimize.rs index b28124b5..a09a1557 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -594,8 +594,8 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { // for id in expr { block } Stmt::For(iterable, x, _) => { optimize_expr(iterable, state, false); - let body = mem::take(x.1.statements()).into_vec(); - *x.1.statements() = optimize_stmt_block(body, state, false, true, false).into(); + let body = mem::take(x.2.statements()).into_vec(); + *x.2.statements() = optimize_stmt_block(body, state, false, true, false).into(); } // let id = expr; Stmt::Let(expr, _, _, _) => optimize_expr(expr, state, false), diff --git a/src/parse_error.rs b/src/parse_error.rs index 87454019..ea32e35e 100644 --- a/src/parse_error.rs +++ b/src/parse_error.rs @@ -116,6 +116,8 @@ pub enum ParseErrorType { DuplicatedProperty(String), /// A `switch` case is duplicated. DuplicatedSwitchCase, + /// A variable name is duplicated. Wrapped value is the variable name. + DuplicatedVariable(String), /// The default case of a `switch` statement is not the last. WrongSwitchDefaultCase, /// The case condition of a `switch` statement is not appropriate. @@ -200,6 +202,7 @@ impl ParseErrorType { Self::MalformedCapture(_) => "Invalid capturing", Self::DuplicatedProperty(_) => "Duplicated property in object map literal", Self::DuplicatedSwitchCase => "Duplicated switch case", + Self::DuplicatedVariable(_) => "Duplicated variable name", Self::WrongSwitchDefaultCase => "Default switch case is not the last", Self::WrongSwitchCaseCondition => "Default switch case cannot have condition", Self::PropertyExpected => "Expecting name of a property", @@ -247,6 +250,7 @@ impl fmt::Display for ParseErrorType { write!(f, "Duplicated property '{}' for object map literal", s) } Self::DuplicatedSwitchCase => f.write_str(self.desc()), + Self::DuplicatedVariable(s) => write!(f, "Duplicated variable name '{}'", s), Self::ExprExpected(s) => write!(f, "Expecting {} expression", s), diff --git a/src/parser.rs b/src/parser.rs index 9897b613..a843182d 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2144,6 +2144,21 @@ fn parse_for( lib: &mut FunctionsLib, mut settings: ParseSettings, ) -> Result { + fn get_name_pos(input: &mut TokenStream) -> Result<(String, Position), ParseError> { + match input.next().expect(NEVER_ENDS) { + // Variable name + (Token::Identifier(s), pos) => Ok((s, pos)), + // Reserved keyword + (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { + Err(PERR::Reserved(s).into_err(pos)) + } + // Bad identifier + (Token::LexError(err), pos) => Err(err.into_err(pos)), + // Not a variable name + (_, pos) => Err(PERR::VariableExpected.into_err(pos)), + } + } + #[cfg(not(feature = "unchecked"))] settings.ensure_level_within_max_limit(state.max_expr_depth)?; @@ -2151,17 +2166,36 @@ fn parse_for( settings.pos = eat_token(input, Token::For); // for name ... - let (name, name_pos) = match input.next().expect(NEVER_ENDS) { - // Variable name - (Token::Identifier(s), pos) => (s, pos), - // Reserved keyword - (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { - return Err(PERR::Reserved(s).into_err(pos)); + let (name, name_pos, counter_name, counter_pos) = if match_token(input, Token::LeftParen).0 { + // ( name, counter ) + let (name, name_pos) = get_name_pos(input)?; + let (has_comma, pos) = match_token(input, Token::Comma); + if !has_comma { + return Err(PERR::MissingToken( + Token::Comma.into(), + "after the iteration variable name".into(), + ) + .into_err(pos)); } - // Bad identifier - (Token::LexError(err), pos) => return Err(err.into_err(pos)), - // Not a variable name - (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), + let (counter_name, counter_pos) = get_name_pos(input)?; + + if counter_name == name { + return Err(PERR::DuplicatedVariable(counter_name).into_err(counter_pos)); + } + + let (has_close_paren, pos) = match_token(input, Token::RightParen); + if !has_close_paren { + return Err(PERR::MissingToken( + Token::RightParen.into(), + "to close the iteration variable".into(), + ) + .into_err(pos)); + } + (name, name_pos, Some(counter_name), Some(counter_pos)) + } else { + // name + let (name, name_pos) = get_name_pos(input)?; + (name, name_pos, None, None) }; // for name in ... @@ -2180,8 +2214,18 @@ fn parse_for( ensure_not_statement_expr(input, "a boolean")?; let expr = parse_expr(input, state, lib, settings.level_up())?; - let loop_var = state.get_identifier(name); let prev_stack_len = state.stack.len(); + + let counter_var = if let Some(name) = counter_name { + let counter_var = state.get_identifier(name); + state + .stack + .push((counter_var.clone(), AccessMode::ReadWrite)); + Some(counter_var) + } else { + None + }; + let loop_var = state.get_identifier(name); state.stack.push((loop_var.clone(), AccessMode::ReadWrite)); settings.is_breakable = true; @@ -2196,6 +2240,10 @@ fn parse_for( name: loop_var, pos: name_pos, }, + counter_var.map(|name| Ident { + name, + pos: counter_pos.expect("never fails because `counter_var` is `Some`"), + }), body.into(), )), settings.pos, @@ -2342,10 +2390,19 @@ fn parse_export( let rename = if match_token(input, Token::As).0 { match input.next().expect(NEVER_ENDS) { - (Token::Identifier(s), pos) => Some(Ident { - name: state.get_identifier(s), - pos, - }), + (Token::Identifier(s), pos) => { + if exports.iter().any(|(_, alias)| match alias { + Some(Ident { name, .. }) if name == &s => true, + _ => false, + }) { + return Err(PERR::DuplicatedVariable(s).into_err(pos)); + } + + Some(Ident { + name: state.get_identifier(s), + pos, + }) + } (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { return Err(PERR::Reserved(s).into_err(pos)); } diff --git a/tests/for.rs b/tests/for.rs index 59986344..8ec462ec 100644 --- a/tests/for.rs +++ b/tests/for.rs @@ -37,6 +37,22 @@ fn test_for_loop() -> Result<(), Box> { 35 ); + #[cfg(not(feature = "no_index"))] + assert_eq!( + engine.eval::( + " + let sum = 0; + let inputs = [1, 2, 3, 4, 5]; + + for (x, i) in inputs { + sum += x * (i + 1); + } + sum + " + )?, + 55 + ); + assert_eq!( engine.eval::( " From 411b718a3b94586ba013b701dfa75e75e861db6c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 7 Jun 2021 11:21:45 +0800 Subject: [PATCH 08/24] Fix test. --- src/packages/iter_basic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/packages/iter_basic.rs b/src/packages/iter_basic.rs index b88f8a10..4e60e986 100644 --- a/src/packages/iter_basic.rs +++ b/src/packages/iter_basic.rs @@ -229,7 +229,7 @@ impl CharsStream { .chars() .skip(from as usize) .take(len as usize) - .collect, + .collect(), 0, ); } From 3e08160653f79d2ec75ef182033e3e0668d85ddf Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 7 Jun 2021 11:43:00 +0800 Subject: [PATCH 09/24] Simplify variable name parsing. --- src/parser.rs | 170 ++++++++++++++++++-------------------------------- 1 file changed, 60 insertions(+), 110 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index a843182d..8e36faeb 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -264,6 +264,22 @@ fn match_token(input: &mut TokenStream, token: Token) -> (bool, Position) { } } +/// Parse a variable name. +fn parse_var_name(input: &mut TokenStream) -> Result<(String, Position), ParseError> { + match input.next().expect(NEVER_ENDS) { + // Variable name + (Token::Identifier(s), pos) => Ok((s, pos)), + // Reserved keyword + (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { + Err(PERR::Reserved(s).into_err(pos)) + } + // Bad identifier + (Token::LexError(err), pos) => Err(err.into_err(pos)), + // Not a variable name + (_, pos) => Err(PERR::VariableExpected.into_err(pos)), + } +} + /// Parse ( expr ) fn parse_paren_expr( input: &mut TokenStream, @@ -1232,33 +1248,26 @@ fn parse_primary( } // module access (Expr::Variable(_, var_pos, x), Token::DoubleColon) => { - match input.next().expect(NEVER_ENDS) { - (Token::Identifier(id2), pos2) => { - let (_, mut namespace, var_name) = *x; - let var_name_def = Ident { - name: var_name, - pos: var_pos, - }; + let (id2, pos2) = parse_var_name(input)?; + let (_, mut namespace, var_name) = *x; + let var_name_def = Ident { + name: var_name, + pos: var_pos, + }; - if let Some((_, ref mut namespace)) = namespace { - namespace.push(var_name_def); - } else { - let mut ns: NamespaceRef = Default::default(); - ns.push(var_name_def); - namespace = Some((42, ns)); - } - - Expr::Variable( - None, - pos2, - Box::new((None, namespace, state.get_identifier(id2))), - ) - } - (Token::Reserved(id2), pos2) if is_valid_identifier(id2.chars()) => { - return Err(PERR::Reserved(id2).into_err(pos2)); - } - (_, pos2) => return Err(PERR::VariableExpected.into_err(pos2)), + if let Some((_, ref mut namespace)) = namespace { + namespace.push(var_name_def); + } else { + let mut ns: NamespaceRef = Default::default(); + ns.push(var_name_def); + namespace = Some((42, ns)); } + + Expr::Variable( + None, + pos2, + Box::new((None, namespace, state.get_identifier(id2))), + ) } // Indexing #[cfg(not(feature = "no_index"))] @@ -1886,18 +1895,13 @@ fn parse_custom_syntax( }; match required_token.as_str() { - MARKER_IDENT => match input.next().expect(NEVER_ENDS) { - (Token::Identifier(s), pos) => { - let name = state.get_identifier(s); - segments.push(name.clone().into()); - tokens.push(state.get_identifier(MARKER_IDENT)); - keywords.push(Expr::Variable(None, pos, Box::new((None, None, name)))); - } - (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { - return Err(PERR::Reserved(s).into_err(pos)); - } - (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), - }, + MARKER_IDENT => { + let (name, pos) = parse_var_name(input)?; + let name = state.get_identifier(name); + segments.push(name.clone().into()); + tokens.push(state.get_identifier(MARKER_IDENT)); + keywords.push(Expr::Variable(None, pos, Box::new((None, None, name)))); + } MARKER_EXPR => { keywords.push(parse_expr(input, state, lib, settings)?); let keyword = state.get_identifier(MARKER_EXPR); @@ -2144,21 +2148,6 @@ fn parse_for( lib: &mut FunctionsLib, mut settings: ParseSettings, ) -> Result { - fn get_name_pos(input: &mut TokenStream) -> Result<(String, Position), ParseError> { - match input.next().expect(NEVER_ENDS) { - // Variable name - (Token::Identifier(s), pos) => Ok((s, pos)), - // Reserved keyword - (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { - Err(PERR::Reserved(s).into_err(pos)) - } - // Bad identifier - (Token::LexError(err), pos) => Err(err.into_err(pos)), - // Not a variable name - (_, pos) => Err(PERR::VariableExpected.into_err(pos)), - } - } - #[cfg(not(feature = "unchecked"))] settings.ensure_level_within_max_limit(state.max_expr_depth)?; @@ -2168,7 +2157,7 @@ fn parse_for( // for name ... let (name, name_pos, counter_name, counter_pos) = if match_token(input, Token::LeftParen).0 { // ( name, counter ) - let (name, name_pos) = get_name_pos(input)?; + let (name, name_pos) = parse_var_name(input)?; let (has_comma, pos) = match_token(input, Token::Comma); if !has_comma { return Err(PERR::MissingToken( @@ -2177,7 +2166,7 @@ fn parse_for( ) .into_err(pos)); } - let (counter_name, counter_pos) = get_name_pos(input)?; + let (counter_name, counter_pos) = parse_var_name(input)?; if counter_name == name { return Err(PERR::DuplicatedVariable(counter_name).into_err(counter_pos)); @@ -2194,7 +2183,7 @@ fn parse_for( (name, name_pos, Some(counter_name), Some(counter_pos)) } else { // name - let (name, name_pos) = get_name_pos(input)?; + let (name, name_pos) = parse_var_name(input)?; (name, name_pos, None, None) }; @@ -2266,14 +2255,7 @@ fn parse_let( settings.pos = input.next().expect(NEVER_ENDS).1; // let name ... - let (name, pos) = match input.next().expect(NEVER_ENDS) { - (Token::Identifier(s), pos) => (s, pos), - (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { - return Err(PERR::Reserved(s).into_err(pos)); - } - (Token::LexError(err), pos) => return Err(err.into_err(pos)), - (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), - }; + let (name, pos) = parse_var_name(input)?; let name = state.get_identifier(name); let var_def = Ident { @@ -2322,15 +2304,7 @@ fn parse_import( } // import expr as name ... - let (name, name_pos) = match input.next().expect(NEVER_ENDS) { - (Token::Identifier(s), pos) => (s, pos), - (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { - return Err(PERR::Reserved(s).into_err(pos)); - } - (Token::LexError(err), pos) => return Err(err.into_err(pos)), - (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), - }; - + let (name, name_pos) = parse_var_name(input)?; let name = state.get_identifier(name); state.modules.push(name.clone()); @@ -2379,36 +2353,18 @@ fn parse_export( let mut exports = Vec::with_capacity(4); loop { - let (id, id_pos) = match input.next().expect(NEVER_ENDS) { - (Token::Identifier(s), pos) => (s.clone(), pos), - (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { - return Err(PERR::Reserved(s).into_err(pos)); - } - (Token::LexError(err), pos) => return Err(err.into_err(pos)), - (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), - }; + let (id, id_pos) = parse_var_name(input)?; let rename = if match_token(input, Token::As).0 { - match input.next().expect(NEVER_ENDS) { - (Token::Identifier(s), pos) => { - if exports.iter().any(|(_, alias)| match alias { - Some(Ident { name, .. }) if name == &s => true, - _ => false, - }) { - return Err(PERR::DuplicatedVariable(s).into_err(pos)); - } - - Some(Ident { - name: state.get_identifier(s), - pos, - }) - } - (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { - return Err(PERR::Reserved(s).into_err(pos)); - } - (Token::LexError(err), pos) => return Err(err.into_err(pos)), - (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), + let (name, pos) = parse_var_name(input)?; + if exports.iter().any(|(_, alias)| match alias { + Some(Ident { name: alias, .. }) if alias == &name => true, + _ => false, + }) { + return Err(PERR::DuplicatedVariable(name).into_err(pos)); } + let name = state.get_identifier(name); + Some(Ident { name, pos }) } else { None }; @@ -2781,25 +2737,19 @@ fn parse_try_catch( // try { body } catch ( let var_def = if match_token(input, Token::LeftParen).0 { - let id = match input.next().expect(NEVER_ENDS) { - (Token::Identifier(s), pos) => Ident { - name: state.get_identifier(s), - pos, - }, - (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), - }; - - let (matched, pos) = match_token(input, Token::RightParen); + let (name, pos) = parse_var_name(input)?; + let (matched, err_pos) = match_token(input, Token::RightParen); if !matched { return Err(PERR::MissingToken( Token::RightParen.into(), "to enclose the catch variable".into(), ) - .into_err(pos)); + .into_err(err_pos)); } - Some(id) + let name = state.get_identifier(name); + Some(Ident { name, pos }) } else { None }; From 65b7135324396923c0f4fc642a384be6a9a4ed69 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 7 Jun 2021 11:54:10 +0800 Subject: [PATCH 10/24] Fix tests. --- .../rhai_fn_non_clonable_return.stderr | 18 +++++++++--------- .../rhai_mod_non_clonable_return.stderr | 18 +++++++++--------- tests/for.rs | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/codegen/ui_tests/rhai_fn_non_clonable_return.stderr b/codegen/ui_tests/rhai_fn_non_clonable_return.stderr index b9016a12..28d43e99 100644 --- a/codegen/ui_tests/rhai_fn_non_clonable_return.stderr +++ b/codegen/ui_tests/rhai_fn_non_clonable_return.stderr @@ -1,10 +1,10 @@ error[E0277]: the trait bound `NonClonable: Clone` is not satisfied - --> $DIR/rhai_fn_non_clonable_return.rs:11:8 - | -11 | pub fn test_fn(input: f32) -> NonClonable { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `NonClonable` - | - ::: $WORKSPACE/src/dynamic.rs - | - | pub fn from(mut value: T) -> Self { - | ----- required by this bound in `rhai::Dynamic::from` + --> $DIR/rhai_fn_non_clonable_return.rs:11:8 + | +11 | pub fn test_fn(input: f32) -> NonClonable { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `NonClonable` + | + ::: $WORKSPACE/src/dynamic.rs + | + | pub fn from(mut value: T) -> Self { + | ----- required by this bound in `rhai::Dynamic::from` diff --git a/codegen/ui_tests/rhai_mod_non_clonable_return.stderr b/codegen/ui_tests/rhai_mod_non_clonable_return.stderr index 900efa16..73ba7569 100644 --- a/codegen/ui_tests/rhai_mod_non_clonable_return.stderr +++ b/codegen/ui_tests/rhai_mod_non_clonable_return.stderr @@ -1,10 +1,10 @@ error[E0277]: the trait bound `NonClonable: Clone` is not satisfied - --> $DIR/rhai_mod_non_clonable_return.rs:12:12 - | -12 | pub fn test_fn(input: f32) -> NonClonable { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `NonClonable` - | - ::: $WORKSPACE/src/dynamic.rs - | - | pub fn from(mut value: T) -> Self { - | ----- required by this bound in `rhai::Dynamic::from` + --> $DIR/rhai_mod_non_clonable_return.rs:12:12 + | +12 | pub fn test_fn(input: f32) -> NonClonable { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `NonClonable` + | + ::: $WORKSPACE/src/dynamic.rs + | + | pub fn from(mut value: T) -> Self { + | ----- required by this bound in `rhai::Dynamic::from` diff --git a/tests/for.rs b/tests/for.rs index 8ec462ec..36adf7f2 100644 --- a/tests/for.rs +++ b/tests/for.rs @@ -251,7 +251,7 @@ fn test_for_string() -> Result<(), Box> { let s = "hello"; let sum = 0; - for ch in s.chars() { + for ch in chars(s) { sum += to_int(ch); } From c4b3ad7c7b8a4491f3f938aa97f7fa5be183f34a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 7 Jun 2021 14:52:45 +0800 Subject: [PATCH 11/24] Improve for-loop scripts. --- scripts/for1.rhai | 12 +++++++----- scripts/for2.rhai | 4 +++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/scripts/for1.rhai b/scripts/for1.rhai index 39ac7009..1e215da5 100644 --- a/scripts/for1.rhai +++ b/scripts/for1.rhai @@ -1,10 +1,12 @@ // This script runs for-loops -let arr = [1, 2, 3, 4]; +let arr = [1, true, 123.456, "hello", 3, 42]; -for a in arr { - for b in [10, 20] { - print(`${a}, ${b}`); +for (a, i) in arr { + for (b, j) in ['x', 42, (), 123, 99, 0.5] { + if b > 100 { continue; } + + print(`(${i}, ${j}) = (${a}, ${b})`); } if a == 3 { break; } @@ -12,6 +14,6 @@ for a in arr { //print(a); // <- if you uncomment this line, the script will fail to run // because 'a' is not defined here -for i in range(0, 5) { // runs through a range from 0 to 4 +for i in range(5, 0, -1) { // runs from 5 down to 1 print(i); } diff --git a/scripts/for2.rhai b/scripts/for2.rhai index c7b6b816..0f4cdce7 100644 --- a/scripts/for2.rhai +++ b/scripts/for2.rhai @@ -12,6 +12,8 @@ for i in range(0, MAX) { list.push(i); } +print(`Time = ${now.elapsed} seconds...`); + let sum = 0; for i in list { @@ -19,4 +21,4 @@ for i in list { } print(`Sum = ${sum}`); -print(`Finished. Run time = ${now.elapsed} seconds.`); +print(`Finished. Total run time = ${now.elapsed} seconds.`); From bed5256e2e554a03a60bd228576a153432a700b8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 7 Jun 2021 20:15:06 +0800 Subject: [PATCH 12/24] Add new example scripts. --- scripts/function_decl3.rhai | 4 ++-- scripts/function_decl4.rhai | 12 ++++++++++++ scripts/if2.rhai | 10 ++++++++++ scripts/switch.rhai | 11 +++++++++++ 4 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 scripts/function_decl4.rhai create mode 100644 scripts/if2.rhai create mode 100644 scripts/switch.rhai diff --git a/scripts/function_decl3.rhai b/scripts/function_decl3.rhai index 872b9745..c1c84631 100644 --- a/scripts/function_decl3.rhai +++ b/scripts/function_decl3.rhai @@ -6,6 +6,6 @@ fn f(a, b, c, d, e, f) { a - b * c - d * e - f + global::KEY } -print("f() call should be 42:"); +let result = f(100, 5, 2, 9, 6, 32); -print(f(100, 5, 2, 9, 6, 32)); +print(`result should be 42: ${result}`); diff --git a/scripts/function_decl4.rhai b/scripts/function_decl4.rhai new file mode 100644 index 00000000..a09ed3fc --- /dev/null +++ b/scripts/function_decl4.rhai @@ -0,0 +1,12 @@ +// This script defines a function that acts as a method + +// Use 'this' to refer to the object of a method call +fn action(x, y) { + this = this.abs() + x * y; // 'this' can be modified +} + +let obj = -40; + +obj.action(1, 2); // call 'action' as method + +print(`obj should now be 42: ${obj}`); diff --git a/scripts/if2.rhai b/scripts/if2.rhai new file mode 100644 index 00000000..a42bb337 --- /dev/null +++ b/scripts/if2.rhai @@ -0,0 +1,10 @@ +let a = 42; +let b = 123; + +let x = if a <= b { // if-expression + b - a +} else { + a - b +} * 10; + +print(`x should be 810: ${x}`); diff --git a/scripts/switch.rhai b/scripts/switch.rhai new file mode 100644 index 00000000..a252fa66 --- /dev/null +++ b/scripts/switch.rhai @@ -0,0 +1,11 @@ +let arr = [42, 123.456, "hello", true, 'x', 999, 1]; + +for item in arr { + switch item { + 42 => print("The Answer!"), + 123.456 => print(`Floating point... ${item}`), + "hello" => print(`${item} world!`), + 999 => print(`A number: ${item}`), + _ => print(`Something else: <${item}>`) + } +} From 2c21928f679493befae23229a62f1cb286e5ed43 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 8 Jun 2021 14:46:49 +0800 Subject: [PATCH 13/24] Simplify constant function call arguments. --- src/ast.rs | 45 +++++++++----- src/engine.rs | 50 ++++++++------- src/fn_call.rs | 159 ++++++++++++++++++++++-------------------------- src/optimize.rs | 103 ++++++++++++++++--------------- src/parser.rs | 10 +-- 5 files changed, 187 insertions(+), 180 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 4d6b3b1c..a6b3fdb7 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1,5 +1,6 @@ //! Module defining the AST (abstract syntax tree). +use crate::dynamic::Union; use crate::fn_native::shared_make_mut; use crate::module::NamespaceRef; use crate::token::Token; @@ -1519,7 +1520,7 @@ pub struct FnCallExpr { /// List of function call argument expressions. pub args: StaticVec, /// List of function call arguments that are constants. - pub literal_args: smallvec::SmallVec<[(Dynamic, Position); 2]>, + pub constants: smallvec::SmallVec<[Dynamic; 2]>, /// Function name. pub name: Identifier, /// Does this function call capture the parent scope? @@ -1532,16 +1533,6 @@ impl FnCallExpr { pub fn is_qualified(&self) -> bool { self.namespace.is_some() } - /// Are there no arguments to this function call? - #[inline(always)] - pub fn is_args_empty(&self) -> bool { - self.args.is_empty() && self.literal_args.is_empty() - } - /// Get the number of arguments to this function call. - #[inline(always)] - pub fn args_count(&self) -> usize { - self.args.len() + self.literal_args.len() - } } /// A type that wraps a floating-point number and implements [`Hash`]. @@ -1718,6 +1709,8 @@ pub enum Expr { (ImmutableString, Position), )>, ), + /// Stack slot + Stack(usize, Position), /// { [statement][Stmt] ... } Stmt(Box), /// func `(` expr `,` ... `)` @@ -1783,6 +1776,7 @@ impl fmt::Debug for Expr { f.write_str(")") } Self::Property(x) => write!(f, "Property({})", (x.2).0), + Self::Stack(x, _) => write!(f, "StackSlot({})", x), Self::Stmt(x) => { f.write_str("ExprStmtBlock")?; f.debug_list().entries(x.0.iter()).finish() @@ -1793,8 +1787,8 @@ impl fmt::Debug for Expr { ff.field("name", &x.name) .field("hash", &x.hashes) .field("args", &x.args); - if !x.literal_args.is_empty() { - ff.field("literal_args", &x.literal_args); + if !x.constants.is_empty() { + ff.field("constants", &x.constants); } if x.capture { ff.field("capture", &x.capture); @@ -1865,6 +1859,22 @@ impl Expr { _ => return None, }) } + /// Create an [`Expr`] from a [`Dynamic`] value. + #[inline] + pub fn from_dynamic(value: Dynamic, pos: Position) -> Self { + match value.0 { + Union::Unit(_, _, _) => Self::Unit(pos), + Union::Bool(b, _, _) => Self::BoolConstant(b, pos), + Union::Str(s, _, _) => Self::StringConstant(s, pos), + Union::Char(c, _, _) => Self::CharConstant(c, pos), + Union::Int(i, _, _) => Self::IntegerConstant(i, pos), + + #[cfg(not(feature = "no_float"))] + Union::Float(f, _, _) => Self::FloatConstant(f, pos), + + _ => Self::DynamicConstant(Box::new(value), pos), + } + } /// Is the expression a simple variable access? #[inline(always)] pub(crate) fn is_variable_access(&self, non_qualified: bool) -> bool { @@ -1897,6 +1907,7 @@ impl Expr { | Self::Array(_, pos) | Self::Map(_, pos) | Self::Variable(_, pos, _) + | Self::Stack(_, pos) | Self::FnCall(_, pos) | Self::Custom(_, pos) => *pos, @@ -1935,6 +1946,7 @@ impl Expr { | Self::Dot(_, pos) | Self::Index(_, pos) | Self::Variable(_, pos, _) + | Self::Stack(_, pos) | Self::FnCall(_, pos) | Self::Custom(_, pos) => *pos = new_pos, @@ -1964,7 +1976,7 @@ impl Expr { Self::Stmt(x) => x.0.iter().all(Stmt::is_pure), - Self::Variable(_, _, _) => true, + Self::Variable(_, _, _) | Self::Stack(_, _) => true, _ => self.is_constant(), } @@ -1989,7 +2001,8 @@ impl Expr { | Self::IntegerConstant(_, _) | Self::CharConstant(_, _) | Self::StringConstant(_, _) - | Self::Unit(_) => true, + | Self::Unit(_) + | Self::Stack(_, _) => true, Self::InterpolatedString(x) | Self::Array(x, _) => x.iter().all(Self::is_constant), @@ -2049,6 +2062,8 @@ impl Expr { }, Self::Custom(_, _) => false, + + Self::Stack(_, _) => unreachable!("Expr::Stack should not occur naturally"), } } /// Recursively walk this expression. diff --git a/src/engine.rs b/src/engine.rs index 51edbe01..dc4323a5 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1670,19 +1670,18 @@ impl Engine { let arg_values = x .args .iter() - .map(|arg_expr| { - self.eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level) - .map(Dynamic::flatten) + .map(|arg_expr| match arg_expr { + Expr::Stack(slot, _) => Ok(x.constants[*slot].clone()), + _ => self + .eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level) + .map(Dynamic::flatten), }) - .chain(x.literal_args.iter().map(|(v, _)| Ok(v.clone()))) .collect::, _>>()?; let pos = x .args - .iter() + .get(0) .map(|arg_expr| arg_expr.position()) - .chain(x.literal_args.iter().map(|(_, pos)| *pos)) - .next() .unwrap_or_default(); idx_values.push((arg_values, pos).into()); @@ -1716,19 +1715,18 @@ impl Engine { let arg_values = x .args .iter() - .map(|arg_expr| { - self.eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level) - .map(Dynamic::flatten) + .map(|arg_expr| match arg_expr { + Expr::Stack(slot, _) => Ok(x.constants[*slot].clone()), + _ => self + .eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level) + .map(Dynamic::flatten), }) - .chain(x.literal_args.iter().map(|(v, _)| Ok(v.clone()))) .collect::, _>>()?; let pos = x .args - .iter() + .get(0) .map(|arg_expr| arg_expr.position()) - .chain(x.literal_args.iter().map(|(_, pos)| *pos)) - .next() .unwrap_or_default(); (arg_values, pos).into() @@ -2055,7 +2053,7 @@ impl Engine { namespace, hashes, args, - literal_args: c_args, + constants, .. } = x.as_ref(); let namespace = namespace @@ -2063,8 +2061,8 @@ impl Engine { .expect("never fails because function call is qualified"); let hash = hashes.native_hash(); self.make_qualified_function_call( - scope, mods, state, lib, this_ptr, namespace, name, args, c_args, hash, *pos, - level, + scope, mods, state, lib, this_ptr, namespace, name, args, constants, hash, + *pos, level, ) } @@ -2075,12 +2073,12 @@ impl Engine { capture, hashes, args, - literal_args: c_args, + constants, .. } = x.as_ref(); self.make_function_call( - scope, mods, state, lib, this_ptr, name, args, c_args, *hashes, *pos, *capture, - level, + scope, mods, state, lib, this_ptr, name, args, constants, *hashes, *pos, + *capture, level, ) } @@ -2643,7 +2641,7 @@ impl Engine { namespace, hashes, args, - literal_args: c_args, + constants, .. } = x.as_ref(); let namespace = namespace @@ -2651,8 +2649,8 @@ impl Engine { .expect("never fails because function call is qualified"); let hash = hashes.native_hash(); self.make_qualified_function_call( - scope, mods, state, lib, this_ptr, namespace, name, args, c_args, hash, *pos, - level, + scope, mods, state, lib, this_ptr, namespace, name, args, constants, hash, + *pos, level, ) } @@ -2663,12 +2661,12 @@ impl Engine { capture, hashes, args, - literal_args: c_args, + constants, .. } = x.as_ref(); self.make_function_call( - scope, mods, state, lib, this_ptr, name, args, c_args, *hashes, *pos, *capture, - level, + scope, mods, state, lib, this_ptr, name, args, constants, *hashes, *pos, + *capture, level, ) } diff --git a/src/fn_call.rs b/src/fn_call.rs index d1f3d3bc..201e31b1 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -1039,6 +1039,27 @@ impl Engine { Ok((result, updated)) } + #[inline(always)] + fn get_arg_value( + &self, + scope: &mut Scope, + mods: &mut Imports, + state: &mut State, + lib: &[&Module], + this_ptr: &mut Option<&mut Dynamic>, + level: usize, + args_expr: &[Expr], + constants: &[Dynamic], + index: usize, + ) -> Result<(Dynamic, Position), Box> { + match args_expr[index] { + Expr::Stack(slot, pos) => Ok((constants[slot].clone(), pos)), + ref arg => self + .eval_expr(scope, mods, state, lib, this_ptr, arg, level) + .map(|v| (v, arg.position())), + } + } + /// Call a function in normal function-call style. pub(crate) fn make_function_call( &self, @@ -1049,7 +1070,7 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, fn_name: &str, args_expr: &[Expr], - literal_args: &[(Dynamic, Position)], + constants: &[Dynamic], mut hashes: FnCallHashes, pos: Position, capture_scope: bool, @@ -1058,20 +1079,15 @@ impl Engine { // Handle call() - Redirect function call let redirected; let mut args_expr = args_expr; - let mut literal_args = literal_args; - let mut total_args = args_expr.len() + literal_args.len(); + let mut total_args = args_expr.len(); let mut curry = StaticVec::new(); let mut name = fn_name; match name { // Handle call() KEYWORD_FN_PTR_CALL if total_args >= 1 => { - let (arg, arg_pos) = args_expr.get(0).map_or_else( - || Ok(literal_args[0].clone()), - |arg| { - self.eval_expr(scope, mods, state, lib, this_ptr, arg, level) - .map(|v| (v, arg.position())) - }, + let (arg, arg_pos) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, )?; if !arg.is::() { @@ -1089,11 +1105,7 @@ impl Engine { name = &redirected; // Skip the first argument - if !args_expr.is_empty() { - args_expr = &args_expr[1..]; - } else { - literal_args = &literal_args[1..]; - } + args_expr = &args_expr[1..]; total_args -= 1; // Recalculate hash @@ -1106,12 +1118,8 @@ impl Engine { } // Handle Fn() KEYWORD_FN_PTR if total_args == 1 => { - let (arg, arg_pos) = args_expr.get(0).map_or_else( - || Ok(literal_args[0].clone()), - |arg| { - self.eval_expr(scope, mods, state, lib, this_ptr, arg, level) - .map(|v| (v, arg.position())) - }, + let (arg, arg_pos) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, )?; // Fn - only in function call style @@ -1125,12 +1133,8 @@ impl Engine { // Handle curry() KEYWORD_FN_PTR_CURRY if total_args > 1 => { - let (arg, arg_pos) = args_expr.get(0).map_or_else( - || Ok(literal_args[0].clone()), - |arg| { - self.eval_expr(scope, mods, state, lib, this_ptr, arg, level) - .map(|v| (v, arg.position())) - }, + let (arg, arg_pos) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, )?; if !arg.is::() { @@ -1143,15 +1147,12 @@ impl Engine { let (name, mut fn_curry) = arg.cast::().take_data(); // Append the new curried arguments to the existing list. - if !args_expr.is_empty() { - args_expr.iter().skip(1).try_for_each(|expr| { - self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) - .map(|value| fn_curry.push(value)) - })?; - fn_curry.extend(literal_args.iter().map(|(v, _)| v.clone())); - } else { - fn_curry.extend(literal_args.iter().skip(1).map(|(v, _)| v.clone())); - } + args_expr.iter().skip(1).try_for_each(|expr| match expr { + Expr::Stack(slot, _) => Ok(fn_curry.push(constants[*slot].clone())), + _ => self + .eval_expr(scope, mods, state, lib, this_ptr, expr, level) + .map(|value| fn_curry.push(value)), + })?; return Ok(FnPtr::new_unchecked(name, fn_curry).into()); } @@ -1159,9 +1160,8 @@ impl Engine { // Handle is_shared() #[cfg(not(feature = "no_closure"))] crate::engine::KEYWORD_IS_SHARED if total_args == 1 => { - let arg = args_expr.get(0).map_or_else( - || Ok(literal_args[0].0.clone()), - |arg| self.eval_expr(scope, mods, state, lib, this_ptr, arg, level), + let (arg, _) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, )?; return Ok(arg.is_shared().into()); } @@ -1169,27 +1169,17 @@ impl Engine { // Handle is_def_fn() #[cfg(not(feature = "no_function"))] crate::engine::KEYWORD_IS_DEF_FN if total_args == 2 => { - let (arg, arg_pos) = if !args_expr.is_empty() { - ( - self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?, - args_expr[0].position(), - ) - } else { - literal_args[0].clone() - }; + let (arg, arg_pos) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, + )?; let fn_name = arg .take_immutable_string() .map_err(|err| self.make_type_mismatch_err::(err, arg_pos))?; - let (arg, arg_pos) = if args_expr.len() > 1 { - ( - self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[1], level)?, - args_expr[1].position(), - ) - } else { - literal_args[if args_expr.is_empty() { 1 } else { 0 }].clone() - }; + let (arg, arg_pos) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, 1, + )?; let num_params = arg .as_int() @@ -1206,12 +1196,8 @@ impl Engine { // Handle is_def_var() KEYWORD_IS_DEF_VAR if total_args == 1 => { - let (arg, arg_pos) = args_expr.get(0).map_or_else( - || Ok(literal_args[0].clone()), - |arg| { - self.eval_expr(scope, mods, state, lib, this_ptr, arg, level) - .map(|v| (v, arg.position())) - }, + let (arg, arg_pos) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, )?; let var_name = arg .take_immutable_string() @@ -1223,12 +1209,8 @@ impl Engine { KEYWORD_EVAL if total_args == 1 => { // eval - only in function call style let prev_len = scope.len(); - let (script, script_pos) = args_expr.get(0).map_or_else( - || Ok(literal_args[0].clone()), - |script_expr| { - self.eval_expr(scope, mods, state, lib, this_ptr, script_expr, level) - .map(|v| (v, script_expr.position())) - }, + let (script, script_pos) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, )?; let script = script.take_immutable_string().map_err(|typ| { self.make_type_mismatch_err::(typ, script_pos) @@ -1276,7 +1258,7 @@ impl Engine { None }; - if args_expr.is_empty() && literal_args.is_empty() && curry.is_empty() { + if args_expr.is_empty() && curry.is_empty() { // No arguments args = Default::default(); } else { @@ -1288,11 +1270,12 @@ impl Engine { arg_values = args_expr .iter() .skip(1) - .map(|expr| { - self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) - .map(Dynamic::flatten) + .map(|expr| match expr { + Expr::Stack(slot, _) => Ok(constants[*slot].clone()), + _ => self + .eval_expr(scope, mods, state, lib, this_ptr, expr, level) + .map(Dynamic::flatten), }) - .chain(literal_args.iter().map(|(v, _)| Ok(v.clone()))) .collect::>()?; let (mut target, _pos) = @@ -1323,11 +1306,12 @@ impl Engine { // func(..., ...) arg_values = args_expr .iter() - .map(|expr| { - self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) - .map(Dynamic::flatten) + .map(|expr| match expr { + Expr::Stack(slot, _) => Ok(constants[*slot].clone()), + _ => self + .eval_expr(scope, mods, state, lib, this_ptr, expr, level) + .map(Dynamic::flatten), }) - .chain(literal_args.iter().map(|(v, _)| Ok(v.clone()))) .collect::>()?; args = curry.iter_mut().chain(arg_values.iter_mut()).collect(); @@ -1351,7 +1335,7 @@ impl Engine { namespace: &NamespaceRef, fn_name: &str, args_expr: &[Expr], - literal_args: &[(Dynamic, Position)], + constants: &[Dynamic], hash: u64, pos: Position, level: usize, @@ -1360,7 +1344,7 @@ impl Engine { let mut first_arg_value = None; let mut args: StaticVec<_>; - if args_expr.is_empty() && literal_args.is_empty() { + if args_expr.is_empty() { // No arguments args = Default::default(); } else { @@ -1375,13 +1359,15 @@ impl Engine { .map(|(i, expr)| { // Skip the first argument if i == 0 { - Ok(Default::default()) - } else { - self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) - .map(Dynamic::flatten) + return Ok(Default::default()); + } + match expr { + Expr::Stack(slot, _) => Ok(constants[*slot].clone()), + _ => self + .eval_expr(scope, mods, state, lib, this_ptr, expr, level) + .map(Dynamic::flatten), } }) - .chain(literal_args.iter().map(|(v, _)| Ok(v.clone()))) .collect::>()?; // Get target reference to first argument @@ -1412,11 +1398,12 @@ impl Engine { // func(..., ...) or func(mod::x, ...) arg_values = args_expr .iter() - .map(|expr| { - self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) - .map(Dynamic::flatten) + .map(|expr| match expr { + Expr::Stack(slot, _) => Ok(constants[*slot].clone()), + _ => self + .eval_expr(scope, mods, state, lib, this_ptr, expr, level) + .map(Dynamic::flatten), }) - .chain(literal_args.iter().map(|(v, _)| Ok(v.clone()))) .collect::>()?; args = arg_values.iter_mut().collect(); diff --git a/src/optimize.rs b/src/optimize.rs index a09a1557..2ce023fa 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -369,7 +369,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { && x.0.is_variable_access(true) && matches!(&x.2, Expr::FnCall(x2, _) if Token::lookup_from_syntax(&x2.name).map(|t| t.has_op_assignment()).unwrap_or(false) - && x2.args_count() == 2 && x2.args.len() >= 1 + && x2.args.len() == 2 && x2.args[0].get_variable_name(true) == x.0.get_variable_name(true) ) => { @@ -379,12 +379,15 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { let op = Token::lookup_from_syntax(&x2.name).unwrap(); let op_assignment = op.make_op_assignment().unwrap(); x.1 = Some(OpAssignment::new(op_assignment)); - x.2 = if x2.args.len() > 1 { - mem::take(&mut x2.args[1]) + + let value = mem::take(&mut x2.args[1]); + + if let Expr::Stack(slot, pos) = value { + let value = mem::take(x2.constants.get_mut(slot).unwrap()); + x.2 = Expr::from_dynamic(value, pos); } else { - let (value, pos) = mem::take(&mut x2.literal_args[0]); - Expr::DynamicConstant(Box::new(value), pos) - }; + x.2 = value; + } } _ => unreachable!(), } @@ -905,14 +908,23 @@ fn optimize_expr(expr: &mut Expr, state: &mut State, _chaining: bool) { Expr::FnCall(x, pos) if !x.is_qualified() // Non-qualified && state.optimization_level == OptimizationLevel::Simple // simple optimizations - && x.args_count() == 1 - && x.literal_args.len() == 1 - && x.literal_args[0].0.is::() + && x.args.len() == 1 + && x.args[0].is_constant() && x.name == KEYWORD_FN_PTR => { - state.set_dirty(); - let fn_ptr = FnPtr::new_unchecked(mem::take(&mut x.literal_args[0].0).as_str_ref().unwrap().into(), Default::default()); - *expr = Expr::DynamicConstant(Box::new(fn_ptr.into()), *pos); + let fn_name = match x.args[0] { + Expr::Stack(slot, _) => Some(x.constants[slot].clone()), + Expr::StringConstant(ref s, _) => Some(s.clone().into()), + _ => None + }; + + if let Some(fn_name) = fn_name { + if fn_name.is::() { + state.set_dirty(); + let fn_ptr = FnPtr::new_unchecked(fn_name.as_str_ref().unwrap().into(), Default::default()); + *expr = Expr::DynamicConstant(Box::new(fn_ptr.into()), *pos); + } + } } // Do not call some special keywords @@ -924,13 +936,14 @@ fn optimize_expr(expr: &mut Expr, state: &mut State, _chaining: bool) { Expr::FnCall(x, pos) if !x.is_qualified() // Non-qualified && state.optimization_level == OptimizationLevel::Simple // simple optimizations - && x.args_count() == 2 // binary call + && x.args.len() == 2 // binary call && x.args.iter().all(Expr::is_constant) // all arguments are constants //&& !is_valid_identifier(x.name.chars()) // cannot be scripted => { - let mut arg_values: StaticVec<_> = x.args.iter().map(|e| e.get_constant_value().unwrap()) - .chain(x.literal_args.iter().map(|(v, _)| v).cloned()) - .collect(); + let mut arg_values: StaticVec<_> = x.args.iter().map(|e| match e { + Expr::Stack(slot, _) => x.constants[*slot].clone(), + _ => e.get_constant_value().unwrap() + }).collect(); let arg_types: StaticVec<_> = arg_values.iter().map(Dynamic::type_id).collect(); @@ -953,15 +966,14 @@ fn optimize_expr(expr: &mut Expr, state: &mut State, _chaining: bool) { x.args.iter_mut().for_each(|a| optimize_expr(a, state, false)); - // Move constant arguments to the right - while x.args.last().map(Expr::is_constant).unwrap_or(false) { - let arg = x.args.pop().unwrap(); - let arg_pos = arg.position(); - x.literal_args.insert(0, (arg.get_constant_value().unwrap(), arg_pos)); + // Move constant arguments + for arg in x.args.iter_mut() { + if let Some(value) = arg.get_constant_value() { + state.set_dirty(); + x.constants.push(value); + *arg = Expr::Stack(x.constants.len()-1, arg.position()); + } } - - x.args.shrink_to_fit(); - x.literal_args.shrink_to_fit(); } // Eagerly call functions @@ -972,14 +984,15 @@ fn optimize_expr(expr: &mut Expr, state: &mut State, _chaining: bool) { => { // First search for script-defined functions (can override built-in) #[cfg(not(feature = "no_function"))] - let has_script_fn = state.lib.iter().any(|&m| m.get_script_fn(x.name.as_ref(), x.args_count()).is_some()); + let has_script_fn = state.lib.iter().any(|&m| m.get_script_fn(x.name.as_ref(), x.args.len()).is_some()); #[cfg(feature = "no_function")] let has_script_fn = false; if !has_script_fn { - let mut arg_values: StaticVec<_> = x.args.iter().map(|e| e.get_constant_value().unwrap()) - .chain(x.literal_args.iter().map(|(v, _)| v).cloned()) - .collect(); + let mut arg_values: StaticVec<_> = x.args.iter().map(|e| match e { + Expr::Stack(slot, _) => x.constants[*slot].clone(), + _ => e.get_constant_value().unwrap() + }).collect(); // Save the typename of the first argument if it is `type_of()` // This is to avoid `call_args` being passed into the closure @@ -990,15 +1003,12 @@ fn optimize_expr(expr: &mut Expr, state: &mut State, _chaining: bool) { }; if let Some(mut result) = call_fn_with_constant_arguments(&state, x.name.as_ref(), &mut arg_values) - .or_else(|| { - if !arg_for_type_of.is_empty() { - // Handle `type_of()` - Some(arg_for_type_of.to_string().into()) - } else { - None - } - }) - .map(Expr::from) + .or_else(|| if !arg_for_type_of.is_empty() { + // Handle `type_of()` + Some(arg_for_type_of.to_string().into()) + } else { + None + }).map(Expr::from) { state.set_dirty(); result.set_position(*pos); @@ -1011,19 +1021,16 @@ fn optimize_expr(expr: &mut Expr, state: &mut State, _chaining: bool) { } // id(args ..) -> optimize function call arguments - Expr::FnCall(x, _) => { - x.args.iter_mut().for_each(|a| optimize_expr(a, state, false)); + Expr::FnCall(x, _) => for arg in x.args.iter_mut() { + optimize_expr(arg, state, false); - // Move constant arguments to the right - while x.args.last().map(Expr::is_constant).unwrap_or(false) { - let arg = x.args.pop().unwrap(); - let arg_pos = arg.position(); - x.literal_args.insert(0, (arg.get_constant_value().unwrap(), arg_pos)); + // Move constant arguments + if let Some(value) = arg.get_constant_value() { + state.set_dirty(); + x.constants.push(value); + *arg = Expr::Stack(x.constants.len()-1, arg.position()); } - - x.args.shrink_to_fit(); - x.literal_args.shrink_to_fit(); - } + }, // constant-name Expr::Variable(_, pos, x) if x.1.is_none() && state.find_constant(&x.2).is_some() => { diff --git a/src/parser.rs b/src/parser.rs index 8e36faeb..0d1e4f74 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1598,8 +1598,8 @@ fn make_dot_expr( Expr::FnCall(mut func, func_pos) => { // Recalculate hash func.hashes = FnCallHashes::from_script_and_native( - calc_fn_hash(&func.name, func.args_count()), - calc_fn_hash(&func.name, func.args_count() + 1), + calc_fn_hash(&func.name, func.args.len()), + calc_fn_hash(&func.name, func.args.len() + 1), ); let rhs = Expr::Dot( @@ -1630,7 +1630,7 @@ fn make_dot_expr( } // lhs.Fn() or lhs.eval() (_, Expr::FnCall(x, pos)) - if x.is_args_empty() + if x.args.is_empty() && [crate::engine::KEYWORD_FN_PTR, crate::engine::KEYWORD_EVAL] .contains(&x.name.as_ref()) => { @@ -1654,8 +1654,8 @@ fn make_dot_expr( (lhs, Expr::FnCall(mut func, func_pos)) => { // Recalculate hash func.hashes = FnCallHashes::from_script_and_native( - calc_fn_hash(&func.name, func.args_count()), - calc_fn_hash(&func.name, func.args_count() + 1), + calc_fn_hash(&func.name, func.args.len()), + calc_fn_hash(&func.name, func.args.len() + 1), ); let rhs = Expr::FnCall(func, func_pos); Expr::Dot(Box::new(BinaryExpr { lhs, rhs }), op_pos) From ae9f4b5b71796d9ac5cb06832c0dd9dcbbb19cfd Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 8 Jun 2021 15:48:55 +0800 Subject: [PATCH 14/24] Remove collect() with exact sizes. --- src/engine.rs | 54 ++++++++--------- src/fn_call.rs | 147 ++++++++++++++++++++++------------------------- src/fn_native.rs | 29 ++++------ 3 files changed, 104 insertions(+), 126 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index dc4323a5..9097c7df 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1667,22 +1667,19 @@ impl Engine { match expr { #[cfg(not(feature = "no_object"))] Expr::FnCall(x, _) if _parent_chain_type == ChainType::Dot && !x.is_qualified() => { - let arg_values = x - .args - .iter() - .map(|arg_expr| match arg_expr { - Expr::Stack(slot, _) => Ok(x.constants[*slot].clone()), - _ => self - .eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level) - .map(Dynamic::flatten), - }) - .collect::, _>>()?; + let crate::ast::FnCallExpr { + args, constants, .. + } = x.as_ref(); + let mut arg_values = StaticVec::with_capacity(args.len()); - let pos = x - .args - .get(0) - .map(|arg_expr| arg_expr.position()) - .unwrap_or_default(); + for index in 0..args.len() { + let (value, _) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args, constants, index, + )?; + arg_values.push(value.flatten()); + } + + let pos = x.args.get(0).map(Expr::position).unwrap_or_default(); idx_values.push((arg_values, pos).into()); } @@ -1712,22 +1709,19 @@ impl Engine { Expr::FnCall(x, _) if _parent_chain_type == ChainType::Dot && !x.is_qualified() => { - let arg_values = x - .args - .iter() - .map(|arg_expr| match arg_expr { - Expr::Stack(slot, _) => Ok(x.constants[*slot].clone()), - _ => self - .eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level) - .map(Dynamic::flatten), - }) - .collect::, _>>()?; + let crate::ast::FnCallExpr { + args, constants, .. + } = x.as_ref(); + let mut arg_values = StaticVec::with_capacity(args.len()); - let pos = x - .args - .get(0) - .map(|arg_expr| arg_expr.position()) - .unwrap_or_default(); + for index in 0..args.len() { + let (value, _) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args, constants, index, + )?; + arg_values.push(value.flatten()); + } + + let pos = x.args.get(0).map(Expr::position).unwrap_or_default(); (arg_values, pos).into() } diff --git a/src/fn_call.rs b/src/fn_call.rs index 201e31b1..e3899089 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -24,7 +24,6 @@ use std::prelude::v1::*; use std::{ any::{type_name, TypeId}, convert::TryFrom, - iter::once, mem, }; @@ -515,11 +514,12 @@ impl Engine { ); // Merge in encapsulated environment, if any - let lib_merged: StaticVec<_>; + let mut lib_merged = StaticVec::with_capacity(lib.len() + 1); let (unified_lib, unified) = if let Some(ref env_lib) = fn_def.lib { state.push_fn_resolution_cache(); - lib_merged = once(env_lib.as_ref()).chain(lib.iter().cloned()).collect(); + lib_merged.push(env_lib.as_ref()); + lib_merged.extend(lib.iter().cloned()); (lib_merged.as_ref(), true) } else { (lib, false) @@ -911,8 +911,11 @@ impl Engine { // Recalculate hashes let new_hash = FnCallHashes::from_script(calc_fn_hash(fn_name, args_len)); // Arguments are passed as-is, adding the curried arguments - let mut curry: StaticVec<_> = fn_ptr.curry().iter().cloned().collect(); - let mut args: StaticVec<_> = curry.iter_mut().chain(call_args.iter_mut()).collect(); + let mut curry = StaticVec::with_capacity(fn_ptr.num_curried()); + curry.extend(fn_ptr.curry().iter().cloned()); + let mut args = StaticVec::with_capacity(curry.len() + call_args.len()); + args.extend(curry.iter_mut()); + args.extend(call_args.iter_mut()); // Map it to name(args) in function-call style self.exec_fn_call( @@ -945,11 +948,12 @@ impl Engine { calc_fn_hash(fn_name, args_len + 1), ); // Replace the first argument with the object pointer, adding the curried arguments - let mut curry: StaticVec<_> = fn_ptr.curry().iter().cloned().collect(); - let mut args: StaticVec<_> = once(target.as_mut()) - .chain(curry.iter_mut()) - .chain(call_args.iter_mut()) - .collect(); + let mut curry = StaticVec::with_capacity(fn_ptr.num_curried()); + curry.extend(fn_ptr.curry().iter().cloned()); + let mut args = StaticVec::with_capacity(curry.len() + call_args.len() + 1); + args.push(target.as_mut()); + args.extend(curry.iter_mut()); + args.extend(call_args.iter_mut()); // Map it to name(args) in function-call style self.exec_fn_call( @@ -1020,8 +1024,9 @@ impl Engine { }; // Attached object pointer in front of the arguments - let mut args: StaticVec<_> = - once(target.as_mut()).chain(call_args.iter_mut()).collect(); + let mut args = StaticVec::with_capacity(call_args.len() + 1); + args.push(target.as_mut()); + args.extend(call_args.iter_mut()); self.exec_fn_call( mods, state, lib, fn_name, hash, &mut args, is_ref, true, pos, None, level, @@ -1039,8 +1044,9 @@ impl Engine { Ok((result, updated)) } + /// Evaluate an argument. #[inline(always)] - fn get_arg_value( + pub(crate) fn get_arg_value( &self, scope: &mut Scope, mods: &mut Imports, @@ -1147,12 +1153,12 @@ impl Engine { let (name, mut fn_curry) = arg.cast::().take_data(); // Append the new curried arguments to the existing list. - args_expr.iter().skip(1).try_for_each(|expr| match expr { - Expr::Stack(slot, _) => Ok(fn_curry.push(constants[*slot].clone())), - _ => self - .eval_expr(scope, mods, state, lib, this_ptr, expr, level) - .map(|value| fn_curry.push(value)), - })?; + for index in 1..args_expr.len() { + let (value, _) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, index, + )?; + fn_curry.push(value); + } return Ok(FnPtr::new_unchecked(name, fn_curry).into()); } @@ -1249,8 +1255,8 @@ impl Engine { } // Normal function call - except for Fn, curry, call and eval (handled above) - let mut arg_values: StaticVec<_>; - let mut args: StaticVec<_>; + let mut arg_values = StaticVec::with_capacity(args_expr.len()); + let mut args = StaticVec::with_capacity(args_expr.len() + curry.len()); let mut is_ref = false; let capture = if capture_scope && !scope.is_empty() { Some(scope.clone_visible()) @@ -1260,23 +1266,18 @@ impl Engine { if args_expr.is_empty() && curry.is_empty() { // No arguments - args = Default::default(); } else { // If the first argument is a variable, and there is no curried arguments, // convert to method-call style in order to leverage potential &mut first argument and // avoid cloning the value if curry.is_empty() && !args_expr.is_empty() && args_expr[0].is_variable_access(false) { // func(x, ...) -> x.func(...) - arg_values = args_expr - .iter() - .skip(1) - .map(|expr| match expr { - Expr::Stack(slot, _) => Ok(constants[*slot].clone()), - _ => self - .eval_expr(scope, mods, state, lib, this_ptr, expr, level) - .map(Dynamic::flatten), - }) - .collect::>()?; + for index in 1..args_expr.len() { + let (value, _) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, index, + )?; + arg_values.push(value.flatten()); + } let (mut target, _pos) = self.search_namespace(scope, mods, state, lib, this_ptr, &args_expr[0])?; @@ -1293,28 +1294,26 @@ impl Engine { #[cfg(feature = "no_closure")] let target_is_shared = false; - args = if target_is_shared || target.is_value() { + if target_is_shared || target.is_value() { arg_values.insert(0, target.take_or_clone().flatten()); - arg_values.iter_mut().collect() + args.extend(arg_values.iter_mut()) } else { // Turn it into a method call only if the object is not shared and not a simple value is_ref = true; let obj_ref = target.take_ref().expect("never fails because `target` is a reference if it is not a value and not shared"); - once(obj_ref).chain(arg_values.iter_mut()).collect() - }; + args.push(obj_ref); + args.extend(arg_values.iter_mut()); + } } else { // func(..., ...) - arg_values = args_expr - .iter() - .map(|expr| match expr { - Expr::Stack(slot, _) => Ok(constants[*slot].clone()), - _ => self - .eval_expr(scope, mods, state, lib, this_ptr, expr, level) - .map(Dynamic::flatten), - }) - .collect::>()?; - - args = curry.iter_mut().chain(arg_values.iter_mut()).collect(); + for index in 0..args_expr.len() { + let (value, _) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, index, + )?; + arg_values.push(value.flatten()); + } + args.extend(curry.iter_mut()); + args.extend(arg_values.iter_mut()); } } @@ -1340,35 +1339,28 @@ impl Engine { pos: Position, level: usize, ) -> RhaiResult { - let mut arg_values: StaticVec<_>; + let mut arg_values = StaticVec::with_capacity(args_expr.len()); + let mut args = StaticVec::with_capacity(args_expr.len()); let mut first_arg_value = None; - let mut args: StaticVec<_>; if args_expr.is_empty() { // No arguments - args = Default::default(); } else { // See if the first argument is a variable (not namespace-qualified). // If so, convert to method-call style in order to leverage potential // &mut first argument and avoid cloning the value if !args_expr.is_empty() && args_expr[0].is_variable_access(true) { // func(x, ...) -> x.func(...) - arg_values = args_expr - .iter() - .enumerate() - .map(|(i, expr)| { - // Skip the first argument - if i == 0 { - return Ok(Default::default()); - } - match expr { - Expr::Stack(slot, _) => Ok(constants[*slot].clone()), - _ => self - .eval_expr(scope, mods, state, lib, this_ptr, expr, level) - .map(Dynamic::flatten), - } - }) - .collect::>()?; + for index in 0..args_expr.len() { + if index == 0 { + arg_values.push(Default::default()); + } else { + let (value, _) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, index, + )?; + arg_values.push(value.flatten()); + } + } // Get target reference to first argument let (target, _pos) = @@ -1384,7 +1376,7 @@ impl Engine { if target_is_shared || target.is_value() { arg_values[0] = target.take_or_clone().flatten(); - args = arg_values.iter_mut().collect(); + args.extend(arg_values.iter_mut()); } else { // Turn it into a method call only if the object is not shared and not a simple value let (first, rest) = arg_values @@ -1392,21 +1384,18 @@ impl Engine { .expect("never fails because the arguments list is not empty"); first_arg_value = Some(first); let obj_ref = target.take_ref().expect("never fails because `target` is a reference if it is not a value and not shared"); - args = once(obj_ref).chain(rest.iter_mut()).collect(); + args.push(obj_ref); + args.extend(rest.iter_mut()); } } else { // func(..., ...) or func(mod::x, ...) - arg_values = args_expr - .iter() - .map(|expr| match expr { - Expr::Stack(slot, _) => Ok(constants[*slot].clone()), - _ => self - .eval_expr(scope, mods, state, lib, this_ptr, expr, level) - .map(Dynamic::flatten), - }) - .collect::>()?; - - args = arg_values.iter_mut().collect(); + for index in 0..args_expr.len() { + let (value, _) = self.get_arg_value( + scope, mods, state, lib, this_ptr, level, args_expr, constants, index, + )?; + arg_values.push(value.flatten()); + } + args.extend(arg_values.iter_mut()); } } diff --git a/src/fn_native.rs b/src/fn_native.rs index 06dccf38..362874d8 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -344,28 +344,23 @@ impl FnPtr { this_ptr: Option<&mut Dynamic>, mut arg_values: impl AsMut<[Dynamic]>, ) -> RhaiResult { - let mut args_data: StaticVec<_>; + let mut arg_values = arg_values.as_mut(); + let mut args_data; - let arg_values = if self.curry().is_empty() { - arg_values.as_mut() - } else { - args_data = self - .curry() - .iter() - .cloned() - .chain(arg_values.as_mut().iter_mut().map(mem::take)) - .collect(); - - args_data.as_mut() + if self.num_curried() > 0 { + args_data = StaticVec::with_capacity(self.num_curried() + arg_values.len()); + args_data.extend(self.curry().iter().cloned()); + args_data.extend(arg_values.iter_mut().map(mem::take)); + arg_values = args_data.as_mut(); }; let is_method = this_ptr.is_some(); - let mut args: StaticVec<_> = if let Some(obj) = this_ptr { - once(obj).chain(arg_values.iter_mut()).collect() - } else { - arg_values.iter_mut().collect() - }; + let mut args = StaticVec::with_capacity(arg_values.len() + 1); + if let Some(obj) = this_ptr { + args.push(obj); + } + args.extend(arg_values.iter_mut()); ctx.call_fn_dynamic_raw(self.fn_name(), is_method, &mut args) } From bcf82dafcbd026139165a9300f90499c00c750d3 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 8 Jun 2021 19:30:13 +0800 Subject: [PATCH 15/24] Fix tests. --- src/fn_native.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/fn_native.rs b/src/fn_native.rs index 362874d8..4fd1acb1 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -12,9 +12,7 @@ use crate::{ use std::prelude::v1::*; use std::{ convert::{TryFrom, TryInto}, - fmt, - iter::once, - mem, + fmt, mem, }; /// Trait that maps to `Send + Sync` only under the `sync` feature. From 6397ce671ee35e63b0bf9cd8f2fbce1eca506499 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 8 Jun 2021 23:40:10 +0800 Subject: [PATCH 16/24] Simplify code. --- src/fn_call.rs | 80 +++++++++++++++----------------------------------- 1 file changed, 23 insertions(+), 57 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index e3899089..0fad230e 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -17,7 +17,7 @@ use crate::{ }; use crate::{ calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, Engine, EvalAltResult, FnPtr, - ImmutableString, Module, ParseErrorType, Position, Scope, StaticVec, + Identifier, ImmutableString, Module, ParseErrorType, Position, Scope, StaticVec, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -627,6 +627,12 @@ impl Engine { _capture_scope: Option, _level: usize, ) -> Result<(Dynamic, bool), Box> { + #[inline(always)] + fn no_method_err(name: &str, pos: Position) -> Result<(Dynamic, bool), Box> { + let msg = format!("'{0}' should not be called this way. Try {0}(...);", name); + EvalAltResult::ErrorRuntime(msg.into(), pos).into() + } + // Check for data race. #[cfg(not(feature = "no_closure"))] ensure_no_data_race(fn_name, args, is_ref)?; @@ -638,7 +644,7 @@ impl Engine { return Ok(( self.map_type_name(args[0].type_name()).to_string().into(), false, - )); + )) } // Handle is_def_fn() @@ -668,39 +674,15 @@ impl Engine { // Handle is_shared() #[cfg(not(feature = "no_closure"))] crate::engine::KEYWORD_IS_SHARED if args.len() == 1 => { - return EvalAltResult::ErrorRuntime( - format!( - "'{}' should not be called this way. Try {}(...);", - fn_name, fn_name - ) - .into(), - pos, - ) - .into() + return no_method_err(fn_name, pos) } KEYWORD_FN_PTR | KEYWORD_EVAL | KEYWORD_IS_DEF_VAR if args.len() == 1 => { - return EvalAltResult::ErrorRuntime( - format!( - "'{}' should not be called this way. Try {}(...);", - fn_name, fn_name - ) - .into(), - pos, - ) - .into() + return no_method_err(fn_name, pos) } KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY if !args.is_empty() => { - return EvalAltResult::ErrorRuntime( - format!( - "'{}' should not be called this way. Try {}(...);", - fn_name, fn_name - ) - .into(), - pos, - ) - .into() + return no_method_err(fn_name, pos) } _ => (), @@ -798,17 +780,8 @@ impl Engine { } // Native function call - self.call_native_fn( - mods, - state, - lib, - fn_name, - hash.native_hash(), - args, - is_ref, - false, - pos, - ) + let hash = hash.native_hash(); + self.call_native_fn(mods, state, lib, fn_name, hash, args, is_ref, false, pos) } /// Evaluate a list of statements with no `this` pointer. @@ -983,7 +956,7 @@ impl Engine { .curry() .iter() .cloned() - .chain(call_args.iter_mut().map(|v| mem::take(v))) + .chain(call_args.iter_mut().map(mem::take)) .collect(), ) } @@ -1215,21 +1188,14 @@ impl Engine { KEYWORD_EVAL if total_args == 1 => { // eval - only in function call style let prev_len = scope.len(); - let (script, script_pos) = self.get_arg_value( + let (value, pos) = self.get_arg_value( scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, )?; - let script = script.take_immutable_string().map_err(|typ| { - self.make_type_mismatch_err::(typ, script_pos) - })?; - let result = self.eval_script_expr_in_place( - scope, - mods, - state, - lib, - &script, - script_pos, - level + 1, - ); + let script = &value + .take_immutable_string() + .map_err(|typ| self.make_type_mismatch_err::(typ, pos))?; + let result = + self.eval_script_expr_in_place(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. @@ -1243,7 +1209,7 @@ impl Engine { state .source .as_ref() - .map(|s| s.to_string()) + .map(Identifier::to_string) .unwrap_or_default(), err, pos, @@ -1294,7 +1260,7 @@ impl Engine { #[cfg(feature = "no_closure")] let target_is_shared = false; - if target_is_shared || target.is_value() { + if target_is_shared || target.is_temp_value() { arg_values.insert(0, target.take_or_clone().flatten()); args.extend(arg_values.iter_mut()) } else { @@ -1374,7 +1340,7 @@ impl Engine { #[cfg(feature = "no_closure")] let target_is_shared = false; - if target_is_shared || target.is_value() { + if target_is_shared || target.is_temp_value() { arg_values[0] = target.take_or_clone().flatten(); args.extend(arg_values.iter_mut()); } else { From c3eb6d65f617a5abcc31149ac633dab5bcb633ce Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 8 Jun 2021 23:40:21 +0800 Subject: [PATCH 17/24] Refine Target variant names. --- src/engine.rs | 97 +++++++++++++++++++++++++-------------------------- 1 file changed, 47 insertions(+), 50 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 9097c7df..0333d4b0 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -328,13 +328,13 @@ impl From<(Dynamic, Position)> for ChainArgument { #[derive(Debug)] pub enum Target<'a> { /// The target is a mutable reference to a `Dynamic` value somewhere. - Ref(&'a mut Dynamic), + RefMut(&'a mut Dynamic), /// The target is a mutable reference to a Shared `Dynamic` value. /// It holds both the access guard and the original shared value. #[cfg(not(feature = "no_closure"))] LockGuard((crate::dynamic::DynamicWriteLock<'a, Dynamic>, Dynamic)), /// The target is a temporary `Dynamic` value (i.e. the mutation can cause no side effects). - Value(Dynamic), + TempValue(Dynamic), /// The target is a bit inside an [`INT`][crate::INT]. /// This is necessary because directly pointing to a bit inside an [`INT`][crate::INT] is impossible. #[cfg(not(feature = "no_index"))] @@ -351,25 +351,24 @@ impl<'a> Target<'a> { #[inline(always)] pub fn is_ref(&self) -> bool { match self { - Self::Ref(_) => true, + Self::RefMut(_) => true, #[cfg(not(feature = "no_closure"))] Self::LockGuard(_) => true, - Self::Value(_) => false, + Self::TempValue(_) => false, #[cfg(not(feature = "no_index"))] Self::BitField(_, _, _) => false, #[cfg(not(feature = "no_index"))] Self::StringChar(_, _, _) => false, } } - /// Is the `Target` an owned value? - #[allow(dead_code)] + /// Is the `Target` a temp value? #[inline(always)] - pub fn is_value(&self) -> bool { + pub fn is_temp_value(&self) -> bool { match self { - Self::Ref(_) => false, + Self::RefMut(_) => false, #[cfg(not(feature = "no_closure"))] Self::LockGuard(_) => false, - Self::Value(_) => true, + Self::TempValue(_) => true, #[cfg(not(feature = "no_index"))] Self::BitField(_, _, _) => false, #[cfg(not(feature = "no_index"))] @@ -381,10 +380,10 @@ impl<'a> Target<'a> { #[inline(always)] pub fn is_shared(&self) -> bool { match self { - Self::Ref(r) => r.is_shared(), + Self::RefMut(r) => r.is_shared(), #[cfg(not(feature = "no_closure"))] Self::LockGuard(_) => true, - Self::Value(r) => r.is_shared(), + Self::TempValue(r) => r.is_shared(), #[cfg(not(feature = "no_index"))] Self::BitField(_, _, _) => false, #[cfg(not(feature = "no_index"))] @@ -396,10 +395,10 @@ impl<'a> Target<'a> { #[inline(always)] pub fn is(&self) -> bool { match self { - Self::Ref(r) => r.is::(), + Self::RefMut(r) => r.is::(), #[cfg(not(feature = "no_closure"))] Self::LockGuard((r, _)) => r.is::(), - Self::Value(r) => r.is::(), + Self::TempValue(r) => r.is::(), #[cfg(not(feature = "no_index"))] Self::BitField(_, _, _) => TypeId::of::() == TypeId::of::(), #[cfg(not(feature = "no_index"))] @@ -410,10 +409,10 @@ impl<'a> Target<'a> { #[inline(always)] pub fn take_or_clone(self) -> Dynamic { match self { - Self::Ref(r) => r.clone(), // Referenced value is cloned + Self::RefMut(r) => r.clone(), // Referenced value is cloned #[cfg(not(feature = "no_closure"))] Self::LockGuard((_, orig)) => orig, // Original value is simply taken - Self::Value(v) => v, // Owned value is simply taken + Self::TempValue(v) => v, // Owned value is simply taken #[cfg(not(feature = "no_index"))] Self::BitField(_, _, value) => value, // Boolean is taken #[cfg(not(feature = "no_index"))] @@ -424,7 +423,7 @@ impl<'a> Target<'a> { #[inline(always)] pub fn take_ref(self) -> Option<&'a mut Dynamic> { match self { - Self::Ref(r) => Some(r), + Self::RefMut(r) => Some(r), _ => None, } } @@ -438,32 +437,28 @@ impl<'a> Target<'a> { #[inline(always)] pub fn propagate_changed_value(&mut self) -> Result<(), Box> { match self { - Self::Ref(_) | Self::Value(_) => Ok(()), + Self::RefMut(_) | Self::TempValue(_) => Ok(()), #[cfg(not(feature = "no_closure"))] Self::LockGuard(_) => Ok(()), #[cfg(not(feature = "no_index"))] Self::BitField(_, _, value) => { let new_val = value.clone(); - self.set_value(new_val, Position::NONE) + self.set_value(new_val) } #[cfg(not(feature = "no_index"))] Self::StringChar(_, _, ch) => { let new_val = ch.clone(); - self.set_value(new_val, Position::NONE) + self.set_value(new_val) } } } /// Update the value of the `Target`. - pub fn set_value( - &mut self, - new_val: Dynamic, - _pos: Position, - ) -> Result<(), Box> { + fn set_value(&mut self, new_val: Dynamic) -> Result<(), Box> { match self { - Self::Ref(r) => **r = new_val, + Self::RefMut(r) => **r = new_val, #[cfg(not(feature = "no_closure"))] Self::LockGuard((r, _)) => **r = new_val, - Self::Value(_) => panic!("cannot update a value"), + Self::TempValue(_) => panic!("cannot update a value"), #[cfg(not(feature = "no_index"))] Self::BitField(value, index, _) => { let value = &mut *value @@ -475,7 +470,7 @@ impl<'a> Target<'a> { Box::new(EvalAltResult::ErrorMismatchDataType( "bool".to_string(), err.to_string(), - _pos, + Position::NONE, )) })?; @@ -501,7 +496,7 @@ impl<'a> Target<'a> { Box::new(EvalAltResult::ErrorMismatchDataType( "char".to_string(), err.to_string(), - _pos, + Position::NONE, )) })?; @@ -534,7 +529,7 @@ impl<'a> From<&'a mut Dynamic> for Target<'a> { )); } - Self::Ref(value) + Self::RefMut(value) } } @@ -544,10 +539,10 @@ impl Deref for Target<'_> { #[inline(always)] fn deref(&self) -> &Dynamic { match self { - Self::Ref(r) => *r, + Self::RefMut(r) => *r, #[cfg(not(feature = "no_closure"))] Self::LockGuard((r, _)) => &**r, - Self::Value(ref r) => r, + Self::TempValue(ref r) => r, #[cfg(not(feature = "no_index"))] Self::BitField(_, _, ref r) => r, #[cfg(not(feature = "no_index"))] @@ -567,10 +562,10 @@ impl DerefMut for Target<'_> { #[inline(always)] fn deref_mut(&mut self) -> &mut Dynamic { match self { - Self::Ref(r) => *r, + Self::RefMut(r) => *r, #[cfg(not(feature = "no_closure"))] Self::LockGuard((r, _)) => r.deref_mut(), - Self::Value(ref mut r) => r, + Self::TempValue(ref mut r) => r, #[cfg(not(feature = "no_index"))] Self::BitField(_, _, ref mut r) => r, #[cfg(not(feature = "no_index"))] @@ -589,7 +584,7 @@ impl AsMut for Target<'_> { impl> From for Target<'_> { #[inline(always)] fn from(value: T) -> Self { - Self::Value(value.into()) + Self::TempValue(value.into()) } } @@ -1252,8 +1247,8 @@ impl Engine { Ok(obj_ptr) => { self.eval_op_assignment( mods, state, lib, op_info, op_pos, obj_ptr, root, new_val, - new_pos, - )?; + ) + .map_err(|err| err.fill_position(new_pos))?; return Ok((Dynamic::UNIT, true)); } // Can't index - try to call an index setter @@ -1315,8 +1310,9 @@ impl Engine { let ((new_val, new_pos), (op_info, op_pos)) = new_val.expect("never fails because `new_val` is `Some`"); self.eval_op_assignment( - mods, state, lib, op_info, op_pos, val, root, new_val, new_pos, - )?; + mods, state, lib, op_info, op_pos, val, root, new_val, + ) + .map_err(|err| err.fill_position(new_pos))?; Ok((Dynamic::UNIT, true)) } // {xxx:map}.id @@ -1363,8 +1359,9 @@ impl Engine { })?; let obj_ptr = (&mut orig_val).into(); self.eval_op_assignment( - mods, state, lib, op_info, op_pos, obj_ptr, root, new_val, new_pos, - )?; + mods, state, lib, op_info, op_pos, obj_ptr, root, new_val, + ) + .map_err(|err| err.fill_position(new_pos))?; new_val = orig_val; } @@ -1771,7 +1768,7 @@ impl Engine { } /// Get the value at the indexed position of a base type. - /// [`Position`] in [`EvalAltResult`] may be None and should be set afterwards. + /// [`Position`] in [`EvalAltResult`] may be [`NONE`][Position::NONE] and should be set afterwards. #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] fn get_indexed_mut<'t>( &self, @@ -2001,8 +1998,8 @@ impl Engine { (&mut result).into(), ("", Position::NONE), item, - expr.position(), - )?; + ) + .map_err(|err| err.fill_position(expr.position()))?; pos = expr.position(); } @@ -2211,6 +2208,8 @@ impl Engine { result } + /// Evaluate an op-assignment statement. + /// [`Position`] in [`EvalAltResult`] is [`NONE`][Position::NONE] and should be set afterwards. pub(crate) fn eval_op_assignment( &self, mods: &mut Imports, @@ -2221,7 +2220,6 @@ impl Engine { mut target: Target, root: (&str, Position), mut new_val: Dynamic, - new_val_pos: Position, ) -> Result<(), Box> { if target.is_read_only() { // Assignment to constant variable @@ -2271,14 +2269,12 @@ impl Engine { err => return err.map(|_| ()), } } - - target.propagate_changed_value()?; } else { // Normal assignment - target.set_value(new_val, new_val_pos)?; + *target.as_mut() = new_val; } - Ok(()) + target.propagate_changed_value() } /// Evaluate a statement. @@ -2339,8 +2335,9 @@ impl Engine { lhs_ptr, (var_name, pos), rhs_val, - rhs_expr.position(), - )?; + ) + .map_err(|err| err.fill_position(rhs_expr.position()))?; + Ok(Dynamic::UNIT) } From a5031969ca7a9d7c33029674990306ca323b14aa Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 10 Jun 2021 10:16:39 +0800 Subject: [PATCH 18/24] New custom syntax expression types. --- CHANGELOG.md | 1 + src/ast.rs | 10 ++++---- src/optimize.rs | 16 ++++++------ src/parse_error.rs | 4 +++ src/parser.rs | 63 +++++++++++++++++++++++++++++++++++++++++++--- src/syntax.rs | 24 +++++++++++++++--- tests/syntax.rs | 23 ++++++++++------- 7 files changed, 113 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e4cfb75..844eefc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ New features * New syntax for `for` statement to include counter variable. * An integer value can now be indexed to get/set a single bit. * The `bits` method of an integer can be used to iterate through its bits. +* New `$bool$`, `$int$`, `$float$` and `$string$` expression types for custom syntax. Version 0.20.2 diff --git a/src/ast.rs b/src/ast.rs index a6b3fdb7..1834a965 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1819,11 +1819,11 @@ impl fmt::Debug for Expr { } impl Expr { - /// Get the [`Dynamic`] value of a constant expression. + /// Get the [`Dynamic`] value of a literal constant expression. /// - /// Returns [`None`] if the expression is not constant. + /// Returns [`None`] if the expression is not a literal constant. #[inline] - pub fn get_constant_value(&self) -> Option { + pub fn get_literal_value(&self) -> Option { Some(match self { Self::DynamicConstant(x, _) => x.as_ref().clone(), Self::IntegerConstant(x, _) => (*x).into(), @@ -1838,7 +1838,7 @@ impl Expr { Self::Array(x, _) if self.is_constant() => { let mut arr = Array::with_capacity(x.len()); arr.extend(x.iter().map(|v| { - v.get_constant_value() + v.get_literal_value() .expect("never fails because a constant array always has a constant value") })); Dynamic::from_array(arr) @@ -1850,7 +1850,7 @@ impl Expr { x.0.iter().for_each(|(k, v)| { *map.get_mut(k.name.as_str()) .expect("never fails because the template should contain all the keys") = v - .get_constant_value() + .get_literal_value() .expect("never fails because a constant map always has a constant value") }); Dynamic::from_map(map) diff --git a/src/optimize.rs b/src/optimize.rs index 2ce023fa..7e1d6d40 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -208,7 +208,7 @@ fn optimize_stmt_block( state.push_var( &x.name, AccessMode::ReadOnly, - value_expr.get_constant_value(), + value_expr.get_literal_value(), ); } } @@ -454,7 +454,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { // switch const { ... } Stmt::Switch(match_expr, x, pos) if match_expr.is_constant() => { - let value = match_expr.get_constant_value().unwrap(); + let value = match_expr.get_literal_value().unwrap(); let hasher = &mut get_hasher(); value.hash(hasher); let hash = hasher.finish(); @@ -841,7 +841,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State, _chaining: bool) { #[cfg(not(feature = "no_index"))] Expr::Array(_, _) if expr.is_constant() => { state.set_dirty(); - *expr = Expr::DynamicConstant(Box::new(expr.get_constant_value().unwrap()), expr.position()); + *expr = Expr::DynamicConstant(Box::new(expr.get_literal_value().unwrap()), expr.position()); } // [ items .. ] #[cfg(not(feature = "no_index"))] @@ -850,7 +850,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State, _chaining: bool) { #[cfg(not(feature = "no_object"))] Expr::Map(_, _) if expr.is_constant() => { state.set_dirty(); - *expr = Expr::DynamicConstant(Box::new(expr.get_constant_value().unwrap()), expr.position()); + *expr = Expr::DynamicConstant(Box::new(expr.get_literal_value().unwrap()), expr.position()); } // #{ key:value, .. } #[cfg(not(feature = "no_object"))] @@ -942,7 +942,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State, _chaining: bool) { => { let mut arg_values: StaticVec<_> = x.args.iter().map(|e| match e { Expr::Stack(slot, _) => x.constants[*slot].clone(), - _ => e.get_constant_value().unwrap() + _ => e.get_literal_value().unwrap() }).collect(); let arg_types: StaticVec<_> = arg_values.iter().map(Dynamic::type_id).collect(); @@ -968,7 +968,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State, _chaining: bool) { // Move constant arguments for arg in x.args.iter_mut() { - if let Some(value) = arg.get_constant_value() { + if let Some(value) = arg.get_literal_value() { state.set_dirty(); x.constants.push(value); *arg = Expr::Stack(x.constants.len()-1, arg.position()); @@ -991,7 +991,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State, _chaining: bool) { if !has_script_fn { let mut arg_values: StaticVec<_> = x.args.iter().map(|e| match e { Expr::Stack(slot, _) => x.constants[*slot].clone(), - _ => e.get_constant_value().unwrap() + _ => e.get_literal_value().unwrap() }).collect(); // Save the typename of the first argument if it is `type_of()` @@ -1025,7 +1025,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State, _chaining: bool) { optimize_expr(arg, state, false); // Move constant arguments - if let Some(value) = arg.get_constant_value() { + if let Some(value) = arg.get_literal_value() { state.set_dirty(); x.constants.push(value); *arg = Expr::Stack(x.constants.len()-1, arg.position()); diff --git a/src/parse_error.rs b/src/parse_error.rs index ea32e35e..468cbd79 100644 --- a/src/parse_error.rs +++ b/src/parse_error.rs @@ -93,6 +93,8 @@ pub enum ParseErrorType { UnknownOperator(String), /// Expecting a particular token but not finding one. Wrapped values are the token and description. MissingToken(String, String), + /// Expecting a particular symbol but not finding one. Wrapped value is the description. + MissingSymbol(String), /// An expression in function call arguments `()` has syntax error. Wrapped value is the error /// description (if any). MalformedCallExpr(String), @@ -196,6 +198,7 @@ impl ParseErrorType { Self::BadInput(err) => err.desc(), Self::UnknownOperator(_) => "Unknown operator", Self::MissingToken(_, _) => "Expecting a certain token that is missing", + Self::MissingSymbol(_) => "Expecting a certain symbol that is missing", Self::MalformedCallExpr(_) => "Invalid expression in function call arguments", Self::MalformedIndexExpr(_) => "Invalid index in indexing expression", Self::MalformedInExpr(_) => "Invalid 'in' expression", @@ -268,6 +271,7 @@ impl fmt::Display for ParseErrorType { } Self::MissingToken(token, s) => write!(f, "Expecting '{}' {}", token, s), + Self::MissingSymbol(s) => f.write_str(s), Self::AssignmentToConstant(s) if s.is_empty() => f.write_str(self.desc()), Self::AssignmentToConstant(s) => write!(f, "Cannot assign to constant '{}'", s), diff --git a/src/parser.rs b/src/parser.rs index 0d1e4f74..998dec80 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -9,7 +9,10 @@ use crate::engine::{Precedence, KEYWORD_THIS, OP_CONTAINS}; use crate::module::NamespaceRef; use crate::optimize::optimize_into_ast; use crate::optimize::OptimizationLevel; -use crate::syntax::{CustomSyntax, MARKER_BLOCK, MARKER_EXPR, MARKER_IDENT}; +use crate::syntax::{ + CustomSyntax, MARKER_BLOCK, MARKER_BOOL, MARKER_EXPR, MARKER_IDENT, MARKER_INT, MARKER_STRING, +}; + use crate::token::{ is_keyword_function, is_valid_identifier, Token, TokenStream, TokenizerControl, }; @@ -27,7 +30,7 @@ use std::{ }; #[cfg(not(feature = "no_float"))] -use crate::FLOAT; +use crate::{syntax::MARKER_FLOAT, FLOAT}; #[cfg(not(feature = "no_function"))] use crate::FnAccess; @@ -876,7 +879,7 @@ fn parse_switch( }; let hash = if let Some(expr) = expr { - if let Some(value) = expr.get_constant_value() { + if let Some(value) = expr.get_literal_value() { let hasher = &mut get_hasher(); value.hash(hasher); let hash = hasher.finish(); @@ -1917,6 +1920,60 @@ fn parse_custom_syntax( } stmt => unreachable!("expecting Stmt::Block, but gets {:?}", stmt), }, + MARKER_BOOL => match input.next().expect(NEVER_ENDS) { + (b @ Token::True, pos) | (b @ Token::False, pos) => { + keywords.push(Expr::BoolConstant(b == Token::True, pos)); + let keyword = state.get_identifier(MARKER_BOOL); + segments.push(keyword.clone().into()); + tokens.push(keyword); + } + (_, pos) => { + return Err( + PERR::MissingSymbol("Expecting 'true' or 'false'".to_string()) + .into_err(pos), + ) + } + }, + MARKER_INT => match input.next().expect(NEVER_ENDS) { + (Token::IntegerConstant(i), pos) => { + keywords.push(Expr::IntegerConstant(i, pos)); + let keyword = state.get_identifier(MARKER_INT); + segments.push(keyword.clone().into()); + tokens.push(keyword); + } + (_, pos) => { + return Err( + PERR::MissingSymbol("Expecting an integer number".to_string()) + .into_err(pos), + ) + } + }, + #[cfg(not(feature = "no_float"))] + MARKER_FLOAT => match input.next().expect(NEVER_ENDS) { + (Token::FloatConstant(f), pos) => { + keywords.push(Expr::FloatConstant(f, pos)); + let keyword = state.get_identifier(MARKER_FLOAT); + segments.push(keyword.clone().into()); + tokens.push(keyword); + } + (_, pos) => { + return Err(PERR::MissingSymbol( + "Expecting a floating-point number".to_string(), + ) + .into_err(pos)) + } + }, + MARKER_STRING => match input.next().expect(NEVER_ENDS) { + (Token::StringConstant(s), pos) => { + keywords.push(Expr::StringConstant(state.get_identifier(s).into(), pos)); + let keyword = state.get_identifier(MARKER_STRING); + segments.push(keyword.clone().into()); + tokens.push(keyword); + } + (_, pos) => { + return Err(PERR::MissingSymbol("Expecting a string".to_string()).into_err(pos)) + } + }, s => match input.next().expect(NEVER_ENDS) { (Token::LexError(err), pos) => return Err(err.into_err(pos)), (t, _) if t.syntax().as_ref() == s => { diff --git a/src/syntax.rs b/src/syntax.rs index 34632f9e..be664bcf 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -5,8 +5,8 @@ use crate::engine::EvalContext; use crate::fn_native::SendSync; use crate::token::{is_valid_identifier, Token}; use crate::{ - Engine, Identifier, ImmutableString, LexError, ParseError, Position, RhaiResult, Shared, - StaticVec, + Dynamic, Engine, Identifier, ImmutableString, LexError, ParseError, Position, RhaiResult, + Shared, StaticVec, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -14,6 +14,11 @@ use std::prelude::v1::*; pub const MARKER_EXPR: &str = "$expr$"; pub const MARKER_BLOCK: &str = "$block$"; pub const MARKER_IDENT: &str = "$ident$"; +pub const MARKER_STRING: &str = "$string$"; +pub const MARKER_INT: &str = "$int$"; +#[cfg(not(feature = "no_float"))] +pub const MARKER_FLOAT: &str = "$float$"; +pub const MARKER_BOOL: &str = "$bool$"; /// A general expression evaluation trait object. #[cfg(not(feature = "sync"))] @@ -58,6 +63,11 @@ impl Expression<'_> { pub fn position(&self) -> Position { self.0.position() } + /// Get the value of this expression if it is a literal constant. + #[inline(always)] + pub fn get_literal_value(&self) -> Option { + self.0.get_literal_value() + } } impl EvalContext<'_, '_, '_, '_, '_, '_, '_> { @@ -132,7 +142,15 @@ impl Engine { let seg = match s { // Markers not in first position - MARKER_IDENT | MARKER_EXPR | MARKER_BLOCK if !segments.is_empty() => s.into(), + MARKER_IDENT | MARKER_EXPR | MARKER_BLOCK | MARKER_BOOL | MARKER_INT + | MARKER_STRING + if !segments.is_empty() => + { + s.into() + } + // Markers not in first position + #[cfg(not(feature = "no_float"))] + MARKER_FLOAT if !segments.is_empty() => s.into(), // Standard or reserved keyword/symbol not in first position s if !segments.is_empty() && token.is_some() => { // Make it a custom keyword/symbol if it is disabled or reserved diff --git a/tests/syntax.rs b/tests/syntax.rs index 1b8857e9..8fbeedb5 100644 --- a/tests/syntax.rs +++ b/tests/syntax.rs @@ -19,19 +19,24 @@ fn test_custom_syntax() -> Result<(), Box> { engine.register_custom_syntax( &[ - "exec", "[", "$ident$", "]", "->", "$block$", "while", "$expr$", + "exec", "[", "$ident$", ";", "$int$", "]", "->", "$block$", "while", "$expr$", ], true, |context, inputs| { let var_name = inputs[0].get_variable_name().unwrap().to_string(); - let stmt = inputs.get(1).unwrap(); - let condition = inputs.get(2).unwrap(); + let max = inputs[1].get_literal_value().unwrap().as_int().unwrap(); + let stmt = inputs.get(2).unwrap(); + let condition = inputs.get(3).unwrap(); context.scope_mut().push(var_name.clone(), 0 as INT); let mut count: INT = 0; loop { + if count >= max { + break; + } + context.eval_expression_tree(stmt)?; count += 1; @@ -63,17 +68,17 @@ fn test_custom_syntax() -> Result<(), Box> { engine.eval::( " let x = 0; - let foo = (exec [x] -> { x += 2 } while x < 42) * 10; + let foo = (exec [x;15] -> { x += 2 } while x < 42) * 10; foo " )?, - 210 + 150 ); assert_eq!( engine.eval::( " let x = 0; - exec [x] -> { x += 1 } while x < 42; + exec [x;100] -> { x += 1 } while x < 42; x " )?, @@ -82,7 +87,7 @@ fn test_custom_syntax() -> Result<(), Box> { assert_eq!( engine.eval::( " - exec [x] -> { x += 1 } while x < 42; + exec [x;100] -> { x += 1 } while x < 42; x " )?, @@ -92,11 +97,11 @@ fn test_custom_syntax() -> Result<(), Box> { engine.eval::( " let foo = 123; - exec [x] -> { x += 1 } while x < 42; + exec [x;15] -> { x += 1 } while x < 42; foo + x + x1 + x2 + x3 " )?, - 171 + 144 ); // The first symbol must be an identifier From 79d9977cd5d65d55601b33c37525d9d5badeb87f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 10 Jun 2021 10:45:44 +0800 Subject: [PATCH 19/24] Change take_string and take_immutable_string to as_XXX. --- CHANGELOG.md | 1 + codegen/src/function.rs | 4 ++-- codegen/src/test/function.rs | 2 +- codegen/src/test/module.rs | 4 ++-- src/dynamic.rs | 36 +++++++++++++++++++++++++++++++++--- src/fn_call.rs | 12 ++++++------ src/fn_register.rs | 2 +- src/packages/string_basic.rs | 2 +- src/serde/ser.rs | 4 ++-- tests/closures.rs | 2 +- tests/serde.rs | 24 ++++++++++++------------ 11 files changed, 62 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 844eefc8..d9442f60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Breaking changes ---------------- * To keep the API consistent, strings are no longer iterable by default. Use the `chars` method to iterator the characters in a string. +* `Dynamic::take_string` and `Dynamic::take_immutable_string` are renamed to `Dynamic::as_string` and `Dynamic::as_immutable_string` respectively. New features ------------ diff --git a/codegen/src/function.rs b/codegen/src/function.rs index 178f3645..8b4c106a 100644 --- a/codegen/src/function.rs +++ b/codegen/src/function.rs @@ -744,7 +744,7 @@ impl ExportedFn { is_string = true; is_ref = true; quote_spanned!(arg_type.span() => - mem::take(args[#i]).take_immutable_string().unwrap() + mem::take(args[#i]).as_immutable_string().unwrap() ) } _ => panic!("internal error: why wasn't this found earlier!?"), @@ -753,7 +753,7 @@ impl ExportedFn { is_string = true; is_ref = false; quote_spanned!(arg_type.span() => - mem::take(args[#i]).take_string().unwrap() + mem::take(args[#i]).as_string().unwrap() ) } _ => { diff --git a/codegen/src/test/function.rs b/codegen/src/test/function.rs index 7fb6dc3a..6d9eaa33 100644 --- a/codegen/src/test/function.rs +++ b/codegen/src/test/function.rs @@ -525,7 +525,7 @@ mod generate_tests { impl PluginFunction for Token { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { - let arg0 = mem::take(args[0usize]).take_immutable_string().unwrap(); + let arg0 = mem::take(args[0usize]).as_immutable_string().unwrap(); Ok(Dynamic::from(special_print(&arg0))) } diff --git a/codegen/src/test/module.rs b/codegen/src/test/module.rs index 6e62c4a6..620b3a1c 100644 --- a/codegen/src/test/module.rs +++ b/codegen/src/test/module.rs @@ -934,7 +934,7 @@ mod generate_tests { impl PluginFunction for print_out_to_token { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { - let arg0 = mem::take(args[0usize]).take_immutable_string().unwrap(); + let arg0 = mem::take(args[0usize]).as_immutable_string().unwrap(); Ok(Dynamic::from(print_out_to(&arg0))) } @@ -987,7 +987,7 @@ mod generate_tests { impl PluginFunction for print_out_to_token { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { - let arg0 = mem::take(args[0usize]).take_string().unwrap(); + let arg0 = mem::take(args[0usize]).as_string().unwrap(); Ok(Dynamic::from(print_out_to(arg0))) } diff --git a/src/dynamic.rs b/src/dynamic.rs index 66c01668..a95cab1f 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -1796,15 +1796,45 @@ impl Dynamic { /// Convert the [`Dynamic`] into a [`String`] and return it. /// If there are other references to the same string, a cloned copy is returned. /// Returns the name of the actual type if the cast fails. + /// + /// # Deprecated + /// + /// This method is deprecated and will be removed in the future. + /// Use [`as_string`][Dynamic::as_string] instead. #[inline(always)] + #[deprecated( + since = "0.20.3", + note = "this method is deprecated and will be removed in the future" + )] pub fn take_string(self) -> Result { - self.take_immutable_string() - .map(ImmutableString::into_owned) + self.as_string() + } + /// Convert the [`Dynamic`] into a [`String`] and return it. + /// If there are other references to the same string, a cloned copy is returned. + /// Returns the name of the actual type if the cast fails. + #[inline(always)] + pub fn as_string(self) -> Result { + self.as_immutable_string().map(ImmutableString::into_owned) + } + /// Convert the [`Dynamic`] into an [`ImmutableString`] and return it. + /// Returns the name of the actual type if the cast fails. + /// + /// # Deprecated + /// + /// This method is deprecated and will be removed in the future. + /// Use [`as_immutable_string`][Dynamic::as_immutable_string] instead. + #[inline(always)] + #[deprecated( + since = "0.20.3", + note = "this method is deprecated and will be removed in the future" + )] + pub fn take_immutable_string(self) -> Result { + self.as_immutable_string() } /// Convert the [`Dynamic`] into an [`ImmutableString`] and return it. /// Returns the name of the actual type if the cast fails. #[inline(always)] - pub fn take_immutable_string(self) -> Result { + pub fn as_immutable_string(self) -> Result { match self.0 { Union::Str(s, _, _) => Ok(s), #[cfg(not(feature = "no_closure"))] diff --git a/src/fn_call.rs b/src/fn_call.rs index 0fad230e..2536c8fd 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -346,7 +346,7 @@ impl Engine { // See if the function match print/debug (which requires special processing) return Ok(match fn_name { KEYWORD_PRINT => { - let text = result.take_immutable_string().map_err(|typ| { + let text = result.as_immutable_string().map_err(|typ| { EvalAltResult::ErrorMismatchOutputType( self.map_type_name(type_name::()).into(), typ.into(), @@ -356,7 +356,7 @@ impl Engine { ((self.print)(&text).into(), false) } KEYWORD_DEBUG => { - let text = result.take_immutable_string().map_err(|typ| { + let text = result.as_immutable_string().map_err(|typ| { EvalAltResult::ErrorMismatchOutputType( self.map_type_name(type_name::()).into(), typ.into(), @@ -1103,7 +1103,7 @@ impl Engine { // Fn - only in function call style return arg - .take_immutable_string() + .as_immutable_string() .map_err(|typ| self.make_type_mismatch_err::(typ, arg_pos)) .and_then(|s| FnPtr::try_from(s)) .map(Into::::into) @@ -1153,7 +1153,7 @@ impl Engine { )?; let fn_name = arg - .take_immutable_string() + .as_immutable_string() .map_err(|err| self.make_type_mismatch_err::(err, arg_pos))?; let (arg, arg_pos) = self.get_arg_value( @@ -1179,7 +1179,7 @@ impl Engine { scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, )?; let var_name = arg - .take_immutable_string() + .as_immutable_string() .map_err(|err| self.make_type_mismatch_err::(err, arg_pos))?; return Ok(scope.contains(&var_name).into()); } @@ -1192,7 +1192,7 @@ impl Engine { scope, mods, state, lib, this_ptr, level, args_expr, constants, 0, )?; let script = &value - .take_immutable_string() + .as_immutable_string() .map_err(|typ| self.make_type_mismatch_err::(typ, pos))?; let result = self.eval_script_expr_in_place(scope, mods, state, lib, script, pos, level + 1); diff --git a/src/fn_register.rs b/src/fn_register.rs index 80cfb2cf..28737808 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -50,7 +50,7 @@ pub fn by_value(data: &mut Dynamic) -> T { } else if TypeId::of::() == TypeId::of::() { // If T is `String`, data must be `ImmutableString`, so map directly to it let value = mem::take(data) - .take_string() + .as_string() .expect("never fails because the type was checked"); unsafe_try_cast(value).expect("never fails because the type was checked") } else { diff --git a/src/packages/string_basic.rs b/src/packages/string_basic.rs index be07f1e0..d577fe2e 100644 --- a/src/packages/string_basic.rs +++ b/src/packages/string_basic.rs @@ -28,7 +28,7 @@ pub fn print_with_func( ) -> crate::ImmutableString { match ctx.call_fn_dynamic_raw(fn_name, true, &mut [value]) { Ok(result) if result.is::() => result - .take_immutable_string() + .as_immutable_string() .expect("never fails as the result is `ImmutableString`"), Ok(result) => ctx.engine().map_type_name(result.type_name()).into(), Err(_) => ctx.engine().map_type_name(value.type_name()).into(), diff --git a/src/serde/ser.rs b/src/serde/ser.rs index 147f290f..779a5487 100644 --- a/src/serde/ser.rs +++ b/src/serde/ser.rs @@ -542,7 +542,7 @@ impl SerializeMap for DynamicSerializer { #[cfg(not(feature = "no_object"))] { let key = std::mem::take(&mut self._key) - .take_immutable_string() + .as_immutable_string() .map_err(|typ| { EvalAltResult::ErrorMismatchDataType( "string".into(), @@ -572,7 +572,7 @@ impl SerializeMap for DynamicSerializer { #[cfg(not(feature = "no_object"))] { let _key: Dynamic = _key.serialize(&mut *self)?; - let _key = _key.take_immutable_string().map_err(|typ| { + let _key = _key.as_immutable_string().map_err(|typ| { EvalAltResult::ErrorMismatchDataType("string".into(), typ.into(), Position::NONE) })?; let _value = _value.serialize(&mut *self)?; diff --git a/tests/closures.rs b/tests/closures.rs index 51a728e9..922a3cd3 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -343,7 +343,7 @@ fn test_closures_external() -> Result<(), Box> { // Closure 'f' captures: the engine, the AST, and the curried function pointer let f = move |x: INT| fn_ptr.call_dynamic(&context, None, [x.into()]); - assert_eq!(f(42)?.take_string(), Ok("hello42".to_string())); + assert_eq!(f(42)?.as_string(), Ok("hello42".to_string())); Ok(()) } diff --git a/tests/serde.rs b/tests/serde.rs index f8557062..a02645f3 100644 --- a/tests/serde.rs +++ b/tests/serde.rs @@ -102,7 +102,7 @@ fn test_serde_ser_struct() -> Result<(), Box> { assert!(obj["b"].as_bool().unwrap()); assert_eq!(Ok(42), map["int"].as_int()); assert_eq!(seq.len(), 3); - assert_eq!("kitty", seq.remove(1).take_string().unwrap()); + assert_eq!("kitty", seq.remove(1).as_string().unwrap()); Ok(()) } @@ -116,10 +116,10 @@ fn test_serde_ser_unit_enum() -> Result<(), Box> { } let d = to_dynamic(MyEnum::VariantFoo)?; - assert_eq!("VariantFoo", d.take_string().unwrap()); + assert_eq!("VariantFoo", d.as_string().unwrap()); let d = to_dynamic(MyEnum::VariantBar)?; - assert_eq!("VariantBar", d.take_string().unwrap()); + assert_eq!("VariantBar", d.as_string().unwrap()); Ok(()) } @@ -145,7 +145,7 @@ fn test_serde_ser_externally_tagged_enum() -> Result<(), Box> { assert_eq!( "VariantUnit", to_dynamic(MyEnum::VariantUnit)? - .take_immutable_string() + .as_immutable_string() .unwrap() .as_str() ); @@ -203,7 +203,7 @@ fn test_serde_ser_internally_tagged_enum() -> Result<(), Box> { "VariantEmptyStruct", map.remove("tag") .unwrap() - .take_immutable_string() + .as_immutable_string() .unwrap() .as_str() ); @@ -214,7 +214,7 @@ fn test_serde_ser_internally_tagged_enum() -> Result<(), Box> { "VariantStruct", map.remove("tag") .unwrap() - .take_immutable_string() + .as_immutable_string() .unwrap() .as_str() ); @@ -247,7 +247,7 @@ fn test_serde_ser_adjacently_tagged_enum() -> Result<(), Box> { "VariantUnit", map.remove("tag") .unwrap() - .take_immutable_string() + .as_immutable_string() .unwrap() .as_str() ); @@ -260,7 +260,7 @@ fn test_serde_ser_adjacently_tagged_enum() -> Result<(), Box> { "VariantUnitTuple", map.remove("tag") .unwrap() - .take_immutable_string() + .as_immutable_string() .unwrap() .as_str() ); @@ -274,7 +274,7 @@ fn test_serde_ser_adjacently_tagged_enum() -> Result<(), Box> { "VariantNewtype", map.remove("tag") .unwrap() - .take_immutable_string() + .as_immutable_string() .unwrap() .as_str() ); @@ -289,7 +289,7 @@ fn test_serde_ser_adjacently_tagged_enum() -> Result<(), Box> { "VariantTuple", map.remove("tag") .unwrap() - .take_immutable_string() + .as_immutable_string() .unwrap() .as_str() ); @@ -305,7 +305,7 @@ fn test_serde_ser_adjacently_tagged_enum() -> Result<(), Box> { "VariantEmptyStruct", map.remove("tag") .unwrap() - .take_immutable_string() + .as_immutable_string() .unwrap() .as_str() ); @@ -316,7 +316,7 @@ fn test_serde_ser_adjacently_tagged_enum() -> Result<(), Box> { let mut map = to_dynamic(MyEnum::VariantStruct { a: 123 })?.cast::(); assert_eq!( "VariantStruct", - map.remove("tag").unwrap().take_string().unwrap() + map.remove("tag").unwrap().as_string().unwrap() ); let mut map_inner = map.remove("content").unwrap().cast::(); assert!(map.is_empty()); From a1950a9562e9ac7a473e676c2e89aceafae6c084 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 10 Jun 2021 19:09:27 +0800 Subject: [PATCH 20/24] Bump versions. --- Cargo.toml | 2 +- codegen/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6151739d..4f6084ca 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ smallvec = { version = "1.6", default-features = false, features = ["union"] } ahash = { version = "0.7", default-features = false } num-traits = { version = "0.2", default_features = false } smartstring = { version = "0.2.6", default_features = false } -rhai_codegen = { version = "0.3.7", path = "codegen", default_features = false } +rhai_codegen = { version = "0.4.0", path = "codegen", default_features = false } [features] default = ["smartstring/std", "ahash/std", "num-traits/std"] # remove 'smartstring/std' when smartstring is updated to support no-std diff --git a/codegen/Cargo.toml b/codegen/Cargo.toml index cfb9d004..53b6ab47 100644 --- a/codegen/Cargo.toml +++ b/codegen/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rhai_codegen" -version = "0.3.7" +version = "0.4.0" edition = "2018" authors = ["jhwgh1968", "Stephen Chung"] description = "Procedural macros support package for Rhai, a scripting language and engine for Rust" From 2a0ab4739f83e1951380e99a183df6c4c2f173a8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 11 Jun 2021 12:29:03 +0800 Subject: [PATCH 21/24] Rename default_features to default-features. --- Cargo.toml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4f6084ca..7e6ec552 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,9 +18,9 @@ categories = ["no-std", "embedded", "wasm", "parser-implementations"] [dependencies] smallvec = { version = "1.6", default-features = false, features = ["union"] } ahash = { version = "0.7", default-features = false } -num-traits = { version = "0.2", default_features = false } -smartstring = { version = "0.2.6", default_features = false } -rhai_codegen = { version = "0.4.0", path = "codegen", default_features = false } +num-traits = { version = "0.2", default-features = false } +smartstring = { version = "0.2.6", default-features = false } +rhai_codegen = { version = "0.4.0", path = "codegen", default-features = false } [features] default = ["smartstring/std", "ahash/std", "num-traits/std"] # remove 'smartstring/std' when smartstring is updated to support no-std @@ -59,41 +59,41 @@ codegen-units = 1 [dependencies.no-std-compat] version = "0.4" -default_features = false +default-features = false features = ["alloc"] optional = true [dependencies.libm] version = "0.2" -default_features = false +default-features = false optional = true [dependencies.core-error] version = "0.0" -default_features = false +default-features = false features = ["alloc"] optional = true [dependencies.serde] version = "1.0" -default_features = false +default-features = false features = ["derive", "alloc"] optional = true [dependencies.serde_json] version = "1.0" -default_features = false +default-features = false features = ["alloc"] optional = true [dependencies.unicode-xid] version = "0.2" -default_features = false +default-features = false optional = true [dependencies.rust_decimal] version = "1.14" -default_features = false +default-features = false features = ["maths"] optional = true From 07d25b4fb62c2c082a9c77132cf29b113d50afa2 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 11 Jun 2021 15:12:01 +0800 Subject: [PATCH 22/24] Update rust_decimal. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 7e6ec552..3de0c467 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -92,7 +92,7 @@ default-features = false optional = true [dependencies.rust_decimal] -version = "1.14" +version = "1.14.2" default-features = false features = ["maths"] optional = true From f9dcfeb1ad4a227173fe9ce1015b85c03c97107b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 11 Jun 2021 19:59:50 +0800 Subject: [PATCH 23/24] Check data size after assignments. --- CHANGELOG.md | 1 + src/engine.rs | 150 ++++++++++++++++++++++++++++++------------------- src/fn_call.rs | 1 + 3 files changed, 93 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9442f60..20d377cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Bug fixes --------- * Fixed incorrect optimization regarding chain-indexing with non-numeric index. +* Variable values are checked for over-sized violations after assignments and setters. Breaking changes ---------------- diff --git a/src/engine.rs b/src/engine.rs index 0333d4b0..fd508a75 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1233,40 +1233,47 @@ impl Engine { } // xxx[rhs] op= new_val _ if new_val.is_some() => { - let ((mut new_val, new_pos), (op_info, op_pos)) = + let ((new_val, new_pos), (op_info, op_pos)) = new_val.expect("never fails because `new_val` is `Some`"); let idx_val = idx_val.as_index_value(); #[cfg(not(feature = "no_index"))] let mut idx_val_for_setter = idx_val.clone(); - match self.get_indexed_mut( + let try_setter = match self.get_indexed_mut( mods, state, lib, target, idx_val, pos, true, false, level, ) { // Indexed value is a reference - update directly - Ok(obj_ptr) => { + Ok(ref mut obj_ptr) => { self.eval_op_assignment( mods, state, lib, op_info, op_pos, obj_ptr, root, new_val, ) .map_err(|err| err.fill_position(new_pos))?; - return Ok((Dynamic::UNIT, true)); + None } // Can't index - try to call an index setter #[cfg(not(feature = "no_index"))] - Err(err) if matches!(*err, EvalAltResult::ErrorIndexingType(_, _)) => {} + Err(err) if matches!(*err, EvalAltResult::ErrorIndexingType(_, _)) => { + Some(new_val) + } // Any other error Err(err) => return Err(err), + }; + + if let Some(mut new_val) = try_setter { + // Try to call index setter + let hash_set = + FnCallHashes::from_native(crate::calc_fn_hash(FN_IDX_SET, 3)); + let args = &mut [target, &mut idx_val_for_setter, &mut new_val]; + + self.exec_fn_call( + mods, state, lib, FN_IDX_SET, hash_set, args, is_ref, true, + new_pos, None, level, + )?; } - // Try to call index setter - let hash_set = - FnCallHashes::from_native(crate::calc_fn_hash(FN_IDX_SET, 3)); - let args = &mut [target, &mut idx_val_for_setter, &mut new_val]; - - self.exec_fn_call( - mods, state, lib, FN_IDX_SET, hash_set, args, is_ref, true, new_pos, - None, level, - )?; + self.check_data_size(target.as_ref()) + .map_err(|err| err.fill_position(root.1))?; Ok((Dynamic::UNIT, true)) } @@ -1304,15 +1311,19 @@ impl Engine { Expr::Property(x) if target.is::() && new_val.is_some() => { let (name, pos) = &x.2; let index = name.into(); - let val = self.get_indexed_mut( - mods, state, lib, target, index, *pos, true, false, level, - )?; - let ((new_val, new_pos), (op_info, op_pos)) = - new_val.expect("never fails because `new_val` is `Some`"); - self.eval_op_assignment( - mods, state, lib, op_info, op_pos, val, root, new_val, - ) - .map_err(|err| err.fill_position(new_pos))?; + { + let mut val = self.get_indexed_mut( + mods, state, lib, target, index, *pos, true, false, level, + )?; + let ((new_val, new_pos), (op_info, op_pos)) = + new_val.expect("never fails because `new_val` is `Some`"); + self.eval_op_assignment( + mods, state, lib, op_info, op_pos, &mut val, root, new_val, + ) + .map_err(|err| err.fill_position(new_pos))?; + } + self.check_data_size(target.as_ref()) + .map_err(|err| err.fill_position(root.1))?; Ok((Dynamic::UNIT, true)) } // {xxx:map}.id @@ -1322,7 +1333,6 @@ impl Engine { let val = self.get_indexed_mut( mods, state, lib, target, index, *pos, false, false, level, )?; - Ok((val.take_or_clone(), false)) } // xxx.id op= ??? @@ -1357,11 +1367,22 @@ impl Engine { } _ => Err(err), })?; - let obj_ptr = (&mut orig_val).into(); + self.eval_op_assignment( - mods, state, lib, op_info, op_pos, obj_ptr, root, new_val, + mods, + state, + lib, + op_info, + op_pos, + &mut (&mut orig_val).into(), + root, + new_val, ) .map_err(|err| err.fill_position(new_pos))?; + + self.check_data_size(target.as_ref()) + .map_err(|err| err.fill_position(root.1))?; + new_val = orig_val; } @@ -1544,6 +1565,8 @@ impl Engine { _ => Err(err), }, )?; + self.check_data_size(target.as_ref()) + .map_err(|err| err.fill_position(root.1))?; } Ok((result, may_be_changed)) @@ -1989,18 +2012,23 @@ impl Engine { for expr in x.iter() { let item = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + self.eval_op_assignment( mods, state, lib, Some(OpAssignment::new(TOKEN_OP_CONCAT)), pos, - (&mut result).into(), + &mut (&mut result).into(), ("", Position::NONE), item, ) .map_err(|err| err.fill_position(expr.position()))?; + pos = expr.position(); + + self.check_data_size(&result) + .map_err(|err| err.fill_position(pos))?; } assert!( @@ -2125,11 +2153,8 @@ impl Engine { _ => unreachable!("expression cannot be evaluated: {:?}", expr), }; - #[cfg(not(feature = "unchecked"))] - self.check_data_size(&result) - .map_err(|err| err.fill_position(expr.position()))?; - - result + self.check_return_value(result) + .map_err(|err| err.fill_position(expr.position())) } /// Evaluate a statements block. @@ -2217,7 +2242,7 @@ impl Engine { lib: &[&Module], op_info: Option, op_pos: Position, - mut target: Target, + target: &mut Target, root: (&str, Position), mut new_val: Dynamic, ) -> Result<(), Box> { @@ -2311,7 +2336,7 @@ impl Engine { let rhs_val = self .eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)? .flatten(); - let (lhs_ptr, pos) = + let (mut lhs_ptr, pos) = self.search_namespace(scope, mods, state, lib, this_ptr, lhs_expr)?; let var_name = lhs_expr @@ -2332,12 +2357,15 @@ impl Engine { lib, op_info.clone(), *op_pos, - lhs_ptr, + &mut lhs_ptr, (var_name, pos), rhs_val, ) .map_err(|err| err.fill_position(rhs_expr.position()))?; + self.check_data_size(lhs_ptr.as_ref()) + .map_err(|err| err.fill_position(lhs_expr.position()))?; + Ok(Dynamic::UNIT) } @@ -2914,36 +2942,25 @@ impl Engine { } }; - #[cfg(not(feature = "unchecked"))] - self.check_data_size(&result) - .map_err(|err| err.fill_position(stmt.position()))?; + self.check_return_value(result) + .map_err(|err| err.fill_position(stmt.position())) + } + /// Check a result to ensure that the data size is within allowable limit. + #[cfg(feature = "unchecked")] + #[inline(always)] + fn check_return_value(&self, result: RhaiResult) -> RhaiResult { result } /// Check a result to ensure that the data size is within allowable limit. #[cfg(not(feature = "unchecked"))] - fn check_data_size(&self, result: &RhaiResult) -> Result<(), Box> { - let result = match result { - Err(_) => return Ok(()), - Ok(r) => r, - }; - - // If no data size limits, just return - let mut _has_limit = self.limits.max_string_size.is_some(); - #[cfg(not(feature = "no_index"))] - { - _has_limit = _has_limit || self.limits.max_array_size.is_some(); - } - #[cfg(not(feature = "no_object"))] - { - _has_limit = _has_limit || self.limits.max_map_size.is_some(); - } - - if !_has_limit { - return Ok(()); - } + #[inline(always)] + fn check_return_value(&self, result: RhaiResult) -> RhaiResult { + result.and_then(|r| self.check_data_size(&r).map(|_| r)) + } + fn check_data_size(&self, value: &Dynamic) -> Result<(), Box> { // Recursively calculate the size of a value (especially `Array` and `Map`) fn calc_size(value: &Dynamic) -> (usize, usize, usize) { match value { @@ -2996,7 +3013,22 @@ impl Engine { } } - let (_arr, _map, s) = calc_size(result); + // If no data size limits, just return + let mut _has_limit = self.limits.max_string_size.is_some(); + #[cfg(not(feature = "no_index"))] + { + _has_limit = _has_limit || self.limits.max_array_size.is_some(); + } + #[cfg(not(feature = "no_object"))] + { + _has_limit = _has_limit || self.limits.max_map_size.is_some(); + } + + if !_has_limit { + return Ok(()); + } + + let (_arr, _map, s) = calc_size(value); if s > self .limits diff --git a/src/fn_call.rs b/src/fn_call.rs index 2536c8fd..be0fe520 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -331,6 +331,7 @@ impl Engine { .as_ref() .or_else(|| state_source.as_ref()) .map(|s| s.as_str()); + let result = if func.is_plugin_fn() { func.get_plugin_fn() .call((self, fn_name, source, mods, lib).into(), args) From 68ea8c27fdf3d4b937bb43b269d6a29ce96bee5d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 12 Jun 2021 10:26:50 +0800 Subject: [PATCH 24/24] Fix unchecked. Do not duplicate data size checking. --- codegen/src/rhai_module.rs | 2 +- src/ast.rs | 10 +-- src/engine.rs | 140 ++++++++++++++++--------------------- src/optimize.rs | 2 +- src/token.rs | 2 +- tests/data_size.rs | 12 ++++ 6 files changed, 80 insertions(+), 88 deletions(-) diff --git a/codegen/src/rhai_module.rs b/codegen/src/rhai_module.rs index 77025db1..17a29fd6 100644 --- a/codegen/src/rhai_module.rs +++ b/codegen/src/rhai_module.rs @@ -130,7 +130,7 @@ pub fn generate_body( let mut namespace = FnNamespaceAccess::Internal; match function.params().special { - FnSpecialAccess::None => {} + FnSpecialAccess::None => (), FnSpecialAccess::Index(_) | FnSpecialAccess::Property(_) => { let reg_name = fn_literal.value(); if reg_name.starts_with(FN_GET) diff --git a/src/ast.rs b/src/ast.rs index 1834a965..0a75dd9d 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -952,7 +952,7 @@ pub enum Stmt { /// \[`export`\] `const` id `=` expr Const(Expr, Box, bool, Position), /// expr op`=` expr - Assignment(Box<(Expr, Option, Expr)>, Position), + Assignment(Box<(Expr, Option>, Expr)>, Position), /// func `(` expr `,` ... `)` /// /// Note - this is a duplicate of [`Expr::FnCall`] to cover the very common pattern of a single @@ -1384,14 +1384,14 @@ pub struct BinaryExpr { /// # Volatile Data Structure /// /// This type is volatile and may change. -#[derive(Debug, Clone, Eq, PartialEq, Hash)] -pub struct OpAssignment { +#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] +pub struct OpAssignment<'a> { pub hash_op_assign: u64, pub hash_op: u64, - pub op: &'static str, + pub op: &'a str, } -impl OpAssignment { +impl OpAssignment<'_> { /// Create a new [`OpAssignment`]. /// /// # Panics diff --git a/src/engine.rs b/src/engine.rs index fd508a75..905d7e5f 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -437,34 +437,11 @@ impl<'a> Target<'a> { #[inline(always)] pub fn propagate_changed_value(&mut self) -> Result<(), Box> { match self { - Self::RefMut(_) | Self::TempValue(_) => Ok(()), + Self::RefMut(_) | Self::TempValue(_) => (), #[cfg(not(feature = "no_closure"))] - Self::LockGuard(_) => Ok(()), + Self::LockGuard(_) => (), #[cfg(not(feature = "no_index"))] - Self::BitField(_, _, value) => { - let new_val = value.clone(); - self.set_value(new_val) - } - #[cfg(not(feature = "no_index"))] - Self::StringChar(_, _, ch) => { - let new_val = ch.clone(); - self.set_value(new_val) - } - } - } - /// Update the value of the `Target`. - fn set_value(&mut self, new_val: Dynamic) -> Result<(), Box> { - match self { - Self::RefMut(r) => **r = new_val, - #[cfg(not(feature = "no_closure"))] - Self::LockGuard((r, _)) => **r = new_val, - Self::TempValue(_) => panic!("cannot update a value"), - #[cfg(not(feature = "no_index"))] - Self::BitField(value, index, _) => { - let value = &mut *value - .write_lock::() - .expect("never fails because `BitField` always holds an `INT`"); - + Self::BitField(value, index, new_val) => { // Replace the bit at the specified index position let new_bit = new_val.as_bool().map_err(|err| { Box::new(EvalAltResult::ErrorMismatchDataType( @@ -474,6 +451,10 @@ impl<'a> Target<'a> { )) })?; + let value = &mut *value + .write_lock::() + .expect("never fails because `BitField` always holds an `INT`"); + let index = *index; if index < std::mem::size_of_val(value) * 8 { @@ -483,14 +464,12 @@ impl<'a> Target<'a> { } else { *value &= !mask; } + } else { + unreachable!("bit-field index out of bounds: {}", index); } } #[cfg(not(feature = "no_index"))] - Self::StringChar(s, index, _) => { - let s = &mut *s - .write_lock::() - .expect("never fails because `StringChar` always holds an `ImmutableString`"); - + Self::StringChar(s, index, new_val) => { // Replace the character at the specified index position let new_ch = new_val.as_char().map_err(|err| { Box::new(EvalAltResult::ErrorMismatchDataType( @@ -500,6 +479,10 @@ impl<'a> Target<'a> { )) })?; + let s = &mut *s + .write_lock::() + .expect("never fails because `StringChar` always holds an `ImmutableString`"); + let index = *index; *s = s @@ -1236,8 +1219,6 @@ impl Engine { let ((new_val, new_pos), (op_info, op_pos)) = new_val.expect("never fails because `new_val` is `Some`"); let idx_val = idx_val.as_index_value(); - - #[cfg(not(feature = "no_index"))] let mut idx_val_for_setter = idx_val.clone(); let try_setter = match self.get_indexed_mut( @@ -1310,13 +1291,13 @@ impl Engine { // {xxx:map}.id op= ??? Expr::Property(x) if target.is::() && new_val.is_some() => { let (name, pos) = &x.2; + let ((new_val, new_pos), (op_info, op_pos)) = + new_val.expect("never fails because `new_val` is `Some`"); let index = name.into(); { let mut val = self.get_indexed_mut( mods, state, lib, target, index, *pos, true, false, level, )?; - let ((new_val, new_pos), (op_info, op_pos)) = - new_val.expect("never fails because `new_val` is `Some`"); self.eval_op_assignment( mods, state, lib, op_info, op_pos, &mut val, root, new_val, ) @@ -2355,7 +2336,7 @@ impl Engine { mods, state, lib, - op_info.clone(), + *op_info, *op_pos, &mut lhs_ptr, (var_name, pos), @@ -2363,8 +2344,10 @@ impl Engine { ) .map_err(|err| err.fill_position(rhs_expr.position()))?; - self.check_data_size(lhs_ptr.as_ref()) - .map_err(|err| err.fill_position(lhs_expr.position()))?; + if op_info.is_some() { + self.check_data_size(lhs_ptr.as_ref()) + .map_err(|err| err.fill_position(lhs_expr.position()))?; + } Ok(Dynamic::UNIT) } @@ -2960,55 +2943,52 @@ impl Engine { result.and_then(|r| self.check_data_size(&r).map(|_| r)) } + #[cfg(feature = "unchecked")] + #[inline(always)] + fn check_data_size(&self, _value: &Dynamic) -> Result<(), Box> { + Ok(()) + } + + #[cfg(not(feature = "unchecked"))] fn check_data_size(&self, value: &Dynamic) -> Result<(), Box> { // Recursively calculate the size of a value (especially `Array` and `Map`) fn calc_size(value: &Dynamic) -> (usize, usize, usize) { - match value { + match value.0 { #[cfg(not(feature = "no_index"))] - Dynamic(Union::Array(arr, _, _)) => { - let mut arrays = 0; - let mut maps = 0; - - arr.iter().for_each(|value| match value { - Dynamic(Union::Array(_, _, _)) => { - let (a, m, _) = calc_size(value); - arrays += a; - maps += m; - } - #[cfg(not(feature = "no_object"))] - Dynamic(Union::Map(_, _, _)) => { - let (a, m, _) = calc_size(value); - arrays += a; - maps += m; - } - _ => arrays += 1, - }); - - (arrays, maps, 0) + Union::Array(ref arr, _, _) => { + arr.iter() + .fold((0, 0, 0), |(arrays, maps, strings), value| match value.0 { + Union::Array(_, _, _) => { + let (a, m, s) = calc_size(value); + (arrays + a + 1, maps + m, strings + s) + } + #[cfg(not(feature = "no_object"))] + Union::Map(_, _, _) => { + let (a, m, s) = calc_size(value); + (arrays + a + 1, maps + m, strings + s) + } + Union::Str(ref s, _, _) => (arrays + 1, maps, strings + s.len()), + _ => (arrays + 1, maps, strings), + }) } #[cfg(not(feature = "no_object"))] - 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(_, _, _)) => { - let (a, m, _) = calc_size(value); - arrays += a; - maps += m; - } - Dynamic(Union::Map(_, _, _)) => { - let (a, m, _) = calc_size(value); - arrays += a; - maps += m; - } - _ => maps += 1, - }); - - (arrays, maps, 0) + Union::Map(ref map, _, _) => { + map.values() + .fold((0, 0, 0), |(arrays, maps, strings), value| match value.0 { + #[cfg(not(feature = "no_index"))] + Union::Array(_, _, _) => { + let (a, m, s) = calc_size(value); + (arrays + a, maps + m + 1, strings + s) + } + Union::Map(_, _, _) => { + let (a, m, s) = calc_size(value); + (arrays + a, maps + m + 1, strings + s) + } + Union::Str(ref s, _, _) => (arrays, maps + 1, strings + s.len()), + _ => (arrays, maps + 1, strings), + }) } - Dynamic(Union::Str(s, _, _)) => (0, 0, s.len()), + Union::Str(ref s, _, _) => (0, 0, s.len()), _ => (0, 0, 0), } } diff --git a/src/optimize.rs b/src/optimize.rs index 7e1d6d40..da14b1d2 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -297,7 +297,7 @@ fn optimize_stmt_block( Stmt::Noop(*pos) }; } - [.., second_last_stmt, Stmt::Noop(_)] if second_last_stmt.returns_value() => {} + [.., second_last_stmt, Stmt::Noop(_)] if second_last_stmt.returns_value() => (), [.., second_last_stmt, last_stmt] if !last_stmt.returns_value() && is_pure(last_stmt) => { diff --git a/src/token.rs b/src/token.rs index 992f0601..ad72253c 100644 --- a/src/token.rs +++ b/src/token.rs @@ -1095,7 +1095,7 @@ pub fn parse_string_literal( match next_char { // \r - ignore if followed by \n - '\r' if stream.peek_next().map(|ch| ch == '\n').unwrap_or(false) => {} + '\r' if stream.peek_next().map(|ch| ch == '\n').unwrap_or(false) => (), // \... '\\' if !verbatim && escape.is_empty() => { escape.push('\\'); diff --git a/tests/data_size.rs b/tests/data_size.rs index f3ff3cd6..aa92c91b 100644 --- a/tests/data_size.rs +++ b/tests/data_size.rs @@ -206,6 +206,18 @@ fn test_max_map_size() -> Result<(), Box> { ) ); + assert!(matches!( + *engine + .consume( + " + let x = #{}; + loop { x.a = x; } + " + ) + .expect_err("should error"), + EvalAltResult::ErrorDataTooLarge(_, _) + )); + assert!(matches!( *engine .eval::(