Force-cast local variable names when pushing into scope.

This commit is contained in:
Stephen Chung 2020-05-14 11:21:56 +08:00
parent e6d6a709f0
commit 5c61827c7c
4 changed files with 82 additions and 44 deletions

View File

@ -20,6 +20,7 @@ Rhai's current features set:
* Freely pass variables/constants into a script via an external [`Scope`] * Freely pass variables/constants into a script via an external [`Scope`]
* Fairly efficient (1 million iterations in 0.75 sec on my 5 year old laptop) * Fairly efficient (1 million iterations in 0.75 sec on my 5 year old laptop)
* Low compile-time overhead (~0.6 sec debug/~3 sec release for script runner app) * Low compile-time overhead (~0.6 sec debug/~3 sec release for script runner app)
* Relatively little `unsafe` code (yes there are some for performance reasons)
* [`no-std`](#optional-features) support * [`no-std`](#optional-features) support
* [Function overloading](#function-overloading) * [Function overloading](#function-overloading)
* [Operator overloading](#operator-overloading) * [Operator overloading](#operator-overloading)

View File

@ -299,7 +299,7 @@ impl Default for Dynamic {
} }
/// Cast a type into another type. /// Cast a type into another type.
fn try_cast<A: Any, B: Any>(a: A) -> Option<B> { fn unsafe_try_cast<A: Any, B: Any>(a: A) -> Option<B> {
if TypeId::of::<B>() == a.type_id() { if TypeId::of::<B>() == a.type_id() {
// SAFETY: Just checked we have the right type. We explicitly forget the // SAFETY: Just checked we have the right type. We explicitly forget the
// value immediately after moving out, removing any chance of a destructor // value immediately after moving out, removing any chance of a destructor
@ -315,7 +315,7 @@ fn try_cast<A: Any, B: Any>(a: A) -> Option<B> {
} }
/// Cast a Boxed type into another type. /// Cast a Boxed type into another type.
fn cast_box<X: Variant, T: Variant>(item: Box<X>) -> Result<Box<T>, Box<X>> { fn unsafe_cast_box<X: Variant, T: Variant>(item: Box<X>) -> Result<Box<T>, Box<X>> {
// Only allow casting to the exact same type // Only allow casting to the exact same type
if TypeId::of::<X>() == TypeId::of::<T>() { if TypeId::of::<X>() == TypeId::of::<T>() {
// SAFETY: just checked whether we are pointing to the correct type // SAFETY: just checked whether we are pointing to the correct type
@ -383,17 +383,17 @@ impl Dynamic {
let mut var = Box::new(value); let mut var = Box::new(value);
var = match cast_box::<_, Dynamic>(var) { var = match unsafe_cast_box::<_, Dynamic>(var) {
Ok(d) => return *d, Ok(d) => return *d,
Err(var) => var, Err(var) => var,
}; };
var = match cast_box::<_, String>(var) { var = match unsafe_cast_box::<_, String>(var) {
Ok(s) => return Self(Union::Str(s)), Ok(s) => return Self(Union::Str(s)),
Err(var) => var, Err(var) => var,
}; };
#[cfg(not(feature = "no_index"))] #[cfg(not(feature = "no_index"))]
{ {
var = match cast_box::<_, Array>(var) { var = match unsafe_cast_box::<_, Array>(var) {
Ok(array) => return Self(Union::Array(array)), Ok(array) => return Self(Union::Array(array)),
Err(var) => var, Err(var) => var,
}; };
@ -401,7 +401,7 @@ impl Dynamic {
#[cfg(not(feature = "no_object"))] #[cfg(not(feature = "no_object"))]
{ {
var = match cast_box::<_, Map>(var) { var = match unsafe_cast_box::<_, Map>(var) {
Ok(map) => return Self(Union::Map(map)), Ok(map) => return Self(Union::Map(map)),
Err(var) => var, Err(var) => var,
} }
@ -426,23 +426,23 @@ impl Dynamic {
/// ``` /// ```
pub fn try_cast<T: Variant>(self) -> Option<T> { pub fn try_cast<T: Variant>(self) -> Option<T> {
if TypeId::of::<T>() == TypeId::of::<Dynamic>() { if TypeId::of::<T>() == TypeId::of::<Dynamic>() {
return cast_box::<_, T>(Box::new(self)).ok().map(|v| *v); return unsafe_cast_box::<_, T>(Box::new(self)).ok().map(|v| *v);
} }
match self.0 { match self.0 {
Union::Unit(value) => try_cast(value), Union::Unit(value) => unsafe_try_cast(value),
Union::Bool(value) => try_cast(value), Union::Bool(value) => unsafe_try_cast(value),
Union::Str(value) => cast_box::<_, T>(value).ok().map(|v| *v), Union::Str(value) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v),
Union::Char(value) => try_cast(value), Union::Char(value) => unsafe_try_cast(value),
Union::Int(value) => try_cast(value), Union::Int(value) => unsafe_try_cast(value),
#[cfg(not(feature = "no_float"))] #[cfg(not(feature = "no_float"))]
Union::Float(value) => try_cast(value), Union::Float(value) => unsafe_try_cast(value),
#[cfg(not(feature = "no_index"))] #[cfg(not(feature = "no_index"))]
Union::Array(value) => cast_box::<_, T>(value).ok().map(|v| *v), Union::Array(value) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v),
#[cfg(not(feature = "no_object"))] #[cfg(not(feature = "no_object"))]
Union::Map(value) => cast_box::<_, T>(value).ok().map(|v| *v), Union::Map(value) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v),
#[cfg(not(feature = "no_module"))] #[cfg(not(feature = "no_module"))]
Union::Module(value) => cast_box::<_, T>(value).ok().map(|v| *v), Union::Module(value) => unsafe_cast_box::<_, T>(value).ok().map(|v| *v),
Union::Variant(value) => (*value).as_box_any().downcast().map(|x| *x).ok(), Union::Variant(value) => (*value).as_box_any().downcast().map(|x| *x).ok(),
} }
} }
@ -467,23 +467,23 @@ impl Dynamic {
//self.try_cast::<T>().unwrap() //self.try_cast::<T>().unwrap()
if TypeId::of::<T>() == TypeId::of::<Dynamic>() { if TypeId::of::<T>() == TypeId::of::<Dynamic>() {
return *cast_box::<_, T>(Box::new(self)).unwrap(); return *unsafe_cast_box::<_, T>(Box::new(self)).unwrap();
} }
match self.0 { match self.0 {
Union::Unit(value) => try_cast(value).unwrap(), Union::Unit(value) => unsafe_try_cast(value).unwrap(),
Union::Bool(value) => try_cast(value).unwrap(), Union::Bool(value) => unsafe_try_cast(value).unwrap(),
Union::Str(value) => *cast_box::<_, T>(value).unwrap(), Union::Str(value) => *unsafe_cast_box::<_, T>(value).unwrap(),
Union::Char(value) => try_cast(value).unwrap(), Union::Char(value) => unsafe_try_cast(value).unwrap(),
Union::Int(value) => try_cast(value).unwrap(), Union::Int(value) => unsafe_try_cast(value).unwrap(),
#[cfg(not(feature = "no_float"))] #[cfg(not(feature = "no_float"))]
Union::Float(value) => try_cast(value).unwrap(), Union::Float(value) => unsafe_try_cast(value).unwrap(),
#[cfg(not(feature = "no_index"))] #[cfg(not(feature = "no_index"))]
Union::Array(value) => *cast_box::<_, T>(value).unwrap(), Union::Array(value) => *unsafe_cast_box::<_, T>(value).unwrap(),
#[cfg(not(feature = "no_object"))] #[cfg(not(feature = "no_object"))]
Union::Map(value) => *cast_box::<_, T>(value).unwrap(), Union::Map(value) => *unsafe_cast_box::<_, T>(value).unwrap(),
#[cfg(not(feature = "no_module"))] #[cfg(not(feature = "no_module"))]
Union::Module(value) => *cast_box::<_, T>(value).unwrap(), Union::Module(value) => *unsafe_cast_box::<_, T>(value).unwrap(),
Union::Variant(value) => (*value).as_box_any().downcast().map(|x| *x).unwrap(), Union::Variant(value) => (*value).as_box_any().downcast().map(|x| *x).unwrap(),
} }
} }

View File

@ -21,6 +21,7 @@ use crate::parser::ModuleRef;
use crate::stdlib::{ use crate::stdlib::{
any::TypeId, any::TypeId,
borrow::Cow,
boxed::Box, boxed::Box,
collections::HashMap, collections::HashMap,
format, format,
@ -135,6 +136,10 @@ pub struct State<'a> {
/// In some situation, e.g. after running an `eval` statement, subsequent offsets may become mis-aligned. /// In some situation, e.g. after running an `eval` statement, subsequent offsets may become mis-aligned.
/// When that happens, this flag is turned on to force a scope lookup by name. /// When that happens, this flag is turned on to force a scope lookup by name.
pub always_search: bool, pub always_search: bool,
/// Level of the current scope. The global (root) level is zero, a new block (or function call)
/// is one level higher, and so on.
pub scope_level: usize,
} }
impl<'a> State<'a> { impl<'a> State<'a> {
@ -143,6 +148,7 @@ impl<'a> State<'a> {
Self { Self {
always_search: false, always_search: false,
fn_lib, fn_lib,
scope_level: 0,
} }
} }
/// Does a certain script-defined function exist in the `State`? /// Does a certain script-defined function exist in the `State`?
@ -259,6 +265,24 @@ impl DerefMut for FunctionsLib {
} }
} }
/// A dangerous function that blindly casts a `&str` from one lifetime to a `Cow<str>` of
/// another lifetime. This is mainly used to let us push a block-local variable into the
/// current `Scope` without cloning the variable name. Doing this is safe because all local
/// variables in the `Scope` are cleared out before existing the block.
///
/// Force-casting a local variable lifetime to the current `Scope`'s larger lifetime saves
/// on allocations and string cloning, thus avoids us having to maintain a chain of `Scope`'s.
fn unsafe_cast_var_name<'s>(name: &str, state: &State) -> Cow<'s, str> {
// If not at global level, we can force-cast
if state.scope_level > 0 {
// 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::<_, &'s str>(name) }.into()
} else {
name.to_string().into()
}
}
/// Rhai main scripting engine. /// Rhai main scripting engine.
/// ///
/// ``` /// ```
@ -656,9 +680,9 @@ impl Engine {
/// Function call arguments may be _consumed_ when the function requires them to be passed by value. /// Function call arguments may be _consumed_ when the function requires them to be passed by value.
/// All function arguments not in the first position are always passed by value and thus consumed. /// All function arguments not in the first position are always passed by value and thus consumed.
/// **DO NOT** reuse the argument values unless for the first `&mut` argument - all others are silently replaced by `()`! /// **DO NOT** reuse the argument values unless for the first `&mut` argument - all others are silently replaced by `()`!
pub(crate) fn call_script_fn<'a>( pub(crate) fn call_script_fn<'s>(
&self, &self,
scope: Option<&mut Scope>, scope: Option<&mut Scope<'s>>,
state: &State, state: &State,
fn_name: &str, fn_name: &str,
fn_def: &FnDef, fn_def: &FnDef,
@ -672,7 +696,9 @@ impl Engine {
let scope_len = scope.len(); let scope_len = scope.len();
let mut state = State::new(state.fn_lib); let mut state = State::new(state.fn_lib);
// Put arguments into scope as variables - variable name is copied state.scope_level += 1;
// Put arguments into scope as variables
scope.extend( scope.extend(
fn_def fn_def
.params .params
@ -681,7 +707,10 @@ impl Engine {
// Actually consume the arguments instead of cloning them // Actually consume the arguments instead of cloning them
args.into_iter().map(|v| mem::take(*v)), args.into_iter().map(|v| mem::take(*v)),
) )
.map(|(name, value)| (name.clone(), ScopeEntryType::Normal, value)), .map(|(name, value)| {
let var_name = unsafe_cast_var_name(name.as_str(), &state);
(var_name, ScopeEntryType::Normal, value)
}),
); );
// Evaluate the function at one higher level of call depth // Evaluate the function at one higher level of call depth
@ -704,6 +733,8 @@ impl Engine {
))), ))),
}); });
// Remove all local variables
// No need to reset `state.scope_level` because it is thrown away
scope.rewind(scope_len); scope.rewind(scope_len);
return result; return result;
@ -712,6 +743,7 @@ impl Engine {
_ => { _ => {
let mut scope = Scope::new(); let mut scope = Scope::new();
let mut state = State::new(state.fn_lib); let mut state = State::new(state.fn_lib);
state.scope_level += 1;
// Put arguments into scope as variables // Put arguments into scope as variables
scope.extend( scope.extend(
@ -726,6 +758,7 @@ impl Engine {
); );
// Evaluate the function at one higher level of call depth // Evaluate the function at one higher level of call depth
// No need to reset `state.scope_level` because it is thrown away
return self return self
.eval_stmt(&mut scope, &mut state, &fn_def.body, level + 1) .eval_stmt(&mut scope, &mut state, &fn_def.body, level + 1)
.or_else(|err| match *err { .or_else(|err| match *err {
@ -1510,9 +1543,9 @@ impl Engine {
} }
/// Evaluate a statement /// Evaluate a statement
pub(crate) fn eval_stmt( pub(crate) fn eval_stmt<'s>(
&self, &self,
scope: &mut Scope, scope: &mut Scope<'s>,
state: &mut State, state: &mut State,
stmt: &Stmt, stmt: &Stmt,
level: usize, level: usize,
@ -1536,12 +1569,14 @@ impl Engine {
// Block scope // Block scope
Stmt::Block(x) => { Stmt::Block(x) => {
let prev_len = scope.len(); let prev_len = scope.len();
state.scope_level += 1;
let result = x.0.iter().try_fold(Default::default(), |_, stmt| { let result = x.0.iter().try_fold(Default::default(), |_, stmt| {
self.eval_stmt(scope, state, stmt, level) self.eval_stmt(scope, state, stmt, level)
}); });
scope.rewind(prev_len); scope.rewind(prev_len);
state.scope_level -= 1;
// The impact of an eval statement goes away at the end of a block // The impact of an eval statement goes away at the end of a block
// because any new variables introduced will go out of scope // because any new variables introduced will go out of scope
@ -1604,9 +1639,10 @@ impl Engine {
.or_else(|| self.packages.get_iter(tid)) .or_else(|| self.packages.get_iter(tid))
{ {
// Add the loop variable // Add the loop variable
let name = x.0.clone(); let var_name = unsafe_cast_var_name(&x.0, &state);
scope.push(name, ()); scope.push(var_name, ());
let index = scope.len() - 1; let index = scope.len() - 1;
state.scope_level += 1;
for loop_var in iter_fn(iter_type) { for loop_var in iter_fn(iter_type) {
*scope.get_mut(index).0 = loop_var; *scope.get_mut(index).0 = loop_var;
@ -1622,6 +1658,7 @@ impl Engine {
} }
scope.rewind(scope.len() - 1); scope.rewind(scope.len() - 1);
state.scope_level -= 1;
Ok(Default::default()) Ok(Default::default())
} else { } else {
Err(Box::new(EvalAltResult::ErrorFor(x.1.position()))) Err(Box::new(EvalAltResult::ErrorFor(x.1.position())))
@ -1667,15 +1704,15 @@ impl Engine {
Stmt::Let(x) if x.1.is_some() => { Stmt::Let(x) if x.1.is_some() => {
let ((var_name, _), expr) = x.as_ref(); let ((var_name, _), expr) = x.as_ref();
let val = self.eval_expr(scope, state, expr.as_ref().unwrap(), level)?; let val = self.eval_expr(scope, state, expr.as_ref().unwrap(), level)?;
// TODO - avoid copying variable name in inner block? let var_name = unsafe_cast_var_name(var_name, &state);
scope.push_dynamic_value(var_name.clone(), ScopeEntryType::Normal, val, false); scope.push_dynamic_value(var_name, ScopeEntryType::Normal, val, false);
Ok(Default::default()) Ok(Default::default())
} }
Stmt::Let(x) => { Stmt::Let(x) => {
let ((var_name, _), _) = x.as_ref(); let ((var_name, _), _) = x.as_ref();
// TODO - avoid copying variable name in inner block? let var_name = unsafe_cast_var_name(var_name, &state);
scope.push(var_name.clone(), ()); scope.push(var_name, ());
Ok(Default::default()) Ok(Default::default())
} }
@ -1683,8 +1720,8 @@ impl Engine {
Stmt::Const(x) if x.1.is_constant() => { Stmt::Const(x) if x.1.is_constant() => {
let ((var_name, _), expr) = x.as_ref(); let ((var_name, _), expr) = x.as_ref();
let val = self.eval_expr(scope, state, &expr, level)?; let val = self.eval_expr(scope, state, &expr, level)?;
// TODO - avoid copying variable name in inner block? let var_name = unsafe_cast_var_name(var_name, &state);
scope.push_dynamic_value(var_name.clone(), ScopeEntryType::Constant, val, true); scope.push_dynamic_value(var_name, ScopeEntryType::Constant, val, true);
Ok(Default::default()) Ok(Default::default())
} }
@ -1709,8 +1746,8 @@ impl Engine {
let module = let module =
resolver.resolve(self, Scope::new(), &path, expr.position())?; resolver.resolve(self, Scope::new(), &path, expr.position())?;
// TODO - avoid copying module name in inner block? let mod_name = unsafe_cast_var_name(name, &state);
scope.push_module(name.clone(), module); scope.push_module(mod_name, module);
Ok(Default::default()) Ok(Default::default())
} else { } else {
Err(Box::new(EvalAltResult::ErrorModuleNotFound( Err(Box::new(EvalAltResult::ErrorModuleNotFound(

View File

@ -1,6 +1,4 @@
//! Module containing various utility types and functions. //! Module containing various utility types and functions.
//
// TODO - remove unsafe code
use crate::stdlib::{ use crate::stdlib::{
any::TypeId, any::TypeId,
@ -55,6 +53,8 @@ pub fn calc_fn_spec<'a>(
/// # Safety /// # Safety
/// ///
/// This type uses some unsafe code (mainly to zero out unused array slots) for efficiency. /// This type uses some unsafe code (mainly to zero out unused array slots) for efficiency.
//
// TODO - remove unsafe code
#[derive(Clone, Hash)] #[derive(Clone, Hash)]
pub struct StaticVec<T> { pub struct StaticVec<T> {
/// Total number of values held. /// Total number of values held.