kjuulh/rhai
1
0
Fork 0
Code Issues 1 Pull Requests 1 Packages Projects Releases Wiki Activity

Do not flatten arguments passed by value.

Browse Source
This commit is contained in:
Stephen Chung 2021-03-06 21:25:49 +08:00
parent a126d05c3f
commit 4da5af8aae
4 changed files with 93 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

106
src/dynamic.rs
View File

@ -286,10 +286,10 @@ impl Dynamic {
pub fn is_shared(&self) -> bool {
#[cfg(not(feature = "no_closure"))]
match self.0 {
Union::Shared(_, _) => true,
_ => false,
Union::Shared(_, _) => return true,
_ => (),
}
#[cfg(feature = "no_closure")]
false
}
/// Is the value held by this [`Dynamic`] a particular type?
@ -749,27 +749,26 @@ impl Dynamic {
/// if its value is going to be modified. This safe-guards constant values from being modified
/// from within Rust functions.
pub fn is_read_only(&self) -> bool {
#[cfg(not(feature = "no_closure"))]
match self.0 {
#[cfg(not(feature = "no_closure"))]
Union::Shared(_, AccessMode::ReadOnly) => true,
#[cfg(not(feature = "no_closure"))]
Union::Shared(_, AccessMode::ReadOnly) => return true,
Union::Shared(ref cell, _) => {
#[cfg(not(feature = "sync"))]
let value = cell.borrow();
#[cfg(feature = "sync")]
let value = cell.read().unwrap();
match value.access_mode() {
return match value.access_mode() {
AccessMode::ReadWrite => false,
AccessMode::ReadOnly => true,
};
}
_ => (),
}
_ => match self.access_mode() {
match self.access_mode() {
AccessMode::ReadWrite => false,
AccessMode::ReadOnly => true,
},
}
}
/// Can this [`Dynamic`] be hashed?
@ -949,13 +948,10 @@ impl Dynamic {
pub fn into_shared(self) -> Self {
let _access = self.access_mode();
return match self.0 {
match self.0 {
Union::Shared(_, _) => self,
_ => Self(Union::Shared(crate::Locked::new(self).into(), _access)),
};
#[cfg(feature = "no_closure")]
panic!("converting into a shared value is not supported under 'no_closure'");
}
}
/// Convert the [`Dynamic`] value into specific type.
///
@ -1139,18 +1135,20 @@ impl Dynamic {
/// If the [`Dynamic`] is a shared value, it a cloned copy of the shared value.
#[inline(always)]
pub fn flatten_clone(&self) -> Self {
match &self.0 {
#[cfg(not(feature = "no_closure"))]
match &self.0 {
Union::Shared(cell, _) => {
#[cfg(not(feature = "sync"))]
let value = cell.borrow();
#[cfg(feature = "sync")]
let value = cell.read().unwrap();
value.clone()
return value.clone();
}
_ => self.clone(),
_ => (),
}
self.clone()
}
/// Flatten the [`Dynamic`].
///
@ -1160,9 +1158,10 @@ impl Dynamic {
/// outstanding references, or a cloned copy.
#[inline(always)]
pub fn flatten(self) -> Self {
match self.0 {
#[cfg(not(feature = "no_closure"))]
Union::Shared(cell, _) => crate::fn_native::shared_try_take(cell).map_or_else(
match self.0 {
Union::Shared(cell, _) => {
return crate::fn_native::shared_try_take(cell).map_or_else(
|cell| {
#[cfg(not(feature = "sync"))]
let value = cell.borrow();
@ -1177,8 +1176,39 @@ impl Dynamic {
#[cfg(feature = "sync")]
return value.into_inner().unwrap();
},
),
_ => self,
)
}
_ => (),
}
self
}
/// Flatten the [`Dynamic`] in place.
///
/// If the [`Dynamic`] is not a shared value, it does nothing.
///
/// If the [`Dynamic`] is a shared value, it is set to the shared value if there are no
/// outstanding references, or a cloned copy otherwise.
#[inline(always)]
pub(crate) fn flatten_in_place(&mut self) {
#[cfg(not(feature = "no_closure"))]
match self.0 {
Union::Shared(_, _) => match crate::stdlib::mem::take(self).0 {
Union::Shared(cell, _) => {
let value = crate::fn_native::shared_take_or_clone(cell);
#[cfg(not(feature = "sync"))]
{
*self = value.into_inner();
}
#[cfg(feature = "sync")]
{
*self = value.into_inner().unwrap();
}
}
_ => unreachable!(),
},
_ => (),
}
}
/// Is the [`Dynamic`] a shared value that is locked?
@ -1190,8 +1220,8 @@ impl Dynamic {
/// So this method always returns [`false`] under [`Sync`].
#[inline(always)]
pub fn is_locked(&self) -> bool {
match self.0 {
#[cfg(not(feature = "no_closure"))]
match self.0 {
Union::Shared(ref _cell, _) => {
#[cfg(not(feature = "sync"))]
return _cell.try_borrow().is_err();
@ -1199,8 +1229,10 @@ impl Dynamic {
#[cfg(feature = "sync")]
return false;
}
_ => false,
_ => (),
}
false
}
/// Get a reference of a specific type to the [`Dynamic`].
/// Casting to [`Dynamic`] just returns a reference to it.
@ -1224,15 +1256,16 @@ impl Dynamic {
let type_id = (*value).type_id();
if type_id != TypeId::of::<T>() && TypeId::of::<Dynamic>() != TypeId::of::<T>() {
None
return None;
} else {
Some(DynamicReadLock(DynamicReadLockInner::Guard(value)))
return Some(DynamicReadLock(DynamicReadLockInner::Guard(value)));
}
}
_ => self
.downcast_ref()
.map(|r| DynamicReadLock(DynamicReadLockInner::Reference(r))),
_ => (),
}
self.downcast_ref()
.map(|r| DynamicReadLock(DynamicReadLockInner::Reference(r)))
}
/// Get a mutable reference of a specific type to the [`Dynamic`].
/// Casting to [`Dynamic`] just returns a mutable reference to it.
@ -1256,15 +1289,16 @@ impl Dynamic {
let type_id = (*value).type_id();
if type_id != TypeId::of::<T>() && TypeId::of::<Dynamic>() != TypeId::of::<T>() {
None
return None;
} else {
Some(DynamicWriteLock(DynamicWriteLockInner::Guard(value)))
return Some(DynamicWriteLock(DynamicWriteLockInner::Guard(value)));
}
}
_ => self
.downcast_mut()
.map(|r| DynamicWriteLock(DynamicWriteLockInner::Reference(r))),
_ => (),
}
self.downcast_mut()
.map(|r| DynamicWriteLock(DynamicWriteLockInner::Reference(r)))
}
/// Get a reference of a specific type to the [`Dynamic`].
/// Casting to [`Dynamic`] just returns a reference to it.
@ -1357,10 +1391,8 @@ impl Dynamic {
match &self.0 {
Union::Variant(value, _) => value.as_ref().as_ref().as_any().downcast_ref::<T>(),
#[cfg(not(feature = "no_closure"))]
Union::Shared(_, _) => None,
_ => None,
}
}
@ -1449,10 +1481,8 @@ impl Dynamic {
match &mut self.0 {
Union::Variant(value, _) => value.as_mut().as_mut_any().downcast_mut::<T>(),
#[cfg(not(feature = "no_closure"))]
Union::Shared(_, _) => None,
_ => None,
}
}

8
src/fn_call.rs
View File

@ -334,12 +334,6 @@ impl Engine {
backup.as_mut().unwrap().change_first_arg_to_copy(args);
}
// Flatten arguments passed by value
args.iter_mut()
.skip(if is_ref { 1 } else { 0 })
.filter(|v| v.is_shared())
.for_each(|v| **v = mem::take(*v).flatten());
// Run external function
let source = src.as_ref().or_else(|| source.as_ref()).map(|s| s.as_str());
let result = if func.is_plugin_fn() {
@ -683,7 +677,7 @@ impl Engine {
crate::engine::KEYWORD_IS_DEF_FN
if args.len() == 2 && args[0].is::<FnPtr>() && args[1].is::<INT>() =>
{
let fn_name = mem::take(args[0]).take_immutable_string().unwrap();
let fn_name = args[0].read_lock::<ImmutableString>().unwrap();
let num_params = args[1].as_int().unwrap();
return Ok((

6
src/fn_register.rs
View File

@ -101,11 +101,7 @@ pub fn by_ref<T: Variant + Clone>(data: &mut Dynamic) -> DynamicWriteLock<T> {
pub fn by_value<T: Variant + Clone>(data: &mut Dynamic) -> T {
if TypeId::of::<T>() == TypeId::of::<&str>() {
// If T is `&str`, data must be `ImmutableString`, so map directly to it
// Beware - `Dynamic::as_str_ref` panics if `data` is shared,
// but this should not happen for argument which is passed by value
assert!(!data.is_shared());
data.flatten_in_place();
let ref_str = data
.as_str_ref()
.expect("argument passed by value should not be shared");

18
tests/modules.rs
View File

@ -395,14 +395,14 @@ fn test_module_export() -> Result<(), Box<EvalAltResult>> {
#[test]
fn test_module_str() -> Result<(), Box<EvalAltResult>> {
fn test_fn(_input: ImmutableString) -> Result<INT, Box<EvalAltResult>> {
Ok(42)
fn test_fn(input: ImmutableString) -> Result<INT, Box<EvalAltResult>> {
Ok(input.len() as INT)
}
fn test_fn2(_input: &str) -> Result<INT, Box<EvalAltResult>> {
Ok(42)
fn test_fn2(input: &str) -> Result<INT, Box<EvalAltResult>> {
Ok(input.len() as INT)
}
fn test_fn3(_input: String) -> Result<INT, Box<EvalAltResult>> {
Ok(42)
fn test_fn3(input: String) -> Result<INT, Box<EvalAltResult>> {
Ok(input.len() as INT)
}
let mut engine = rhai::Engine::new();
@ -417,15 +417,15 @@ fn test_module_str() -> Result<(), Box<EvalAltResult>> {
assert_eq!(
engine.eval::<INT>(r#"import "test" as test; test::test("test");"#)?,
42
4
);
assert_eq!(
engine.eval::<INT>(r#"import "test" as test; test::test2("test");"#)?,
42
4
);
assert_eq!(
engine.eval::<INT>(r#"import "test" as test; test::test3("test");"#)?,
42
4
);
Ok(())