Replace Cow<str> in Scope with SmartString.

This commit is contained in:
Stephen Chung 2022-01-15 11:26:43 +08:00
parent 2a8a8c00f5
commit 00b189d0c6
7 changed files with 75 additions and 103 deletions

View File

@ -17,6 +17,7 @@ Enhancements
------------ ------------
* Formatting of return types in functions metadata info is improved. * Formatting of return types in functions metadata info is improved.
* Use `SmartString` for `Scope` variable names and remove `unsafe` lifetime casting.
Version 1.4.0 Version 1.4.0

View File

@ -3,15 +3,11 @@
use super::{EvalState, GlobalRuntimeState, Target}; use super::{EvalState, GlobalRuntimeState, Target};
use crate::ast::{Expr, Ident, OpAssignment, Stmt, AST_OPTION_FLAGS::*}; use crate::ast::{Expr, Ident, OpAssignment, Stmt, AST_OPTION_FLAGS::*};
use crate::func::get_hasher; use crate::func::get_hasher;
use crate::r#unsafe::unsafe_cast_var_name_to_lifetime;
use crate::types::dynamic::{AccessMode, Union}; use crate::types::dynamic::{AccessMode, Union};
use crate::{Dynamic, Engine, Module, Position, RhaiResult, RhaiResultOf, Scope, ERR, INT}; use crate::{Dynamic, Engine, Module, Position, RhaiResult, RhaiResultOf, Scope, ERR, INT};
use std::hash::{Hash, Hasher};
#[cfg(feature = "no_std")] #[cfg(feature = "no_std")]
use std::prelude::v1::*; use std::prelude::v1::*;
use std::{
borrow::Cow,
hash::{Hash, Hasher},
};
impl Engine { impl Engine {
/// Evaluate a statements block. /// Evaluate a statements block.
@ -492,19 +488,13 @@ impl Engine {
// Add the loop variables // Add the loop variables
let orig_scope_len = scope.len(); let orig_scope_len = scope.len();
let counter_index = if let Some(counter) = counter { let counter_index = if let Some(counter) = counter {
// Loop variables are always removed at the end of the statement scope.push(counter.name.clone(), 0 as INT);
// so this cast is safe.
let counter_name = unsafe_cast_var_name_to_lifetime(&counter.name);
scope.push(counter_name, 0 as INT);
scope.len() - 1 scope.len() - 1
} else { } else {
usize::MAX usize::MAX
}; };
// Loop variables are always removed at the end of the statement scope.push(var_name.clone(), ());
// so this cast is safe.
let var_name = unsafe_cast_var_name_to_lifetime(var_name);
scope.push(var_name, ());
let index = scope.len() - 1; let index = scope.len() - 1;
let mut loop_result = Ok(Dynamic::UNIT); let mut loop_result = Ok(Dynamic::UNIT);
@ -514,11 +504,12 @@ impl Engine {
if counter_index < usize::MAX { if counter_index < usize::MAX {
#[cfg(not(feature = "unchecked"))] #[cfg(not(feature = "unchecked"))]
if x > INT::MAX as usize { if x > INT::MAX as usize {
return Err(ERR::ErrorArithmetic( loop_result = Err(ERR::ErrorArithmetic(
format!("for-loop counter overflow: {}", x), format!("for-loop counter overflow: {}", x),
counter.as_ref().expect("`Some`").pos, counter.as_ref().expect("`Some`").pos,
) )
.into()); .into());
break;
} }
let index_value = (x as INT).into(); let index_value = (x as INT).into();
@ -575,7 +566,10 @@ impl Engine {
Err(err) => match *err { Err(err) => match *err {
ERR::LoopBreak(false, _) => (), ERR::LoopBreak(false, _) => (),
ERR::LoopBreak(true, _) => break, ERR::LoopBreak(true, _) => break,
_ => return Err(err), _ => {
loop_result = Err(err);
break;
}
}, },
} }
} }
@ -645,12 +639,9 @@ impl Engine {
let orig_scope_len = scope.len(); let orig_scope_len = scope.len();
err_var_name.as_ref().map(|Ident { name, .. }| { err_var_name
// Catch error variables are always removed from after the block .as_ref()
// so this cast is safe. .map(|Ident { name, .. }| scope.push(name.clone(), err_value));
let var_name = unsafe_cast_var_name_to_lifetime(name);
scope.push(var_name, err_value)
});
let result = self.eval_stmt_block( let result = self.eval_stmt_block(
scope, global, state, lib, this_ptr, catch_stmt, true, level, scope, global, state, lib, this_ptr, catch_stmt, true, level,
@ -701,7 +692,7 @@ impl Engine {
// Let/const statement // Let/const statement
Stmt::Var(expr, x, options, _) => { Stmt::Var(expr, x, options, _) => {
let name = &x.name; let var_name = &x.name;
let entry_type = if options.contains(AST_OPTION_CONSTANT) { let entry_type = if options.contains(AST_OPTION_CONSTANT) {
AccessMode::ReadOnly AccessMode::ReadOnly
} else { } else {
@ -713,7 +704,7 @@ impl Engine {
.eval_expr(scope, global, state, lib, this_ptr, expr, level)? .eval_expr(scope, global, state, lib, this_ptr, expr, level)?
.flatten(); .flatten();
let (var_name, _alias): (Cow<'_, str>, _) = if !rewind_scope { let _alias = if !rewind_scope {
#[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_function"))]
#[cfg(not(feature = "no_module"))] #[cfg(not(feature = "no_module"))]
if state.scope_level == 0 if state.scope_level == 0
@ -721,23 +712,26 @@ impl Engine {
&& lib.iter().any(|&m| !m.is_empty()) && lib.iter().any(|&m| !m.is_empty())
{ {
// Add a global constant if at top level and there are functions // Add a global constant if at top level and there are functions
global.set_constant(name.clone(), value.clone()); global.set_constant(var_name.clone(), value.clone());
} }
( if export {
name.to_string().into(), Some(var_name)
if export { Some(name.clone()) } else { None }, } else {
) None
}
} else if export { } else if export {
unreachable!("exported variable not on global level"); unreachable!("exported variable not on global level");
} else { } else {
(unsafe_cast_var_name_to_lifetime(name).into(), None) None
}; };
scope.push_dynamic_value(var_name, entry_type, value); scope.push_dynamic_value(var_name.clone(), entry_type, value);
#[cfg(not(feature = "no_module"))] #[cfg(not(feature = "no_module"))]
_alias.map(|alias| scope.add_entry_alias(scope.len() - 1, alias)); if let Some(alias) = _alias {
scope.add_entry_alias(scope.len() - 1, alias.clone());
}
Ok(Dynamic::UNIT) Ok(Dynamic::UNIT)
} }

View File

@ -4,7 +4,6 @@
use super::call::FnCallArgs; use super::call::FnCallArgs;
use crate::ast::ScriptFnDef; use crate::ast::ScriptFnDef;
use crate::eval::{EvalState, GlobalRuntimeState}; use crate::eval::{EvalState, GlobalRuntimeState};
use crate::r#unsafe::unsafe_cast_var_name_to_lifetime;
use crate::{Dynamic, Engine, Module, Position, RhaiError, RhaiResult, Scope, StaticVec, ERR}; use crate::{Dynamic, Engine, Module, Position, RhaiError, RhaiResult, Scope, StaticVec, ERR};
use std::mem; use std::mem;
#[cfg(feature = "no_std")] #[cfg(feature = "no_std")]
@ -74,22 +73,10 @@ impl Engine {
let orig_mods_len = global.num_imported_modules(); let orig_mods_len = global.num_imported_modules();
// Put arguments into scope as variables // Put arguments into scope as variables
scope.extend( scope.extend(fn_def.params.iter().cloned().zip(args.into_iter().map(|v| {
fn_def
.params
.iter()
.zip(args.into_iter().map(|v| {
// Actually consume the arguments instead of cloning them // Actually consume the arguments instead of cloning them
mem::take(*v) mem::take(*v)
})) })));
.map(|(name, value)| {
// Arguments are always removed at the end of the call,
// so this cast is safe.
let var_name: std::borrow::Cow<_> =
unsafe_cast_var_name_to_lifetime(name).into();
(var_name, value)
}),
);
// Merge in encapsulated environment, if any // Merge in encapsulated environment, if any
let mut lib_merged = StaticVec::with_capacity(lib.len() + 1); let mut lib_merged = StaticVec::with_capacity(lib.len() + 1);

View File

@ -23,11 +23,11 @@ fn check_struct_sizes() {
assert_eq!(size_of::<Option<ast::Expr>>(), if PACKED { 12 } else { 16 }); assert_eq!(size_of::<Option<ast::Expr>>(), if PACKED { 12 } else { 16 });
assert_eq!(size_of::<ast::Stmt>(), if PACKED { 24 } else { 32 }); assert_eq!(size_of::<ast::Stmt>(), if PACKED { 24 } else { 32 });
assert_eq!(size_of::<Option<ast::Stmt>>(), if PACKED { 24 } else { 32 }); assert_eq!(size_of::<Option<ast::Stmt>>(), if PACKED { 24 } else { 32 });
assert_eq!(size_of::<FnPtr>(), if PACKED { 40 } else { 80 });
assert_eq!(size_of::<Scope>(), if PACKED { 232 } else { 464 });
#[cfg(target_pointer_width = "64")] #[cfg(target_pointer_width = "64")]
{ {
assert_eq!(size_of::<Scope>(), 400);
assert_eq!(size_of::<FnPtr>(), 80);
assert_eq!(size_of::<LexError>(), 56); assert_eq!(size_of::<LexError>(), 56);
assert_eq!( assert_eq!(
size_of::<ParseError>(), size_of::<ParseError>(),

View File

@ -3,10 +3,12 @@
use super::dynamic::{AccessMode, Variant}; use super::dynamic::{AccessMode, Variant};
use crate::{Dynamic, Identifier, StaticVec}; use crate::{Dynamic, Identifier, StaticVec};
use smallvec::SmallVec; use smallvec::SmallVec;
use std::iter::FromIterator;
#[cfg(feature = "no_std")] #[cfg(feature = "no_std")]
use std::prelude::v1::*; use std::prelude::v1::*;
use std::{borrow::Cow, iter::Extend}; use std::{
iter::{Extend, FromIterator},
marker::PhantomData,
};
/// Keep a number of entries inline (since [`Dynamic`] is usually small enough). /// Keep a number of entries inline (since [`Dynamic`] is usually small enough).
const SCOPE_ENTRIES_INLINED: usize = 8; const SCOPE_ENTRIES_INLINED: usize = 8;
@ -14,6 +16,11 @@ const SCOPE_ENTRIES_INLINED: usize = 8;
/// Type containing information about the current scope. Useful for keeping state between /// Type containing information about the current scope. Useful for keeping state between
/// [`Engine`][crate::Engine] evaluation runs. /// [`Engine`][crate::Engine] evaluation runs.
/// ///
/// # Lifetime
///
/// Currently the lifetime parameter is not used, but it is not guaranteed to remain unused for
/// future versions. Until then, `'static` can be used.
///
/// # Thread Safety /// # Thread Safety
/// ///
/// Currently, [`Scope`] is neither [`Send`] nor [`Sync`]. Turn on the `sync` feature to make it /// Currently, [`Scope`] is neither [`Send`] nor [`Sync`]. Turn on the `sync` feature to make it
@ -47,7 +54,7 @@ const SCOPE_ENTRIES_INLINED: usize = 8;
// //
// [`Scope`] is implemented as two [`Vec`]'s of exactly the same length. Variables data (name, // [`Scope`] is implemented as two [`Vec`]'s of exactly the same length. Variables data (name,
// type, etc.) is manually split into two equal-length arrays. That's because variable names take // type, etc.) is manually split into two equal-length arrays. That's because variable names take
// up the most space, with [`Cow<str>`][Cow] being four words long, but in the vast majority of // up the most space, with [`Identifier`] being four words long, but in the vast majority of
// cases the name is NOT used to look up a variable. Variable lookup is usually via direct // cases the name is NOT used to look up a variable. Variable lookup is usually via direct
// indexing, by-passing the name altogether. // indexing, by-passing the name altogether.
// //
@ -58,25 +65,30 @@ pub struct Scope<'a> {
/// Current value of the entry. /// Current value of the entry.
values: SmallVec<[Dynamic; SCOPE_ENTRIES_INLINED]>, values: SmallVec<[Dynamic; SCOPE_ENTRIES_INLINED]>,
/// (Name, aliases) of the entry. /// (Name, aliases) of the entry.
names: SmallVec<[(Cow<'a, str>, Option<Box<StaticVec<Identifier>>>); SCOPE_ENTRIES_INLINED]>, names: SmallVec<[(Identifier, Option<Box<StaticVec<Identifier>>>); SCOPE_ENTRIES_INLINED]>,
/// Phantom to keep the lifetime parameter in order not to break existing code.
phantom: PhantomData<&'a ()>,
} }
impl<'a> IntoIterator for Scope<'a> { impl IntoIterator for Scope<'_> {
type Item = (Cow<'a, str>, Dynamic); type Item = (String, Dynamic, Vec<Identifier>);
type IntoIter = Box<dyn Iterator<Item = Self::Item> + 'a>; type IntoIter = Box<dyn Iterator<Item = Self::Item>>;
#[inline] #[inline]
fn into_iter(self) -> Self::IntoIter { fn into_iter(self) -> Self::IntoIter {
Box::new( Box::new(self.values.into_iter().zip(self.names.into_iter()).map(
self.values |(value, (name, alias))| {
.into_iter() (
.zip(self.names.into_iter()) name.into(),
.map(|(value, (name, _))| (name, value)), value,
alias.map(|a| a.to_vec()).unwrap_or_default(),
) )
},
))
} }
} }
impl<'a> Scope<'a> { impl Scope<'_> {
/// Create a new [`Scope`]. /// Create a new [`Scope`].
/// ///
/// # Example /// # Example
@ -95,6 +107,7 @@ impl<'a> Scope<'a> {
Self { Self {
values: SmallVec::new_const(), values: SmallVec::new_const(),
names: SmallVec::new_const(), names: SmallVec::new_const(),
phantom: PhantomData,
} }
} }
/// Empty the [`Scope`]. /// Empty the [`Scope`].
@ -171,11 +184,7 @@ impl<'a> Scope<'a> {
/// assert_eq!(my_scope.get_value::<i64>("x").expect("x should exist"), 42); /// assert_eq!(my_scope.get_value::<i64>("x").expect("x should exist"), 42);
/// ``` /// ```
#[inline(always)] #[inline(always)]
pub fn push( pub fn push(&mut self, name: impl Into<Identifier>, value: impl Variant + Clone) -> &mut Self {
&mut self,
name: impl Into<Cow<'a, str>>,
value: impl Variant + Clone,
) -> &mut Self {
self.push_dynamic_value(name, AccessMode::ReadWrite, Dynamic::from(value)) self.push_dynamic_value(name, AccessMode::ReadWrite, Dynamic::from(value))
} }
/// Add (push) a new [`Dynamic`] entry to the [`Scope`]. /// Add (push) a new [`Dynamic`] entry to the [`Scope`].
@ -191,7 +200,7 @@ impl<'a> Scope<'a> {
/// assert_eq!(my_scope.get_value::<i64>("x").expect("x should exist"), 42); /// assert_eq!(my_scope.get_value::<i64>("x").expect("x should exist"), 42);
/// ``` /// ```
#[inline(always)] #[inline(always)]
pub fn push_dynamic(&mut self, name: impl Into<Cow<'a, str>>, value: Dynamic) -> &mut Self { pub fn push_dynamic(&mut self, name: impl Into<Identifier>, value: Dynamic) -> &mut Self {
self.push_dynamic_value(name, value.access_mode(), value) self.push_dynamic_value(name, value.access_mode(), value)
} }
/// Add (push) a new constant to the [`Scope`]. /// Add (push) a new constant to the [`Scope`].
@ -212,7 +221,7 @@ impl<'a> Scope<'a> {
#[inline(always)] #[inline(always)]
pub fn push_constant( pub fn push_constant(
&mut self, &mut self,
name: impl Into<Cow<'a, str>>, name: impl Into<Identifier>,
value: impl Variant + Clone, value: impl Variant + Clone,
) -> &mut Self { ) -> &mut Self {
self.push_dynamic_value(name, AccessMode::ReadOnly, Dynamic::from(value)) self.push_dynamic_value(name, AccessMode::ReadOnly, Dynamic::from(value))
@ -235,7 +244,7 @@ impl<'a> Scope<'a> {
#[inline(always)] #[inline(always)]
pub fn push_constant_dynamic( pub fn push_constant_dynamic(
&mut self, &mut self,
name: impl Into<Cow<'a, str>>, name: impl Into<Identifier>,
value: Dynamic, value: Dynamic,
) -> &mut Self { ) -> &mut Self {
self.push_dynamic_value(name, AccessMode::ReadOnly, value) self.push_dynamic_value(name, AccessMode::ReadOnly, value)
@ -244,7 +253,7 @@ impl<'a> Scope<'a> {
#[inline] #[inline]
pub(crate) fn push_dynamic_value( pub(crate) fn push_dynamic_value(
&mut self, &mut self,
name: impl Into<Cow<'a, str>>, name: impl Into<Identifier>,
access: AccessMode, access: AccessMode,
mut value: Dynamic, mut value: Dynamic,
) -> &mut Self { ) -> &mut Self {
@ -301,7 +310,7 @@ impl<'a> Scope<'a> {
#[inline] #[inline]
#[must_use] #[must_use]
pub fn contains(&self, name: &str) -> bool { pub fn contains(&self, name: &str) -> bool {
self.names.iter().any(|(key, _)| name == key.as_ref()) self.names.iter().any(|(key, _)| name == key)
} }
/// Find an entry in the [`Scope`], starting from the last. /// Find an entry in the [`Scope`], starting from the last.
#[inline] #[inline]
@ -314,7 +323,7 @@ impl<'a> Scope<'a> {
.rev() // Always search a Scope in reverse order .rev() // Always search a Scope in reverse order
.enumerate() .enumerate()
.find_map(|(i, (key, _))| { .find_map(|(i, (key, _))| {
if name == key.as_ref() { if name == key {
let index = len - 1 - i; let index = len - 1 - i;
Some((index, self.values[index].access_mode())) Some((index, self.values[index].access_mode()))
} else { } else {
@ -343,7 +352,7 @@ impl<'a> Scope<'a> {
.iter() .iter()
.rev() .rev()
.enumerate() .enumerate()
.find(|(_, (key, _))| name == key.as_ref()) .find(|(_, (key, _))| name == key)
.and_then(|(index, _)| self.values[len - 1 - index].flatten_clone().try_cast()) .and_then(|(index, _)| self.values[len - 1 - index].flatten_clone().try_cast())
} }
/// Check if the named entry in the [`Scope`] is constant. /// Check if the named entry in the [`Scope`] is constant.
@ -398,7 +407,7 @@ impl<'a> Scope<'a> {
#[inline] #[inline]
pub fn set_or_push( pub fn set_or_push(
&mut self, &mut self,
name: impl AsRef<str> + Into<Cow<'a, str>>, name: impl AsRef<str> + Into<Identifier>,
value: impl Variant + Clone, value: impl Variant + Clone,
) -> &mut Self { ) -> &mut Self {
match self.get_index(name.as_ref()) { match self.get_index(name.as_ref()) {
@ -437,7 +446,7 @@ impl<'a> Scope<'a> {
#[inline] #[inline]
pub fn set_value( pub fn set_value(
&mut self, &mut self,
name: impl AsRef<str> + Into<Cow<'a, str>>, name: impl AsRef<str> + Into<Identifier>,
value: impl Variant + Clone, value: impl Variant + Clone,
) -> &mut Self { ) -> &mut Self {
match self.get_index(name.as_ref()) { match self.get_index(name.as_ref()) {
@ -535,9 +544,7 @@ impl<'a> Scope<'a> {
/// Get an iterator to entries in the [`Scope`]. /// Get an iterator to entries in the [`Scope`].
#[inline] #[inline]
#[allow(dead_code)] #[allow(dead_code)]
pub(crate) fn into_iter( pub(crate) fn into_iter(self) -> impl Iterator<Item = (Identifier, Dynamic, Vec<Identifier>)> {
self,
) -> impl Iterator<Item = (Cow<'a, str>, Dynamic, Vec<Identifier>)> {
self.names self.names
.into_iter() .into_iter()
.zip(self.values.into_iter()) .zip(self.values.into_iter())
@ -597,7 +604,7 @@ impl<'a> Scope<'a> {
} }
} }
impl<'a, K: Into<Cow<'a, str>>> Extend<(K, Dynamic)> for Scope<'a> { impl<K: Into<Identifier>> Extend<(K, Dynamic)> for Scope<'_> {
#[inline] #[inline]
fn extend<T: IntoIterator<Item = (K, Dynamic)>>(&mut self, iter: T) { fn extend<T: IntoIterator<Item = (K, Dynamic)>>(&mut self, iter: T) {
iter.into_iter().for_each(|(name, value)| { iter.into_iter().for_each(|(name, value)| {
@ -606,7 +613,7 @@ impl<'a, K: Into<Cow<'a, str>>> Extend<(K, Dynamic)> for Scope<'a> {
} }
} }
impl<'a, K: Into<Cow<'a, str>>> FromIterator<(K, Dynamic)> for Scope<'a> { impl<K: Into<Identifier>> FromIterator<(K, Dynamic)> for Scope<'_> {
#[inline] #[inline]
fn from_iter<T: IntoIterator<Item = (K, Dynamic)>>(iter: T) -> Self { fn from_iter<T: IntoIterator<Item = (K, Dynamic)>>(iter: T) -> Self {
let mut scope = Self::new(); let mut scope = Self::new();
@ -615,7 +622,7 @@ impl<'a, K: Into<Cow<'a, str>>> FromIterator<(K, Dynamic)> for Scope<'a> {
} }
} }
impl<'a, K: Into<Cow<'a, str>>> Extend<(K, bool, Dynamic)> for Scope<'a> { impl<K: Into<Identifier>> Extend<(K, bool, Dynamic)> for Scope<'_> {
#[inline] #[inline]
fn extend<T: IntoIterator<Item = (K, bool, Dynamic)>>(&mut self, iter: T) { fn extend<T: IntoIterator<Item = (K, bool, Dynamic)>>(&mut self, iter: T) {
iter.into_iter().for_each(|(name, is_constant, value)| { iter.into_iter().for_each(|(name, is_constant, value)| {
@ -632,7 +639,7 @@ impl<'a, K: Into<Cow<'a, str>>> Extend<(K, bool, Dynamic)> for Scope<'a> {
} }
} }
impl<'a, K: Into<Cow<'a, str>>> FromIterator<(K, bool, Dynamic)> for Scope<'a> { impl<K: Into<Identifier>> FromIterator<(K, bool, Dynamic)> for Scope<'_> {
#[inline] #[inline]
fn from_iter<T: IntoIterator<Item = (K, bool, Dynamic)>>(iter: T) -> Self { fn from_iter<T: IntoIterator<Item = (K, bool, Dynamic)>>(iter: T) -> Self {
let mut scope = Self::new(); let mut scope = Self::new();

View File

@ -52,20 +52,3 @@ pub fn unsafe_cast_box<X: Any, T: Any>(item: Box<X>) -> Option<T> {
None None
} }
} }
/// # DANGEROUS!!!
///
/// A dangerous function that blindly casts a `&str` from one lifetime to a `&str` of
/// another lifetime. This is mainly used to let us push a block-local variable into the
/// current [`Scope`][crate::Scope] without cloning the variable name. Doing this is safe because all local
/// variables in the [`Scope`][crate::Scope] are cleared out before existing the block.
///
/// Force-casting a local variable's lifetime to the current [`Scope`][crate::Scope]'s larger lifetime saves
/// on allocations and string cloning, thus avoids us having to maintain a chain of [`Scope`][crate::Scope]'s.
#[inline(always)]
#[must_use]
pub fn unsafe_cast_var_name_to_lifetime<'s>(name: &str) -> &'s str {
// WARNING - force-cast the variable name into the scope's lifetime to avoid cloning it
// this is safe because all local variables are cleared at the end of the block
unsafe { mem::transmute(name) }
}

View File

@ -12,7 +12,7 @@ use rhai::Array;
use rhai::Map; use rhai::Map;
#[cfg(not(feature = "no_float"))] #[cfg(not(feature = "no_float"))]
use rhai::FLOAT; use rhai::FLOAT;
#[cfg(not(feature = "no_float"))] #[cfg(feature = "no_float")]
#[cfg(feature = "decimal")] #[cfg(feature = "decimal")]
use rust_decimal::Decimal; use rust_decimal::Decimal;