From fc782c55630b164fb964a3f617d939e93ecda8ee Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 23 Apr 2021 14:24:53 +0800 Subject: [PATCH 01/16] Refine posistion display. --- src/ast.rs | 160 +++++++++++++++++----------------------------- src/engine.rs | 2 +- src/module/mod.rs | 20 +++--- src/token.rs | 10 +++ tests/maps.rs | 2 +- 5 files changed, 79 insertions(+), 115 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 0b56c64b..47cf4d16 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -780,12 +780,8 @@ pub struct Ident { impl fmt::Debug for Ident { #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - #[cfg(not(feature = "no_position"))] - write!(f, "{:?} @ {:?}", self.name, self.pos)?; - #[cfg(feature = "no_position")] write!(f, "{:?}", self.name)?; - - Ok(()) + self.pos.debug_print(f) } } @@ -879,10 +875,7 @@ impl fmt::Debug for StmtBlock { #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Debug::fmt(&self.0, f)?; - if !self.1.is_none() { - write!(f, " @ {:?}", self.1)?; - } - Ok(()) + self.1.debug_print(f) } } @@ -1691,60 +1684,33 @@ impl Default for Expr { impl fmt::Debug for Expr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - #[cfg(not(feature = "no_position"))] - Self::DynamicConstant(value, pos) => write!(f, "{:?} @ {:?}", value, pos), - #[cfg(not(feature = "no_position"))] - Self::BoolConstant(value, pos) => write!(f, "{:?} @ {:?}", value, pos), - #[cfg(not(feature = "no_position"))] - Self::IntegerConstant(value, pos) => write!(f, "{:?} @ {:?}", value, pos), - #[cfg(not(feature = "no_float"))] - #[cfg(not(feature = "no_position"))] - Self::FloatConstant(value, pos) => write!(f, "{:?} @ {:?}", value, pos), - #[cfg(not(feature = "no_position"))] - Self::CharConstant(value, pos) => write!(f, "{:?} @ {:?}", value, pos), - #[cfg(not(feature = "no_position"))] - Self::StringConstant(value, pos) => write!(f, "{:?} @ {:?}", value, pos), - #[cfg(not(feature = "no_position"))] - Self::Unit(pos) => write!(f, "() @ {:?}", pos), + let mut display_pos = self.position(); - #[cfg(feature = "no_position")] + match self { Self::DynamicConstant(value, _) => write!(f, "{:?}", value), - #[cfg(feature = "no_position")] Self::BoolConstant(value, _) => write!(f, "{:?}", value), - #[cfg(feature = "no_position")] Self::IntegerConstant(value, _) => write!(f, "{:?}", value), #[cfg(not(feature = "no_float"))] - #[cfg(feature = "no_position")] Self::FloatConstant(value, _) => write!(f, "{:?}", value), - #[cfg(feature = "no_position")] Self::CharConstant(value, _) => write!(f, "{:?}", value), - #[cfg(feature = "no_position")] Self::StringConstant(value, _) => write!(f, "{:?}", value), - #[cfg(feature = "no_position")] Self::Unit(_) => f.write_str("()"), Self::InterpolatedString(x) => { f.write_str("InterpolatedString")?; + return f.debug_list().entries(x.iter()).finish(); + } + Self::Array(x, _) => { + f.write_str("Array")?; f.debug_list().entries(x.iter()).finish() } - Self::Array(x, _pos) => { - f.write_str("Array")?; - f.debug_list().entries(x.iter()).finish()?; - #[cfg(not(feature = "no_position"))] - write!(f, " @ {:?}", _pos)?; - Ok(()) - } - Self::Map(x, _pos) => { + Self::Map(x, _) => { f.write_str("Map")?; f.debug_map() .entries(x.0.iter().map(|(k, v)| (k, v))) - .finish()?; - #[cfg(not(feature = "no_position"))] - write!(f, " @ {:?}", _pos)?; - Ok(()) + .finish() } - Self::Variable(i, _pos, x) => { + Self::Variable(i, _, x) => { f.write_str("Variable(")?; match x.1 { Some((_, ref namespace)) => write!(f, "{}", namespace)?, @@ -1755,23 +1721,14 @@ impl fmt::Debug for Expr { Some(n) => write!(f, ", {}", n)?, _ => (), } - f.write_str(")")?; - #[cfg(not(feature = "no_position"))] - write!(f, " @ {:?}", _pos)?; - Ok(()) + f.write_str(")") } - #[cfg(not(feature = "no_position"))] - Self::Property(x) => write!(f, "Property({:?} @ {:?})", x.2.name, x.2.pos), - #[cfg(feature = "no_position")] - Self::Property(x) => write!(f, "Property({:?})", x.2.name), + Self::Property(x) => write!(f, "Property({})", x.2.name), Self::Stmt(x) => { f.write_str("Stmt")?; - f.debug_list().entries(x.0.iter()).finish()?; - #[cfg(not(feature = "no_position"))] - write!(f, " @ {:?}", x.1)?; - Ok(()) + f.debug_list().entries(x.0.iter()).finish() } - Self::FnCall(x, _pos) => { + Self::FnCall(x, _) => { let mut ff = f.debug_struct("FnCall"); if let Some(ref ns) = x.namespace { ff.field("namespace", ns); @@ -1785,12 +1742,9 @@ impl fmt::Debug for Expr { if x.capture { ff.field("capture", &x.capture); } - ff.finish()?; - #[cfg(not(feature = "no_position"))] - write!(f, " @ {:?}", _pos)?; - Ok(()) + ff.finish() } - Self::Dot(x, _pos) | Self::Index(x, _pos) | Self::And(x, _pos) | Self::Or(x, _pos) => { + Self::Dot(x, pos) | Self::Index(x, pos) | Self::And(x, pos) | Self::Or(x, pos) => { let op_name = match self { Self::Dot(_, _) => "Dot", Self::Index(_, _) => "Index", @@ -1799,21 +1753,17 @@ impl fmt::Debug for Expr { _ => unreachable!(), }; + display_pos = *pos; + f.debug_struct(op_name) .field("lhs", &x.lhs) .field("rhs", &x.rhs) - .finish()?; - #[cfg(not(feature = "no_position"))] - write!(f, " @ {:?}", _pos)?; - Ok(()) + .finish() } - Self::Custom(x, _pos) => { - f.debug_tuple("Custom").field(x).finish()?; - #[cfg(not(feature = "no_position"))] - write!(f, " @ {:?}", _pos)?; - Ok(()) - } - } + Self::Custom(x, _) => f.debug_tuple("Custom").field(x).finish(), + }?; + + display_pos.debug_print(f) } } @@ -1875,26 +1825,26 @@ impl Expr { #[cfg(not(feature = "no_float"))] Self::FloatConstant(_, pos) => *pos, - Self::DynamicConstant(_, pos) => *pos, - Self::BoolConstant(_, pos) => *pos, - Self::IntegerConstant(_, pos) => *pos, - Self::CharConstant(_, pos) => *pos, - Self::StringConstant(_, pos) => *pos, + Self::DynamicConstant(_, pos) + | Self::BoolConstant(_, pos) + | Self::IntegerConstant(_, pos) + | Self::CharConstant(_, pos) + | Self::Unit(pos) + | Self::StringConstant(_, pos) + | Self::Array(_, pos) + | Self::Map(_, pos) + | Self::Variable(_, pos, _) + | Self::FnCall(_, pos) + | Self::Custom(_, pos) => *pos, + Self::InterpolatedString(x) => x.first().unwrap().position(), - Self::Array(_, pos) => *pos, - Self::Map(_, pos) => *pos, + Self::Property(x) => (x.2).pos, Self::Stmt(x) => x.1, - Self::Variable(_, pos, _) => *pos, - Self::FnCall(_, pos) => *pos, - Self::And(x, _) | Self::Or(x, _) => x.lhs.position(), - - Self::Unit(pos) => *pos, - - Self::Dot(x, _) | Self::Index(x, _) => x.lhs.position(), - - Self::Custom(_, pos) => *pos, + Self::And(x, _) | Self::Or(x, _) | Self::Dot(x, _) | Self::Index(x, _) => { + x.lhs.position() + } } } /// Override the [position][Position] of the expression. @@ -1904,24 +1854,28 @@ impl Expr { #[cfg(not(feature = "no_float"))] Self::FloatConstant(_, pos) => *pos = new_pos, - Self::DynamicConstant(_, pos) => *pos = new_pos, - Self::BoolConstant(_, pos) => *pos = new_pos, - Self::IntegerConstant(_, pos) => *pos = new_pos, - Self::CharConstant(_, pos) => *pos = new_pos, - Self::StringConstant(_, pos) => *pos = new_pos, + Self::DynamicConstant(_, pos) + | Self::BoolConstant(_, pos) + | Self::IntegerConstant(_, pos) + | Self::CharConstant(_, pos) + | Self::Unit(pos) + | Self::StringConstant(_, pos) + | Self::Array(_, pos) + | Self::Map(_, pos) + | Self::And(_, pos) + | Self::Or(_, pos) + | Self::Dot(_, pos) + | Self::Index(_, pos) + | Self::Variable(_, pos, _) + | Self::FnCall(_, pos) + | Self::Custom(_, pos) => *pos = new_pos, + Self::InterpolatedString(x) => { x.first_mut().unwrap().set_position(new_pos); } - Self::Array(_, pos) => *pos = new_pos, - Self::Map(_, pos) => *pos = new_pos, - Self::Variable(_, pos, _) => *pos = new_pos, + Self::Property(x) => (x.2).pos = new_pos, Self::Stmt(x) => x.1 = new_pos, - Self::FnCall(_, pos) => *pos = new_pos, - Self::And(_, pos) | Self::Or(_, pos) => *pos = new_pos, - Self::Unit(pos) => *pos = new_pos, - Self::Dot(_, pos) | Self::Index(_, pos) => *pos = new_pos, - Self::Custom(_, pos) => *pos = new_pos, } self diff --git a/src/engine.rs b/src/engine.rs index aaaa9300..1251c193 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -838,7 +838,7 @@ fn default_debug(_s: &str, _source: Option<&str>, _pos: Position) { #[cfg(not(feature = "no_std"))] #[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))] if let Some(source) = _source { - println!("{} @ {:?} | {}", source, _pos, _s); + println!("{}{:?} | {}", source, _pos, _s); } else if _pos.is_none() { println!("{}", _s); } else { diff --git a/src/module/mod.rs b/src/module/mod.rs index bfefec28..5f2868f8 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1661,6 +1661,16 @@ impl fmt::Debug for NamespaceRef { } } +impl fmt::Display for NamespaceRef { + #[inline(always)] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + for Ident { name, .. } in self.path.iter() { + write!(f, "{}{}", name, Token::DoubleColon.syntax())?; + } + Ok(()) + } +} + impl Deref for NamespaceRef { type Target = StaticVec; @@ -1677,16 +1687,6 @@ impl DerefMut for NamespaceRef { } } -impl fmt::Display for NamespaceRef { - #[inline(always)] - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - for Ident { name, .. } in self.path.iter() { - write!(f, "{}{}", name, Token::DoubleColon.syntax())?; - } - Ok(()) - } -} - impl From> for NamespaceRef { #[inline(always)] fn from(path: StaticVec) -> Self { diff --git a/src/token.rs b/src/token.rs index 72ee99bc..c777e4d7 100644 --- a/src/token.rs +++ b/src/token.rs @@ -185,6 +185,16 @@ impl Position { #[cfg(feature = "no_position")] return true; } + /// Print this [`Position`] for debug purposes. + #[inline(always)] + pub(crate) fn debug_print(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + #[cfg(not(feature = "no_position"))] + if !self.is_none() { + write!(f, " @ {:?}", self)?; + } + + Ok(()) + } } impl Default for Position { diff --git a/tests/maps.rs b/tests/maps.rs index 4efea70f..10a79591 100644 --- a/tests/maps.rs +++ b/tests/maps.rs @@ -205,7 +205,7 @@ fn test_map_json() -> Result<(), Box> { assert!(matches!( *engine.parse_json(" 123", true).expect_err("should error"), - EvalAltResult::ErrorParsing(ParseErrorType::MissingToken(token, _), pos) + EvalAltResult::ErrorParsing(ParseErrorType::MissingToken(token, _), _) if token == "{" )); From 335ab64a2c2d0fc8ccc2630f8ec807c16743702b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 23 Apr 2021 19:10:10 +0800 Subject: [PATCH 02/16] Use SmartString inside ImmutableString. --- src/dynamic.rs | 22 ++++++------- src/lib.rs | 12 ++++++-- src/utils.rs | 84 ++++++++++++++++++++------------------------------ 3 files changed, 53 insertions(+), 65 deletions(-) diff --git a/src/dynamic.rs b/src/dynamic.rs index 19b71823..2414d31f 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -2,7 +2,7 @@ use crate::fn_native::SendSync; use crate::r#unsafe::{unsafe_cast_box, unsafe_try_cast}; -use crate::{FnPtr, ImmutableString, INT}; +use crate::{FnPtr, ImmutableString, SmartString, INT}; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{ @@ -900,6 +900,12 @@ impl Dynamic { .deref() .into(); } + if TypeId::of::() == TypeId::of::() { + return ::downcast_ref::(&value) + .unwrap() + .clone() + .into(); + } if TypeId::of::() == TypeId::of::<()>() { return ().into(); } @@ -1173,13 +1179,13 @@ impl Dynamic { /// ``` #[inline(always)] pub fn clone_cast(&self) -> T { - self.read_lock::().unwrap().clone() + self.flatten_clone().cast::() } /// Flatten the [`Dynamic`] and clone it. /// /// If the [`Dynamic`] is not a shared value, it returns a cloned copy. /// - /// If the [`Dynamic`] is a shared value, it a cloned copy of the shared value. + /// If the [`Dynamic`] is a shared value, it returns a cloned copy of the shared value. #[inline(always)] pub fn flatten_clone(&self) -> Self { #[cfg(not(feature = "no_closure"))] @@ -1361,7 +1367,7 @@ impl Dynamic { /// /// Returns [`None`] if the cast fails, or if the value is shared. #[inline(always)] - pub(crate) fn downcast_ref(&self) -> Option<&T> { + pub(crate) fn downcast_ref(&self) -> Option<&T> { // Coded this way in order to maximally leverage potentials for dead-code removal. if TypeId::of::() == TypeId::of::() { @@ -1396,12 +1402,6 @@ impl Dynamic { _ => None, }; } - if TypeId::of::() == TypeId::of::() { - return match &self.0 { - Union::Str(value, _) => ::downcast_ref::(value.as_ref() as &String), - _ => None, - }; - } if TypeId::of::() == TypeId::of::() { return match &self.0 { Union::Char(value, _) => ::downcast_ref::(value), @@ -1720,7 +1720,7 @@ impl From<&ImmutableString> for Dynamic { value.clone().into() } } -#[cfg(not(feature = "no_smartstring"))] +#[cfg(not(feature = "no_smartstring_for_identifier"))] impl From<&crate::Identifier> for Dynamic { #[inline(always)] fn from(value: &crate::Identifier) -> Self { diff --git a/src/lib.rs b/src/lib.rs index 6ddb7aab..4b8633bd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -140,11 +140,11 @@ pub use utils::ImmutableString; /// An identifier in Rhai. [`SmartString`](https://crates.io/crates/smartstring) is used because most /// identifiers are ASCII and short, fewer than 23 characters, so they can be stored inline. -#[cfg(not(feature = "no_smartstring"))] -pub type Identifier = smartstring::SmartString; +#[cfg(not(feature = "no_smartstring_for_identifier"))] +pub type Identifier = SmartString; /// An identifier in Rhai. -#[cfg(feature = "no_smartstring")] +#[cfg(feature = "no_smartstring_for_identifier")] pub type Identifier = ImmutableString; /// A trait to enable registering Rust functions. @@ -305,6 +305,12 @@ type StaticVec = smallvec::SmallVec<[T; 4]>; #[cfg(feature = "internals")] pub type StaticVec = smallvec::SmallVec<[T; 4]>; +#[cfg(not(feature = "internals"))] +pub(crate) type SmartString = smartstring::SmartString; + +#[cfg(feature = "internals")] +pub type SmartString = smartstring::SmartString; + // Compiler guards against mutually-exclusive feature flags #[cfg(feature = "no_float")] diff --git a/src/utils.rs b/src/utils.rs index 6b73aa64..98e381c5 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,7 +1,7 @@ //! Module containing various utility types and functions. use crate::fn_native::{shared_make_mut, shared_take}; -use crate::{Identifier, Shared}; +use crate::{Identifier, Shared, SmartString}; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{ @@ -141,10 +141,10 @@ pub(crate) fn combine_hashes(a: u64, b: u64) -> u64 { /// assert_eq!(s, "hello, world!"); /// ``` #[derive(Clone, Eq, Ord, Hash, Default)] -pub struct ImmutableString(Shared); +pub struct ImmutableString(Shared); impl Deref for ImmutableString { - type Target = String; + type Target = SmartString; #[inline(always)] fn deref(&self) -> &Self::Target { @@ -152,9 +152,9 @@ impl Deref for ImmutableString { } } -impl AsRef for ImmutableString { +impl AsRef for ImmutableString { #[inline(always)] - fn as_ref(&self) -> &String { + fn as_ref(&self) -> &SmartString { &self.0 } } @@ -166,9 +166,9 @@ impl AsRef for ImmutableString { } } -impl Borrow for ImmutableString { +impl Borrow for ImmutableString { #[inline(always)] - fn borrow(&self) -> &String { + fn borrow(&self) -> &SmartString { &self.0 } } @@ -183,33 +183,31 @@ impl Borrow for ImmutableString { impl From<&str> for ImmutableString { #[inline(always)] fn from(value: &str) -> Self { - Self(value.to_string().into()) + Self(Into::::into(value).into()) } } impl From<&String> for ImmutableString { #[inline(always)] fn from(value: &String) -> Self { - Self(value.to_string().into()) + Self(Into::::into(value).into()) } } impl From for ImmutableString { #[inline(always)] fn from(value: String) -> Self { + Self(Into::::into(value).into()) + } +} +impl From for ImmutableString { + #[inline(always)] + fn from(value: SmartString) -> Self { Self(value.into()) } } - -impl From> for ImmutableString { +impl From for SmartString { #[inline(always)] - fn from(value: Box) -> Self { - Self(value.into()) - } -} - -impl From for String { - #[inline(always)] - fn from(value: ImmutableString) -> Self { - value.into_owned() + fn from(mut value: ImmutableString) -> Self { + std::mem::take(shared_make_mut(&mut value.0)) } } @@ -218,35 +216,35 @@ impl FromStr for ImmutableString { #[inline(always)] fn from_str(s: &str) -> Result { - Ok(Self(s.to_string().into())) + Ok(Self(Into::::into(s).into())) } } impl FromIterator for ImmutableString { #[inline(always)] fn from_iter>(iter: T) -> Self { - Self(iter.into_iter().collect::().into()) + Self(iter.into_iter().collect::().into()) } } impl<'a> FromIterator<&'a char> for ImmutableString { #[inline(always)] fn from_iter>(iter: T) -> Self { - Self(iter.into_iter().cloned().collect::().into()) + Self(iter.into_iter().cloned().collect::().into()) } } impl<'a> FromIterator<&'a str> for ImmutableString { #[inline(always)] fn from_iter>(iter: T) -> Self { - Self(iter.into_iter().collect::().into()) + Self(iter.into_iter().collect::().into()) } } impl<'a> FromIterator for ImmutableString { #[inline(always)] fn from_iter>(iter: T) -> Self { - Self(iter.into_iter().collect::().into()) + Self(iter.into_iter().collect::().into()) } } @@ -466,7 +464,7 @@ impl SubAssign<&ImmutableString> for ImmutableString { if self.is_empty() { self.0 = rhs.0.clone(); } else { - self.0 = self.replace(rhs.as_str(), "").into(); + self.0 = Into::::into(self.replace(rhs.as_str(), "")).into(); } } } @@ -479,7 +477,7 @@ impl SubAssign for ImmutableString { if self.is_empty() { self.0 = rhs.0; } else { - self.0 = self.replace(rhs.as_str(), "").into(); + self.0 = Into::::into(self.replace(rhs.as_str(), "")).into(); } } } @@ -518,7 +516,7 @@ impl Sub for &ImmutableString { impl SubAssign for ImmutableString { #[inline(always)] fn sub_assign(&mut self, rhs: String) { - self.0 = self.replace(&rhs, "").into(); + self.0 = Into::::into(self.replace(&rhs, "")).into(); } } @@ -543,7 +541,7 @@ impl Sub for &ImmutableString { impl SubAssign for ImmutableString { #[inline(always)] fn sub_assign(&mut self, rhs: char) { - self.0 = self.replace(rhs, "").into(); + self.0 = Into::::into(self.replace(rhs, "")).into(); } } @@ -588,34 +586,18 @@ impl PartialOrd for String { } } -#[cfg(not(feature = "no_smartstring"))] -impl From for Identifier { - #[inline(always)] - fn from(value: ImmutableString) -> Self { - value.into_owned().into() - } -} - -#[cfg(not(feature = "no_smartstring"))] -impl From for ImmutableString { - #[inline(always)] - fn from(value: Identifier) -> Self { - value.to_string().into() - } -} - impl ImmutableString { /// Consume the [`ImmutableString`] and convert it into a [`String`]. /// If there are other references to the same string, a cloned copy is returned. #[inline(always)] pub fn into_owned(mut self) -> String { self.make_mut(); // Make sure it is unique reference - shared_take(self.0) // Should succeed + shared_take(self.0).into() // Should succeed } /// Make sure that the [`ImmutableString`] is unique (i.e. no other outstanding references). - /// Then return a mutable reference to the [`String`]. + /// Then return a mutable reference to the [`SmartString`]. #[inline(always)] - pub fn make_mut(&mut self) -> &mut String { + pub(crate) fn make_mut(&mut self) -> &mut SmartString { shared_make_mut(&mut self.0) } } @@ -630,17 +612,17 @@ impl ImmutableString { /// yet interned. #[derive(Debug, Clone, Default, Hash)] pub struct IdentifierBuilder( - #[cfg(feature = "no_smartstring")] std::collections::BTreeSet, + #[cfg(feature = "no_smartstring_for_identifier")] std::collections::BTreeSet, ); impl IdentifierBuilder { /// Get an identifier from a text string. #[inline(always)] pub fn get(&mut self, text: impl AsRef + Into) -> Identifier { - #[cfg(not(feature = "no_smartstring"))] + #[cfg(not(feature = "no_smartstring_for_identifier"))] return text.as_ref().into(); - #[cfg(feature = "no_smartstring")] + #[cfg(feature = "no_smartstring_for_identifier")] return self.0.get(text.as_ref()).cloned().unwrap_or_else(|| { let s: Identifier = text.into(); self.0.insert(s.clone()); From cc1f94187577ff72bb3aab8c925062c5878b768c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 23 Apr 2021 23:37:10 +0800 Subject: [PATCH 03/16] Optimize op-assignment. --- src/optimize.rs | 30 +++++++++++++++++++++++++++++- src/parser.rs | 14 +------------- src/token.rs | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/src/optimize.rs b/src/optimize.rs index e9b36d27..3e40d019 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -1,10 +1,11 @@ //! Module implementing the [`AST`] optimizer. -use crate::ast::{Expr, Stmt}; +use crate::ast::{Expr, OpAssignment, Stmt}; use crate::dynamic::AccessMode; use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT, KEYWORD_TYPE_OF}; use crate::fn_builtin::get_builtin_binary_op_fn; use crate::parser::map_dynamic_to_expr; +use crate::token::Token; use crate::utils::get_hasher; use crate::{ calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, Engine, FnPtr, ImmutableString, @@ -381,6 +382,33 @@ fn optimize_stmt_block( /// Optimize a [statement][Stmt]. fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { match stmt { + // var = var op expr => var op= expr + Stmt::Assignment(x, _) + if x.1.is_none() + && x.0.is_variable_access(true) + && matches!(&x.2, Expr::FnCall(x2, _) + if Token::lookup_from_syntax(&x2.name).and_then(|t| t.make_op_assignment()).is_some() + && x2.args_count() == 2 && x2.args.len() >= 1 + && x2.args[0].get_variable_name(true) == x.0.get_variable_name(true) + ) => + { + match &mut x.2 { + Expr::FnCall(x2, _) => { + state.set_dirty(); + let op = Token::lookup_from_syntax(&x2.name).unwrap(); + let op_assignment = op.make_op_assignment().unwrap(); + x.1 = Some(OpAssignment::new(op_assignment.keyword_syntax())); + x.2 = if x2.args.len() > 1 { + mem::take(&mut x2.args[1]) + } else { + let (value, pos) = mem::take(&mut x2.constant_args[0]); + Expr::DynamicConstant(Box::new(value), pos) + }; + } + _ => unreachable!(), + } + } + // expr op= expr Stmt::Assignment(x, _) => match x.0 { Expr::Variable(_, _, _) => optimize_expr(&mut x.2, state), diff --git a/src/parser.rs b/src/parser.rs index 87082f7e..b587fb6b 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1518,19 +1518,7 @@ fn parse_op_assignment_stmt( let op = match token { Token::Equals => "", - - Token::PlusAssign - | Token::MinusAssign - | Token::MultiplyAssign - | Token::DivideAssign - | Token::LeftShiftAssign - | Token::RightShiftAssign - | Token::ModuloAssign - | Token::PowerOfAssign - | Token::AndAssign - | Token::OrAssign - | Token::XOrAssign => token.keyword_syntax(), - + _ if token.map_op_assignment().is_some() => token.keyword_syntax(), _ => return Ok(Stmt::Expr(lhs)), }; diff --git a/src/token.rs b/src/token.rs index c777e4d7..20a4bc98 100644 --- a/src/token.rs +++ b/src/token.rs @@ -578,6 +578,42 @@ impl Token { } } + /// Get the corresponding operator of the token if it is an op-assignment operator. + pub fn map_op_assignment(&self) -> Option { + Some(match self { + Self::PlusAssign => Self::Plus, + Self::MinusAssign => Self::Minus, + Self::MultiplyAssign => Self::Multiply, + Self::DivideAssign => Self::Divide, + Self::LeftShiftAssign => Self::LeftShift, + Self::RightShiftAssign => Self::RightShift, + Self::ModuloAssign => Self::Modulo, + Self::PowerOfAssign => Self::PowerOf, + Self::AndAssign => Self::Ampersand, + Self::OrAssign => Self::Pipe, + Self::XOrAssign => Self::XOr, + _ => return None, + }) + } + + /// Get the corresponding op-assignment operator of the token. + pub fn make_op_assignment(&self) -> Option { + Some(match self { + Self::Plus => Self::PlusAssign, + Self::Minus => Self::MinusAssign, + Self::Multiply => Self::MultiplyAssign, + Self::Divide => Self::DivideAssign, + Self::LeftShift => Self::LeftShiftAssign, + Self::RightShift => Self::RightShiftAssign, + Self::Modulo => Self::ModuloAssign, + Self::PowerOf => Self::PowerOfAssign, + Self::Ampersand => Self::AndAssign, + Self::Pipe => Self::OrAssign, + Self::XOr => Self::XOrAssign, + _ => return None, + }) + } + /// Reverse lookup a token from a piece of syntax. pub fn lookup_from_syntax(syntax: &str) -> Option { use Token::*; From dc3a217b2ff73fe53669e62e7a67474f9a09ef88 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 23 Apr 2021 23:37:33 +0800 Subject: [PATCH 04/16] ImmutableString += String optimized. --- src/utils.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/utils.rs b/src/utils.rs index 98e381c5..234ad303 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -395,7 +395,13 @@ impl Add for &ImmutableString { impl AddAssign for ImmutableString { #[inline(always)] fn add_assign(&mut self, rhs: String) { - self.make_mut().push_str(&rhs); + if !rhs.is_empty() { + if self.is_empty() { + self.0 = Into::::into(rhs).into(); + } else { + self.make_mut().push_str(&rhs); + } + } } } From e58b57b6e7014144c90a2bf43e1eec61ee2abaff Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 23 Apr 2021 23:37:44 +0800 Subject: [PATCH 05/16] Change string building benchmarks. --- benches/eval_expression.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/benches/eval_expression.rs b/benches/eval_expression.rs index db7a6528..9ef69f00 100644 --- a/benches/eval_expression.rs +++ b/benches/eval_expression.rs @@ -126,9 +126,9 @@ fn bench_eval_loop_number(bench: &mut Bencher) { #[bench] fn bench_eval_loop_strings_build(bench: &mut Bencher) { let script = r#" - let s = "hello"; + let s; for x in range(0, 10000) { - s += "x"; + s = "hello, world!" + "hello, world!"; } "#; @@ -143,9 +143,9 @@ fn bench_eval_loop_strings_build(bench: &mut Bencher) { #[bench] fn bench_eval_loop_strings_no_build(bench: &mut Bencher) { let script = r#" - let s = "hello"; + let s; for x in range(0, 10000) { - s += ""; + s = "hello" + ""; } "#; From 61b559a58f092dfa8da69a4799c04bee54b05647 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 24 Apr 2021 11:55:40 +0800 Subject: [PATCH 06/16] Refine op-assignment. --- src/ast.rs | 19 ++++++++++++++----- src/engine.rs | 9 +++++---- src/optimize.rs | 4 ++-- src/parser.rs | 15 +++++---------- src/token.rs | 38 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 64 insertions(+), 21 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 47cf4d16..023302a2 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1347,13 +1347,22 @@ pub struct OpAssignment { } impl OpAssignment { - pub fn new(op: &'static str) -> Self { - let op2 = &op[..op.len() - 1]; // extract operator without = + /// Create a new [`OpAssignment`]. + /// + /// # Panics + /// + /// Panics if the operator name is not an op-assignment operator. + pub fn new(op: Token) -> Self { + let op_raw = op + .map_op_assignment() + .expect("token must be an op-assignment operator") + .keyword_syntax(); + let op_assignment = op.keyword_syntax(); Self { - hash_op_assign: calc_fn_hash(empty(), op, 2), - hash_op: calc_fn_hash(empty(), op2, 2), - op, + hash_op_assign: calc_fn_hash(empty(), op_assignment, 2), + hash_op: calc_fn_hash(empty(), op_raw, 2), + op: op_assignment, } } } diff --git a/src/engine.rs b/src/engine.rs index 1251c193..29439d77 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -11,6 +11,7 @@ use crate::optimize::OptimizationLevel; use crate::packages::{Package, StandardPackage}; use crate::r#unsafe::unsafe_cast_var_name_to_lifetime; use crate::syntax::CustomSyntax; +use crate::token::Token; use crate::utils::get_hasher; use crate::{ Dynamic, EvalAltResult, Identifier, ImmutableString, Module, Position, RhaiResult, Scope, @@ -221,14 +222,14 @@ pub const FN_ANONYMOUS: &str = "anon$"; /// Standard equality comparison operator. pub const OP_EQUALS: &str = "=="; -/// Standard concatenation operator. -pub const OP_CONCAT: &str = "+="; - /// Standard method function for containment testing. /// /// The `in` operator is implemented as a call to this method. pub const OP_CONTAINS: &str = "contains"; +/// Standard concatenation operator token. +pub const TOKEN_OP_CONCAT: Token = Token::PlusAssign; + /// Method of chaining. #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] @@ -1792,7 +1793,7 @@ impl Engine { mods, state, lib, - Some(OpAssignment::new(OP_CONCAT)), + Some(OpAssignment::new(TOKEN_OP_CONCAT)), pos, (&mut result).into(), item, diff --git a/src/optimize.rs b/src/optimize.rs index 3e40d019..941aa695 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -387,7 +387,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { if x.1.is_none() && x.0.is_variable_access(true) && matches!(&x.2, Expr::FnCall(x2, _) - if Token::lookup_from_syntax(&x2.name).and_then(|t| t.make_op_assignment()).is_some() + 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[0].get_variable_name(true) == x.0.get_variable_name(true) ) => @@ -397,7 +397,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { state.set_dirty(); let op = Token::lookup_from_syntax(&x2.name).unwrap(); let op_assignment = op.make_op_assignment().unwrap(); - x.1 = Some(OpAssignment::new(op_assignment.keyword_syntax())); + x.1 = Some(OpAssignment::new(op_assignment)); x.2 = if x2.args.len() > 1 { mem::take(&mut x2.args[1]) } else { diff --git a/src/parser.rs b/src/parser.rs index b587fb6b..b33ae36e 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1409,7 +1409,7 @@ fn parse_unary( /// Make an assignment statement. fn make_assignment_stmt<'a>( - op: &'static str, + op: Option, state: &mut ParseState, lhs: Expr, rhs: Expr, @@ -1432,11 +1432,7 @@ fn make_assignment_stmt<'a>( } } - let op_info = if op.is_empty() { - None - } else { - Some(OpAssignment::new(op)) - }; + let op_info = op.map(|v| OpAssignment::new(v)); match &lhs { // const_expr = rhs @@ -1516,13 +1512,12 @@ fn parse_op_assignment_stmt( let (token, token_pos) = input.peek().unwrap(); settings.pos = *token_pos; - let op = match token { - Token::Equals => "", - _ if token.map_op_assignment().is_some() => token.keyword_syntax(), + let (op, pos) = match token { + Token::Equals => (None, input.next().unwrap().1), + _ if token.is_op_assignment() => input.next().map(|(op, pos)| (Some(op), pos)).unwrap(), _ => return Ok(Stmt::Expr(lhs)), }; - let (_, pos) = input.next().unwrap(); let rhs = parse_expr(input, state, lib, settings.level_up())?; make_assignment_stmt(op, state, lhs, rhs, pos) } diff --git a/src/token.rs b/src/token.rs index 20a4bc98..490bc232 100644 --- a/src/token.rs +++ b/src/token.rs @@ -578,6 +578,25 @@ impl Token { } } + /// Is this token an op-assignment operator? + #[inline] + pub fn is_op_assignment(&self) -> bool { + match self { + Self::PlusAssign + | Self::MinusAssign + | Self::MultiplyAssign + | Self::DivideAssign + | Self::LeftShiftAssign + | Self::RightShiftAssign + | Self::ModuloAssign + | Self::PowerOfAssign + | Self::AndAssign + | Self::OrAssign + | Self::XOrAssign => true, + _ => false, + } + } + /// Get the corresponding operator of the token if it is an op-assignment operator. pub fn map_op_assignment(&self) -> Option { Some(match self { @@ -596,6 +615,25 @@ impl Token { }) } + /// Has this token a corresponding op-assignment operator? + #[inline] + pub fn has_op_assignment(&self) -> bool { + match self { + Self::Plus + | Self::Minus + | Self::Multiply + | Self::Divide + | Self::LeftShift + | Self::RightShift + | Self::Modulo + | Self::PowerOf + | Self::Ampersand + | Self::Pipe + | Self::XOr => true, + _ => false, + } + } + /// Get the corresponding op-assignment operator of the token. pub fn make_op_assignment(&self) -> Option { Some(match self { From b5a2937336a43f913c04ccef31ff58a799bcca49 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 24 Apr 2021 13:42:30 +0800 Subject: [PATCH 07/16] Do not treat Expr::Index with pure index as pure. --- CHANGELOG.md | 1 + src/ast.rs | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65c55117..235048e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Bug fixes --------- * Fixed bug when position is zero in `insert` and `split_at` methods for arrays. +* Indexing operations with pure index values are no longer considered pure due to the possibility of indexers. Breaking changes ---------------- diff --git a/src/ast.rs b/src/ast.rs index 023302a2..3d16bf68 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1899,9 +1899,7 @@ impl Expr { Self::Map(x, _) => x.0.iter().map(|(_, v)| v).all(Self::is_pure), - Self::Index(x, _) | Self::And(x, _) | Self::Or(x, _) => { - x.lhs.is_pure() && x.rhs.is_pure() - } + Self::And(x, _) | Self::Or(x, _) => x.lhs.is_pure() && x.rhs.is_pure(), Self::Stmt(x) => x.0.iter().all(Stmt::is_pure), From ce35f7fa725906c7bf294454d5b1d86725047e56 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 24 Apr 2021 13:42:45 +0800 Subject: [PATCH 08/16] Fix off by one error in optimizer. --- src/optimize.rs | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/src/optimize.rs b/src/optimize.rs index 941aa695..77243bbf 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -239,29 +239,18 @@ fn optimize_stmt_block( .find_map(|(i, stmt)| match stmt { stmt if !is_pure(stmt) => Some(i), - Stmt::Noop(_) | Stmt::Return(_, None, _) => None, - - Stmt::Let(e, _, _, _) - | Stmt::Const(e, _, _, _) - | Stmt::Expr(e) - | Stmt::Return(_, Some(e), _) - if e.is_constant() => + Stmt::Let(e, _, _, _) | Stmt::Const(e, _, _, _) | Stmt::Expr(e) + if !e.is_constant() => { - None + Some(i) } #[cfg(not(feature = "no_module"))] - Stmt::Import(e, _, _) if e.is_constant() => None, + Stmt::Import(e, _, _) if !e.is_constant() => Some(i), - #[cfg(not(feature = "no_module"))] - Stmt::Export(_, _) => None, - - #[cfg(not(feature = "no_closure"))] - Stmt::Share(_) => None, - - _ => Some(i), + _ => None, }) - .map_or(0, |n| statements.len() - n); + .map_or(0, |n| statements.len() - n - 1); while index < statements.len() { if preserve_result && index >= statements.len() - 1 { From c82a47ac2697dbba00503c0fd2fba8a5f294208a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 24 Apr 2021 14:47:20 +0800 Subject: [PATCH 09/16] Unchecked index access. --- src/engine.rs | 85 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 29 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 29439d77..dfbe507d 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1632,6 +1632,7 @@ impl Engine { let arr_len = arr.len(); + #[cfg(not(feature = "unchecked"))] let arr_idx = if index < 0 { // Count from end if negative arr_len @@ -1651,10 +1652,20 @@ impl Engine { } else { index as usize }; + #[cfg(feature = "unchecked")] + let arr_idx = if index < 0 { + arr_len - index.abs() as usize + } else { + index as usize + }; - arr.get_mut(arr_idx) - .map(Target::from) - .ok_or_else(|| EvalAltResult::ErrorArrayBounds(arr_len, index, idx_pos).into()) + #[cfg(not(feature = "unchecked"))] + return arr.get_mut(arr_idx).map(Target::from).ok_or_else(|| { + EvalAltResult::ErrorArrayBounds(arr_len, index, idx_pos).into() + }); + + #[cfg(feature = "unchecked")] + return Ok(Target::from(&mut arr[arr_idx])); } #[cfg(not(feature = "no_object"))] @@ -1668,25 +1679,26 @@ impl Engine { map.insert(index.clone().into(), Default::default()); } - Ok(map + return Ok(map .get_mut(index.as_str()) .map(Target::from) - .unwrap_or_else(|| Target::from(()))) + .unwrap_or_else(|| Target::from(()))); } #[cfg(not(feature = "no_index"))] Dynamic(Union::Str(s, _)) => { // val_string[idx] - let chars_len = s.chars().count(); + let _chars_len = s.chars().count(); let index = _idx .as_int() .map_err(|err| self.make_type_mismatch_err::(err, idx_pos))?; + #[cfg(not(feature = "unchecked"))] let (ch, offset) = if index >= 0 { let offset = index as usize; ( s.chars().nth(offset).ok_or_else(|| { - EvalAltResult::ErrorStringBounds(chars_len, index, idx_pos) + EvalAltResult::ErrorStringBounds(_chars_len, index, idx_pos) })?, offset, ) @@ -1694,15 +1706,24 @@ impl Engine { let offset = index as usize; ( s.chars().rev().nth(offset - 1).ok_or_else(|| { - EvalAltResult::ErrorStringBounds(chars_len, index, idx_pos) + EvalAltResult::ErrorStringBounds(_chars_len, index, idx_pos) })?, offset, ) } else { - return EvalAltResult::ErrorStringBounds(chars_len, index, idx_pos).into(); + return EvalAltResult::ErrorStringBounds(_chars_len, index, idx_pos).into(); }; - Ok(Target::StringChar(target, offset, ch.into())) + #[cfg(feature = "unchecked")] + let (ch, offset) = if index >= 0 { + let offset = index as usize; + (s.chars().nth(offset).unwrap(), offset) + } else { + let offset = index.abs() as usize; + (s.chars().rev().nth(offset - 1).unwrap(), offset) + }; + + return Ok(Target::StringChar(target, offset, ch.into())); } #[cfg(not(feature = "no_index"))] @@ -1711,27 +1732,33 @@ impl Engine { let args = &mut [target, &mut _idx]; let hash_get = FnCallHashes::from_native(calc_fn_hash(std::iter::empty(), FN_IDX_GET, 2)); - self.exec_fn_call( - _mods, state, _lib, FN_IDX_GET, hash_get, args, _is_ref, true, idx_pos, None, - _level, - ) - .map(|(v, _)| v.into()) - .map_err(|err| match *err { - EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.ends_with(']') => { - Box::new(EvalAltResult::ErrorIndexingType( - type_name.into(), - Position::NONE, - )) - } - _ => err, - }) + + return self + .exec_fn_call( + _mods, state, _lib, FN_IDX_GET, hash_get, args, _is_ref, true, idx_pos, + None, _level, + ) + .map(|(v, _)| v.into()) + .map_err(|err| match *err { + EvalAltResult::ErrorFunctionNotFound(fn_sig, _) + if fn_sig.ends_with(']') => + { + Box::new(EvalAltResult::ErrorIndexingType( + type_name.into(), + Position::NONE, + )) + } + _ => err, + }); } - _ => EvalAltResult::ErrorIndexingType( - self.map_type_name(target.type_name()).into(), - Position::NONE, - ) - .into(), + _ => { + return EvalAltResult::ErrorIndexingType( + self.map_type_name(target.type_name()).into(), + Position::NONE, + ) + .into() + } } } From 41d3709db1a29aa21a537dd414e93782975bf815 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 24 Apr 2021 15:53:02 +0800 Subject: [PATCH 10/16] Fix decimal build. --- src/packages/iter_basic.rs | 95 ++++++++++++++++------- tests/for.rs | 150 ++++++++++++++++++++++++++++++++----- 2 files changed, 203 insertions(+), 42 deletions(-) diff --git a/src/packages/iter_basic.rs b/src/packages/iter_basic.rs index 961f45e4..42a984b8 100644 --- a/src/packages/iter_basic.rs +++ b/src/packages/iter_basic.rs @@ -5,24 +5,20 @@ use std::ops::Range; use std::prelude::v1::*; #[cfg(not(feature = "unchecked"))] -use num_traits::{CheckedAdd as Add, CheckedSub as Sub}; +use num_traits::{CheckedAdd, CheckedSub}; #[cfg(feature = "unchecked")] use std::ops::{Add, Sub}; -fn get_range(from: T, to: T) -> Result, Box> { - Ok(from..to) -} - // Register range function with step #[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] struct StepRange(T, T, T) where - T: Variant + Copy + PartialOrd + Add + Sub; + T: Variant + Copy + PartialOrd + CheckedAdd + CheckedSub; impl StepRange where - T: Variant + Copy + PartialOrd + Add + Sub, + T: Variant + Copy + PartialOrd + CheckedAdd + CheckedSub, { pub fn new(from: T, to: T, step: T) -> Result> { #[cfg(not(feature = "unchecked"))] @@ -47,7 +43,7 @@ where impl Iterator for StepRange where - T: Variant + Copy + PartialOrd + Add + Sub, + T: Variant + Copy + PartialOrd + CheckedAdd + CheckedSub, { type Item = T; @@ -130,18 +126,11 @@ where } } -fn get_step_range(from: T, to: T, step: T) -> Result, Box> -where - T: Variant + Copy + PartialOrd + Add + Sub, -{ - StepRange::::new(from, to, step) -} - macro_rules! reg_range { ($lib:ident | $x:expr => $( $y:ty ),*) => { $( $lib.set_iterator::>(); - let _hash = $lib.set_native_fn($x, get_range::<$y>); + let _hash = $lib.set_native_fn($x, |from: $y, to: $y| Ok(from..to)); #[cfg(feature = "metadata")] $lib.update_fn_metadata(_hash, &[ @@ -154,7 +143,7 @@ macro_rules! reg_range { ($lib:ident | step $x:expr => $( $y:ty ),*) => { $( $lib.set_iterator::>(); - let _hash = $lib.set_native_fn($x, get_step_range::<$y>); + let _hash = $lib.set_native_fn($x, |from: $y, to: $y, step: $y| StepRange::new(from, to, step)); #[cfg(feature = "metadata")] $lib.update_fn_metadata(_hash, &[ @@ -192,12 +181,72 @@ def_package!(crate:BasicIteratorPackage:"Basic range iterators.", lib, { } } + #[cfg(not(feature = "no_float"))] + { + use crate::FLOAT; + + #[derive(Debug, Clone, Copy, PartialEq)] + struct StepFloatRange(FLOAT, FLOAT, FLOAT); + + impl StepFloatRange { + pub fn new(from: FLOAT, to: FLOAT, step: FLOAT) -> Result> { + #[cfg(not(feature = "unchecked"))] + if step == 0.0 { + return EvalAltResult::ErrorInFunctionCall("range".to_string(), "".to_string(), + Box::new(EvalAltResult::ErrorArithmetic("step value cannot be zero".to_string(), crate::Position::NONE)), + crate::Position::NONE, + ).into(); + } + + Ok(Self(from, to, step)) + } + } + + impl Iterator for StepFloatRange { + type Item = FLOAT; + + fn next(&mut self) -> Option { + if self.0 == self.1 { + None + } else if self.0 < self.1 { + #[cfg(not(feature = "unchecked"))] + if self.2 < 0.0 { + return None; + } + + let v = self.0; + let n = self.0 + self.2; + + self.0 = if n >= self.1 { self.1 } else { n }; + Some(v) + } else { + #[cfg(not(feature = "unchecked"))] + if self.2 > 0.0 { + return None; + } + + let v = self.0; + let n = self.0 + self.2; + + self.0 = if n <= self.1 { self.1 } else { n }; + Some(v) + } + } + } + + impl std::iter::FusedIterator for StepFloatRange {} + + lib.set_iterator::(); + + let _hash = lib.set_native_fn("range", |from, to, step| StepFloatRange::new(from, to, step)); + #[cfg(feature = "metadata")] + lib.update_fn_metadata(_hash, &["from: FLOAT", "to: FLOAT", "step: FLOAT", "Iterator"]); + } + #[cfg(feature = "decimal")] { - use rust_decimal::{ - prelude::{One, Zero}, - Decimal, - }; + use rust_decimal::Decimal; + use num_traits::Zero; #[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] struct StepDecimalRange(Decimal, Decimal, Decimal); @@ -252,10 +301,6 @@ def_package!(crate:BasicIteratorPackage:"Basic range iterators.", lib, { lib.set_iterator::(); - let _hash = lib.set_native_fn("range", |from, to| StepDecimalRange::new(from, to, Decimal::one())); - #[cfg(feature = "metadata")] - lib.update_fn_metadata(_hash, &["from: Decimal", "to: Decimal", "Iterator"]); - let _hash = lib.set_native_fn("range", |from, to, step| StepDecimalRange::new(from, to, step)); #[cfg(feature = "metadata")] lib.update_fn_metadata(_hash, &["from: Decimal", "to: Decimal", "step: Decimal", "Iterator"]); diff --git a/tests/for.rs b/tests/for.rs index 3118b8a7..381809d6 100644 --- a/tests/for.rs +++ b/tests/for.rs @@ -1,31 +1,52 @@ use rhai::{Engine, EvalAltResult, Module, INT}; -#[cfg(not(feature = "no_index"))] +#[cfg(not(feature = "no_float"))] +use rhai::FLOAT; + +#[cfg(feature = "decimal")] +#[cfg(not(feature = "no_float"))] +use rust_decimal::Decimal; + #[test] fn test_for() -> Result<(), Box> { let engine = Engine::new(); - let script = " - let sum1 = 0; - let sum2 = 0; - let inputs = [1, 2, 3, 4, 5]; + #[cfg(not(feature = "no_index"))] + assert_eq!( + engine.eval::( + " + let sum1 = 0; + let sum2 = 0; + let inputs = [1, 2, 3, 4, 5]; - for x in inputs { - sum1 += x; - } + for x in inputs { + sum1 += x; + } - for x in range(1, 6) { - sum2 += x; - } + for x in range(1, 6) { + sum2 += x; + } - for x in range(1, 6, 3) { - sum2 += x; - } + for x in range(1, 6, 3) { + sum2 += x; + } - sum1 + sum2 - "; + sum1 + sum2 + " + )?, + 35 + ); - assert_eq!(engine.eval::(script)?, 35); + assert_eq!( + engine.eval::( + " + let sum = 0; + for x in range(1, 10) { sum += x; } + sum + " + )?, + 45 + ); assert_eq!( engine.eval::( @@ -71,6 +92,101 @@ fn test_for() -> Result<(), Box> { 30 ); + #[cfg(not(feature = "no_float"))] + { + assert_eq!( + engine.eval::( + " + let sum = 0.0; + for x in range(1.0, 10.0, 2.0) { sum += x; } + sum + " + )?, + 25.0 + ); + + assert_eq!( + engine.eval::( + " + let sum = 0.0; + for x in range(10.0, 1.0, 2.0) { sum += x; } + sum + " + )?, + 0.0 + ); + + assert_eq!( + engine.eval::( + " + let sum = 0.0; + for x in range(1.0, 10.0, -2.0) { sum += x; } + sum + " + )?, + 0.0 + ); + + assert_eq!( + engine.eval::( + " + let sum = 0.0; + for x in range(10.0, 1.0, -2.0) { sum += x; } + sum + " + )?, + 30.0 + ); + } + + #[cfg(not(feature = "no_float"))] + #[cfg(feature = "decimal")] + { + assert_eq!( + engine.eval::( + " + let sum = to_decimal(0); + for x in range(to_decimal(1), to_decimal(10), to_decimal(2)) { sum += x; } + sum + " + )?, + Decimal::from(25) + ); + + assert_eq!( + engine.eval::( + " + let sum = to_decimal(0); + for x in range(to_decimal(10), to_decimal(1), to_decimal(2)) { sum += x; } + sum + " + )?, + Decimal::from(0) + ); + + assert_eq!( + engine.eval::( + " + let sum = to_decimal(0); + for x in range(to_decimal(1), to_decimal(10), to_decimal(-2)) { sum += x; } + sum + " + )?, + Decimal::from(0) + ); + + assert_eq!( + engine.eval::( + " + let sum = to_decimal(0); + for x in range(to_decimal(10), to_decimal(1), to_decimal(-2)) { sum += x; } + sum + " + )?, + Decimal::from(30) + ); + } + Ok(()) } From f81e3d6ff8b5737a2b0049b06907ffa4cd331136 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 24 Apr 2021 18:14:48 +0800 Subject: [PATCH 11/16] Fix unchecked builds. --- src/engine_api.rs | 2 +- src/module/resolvers/file.rs | 2 +- src/packages/iter_basic.rs | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/engine_api.rs b/src/engine_api.rs index 973b1a5c..e73accdc 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1074,7 +1074,7 @@ impl Engine { Some(Ok(module_ast)) => { collect_imports(&module_ast, &mut resolver, &mut imports) } - Some(err @ Err(_)) => return err, + Some(err) => return err, None => (), } diff --git a/src/module/resolvers/file.rs b/src/module/resolvers/file.rs index ab3800e6..d933b028 100644 --- a/src/module/resolvers/file.rs +++ b/src/module/resolvers/file.rs @@ -342,7 +342,7 @@ impl ModuleResolver for FileModuleResolver { ast.set_source(path); Some(Ok(ast)) } - err @ Err(_) => Some(err), + err => Some(err), } } } diff --git a/src/packages/iter_basic.rs b/src/packages/iter_basic.rs index 42a984b8..e09d8923 100644 --- a/src/packages/iter_basic.rs +++ b/src/packages/iter_basic.rs @@ -5,7 +5,7 @@ use std::ops::Range; use std::prelude::v1::*; #[cfg(not(feature = "unchecked"))] -use num_traits::{CheckedAdd, CheckedSub}; +use num_traits::{CheckedAdd as Add, CheckedSub as Sub}; #[cfg(feature = "unchecked")] use std::ops::{Add, Sub}; @@ -14,11 +14,11 @@ use std::ops::{Add, Sub}; #[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] struct StepRange(T, T, T) where - T: Variant + Copy + PartialOrd + CheckedAdd + CheckedSub; + T: Variant + Copy + PartialOrd + Add + Sub; impl StepRange where - T: Variant + Copy + PartialOrd + CheckedAdd + CheckedSub, + T: Variant + Copy + PartialOrd + Add + Sub, { pub fn new(from: T, to: T, step: T) -> Result> { #[cfg(not(feature = "unchecked"))] @@ -43,7 +43,7 @@ where impl Iterator for StepRange where - T: Variant + Copy + PartialOrd + CheckedAdd + CheckedSub, + T: Variant + Copy + PartialOrd + Add + Sub, { type Item = T; From 510c28c5579af9c28f300a75018b71ea3e13ef2a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 24 Apr 2021 22:19:41 +0800 Subject: [PATCH 12/16] Refine badges. --- README.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index e748286a..816e59a6 100644 --- a/README.md +++ b/README.md @@ -3,13 +3,15 @@ Rhai - Embedded Scripting for Rust ![GitHub last commit](https://img.shields.io/github/last-commit/rhaiscript/rhai?logo=github) [![Build Status](https://github.com/rhaiscript/rhai/workflows/Build/badge.svg)](https://github.com/rhaiscript/rhai/actions) -[![stars](https://img.shields.io/github/stars/rhaiscript/rhai?logo=github)](https://github.com/rhaiscript/rhai) -[![license](https://img.shields.io/crates/l/rhai)](https://github.com/license/rhaiscript/rhai) +[![Stars](https://img.shields.io/github/stars/rhaiscript/rhai?logo=github)](https://github.com/rhaiscript/rhai) +[![License](https://img.shields.io/crates/l/rhai)](https://github.com/license/rhaiscript/rhai) [![crates.io](https://img.shields.io/crates/v/rhai?logo=rust)](https://crates.io/crates/rhai/) [![crates.io](https://img.shields.io/crates/d/rhai?logo=rust)](https://crates.io/crates/rhai/) [![API Docs](https://docs.rs/rhai/badge.svg?logo=docs.rs)](https://docs.rs/rhai/) -[![chat](https://img.shields.io/discord/767611025456889857.svg?logo=discord)](https://discord.gg/HquqbYFcZ9) -[![Reddit](https://img.shields.io/reddit/subreddit-subscribers/Rhai?logo=reddit)](https://www.reddit.com/r/Rhai) +[![VS Code plugin installs](https://img.shields.io/visual-studio-marketplace/i/rhaiscript.vscode-rhai?label=vs%20code)](https://img.shields.io/visual-studio-marketplace/i/rhaiscript.vscode-rhai) +[![Sublime Text package downloads](https://img.shields.io/packagecontrol/dt/Rhai.svg?label=sublime%20text)](https://packagecontrol.io/packages/Rhai) +[![Discord Chat](https://img.shields.io/discord/767611025456889857.svg?logo=discord&label=discord)](https://discord.gg/HquqbYFcZ9) +[![Reddit Channel](https://img.shields.io/reddit/subreddit-subscribers/Rhai?logo=reddit&label=reddit)](https://www.reddit.com/r/Rhai) [![Rhai logo](https://rhai.rs/book/images/logo/rhai-banner-transparent-colour.svg)](https://rhai.rs) From 24c01f6f9fa2ccbaf222fc95cdbfee2c0e0c361f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 25 Apr 2021 14:39:02 +0800 Subject: [PATCH 13/16] Fix badge url. --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 816e59a6..568b8982 100644 --- a/README.md +++ b/README.md @@ -7,9 +7,9 @@ Rhai - Embedded Scripting for Rust [![License](https://img.shields.io/crates/l/rhai)](https://github.com/license/rhaiscript/rhai) [![crates.io](https://img.shields.io/crates/v/rhai?logo=rust)](https://crates.io/crates/rhai/) [![crates.io](https://img.shields.io/crates/d/rhai?logo=rust)](https://crates.io/crates/rhai/) -[![API Docs](https://docs.rs/rhai/badge.svg?logo=docs.rs)](https://docs.rs/rhai/) -[![VS Code plugin installs](https://img.shields.io/visual-studio-marketplace/i/rhaiscript.vscode-rhai?label=vs%20code)](https://img.shields.io/visual-studio-marketplace/i/rhaiscript.vscode-rhai) -[![Sublime Text package downloads](https://img.shields.io/packagecontrol/dt/Rhai.svg?label=sublime%20text)](https://packagecontrol.io/packages/Rhai) +[![API Docs](https://docs.rs/rhai/badge.svg?logo=docs-rs)](https://docs.rs/rhai/) +[![VS Code plugin installs](https://img.shields.io/visual-studio-marketplace/i/rhaiscript.vscode-rhai?logo=visual-studio-code&label=vs%20code)](https://marketplace.visualstudio.com/items?itemName=rhaiscript.vscode-rhai) +[![Sublime Text package downloads](https://img.shields.io/packagecontrol/dt/Rhai.svg?logo=sublime-text&label=sublime%20text)](https://packagecontrol.io/packages/Rhai) [![Discord Chat](https://img.shields.io/discord/767611025456889857.svg?logo=discord&label=discord)](https://discord.gg/HquqbYFcZ9) [![Reddit Channel](https://img.shields.io/reddit/subreddit-subscribers/Rhai?logo=reddit&label=reddit)](https://www.reddit.com/r/Rhai) From ed18b3f32a11ef9c6a9fb6c35058359affec97b6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 25 Apr 2021 15:27:43 +0800 Subject: [PATCH 14/16] Fix unchecked test. --- tests/for.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/for.rs b/tests/for.rs index 381809d6..0a7cb840 100644 --- a/tests/for.rs +++ b/tests/for.rs @@ -8,7 +8,7 @@ use rhai::FLOAT; use rust_decimal::Decimal; #[test] -fn test_for() -> Result<(), Box> { +fn test_for_loop() -> Result<(), Box> { let engine = Engine::new(); #[cfg(not(feature = "no_index"))] @@ -59,6 +59,7 @@ fn test_for() -> Result<(), Box> { 25 ); + #[cfg(not(feature = "unchecked"))] assert_eq!( engine.eval::( " @@ -70,6 +71,7 @@ fn test_for() -> Result<(), Box> { 0 ); + #[cfg(not(feature = "unchecked"))] assert_eq!( engine.eval::( " @@ -105,6 +107,7 @@ fn test_for() -> Result<(), Box> { 25.0 ); + #[cfg(not(feature = "unchecked"))] assert_eq!( engine.eval::( " @@ -116,6 +119,7 @@ fn test_for() -> Result<(), Box> { 0.0 ); + #[cfg(not(feature = "unchecked"))] assert_eq!( engine.eval::( " @@ -153,6 +157,7 @@ fn test_for() -> Result<(), Box> { Decimal::from(25) ); + #[cfg(not(feature = "unchecked"))] assert_eq!( engine.eval::( " @@ -164,6 +169,7 @@ fn test_for() -> Result<(), Box> { Decimal::from(0) ); + #[cfg(not(feature = "unchecked"))] assert_eq!( engine.eval::( " From a5d4a0abb98675b9dab06c44a94d80f16036babd Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 25 Apr 2021 15:27:58 +0800 Subject: [PATCH 15/16] Disable on_progress with unchecked. --- CHANGELOG.md | 1 + src/engine.rs | 106 ++++++++++++++++++++++------------------------ src/engine_api.rs | 1 + src/fn_call.rs | 18 +++++--- src/fn_native.rs | 2 + 5 files changed, 66 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 235048e2..d8f6f26e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Breaking changes * `Dynamic::is_shared` and `Dynamic::is_locked` are removed under the `no_closure` feature. They used to always return `false`. * `Engine::call_fn` now evaluates the `AST` before calling the function. +* `Engine::on_progress` is disabled with `unchecked`. Enhancements ------------ diff --git a/src/engine.rs b/src/engine.rs index dfbe507d..f85d5b66 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -3,8 +3,7 @@ use crate::ast::{Expr, FnCallExpr, Ident, OpAssignment, ReturnType, Stmt}; use crate::dynamic::{map_std_type_name, AccessMode, Union, Variant}; use crate::fn_native::{ - CallableFunction, IteratorFn, OnDebugCallback, OnPrintCallback, OnProgressCallback, - OnVarCallback, + CallableFunction, IteratorFn, OnDebugCallback, OnPrintCallback, OnVarCallback, }; use crate::module::NamespaceRef; use crate::optimize::OptimizationLevel; @@ -775,7 +774,8 @@ pub struct Engine { /// Callback closure for implementing the `debug` command. pub(crate) debug: OnDebugCallback, /// Callback closure for progress reporting. - pub(crate) progress: Option, + #[cfg(not(feature = "unchecked"))] + pub(crate) progress: Option, /// Optimize the AST after compilation. pub(crate) optimization_level: OptimizationLevel, @@ -879,6 +879,7 @@ impl Engine { debug: Box::new(default_debug), // progress callback + #[cfg(not(feature = "unchecked"))] progress: None, // optimization level @@ -939,6 +940,8 @@ impl Engine { print: Box::new(|_| {}), debug: Box::new(|_, _, _| {}), + + #[cfg(not(feature = "unchecked"))] progress: None, optimization_level: if cfg!(feature = "no_optimize") { @@ -1459,8 +1462,9 @@ impl Engine { match lhs { // id.??? or id[???] - Expr::Variable(_, var_pos, x) => { - self.inc_operations(state, *var_pos)?; + Expr::Variable(_, _var_pos, x) => { + #[cfg(not(feature = "unchecked"))] + self.inc_operations(state, *_var_pos)?; let (target, pos) = self.search_namespace(scope, mods, state, lib, this_ptr, lhs)?; @@ -1511,6 +1515,7 @@ impl Engine { size: usize, level: usize, ) -> Result<(), Box> { + #[cfg(not(feature = "unchecked"))] self.inc_operations(state, expr.position())?; match expr { @@ -1620,6 +1625,7 @@ impl Engine { _indexers: bool, _level: usize, ) -> Result, Box> { + #[cfg(not(feature = "unchecked"))] self.inc_operations(state, Position::NONE)?; match target { @@ -1654,18 +1660,15 @@ impl Engine { }; #[cfg(feature = "unchecked")] let arr_idx = if index < 0 { + // Count from end if negative arr_len - index.abs() as usize } else { index as usize }; - #[cfg(not(feature = "unchecked"))] - return arr.get_mut(arr_idx).map(Target::from).ok_or_else(|| { - EvalAltResult::ErrorArrayBounds(arr_len, index, idx_pos).into() - }); - - #[cfg(feature = "unchecked")] - return Ok(Target::from(&mut arr[arr_idx])); + arr.get_mut(arr_idx) + .map(Target::from) + .ok_or_else(|| EvalAltResult::ErrorArrayBounds(arr_len, index, idx_pos).into()) } #[cfg(not(feature = "no_object"))] @@ -1679,26 +1682,26 @@ impl Engine { map.insert(index.clone().into(), Default::default()); } - return Ok(map + Ok(map .get_mut(index.as_str()) .map(Target::from) - .unwrap_or_else(|| Target::from(()))); + .unwrap_or_else(|| Target::from(Dynamic::UNIT))) } #[cfg(not(feature = "no_index"))] Dynamic(Union::Str(s, _)) => { // val_string[idx] - let _chars_len = s.chars().count(); let index = _idx .as_int() .map_err(|err| self.make_type_mismatch_err::(err, idx_pos))?; - #[cfg(not(feature = "unchecked"))] let (ch, offset) = if index >= 0 { + // Count from end if negative let offset = index as usize; ( s.chars().nth(offset).ok_or_else(|| { - EvalAltResult::ErrorStringBounds(_chars_len, index, idx_pos) + let chars_len = s.chars().count(); + EvalAltResult::ErrorStringBounds(chars_len, index, idx_pos) })?, offset, ) @@ -1706,24 +1709,17 @@ impl Engine { let offset = index as usize; ( s.chars().rev().nth(offset - 1).ok_or_else(|| { - EvalAltResult::ErrorStringBounds(_chars_len, index, idx_pos) + let chars_len = s.chars().count(); + EvalAltResult::ErrorStringBounds(chars_len, index, idx_pos) })?, offset, ) } else { - return EvalAltResult::ErrorStringBounds(_chars_len, index, idx_pos).into(); + let chars_len = s.chars().count(); + return EvalAltResult::ErrorStringBounds(chars_len, index, idx_pos).into(); }; - #[cfg(feature = "unchecked")] - let (ch, offset) = if index >= 0 { - let offset = index as usize; - (s.chars().nth(offset).unwrap(), offset) - } else { - let offset = index.abs() as usize; - (s.chars().rev().nth(offset - 1).unwrap(), offset) - }; - - return Ok(Target::StringChar(target, offset, ch.into())); + Ok(Target::StringChar(target, offset, ch.into())) } #[cfg(not(feature = "no_index"))] @@ -1733,32 +1729,27 @@ impl Engine { let hash_get = FnCallHashes::from_native(calc_fn_hash(std::iter::empty(), FN_IDX_GET, 2)); - return self - .exec_fn_call( - _mods, state, _lib, FN_IDX_GET, hash_get, args, _is_ref, true, idx_pos, - None, _level, - ) - .map(|(v, _)| v.into()) - .map_err(|err| match *err { - EvalAltResult::ErrorFunctionNotFound(fn_sig, _) - if fn_sig.ends_with(']') => - { - Box::new(EvalAltResult::ErrorIndexingType( - type_name.into(), - Position::NONE, - )) - } - _ => err, - }); + self.exec_fn_call( + _mods, state, _lib, FN_IDX_GET, hash_get, args, _is_ref, true, idx_pos, None, + _level, + ) + .map(|(v, _)| v.into()) + .map_err(|err| match *err { + EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.ends_with(']') => { + Box::new(EvalAltResult::ErrorIndexingType( + type_name.into(), + Position::NONE, + )) + } + _ => err, + }) } - _ => { - return EvalAltResult::ErrorIndexingType( - self.map_type_name(target.type_name()).into(), - Position::NONE, - ) - .into() - } + _ => EvalAltResult::ErrorIndexingType( + self.map_type_name(target.type_name()).into(), + Position::NONE, + ) + .into(), } } @@ -1773,6 +1764,7 @@ impl Engine { expr: &Expr, level: usize, ) -> RhaiResult { + #[cfg(not(feature = "unchecked"))] self.inc_operations(state, expr.position())?; let result = match expr { @@ -2110,6 +2102,7 @@ impl Engine { stmt: &Stmt, level: usize, ) -> RhaiResult { + #[cfg(not(feature = "unchecked"))] self.inc_operations(state, stmt.position())?; let result = match stmt { @@ -2138,6 +2131,7 @@ impl Engine { .into(); } + #[cfg(not(feature = "unchecked"))] self.inc_operations(state, pos)?; if lhs_ptr.is_read_only() { @@ -2332,7 +2326,6 @@ impl Engine { // For loop Stmt::For(expr, x, _) => { let (Ident { name, .. }, statements) = x.as_ref(); - let pos = statements.position(); let iter_obj = self .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .flatten(); @@ -2386,7 +2379,8 @@ impl Engine { *loop_var = value; } - self.inc_operations(state, pos)?; + #[cfg(not(feature = "unchecked"))] + self.inc_operations(state, statements.position())?; if statements.is_empty() { continue; @@ -2828,6 +2822,7 @@ impl Engine { } /// Check if the number of operations stay within limit. + #[cfg(not(feature = "unchecked"))] pub(crate) fn inc_operations( &self, state: &mut State, @@ -2835,7 +2830,6 @@ impl Engine { ) -> Result<(), Box> { state.operations += 1; - #[cfg(not(feature = "unchecked"))] // Guard against too many operations if self.max_operations() > 0 && state.operations > self.max_operations() { return EvalAltResult::ErrorTooManyOperations(pos).into(); diff --git a/src/engine_api.rs b/src/engine_api.rs index e73accdc..1722dd76 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -2105,6 +2105,7 @@ impl Engine { /// # Ok(()) /// # } /// ``` + #[cfg(not(feature = "unchecked"))] #[inline(always)] pub fn on_progress( &mut self, diff --git a/src/fn_call.rs b/src/fn_call.rs index 896e934a..fff4dfb6 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -294,6 +294,7 @@ impl Engine { is_op_assignment: bool, pos: Position, ) -> Result<(Dynamic, bool), Box> { + #[cfg(not(feature = "unchecked"))] self.inc_operations(state, pos)?; let state_source = state.source.clone(); @@ -483,6 +484,7 @@ impl Engine { .into() } + #[cfg(not(feature = "unchecked"))] self.inc_operations(state, pos)?; if fn_def.body.is_empty() { @@ -844,10 +846,11 @@ impl Engine { state: &mut State, lib: &[&Module], script: &str, - pos: Position, + _pos: Position, level: usize, ) -> RhaiResult { - self.inc_operations(state, pos)?; + #[cfg(not(feature = "unchecked"))] + self.inc_operations(state, _pos)?; let script = script.trim(); if script.is_empty() { @@ -1305,14 +1308,15 @@ impl Engine { .chain(constant_args.iter().map(|(v, _)| Ok(v.clone()))) .collect::>()?; - let (mut target, pos) = + let (mut target, _pos) = self.search_namespace(scope, mods, state, lib, this_ptr, &args_expr[0])?; if target.as_ref().is_read_only() { target = target.into_owned(); } - self.inc_operations(state, pos)?; + #[cfg(not(feature = "unchecked"))] + self.inc_operations(state, _pos)?; #[cfg(not(feature = "no_closure"))] let target_is_shared = target.is_shared(); @@ -1396,10 +1400,11 @@ impl Engine { .collect::>()?; // Get target reference to first argument - let (target, pos) = + let (target, _pos) = self.search_scope_only(scope, mods, state, lib, this_ptr, &args_expr[0])?; - self.inc_operations(state, pos)?; + #[cfg(not(feature = "unchecked"))] + self.inc_operations(state, _pos)?; #[cfg(not(feature = "no_closure"))] let target_is_shared = target.is_shared(); @@ -1439,6 +1444,7 @@ impl Engine { let func = match module.get_qualified_fn(hash) { // Then search in Rust functions None => { + #[cfg(not(feature = "unchecked"))] self.inc_operations(state, pos)?; let hash_params = calc_fn_params_hash(args.iter().map(|a| a.type_id())); diff --git a/src/fn_native.rs b/src/fn_native.rs index 007ba761..a72d79b0 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -418,9 +418,11 @@ pub type FnPlugin = dyn PluginFunction; pub type FnPlugin = dyn PluginFunction + Send + Sync; /// A standard callback function for progress reporting. +#[cfg(not(feature = "unchecked"))] #[cfg(not(feature = "sync"))] pub type OnProgressCallback = Box Option + 'static>; /// A standard callback function for progress reporting. +#[cfg(not(feature = "unchecked"))] #[cfg(feature = "sync")] pub type OnProgressCallback = Box Option + Send + Sync + 'static>; From b0911133344a90c8d1c4a999c4316c1792a5b5f2 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 27 Apr 2021 22:28:01 +0800 Subject: [PATCH 16/16] Eliminate unnecessary data structures. --- codegen/Cargo.toml | 4 +- src/engine.rs | 156 +++++++++++++++++++++++++++++---------------- src/token.rs | 4 +- 3 files changed, 105 insertions(+), 59 deletions(-) diff --git a/codegen/Cargo.toml b/codegen/Cargo.toml index d02e4388..778a70fe 100644 --- a/codegen/Cargo.toml +++ b/codegen/Cargo.toml @@ -2,8 +2,8 @@ name = "rhai_codegen" version = "0.3.5" edition = "2018" -authors = ["jhwgh1968"] -description = "Procedural macro support package for Rhai, a scripting language for Rust" +authors = ["jhwgh1968", "Stephen Chung"] +description = "Procedural macros support package for Rhai, a scripting language and engine for Rust" homepage = "https://rhai.rs/book/plugins/index.html" repository = "https://github.com/rhaiscript/rhai" license = "MIT OR Apache-2.0" diff --git a/src/engine.rs b/src/engine.rs index f85d5b66..1c696e02 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -51,35 +51,42 @@ pub type Precedence = NonZeroU8; // We cannot use Cow here because `eval` may load a [module][Module] and // the module name will live beyond the AST of the eval script text. // The best we can do is a shared reference. +// +// This implementation splits the module names from the shared modules to improve data locality. +// Most usage will be looking up a particular key from the list and then getting the module that +// corresponds to that key. #[derive(Clone, Default)] -pub struct Imports(StaticVec, StaticVec>); +pub struct Imports { + keys: StaticVec, + modules: StaticVec>, +} impl Imports { /// Get the length of this stack of imported [modules][Module]. #[inline(always)] pub fn len(&self) -> usize { - self.0.len() + self.keys.len() } /// Is this stack of imported [modules][Module] empty? #[inline(always)] pub fn is_empty(&self) -> bool { - self.0.is_empty() + self.keys.is_empty() } /// Get the imported [modules][Module] at a particular index. #[inline(always)] pub fn get(&self, index: usize) -> Option> { - self.1.get(index).cloned() + self.modules.get(index).cloned() } /// Get the imported [modules][Module] at a particular index. #[allow(dead_code)] #[inline(always)] pub(crate) fn get_mut(&mut self, index: usize) -> Option<&mut Shared> { - self.1.get_mut(index) + self.modules.get_mut(index) } /// Get the index of an imported [modules][Module] by name. #[inline(always)] pub fn find(&self, name: &str) -> Option { - self.0 + self.keys .iter() .enumerate() .rev() @@ -88,22 +95,22 @@ impl Imports { /// Push an imported [modules][Module] onto the stack. #[inline(always)] pub fn push(&mut self, name: impl Into, module: impl Into>) { - self.0.push(name.into()); - self.1.push(module.into()); + self.keys.push(name.into()); + self.modules.push(module.into()); } /// Truncate the stack of imported [modules][Module] to a particular length. #[inline(always)] pub fn truncate(&mut self, size: usize) { - self.0.truncate(size); - self.1.truncate(size); + self.keys.truncate(size); + self.modules.truncate(size); } /// Get an iterator to this stack of imported [modules][Module] in reverse order. #[allow(dead_code)] #[inline(always)] pub fn iter(&self) -> impl Iterator { - self.0 + self.keys .iter() - .zip(self.1.iter()) + .zip(self.modules.iter()) .rev() .map(|(name, module)| (name.as_str(), module.as_ref())) } @@ -111,29 +118,32 @@ impl Imports { #[allow(dead_code)] #[inline(always)] pub(crate) fn iter_raw(&self) -> impl Iterator)> { - self.0.iter().rev().zip(self.1.iter().rev()) + self.keys.iter().rev().zip(self.modules.iter().rev()) } /// Get an iterator to this stack of imported [modules][Module] in forward order. #[allow(dead_code)] #[inline(always)] pub(crate) fn scan_raw(&self) -> impl Iterator)> { - self.0.iter().zip(self.1.iter()) + self.keys.iter().zip(self.modules.iter()) } /// Get a consuming iterator to this stack of imported [modules][Module] in reverse order. #[inline(always)] pub fn into_iter(self) -> impl Iterator)> { - self.0.into_iter().rev().zip(self.1.into_iter().rev()) + self.keys + .into_iter() + .rev() + .zip(self.modules.into_iter().rev()) } /// Does the specified function hash key exist in this stack of imported [modules][Module]? #[allow(dead_code)] #[inline(always)] pub fn contains_fn(&self, hash: u64) -> bool { - self.1.iter().any(|m| m.contains_qualified_fn(hash)) + self.modules.iter().any(|m| m.contains_qualified_fn(hash)) } /// Get specified function via its hash key. #[inline(always)] pub fn get_fn(&self, hash: u64) -> Option<(&CallableFunction, Option<&Identifier>)> { - self.1 + self.modules .iter() .rev() .find_map(|m| m.get_qualified_fn(hash).map(|f| (f, m.id_raw()))) @@ -143,12 +153,15 @@ impl Imports { #[allow(dead_code)] #[inline(always)] pub fn contains_iter(&self, id: TypeId) -> bool { - self.1.iter().any(|m| m.contains_qualified_iter(id)) + self.modules.iter().any(|m| m.contains_qualified_iter(id)) } /// Get the specified [`TypeId`][std::any::TypeId] iterator. #[inline(always)] pub fn get_iter(&self, id: TypeId) -> Option { - self.1.iter().rev().find_map(|m| m.get_qualified_iter(id)) + self.modules + .iter() + .rev() + .find_map(|m| m.get_qualified_iter(id)) } } @@ -160,7 +173,7 @@ impl fmt::Debug for Imports { f.debug_map().finish() } else { f.debug_map() - .entries(self.0.iter().zip(self.1.iter())) + .entries(self.keys.iter().zip(self.modules.iter())) .finish() } } @@ -232,24 +245,27 @@ pub const TOKEN_OP_CONCAT: Token = Token::PlusAssign; /// Method of chaining. #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] -pub enum ChainType { - /// Not a chaining type. - NonChaining, +enum ChainType { /// Indexing. + #[cfg(not(feature = "no_index"))] Index, /// Dotting. + #[cfg(not(feature = "no_object"))] Dot, } /// Value of a chaining argument. #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[derive(Debug, Clone, Hash)] -pub enum ChainArgument { +enum ChainArgument { /// Dot-property access. + #[cfg(not(feature = "no_object"))] Property(Position), - /// Arguments to a dot-function call. - FnCallArgs(StaticVec, StaticVec), + /// Arguments to a dot method call. + #[cfg(not(feature = "no_object"))] + MethodCallArgs(StaticVec, StaticVec), /// Index value. + #[cfg(not(feature = "no_index"))] IndexValue(Dynamic, Position), } @@ -264,7 +280,8 @@ impl ChainArgument { #[cfg(not(feature = "no_index"))] pub fn as_index_value(self) -> Dynamic { match self { - Self::Property(_) | Self::FnCallArgs(_, _) => { + #[cfg(not(feature = "no_object"))] + Self::Property(_) | Self::MethodCallArgs(_, _) => { panic!("expecting ChainArgument::IndexValue") } Self::IndexValue(value, _) => value, @@ -274,28 +291,32 @@ impl ChainArgument { /// /// # Panics /// - /// Panics if not `ChainArgument::FnCallArgs`. + /// Panics if not `ChainArgument::MethodCallArgs`. #[inline(always)] #[cfg(not(feature = "no_object"))] pub fn as_fn_call_args(self) -> (StaticVec, StaticVec) { match self { - Self::Property(_) | Self::IndexValue(_, _) => { - panic!("expecting ChainArgument::FnCallArgs") + Self::Property(_) => { + panic!("expecting ChainArgument::MethodCallArgs") } - Self::FnCallArgs(values, positions) => (values, positions), + #[cfg(not(feature = "no_index"))] + Self::IndexValue(_, _) => { + panic!("expecting ChainArgument::MethodCallArgs") + } + Self::MethodCallArgs(values, positions) => (values, positions), } } } -#[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] +#[cfg(not(feature = "no_object"))] impl From<(StaticVec, StaticVec)> for ChainArgument { #[inline(always)] fn from((values, positions): (StaticVec, StaticVec)) -> Self { - Self::FnCallArgs(values, positions) + Self::MethodCallArgs(values, positions) } } -#[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] +#[cfg(not(feature = "no_index"))] impl From<(Dynamic, Position)> for ChainArgument { #[inline(always)] fn from((value, pos): (Dynamic, Position)) -> Self { @@ -1129,14 +1150,14 @@ impl Engine { level: usize, new_val: Option<((Dynamic, Position), (Option, Position))>, ) -> Result<(Dynamic, bool), Box> { - assert!(chain_type != ChainType::NonChaining); - let is_ref = target.is_ref(); - let next_chain = match rhs { - Expr::Index(_, _) => ChainType::Index, - Expr::Dot(_, _) => ChainType::Dot, - _ => ChainType::NonChaining, + let rhs_chain = match rhs { + #[cfg(not(feature = "no_index"))] + Expr::Index(_, _) => Some(ChainType::Index), + #[cfg(not(feature = "no_object"))] + Expr::Dot(_, _) => Some(ChainType::Dot), + _ => None, }; // Pop the last index value @@ -1155,9 +1176,10 @@ impl Engine { let obj_ptr = &mut self.get_indexed_mut( mods, state, lib, target, idx_val, idx_pos, false, is_ref, true, level, )?; + let rhs_chain = rhs_chain.unwrap(); self.eval_dot_index_chain_helper( - mods, state, lib, this_ptr, obj_ptr, &x.rhs, idx_values, next_chain, + mods, state, lib, this_ptr, obj_ptr, &x.rhs, idx_values, rhs_chain, level, new_val, ) .map_err(|err| err.fill_position(*x_pos)) @@ -1340,9 +1362,10 @@ impl Engine { // Others - syntax error expr => unreachable!("invalid dot expression: {:?}", expr), }; + let rhs_chain = rhs_chain.unwrap(); self.eval_dot_index_chain_helper( - mods, state, lib, this_ptr, &mut val, &x.rhs, idx_values, next_chain, + mods, state, lib, this_ptr, &mut val, &x.rhs, idx_values, rhs_chain, level, new_val, ) .map_err(|err| err.fill_position(*x_pos)) @@ -1354,6 +1377,7 @@ impl Engine { Expr::Property(p) => { let ((getter, hash_get), (setter, hash_set), Ident { pos, .. }) = p.as_ref(); + let rhs_chain = rhs_chain.unwrap(); let hash_get = FnCallHashes::from_native(*hash_get); let hash_set = FnCallHashes::from_native(*hash_set); let arg_values = &mut [target.as_mut(), &mut Default::default()]; @@ -1374,7 +1398,7 @@ impl Engine { &mut val.into(), &x.rhs, idx_values, - next_chain, + rhs_chain, level, new_val, ) @@ -1405,6 +1429,7 @@ impl Engine { // xxx.fn_name(arg_expr_list)[expr] | xxx.fn_name(arg_expr_list).expr Expr::FnCall(f, pos) if !f.is_qualified() => { let FnCallExpr { name, hashes, .. } = f.as_ref(); + let rhs_chain = rhs_chain.unwrap(); let mut args = idx_val.as_fn_call_args(); let (mut val, _) = self.make_method_call( mods, state, lib, name, *hashes, target, &mut args, *pos, level, @@ -1414,7 +1439,7 @@ impl Engine { self.eval_dot_index_chain_helper( mods, state, lib, this_ptr, target, &x.rhs, idx_values, - next_chain, level, new_val, + rhs_chain, level, new_val, ) .map_err(|err| err.fill_position(*pos)) } @@ -1430,8 +1455,6 @@ impl Engine { _ => EvalAltResult::ErrorDotExpr("".into(), rhs.position()).into(), } } - - chain_type => unreachable!("invalid ChainType: {:?}", chain_type), } } @@ -1449,7 +1472,9 @@ impl Engine { new_val: Option<((Dynamic, Position), (Option, Position))>, ) -> RhaiResult { let (crate::ast::BinaryExpr { lhs, rhs }, chain_type, op_pos) = match expr { + #[cfg(not(feature = "no_index"))] Expr::Index(x, pos) => (x.as_ref(), ChainType::Index, *pos), + #[cfg(not(feature = "no_object"))] Expr::Dot(x, pos) => (x.as_ref(), ChainType::Dot, *pos), _ => unreachable!("index or dot chain expected, but gets {:?}", expr), }; @@ -1510,7 +1535,7 @@ impl Engine { lib: &[&Module], this_ptr: &mut Option<&mut Dynamic>, expr: &Expr, - parent_chain_type: ChainType, + _parent_chain_type: ChainType, idx_values: &mut StaticVec, size: usize, level: usize, @@ -1519,7 +1544,8 @@ impl Engine { self.inc_operations(state, expr.position())?; match expr { - Expr::FnCall(x, _) if parent_chain_type == ChainType::Dot && !x.is_qualified() => { + #[cfg(not(feature = "no_object"))] + Expr::FnCall(x, _) if _parent_chain_type == ChainType::Dot && !x.is_qualified() => { let mut arg_positions: StaticVec<_> = Default::default(); let mut arg_values = x @@ -1539,11 +1565,13 @@ impl Engine { idx_values.push((arg_values, arg_positions).into()); } - Expr::FnCall(_, _) if parent_chain_type == ChainType::Dot => { + #[cfg(not(feature = "no_object"))] + Expr::FnCall(_, _) if _parent_chain_type == ChainType::Dot => { unreachable!("function call in dot chain should not be namespace-qualified") } - Expr::Property(x) if parent_chain_type == ChainType::Dot => { + #[cfg(not(feature = "no_object"))] + Expr::Property(x) if _parent_chain_type == ChainType::Dot => { idx_values.push(ChainArgument::Property(x.2.pos)) } Expr::Property(_) => unreachable!("unexpected Expr::Property for indexing"), @@ -1553,12 +1581,15 @@ impl Engine { // Evaluate in left-to-right order let lhs_val = match lhs { - Expr::Property(x) if parent_chain_type == ChainType::Dot => { + #[cfg(not(feature = "no_object"))] + Expr::Property(x) if _parent_chain_type == ChainType::Dot => { ChainArgument::Property(x.2.pos) } Expr::Property(_) => unreachable!("unexpected Expr::Property for indexing"), + + #[cfg(not(feature = "no_object"))] Expr::FnCall(x, _) - if parent_chain_type == ChainType::Dot && !x.is_qualified() => + if _parent_chain_type == ChainType::Dot && !x.is_qualified() => { let mut arg_positions: StaticVec<_> = Default::default(); @@ -1579,17 +1610,26 @@ impl Engine { (arg_values, arg_positions).into() } - Expr::FnCall(_, _) if parent_chain_type == ChainType::Dot => { + #[cfg(not(feature = "no_object"))] + Expr::FnCall(_, _) if _parent_chain_type == ChainType::Dot => { unreachable!("function call in dot chain should not be namespace-qualified") } - _ => self + #[cfg(not(feature = "no_object"))] + expr if _parent_chain_type == ChainType::Dot => { + unreachable!("invalid dot expression: {:?}", expr); + } + #[cfg(not(feature = "no_index"))] + _ if _parent_chain_type == ChainType::Index => self .eval_expr(scope, mods, state, lib, this_ptr, lhs, level) .map(|v| (v.flatten(), lhs.position()).into())?, + expr => unreachable!("unknown chained expression: {:?}", expr), }; // Push in reverse order let chain_type = match expr { + #[cfg(not(feature = "no_index"))] Expr::Index(_, _) => ChainType::Index, + #[cfg(not(feature = "no_object"))] Expr::Dot(_, _) => ChainType::Dot, _ => unreachable!("index or dot chain expected, but gets {:?}", expr), }; @@ -1600,10 +1640,16 @@ impl Engine { idx_values.push(lhs_val); } - _ => idx_values.push( + #[cfg(not(feature = "no_object"))] + _ if _parent_chain_type == ChainType::Dot => { + unreachable!("invalid dot expression: {:?}", expr); + } + #[cfg(not(feature = "no_index"))] + _ if _parent_chain_type == ChainType::Index => idx_values.push( self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) .map(|v| (v.flatten(), expr.position()).into())?, ), + _ => unreachable!("unknown chained expression: {:?}", expr), } Ok(()) diff --git a/src/token.rs b/src/token.rs index 490bc232..ad4d6a44 100644 --- a/src/token.rs +++ b/src/token.rs @@ -187,10 +187,10 @@ impl Position { } /// Print this [`Position`] for debug purposes. #[inline(always)] - pub(crate) fn debug_print(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + pub(crate) fn debug_print(&self, _f: &mut fmt::Formatter<'_>) -> fmt::Result { #[cfg(not(feature = "no_position"))] if !self.is_none() { - write!(f, " @ {:?}", self)?; + write!(_f, " @ {:?}", self)?; } Ok(())