Merge pull request #367 from schungx/master

Fixed bug with shared value assignments.
This commit is contained in:
Stephen Chung 2021-03-04 11:00:52 +08:00 committed by GitHub
commit c5f11e8b1f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 44 additions and 27 deletions

View File

@ -9,6 +9,7 @@ Bug fixes
* Errors in native Rust functions now contain the correct function call positions. * Errors in native Rust functions now contain the correct function call positions.
* Fixed error types in `EvalAltResult::ErrorMismatchDataType` which were swapped. * Fixed error types in `EvalAltResult::ErrorMismatchDataType` which were swapped.
* Some expressions involving shared variables now work properly, for example `x in shared_value`, `return shared_value`, `obj.field = shared_value` etc. Previously, the resultant value is still shared which is counter-intuitive.
Breaking changes Breaking changes
---------------- ----------------

View File

@ -944,16 +944,9 @@ impl Dynamic {
pub fn try_cast<T: Variant>(self) -> Option<T> { pub fn try_cast<T: Variant>(self) -> Option<T> {
// Coded this way in order to maximally leverage potentials for dead-code removal. // Coded this way in order to maximally leverage potentials for dead-code removal.
match self.0 { #[cfg(not(feature = "no_closure"))]
#[cfg(not(feature = "no_closure"))] if let Union::Shared(_, _) = self.0 {
#[cfg(not(feature = "sync"))] return self.flatten().try_cast::<T>();
Union::Shared(cell, _) => return cell.borrow().clone().try_cast(),
#[cfg(not(feature = "no_closure"))]
#[cfg(feature = "sync")]
Union::Shared(cell, _) => return cell.read().unwrap().clone().try_cast(),
_ => (),
} }
if TypeId::of::<T>() == TypeId::of::<Dynamic>() { if TypeId::of::<T>() == TypeId::of::<Dynamic>() {
@ -1321,8 +1314,10 @@ impl Dynamic {
match &self.0 { match &self.0 {
Union::Variant(value, _) => value.as_ref().as_ref().as_any().downcast_ref::<T>(), Union::Variant(value, _) => value.as_ref().as_ref().as_any().downcast_ref::<T>(),
#[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "no_closure"))]
Union::Shared(_, _) => None, Union::Shared(_, _) => None,
_ => None, _ => None,
} }
} }
@ -1411,8 +1406,10 @@ impl Dynamic {
match &mut self.0 { match &mut self.0 {
Union::Variant(value, _) => value.as_mut().as_mut_any().downcast_mut::<T>(), Union::Variant(value, _) => value.as_mut().as_mut_any().downcast_mut::<T>(),
#[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "no_closure"))]
Union::Shared(_, _) => None, Union::Shared(_, _) => None,
_ => None, _ => None,
} }
} }

View File

@ -1414,8 +1414,8 @@ impl Engine {
_ if new_val.is_some() => unreachable!("cannot assign to an expression"), _ if new_val.is_some() => unreachable!("cannot assign to an expression"),
// {expr}.??? or {expr}[???] // {expr}.??? or {expr}[???]
expr => { expr => {
let val = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; let value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?;
let obj_ptr = &mut val.into(); let obj_ptr = &mut value.into();
self.eval_dot_index_chain_helper( self.eval_dot_index_chain_helper(
mods, state, lib, this_ptr, obj_ptr, rhs, idx_values, chain_type, level, mods, state, lib, this_ptr, obj_ptr, rhs, idx_values, chain_type, level,
new_val, new_val,
@ -1452,6 +1452,7 @@ impl Engine {
.iter() .iter()
.map(|arg_expr| { .map(|arg_expr| {
self.eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level) self.eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level)
.map(Dynamic::flatten)
}) })
.collect::<Result<StaticVec<_>, _>>()?; .collect::<Result<StaticVec<_>, _>>()?;
@ -1482,6 +1483,7 @@ impl Engine {
.iter() .iter()
.map(|arg_expr| { .map(|arg_expr| {
self.eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level) self.eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level)
.map(Dynamic::flatten)
}) })
.collect::<Result<StaticVec<Dynamic>, _>>()? .collect::<Result<StaticVec<Dynamic>, _>>()?
.into() .into()
@ -1491,6 +1493,7 @@ impl Engine {
} }
_ => self _ => self
.eval_expr(scope, mods, state, lib, this_ptr, lhs, level)? .eval_expr(scope, mods, state, lib, this_ptr, lhs, level)?
.flatten()
.into(), .into(),
}; };
@ -1509,6 +1512,7 @@ impl Engine {
_ => idx_values.push( _ => idx_values.push(
self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)? self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?
.flatten()
.into(), .into(),
), ),
} }
@ -1638,7 +1642,9 @@ impl Engine {
self.inc_operations(state, rhs.position())?; self.inc_operations(state, rhs.position())?;
let lhs_value = self.eval_expr(scope, mods, state, lib, this_ptr, lhs, level)?; let lhs_value = self.eval_expr(scope, mods, state, lib, this_ptr, lhs, level)?;
let rhs_value = self.eval_expr(scope, mods, state, lib, this_ptr, rhs, level)?; let rhs_value = self
.eval_expr(scope, mods, state, lib, this_ptr, rhs, level)?
.flatten();
match rhs_value { match rhs_value {
#[cfg(not(feature = "no_index"))] #[cfg(not(feature = "no_index"))]
@ -1741,7 +1747,10 @@ impl Engine {
let mut arr = let mut arr =
Array::with_capacity(crate::stdlib::cmp::max(TYPICAL_ARRAY_SIZE, x.len())); Array::with_capacity(crate::stdlib::cmp::max(TYPICAL_ARRAY_SIZE, x.len()));
for item in x.as_ref() { for item in x.as_ref() {
arr.push(self.eval_expr(scope, mods, state, lib, this_ptr, item, level)?); arr.push(
self.eval_expr(scope, mods, state, lib, this_ptr, item, level)?
.flatten(),
);
} }
Ok(Dynamic(Union::Array(Box::new(arr), AccessMode::ReadWrite))) Ok(Dynamic(Union::Array(Box::new(arr), AccessMode::ReadWrite)))
} }
@ -1753,7 +1762,8 @@ impl Engine {
for (Ident { name: key, .. }, expr) in x.as_ref() { for (Ident { name: key, .. }, expr) in x.as_ref() {
map.insert( map.insert(
key.clone(), key.clone(),
self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?, self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?
.flatten(),
); );
} }
Ok(Dynamic(Union::Map(Box::new(map), AccessMode::ReadWrite))) Ok(Dynamic(Union::Map(Box::new(map), AccessMode::ReadWrite)))
@ -1997,7 +2007,9 @@ impl Engine {
Stmt::Noop(_) => Ok(Dynamic::UNIT), Stmt::Noop(_) => Ok(Dynamic::UNIT),
// Expression as statement // Expression as statement
Stmt::Expr(expr) => self.eval_expr(scope, mods, state, lib, this_ptr, expr, level), Stmt::Expr(expr) => Ok(self
.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?
.flatten()),
// var op= rhs // var op= rhs
Stmt::Assignment(x, op_pos) if x.0.get_variable_access(false).is_some() => { Stmt::Assignment(x, op_pos) if x.0.get_variable_access(false).is_some() => {
@ -2042,7 +2054,9 @@ impl Engine {
// lhs op= rhs // lhs op= rhs
Stmt::Assignment(x, op_pos) => { Stmt::Assignment(x, op_pos) => {
let (lhs_expr, op, rhs_expr) = x.as_ref(); let (lhs_expr, op, rhs_expr) = x.as_ref();
let rhs_val = self.eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)?; let rhs_val = self
.eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)?
.flatten();
let _new_val = Some(((rhs_val, rhs_expr.position()), (op.as_ref(), *op_pos))); let _new_val = Some(((rhs_val, rhs_expr.position()), (op.as_ref(), *op_pos)));
// Must be either `var[index] op= val` or `var.prop op= val` // Must be either `var[index] op= val` or `var.prop op= val`
@ -2161,7 +2175,9 @@ impl Engine {
// For loop // For loop
Stmt::For(expr, x, _) => { Stmt::For(expr, x, _) => {
let (name, stmt) = x.as_ref(); let (name, stmt) = x.as_ref();
let iter_obj = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; let iter_obj = self
.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?
.flatten();
let iter_type = iter_obj.type_id(); let iter_type = iter_obj.type_id();
// lib should only contain scripts, so technically they cannot have iterators // lib should only contain scripts, so technically they cannot have iterators
@ -2319,11 +2335,12 @@ impl Engine {
} }
// Return value // Return value
Stmt::Return((ReturnType::Return, pos), Some(expr), _) => EvalAltResult::Return( Stmt::Return((ReturnType::Return, pos), Some(expr), _) => {
self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?, let value = self
*pos, .eval_expr(scope, mods, state, lib, this_ptr, expr, level)?
) .flatten();
.into(), EvalAltResult::Return(value, *pos).into()
}
// Empty return // Empty return
Stmt::Return((ReturnType::Return, pos), None, _) => { Stmt::Return((ReturnType::Return, pos), None, _) => {
@ -2332,8 +2349,10 @@ impl Engine {
// Throw value // Throw value
Stmt::Return((ReturnType::Exception, pos), Some(expr), _) => { Stmt::Return((ReturnType::Exception, pos), Some(expr), _) => {
let val = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; let value = self
EvalAltResult::ErrorRuntime(val, *pos).into() .eval_expr(scope, mods, state, lib, this_ptr, expr, level)?
.flatten();
EvalAltResult::ErrorRuntime(value, *pos).into()
} }
// Empty throw // Empty throw
@ -2349,7 +2368,7 @@ impl Engine {
_ => unreachable!("should be Stmt::Let or Stmt::Const, but gets {:?}", stmt), _ => unreachable!("should be Stmt::Let or Stmt::Const, but gets {:?}", stmt),
}; };
let val = if let Some(expr) = expr { let value = if let Some(expr) = expr {
self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)? self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?
.flatten() .flatten()
} else { } else {
@ -2369,7 +2388,7 @@ impl Engine {
} else { } else {
(unsafe_cast_var_name_to_lifetime(&var_def.name).into(), None) (unsafe_cast_var_name_to_lifetime(&var_def.name).into(), None)
}; };
scope.push_dynamic_value(var_name, entry_type, val); scope.push_dynamic_value(var_name, entry_type, value);
#[cfg(not(feature = "no_module"))] #[cfg(not(feature = "no_module"))]
if let Some(alias) = _alias { if let Some(alias) = _alias {