From 941e09d29de2ee0db2ad90dcc56acdd667fd1fb1 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 15 May 2021 11:41:42 +0800 Subject: [PATCH] Guard against setters mutating constants, and allow pure setters. --- CHANGELOG.md | 8 +++- codegen/src/function.rs | 18 +------- codegen/src/test/function.rs | 4 +- codegen/src/test/module.rs | 44 +++++------------- src/ast.rs | 10 ++--- src/dynamic.rs | 10 ++++- src/engine.rs | 86 +++++++++++++++++------------------- src/fn_call.rs | 42 +++++++++--------- src/fn_register.rs | 22 ++++++++- src/packages/iter_basic.rs | 2 +- src/parser.rs | 18 +------- src/serde/de.rs | 2 +- tests/constants.rs | 64 +++++++++++++++++++++++++-- tests/plugins.rs | 8 ++-- tests/syntax.rs | 2 +- 15 files changed, 184 insertions(+), 156 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74c50c1a..0d73c19f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,17 @@ Rhai Release Notes Version 0.20.2 ============== +Bug fixes +--------- + +* Constant propagation during optimization for constants held in a custom scope now works properly instead of always replacing by `()`. + Breaking changes ---------------- * `Engine::disable_doc_comments` is removed because doc-comments are now placed under the `metadata` feature flag. * Registering a custom syntax now only requires specifying whether the `Scope` is adjusted (i.e. whether variables are added or removed). There is no need to specify the number of variables added/removed. +* Assigning to a property of a constant is now allowed and no longer raise an `EvalAltResult::ErrorAssignmentToConstant` error. This is to facilitate the Singleton pattern. Registered setter functions are automatically guarded against setters calling on constants and will continue to raise errors unless the `pure` attribute is present (for plugins). New features ------------ @@ -22,7 +28,7 @@ Enhancements ------------ * Registering a custom syntax now only requires specifying whether the `Scope` is adjusted (i.e. whether variables are added or removed). This allows more flexibility for cases where the number of new variables declared depends on internal logic. -* Putting a `pure` attribute on a plugin property setter now raises a syntax error. +* Putting a `pure` attribute on a plugin property/index setter now enables it to be used on constants. Version 0.20.1 diff --git a/codegen/src/function.rs b/codegen/src/function.rs index 8fc8c93b..ccc97afa 100644 --- a/codegen/src/function.rs +++ b/codegen/src/function.rs @@ -550,13 +550,6 @@ impl ExportedFn { "property setter cannot return any value", )) } - // 3c. Property setters cannot be pure. - FnSpecialAccess::Property(Property::Set(_)) if params.pure.is_some() => { - return Err(syn::Error::new( - params.pure.unwrap(), - "property setter cannot be pure", - )) - } // 4a. Index getters must take the subject and the accessed "index" as arguments. FnSpecialAccess::Index(Index::Get) if self.arg_count() != 2 => { return Err(syn::Error::new( @@ -587,13 +580,6 @@ impl ExportedFn { "index setter cannot return any value", )) } - // 5b. Index setters cannot be pure. - FnSpecialAccess::Index(Index::Set) if params.pure.is_some() => { - return Err(syn::Error::new( - params.pure.unwrap(), - "index setter cannot be pure", - )) - } _ => {} } @@ -711,9 +697,7 @@ impl ExportedFn { unpack_statements.push( syn::parse2::(quote! { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant(#arg_lit_str.to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant(#arg_lit_str.to_string(), Position::NONE).into(); } }) .unwrap(), diff --git a/codegen/src/test/function.rs b/codegen/src/test/function.rs index a8a56d83..2013a120 100644 --- a/codegen/src/test/function.rs +++ b/codegen/src/test/function.rs @@ -492,9 +492,7 @@ mod generate_tests { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE).into(); } let arg1 = mem::take(args[1usize]).cast::(); let arg0 = &mut args[0usize].write_lock::().unwrap(); diff --git a/codegen/src/test/module.rs b/codegen/src/test/module.rs index 410f25ed..959d2700 100644 --- a/codegen/src/test/module.rs +++ b/codegen/src/test/module.rs @@ -1107,9 +1107,7 @@ mod generate_tests { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE).into(); } let arg0 = &mut args[0usize].write_lock::().unwrap(); Ok(Dynamic::from(increment(arg0))) @@ -1169,9 +1167,7 @@ mod generate_tests { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE).into(); } let arg0 = &mut args[0usize].write_lock::().unwrap(); Ok(Dynamic::from(increment(arg0))) @@ -1252,9 +1248,7 @@ mod generate_tests { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE).into(); } let arg0 = &mut args[0usize].write_lock::().unwrap(); Ok(Dynamic::from(increment(arg0))) @@ -1336,9 +1330,7 @@ mod generate_tests { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE).into(); } let arg0 = &mut args[0usize].write_lock::().unwrap(); Ok(Dynamic::from(int_foo(arg0))) @@ -1399,9 +1391,7 @@ mod generate_tests { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE).into(); } let arg0 = &mut args[0usize].write_lock::().unwrap(); Ok(Dynamic::from(int_foo(arg0))) @@ -1459,9 +1449,7 @@ mod generate_tests { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE).into(); } let arg1 = mem::take(args[1usize]).cast::(); let arg0 = &mut args[0usize].write_lock::().unwrap(); @@ -1523,9 +1511,7 @@ mod generate_tests { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE).into(); } let arg1 = mem::take(args[1usize]).cast::(); let arg0 = &mut args[0usize].write_lock::().unwrap(); @@ -1584,9 +1570,7 @@ mod generate_tests { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE).into(); } let arg1 = mem::take(args[1usize]).cast::(); let arg0 = &mut args[0usize].write_lock::().unwrap(); @@ -1648,9 +1632,7 @@ mod generate_tests { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE).into(); } let arg1 = mem::take(args[1usize]).cast::(); let arg0 = &mut args[0usize].write_lock::().unwrap(); @@ -1709,9 +1691,7 @@ mod generate_tests { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE).into(); } let arg1 = mem::take(args[1usize]).cast::(); let arg2 = mem::take(args[2usize]).cast::(); @@ -1774,9 +1754,7 @@ mod generate_tests { #[inline(always)] fn call(&self, context: NativeCallContext, args: &mut [&mut Dynamic]) -> RhaiResult { if args[0usize].is_read_only() { - return Err(Box::new( - EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE) - )); + return EvalAltResult::ErrorAssignmentToConstant("x".to_string(), Position::NONE).into(); } let arg1 = mem::take(args[1usize]).cast::(); let arg2 = mem::take(args[2usize]).cast::(); diff --git a/src/ast.rs b/src/ast.rs index ca0a0c83..21841502 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1523,7 +1523,7 @@ pub struct FnCallExpr { /// List of function call argument expressions. pub args: StaticVec, /// List of function call arguments that are constants. - pub constant_args: smallvec::SmallVec<[(Dynamic, Position); 2]>, + pub literal_args: smallvec::SmallVec<[(Dynamic, Position); 2]>, /// Function name. pub name: Identifier, /// Does this function call capture the parent scope? @@ -1539,12 +1539,12 @@ impl FnCallExpr { /// Are there no arguments to this function call? #[inline(always)] pub fn is_args_empty(&self) -> bool { - self.args.is_empty() && self.constant_args.is_empty() + self.args.is_empty() && self.literal_args.is_empty() } /// Get the number of arguments to this function call. #[inline(always)] pub fn args_count(&self) -> usize { - self.args.len() + self.constant_args.len() + self.args.len() + self.literal_args.len() } } @@ -1793,8 +1793,8 @@ impl fmt::Debug for Expr { ff.field("name", &x.name) .field("hash", &x.hashes) .field("args", &x.args); - if !x.constant_args.is_empty() { - ff.field("constant_args", &x.constant_args); + if !x.literal_args.is_empty() { + ff.field("literal_args", &x.literal_args); } if x.capture { ff.field("capture", &x.capture); diff --git a/src/dynamic.rs b/src/dynamic.rs index f56bf4b0..d90b532d 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -870,9 +870,15 @@ impl Dynamic { #[cfg(feature = "decimal")] Union::Decimal(_, _, access) => *access = typ, #[cfg(not(feature = "no_index"))] - Union::Array(_, _, access) => *access = typ, + Union::Array(a, _, access) => { + *access = typ; + a.iter_mut().for_each(|v| v.set_access_mode(typ)); + } #[cfg(not(feature = "no_object"))] - Union::Map(_, _, access) => *access = typ, + Union::Map(m, _, access) => { + *access = typ; + m.values_mut().for_each(|v| v.set_access_mode(typ)); + } #[cfg(not(feature = "no_std"))] Union::TimeStamp(_, _, access) => *access = typ, #[cfg(not(feature = "no_closure"))] diff --git a/src/engine.rs b/src/engine.rs index 3f3031d3..fe2bcc7a 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1131,6 +1131,7 @@ impl Engine { lib: &[&Module], this_ptr: &mut Option<&mut Dynamic>, target: &mut Target, + root: (&str, Position), rhs: &Expr, idx_values: &mut StaticVec, chain_type: ChainType, @@ -1166,8 +1167,8 @@ impl Engine { let rhs_chain = rhs_chain.unwrap(); self.eval_dot_index_chain_helper( - mods, state, lib, this_ptr, obj_ptr, &x.rhs, idx_values, rhs_chain, - level, new_val, + mods, state, lib, this_ptr, obj_ptr, root, &x.rhs, idx_values, + rhs_chain, level, new_val, ) .map_err(|err| err.fill_position(*x_pos)) } @@ -1186,7 +1187,8 @@ impl Engine { Ok(obj_ptr) => { let ((new_val, new_pos), (op_info, op_pos)) = new_val.unwrap(); self.eval_op_assignment( - mods, state, lib, op_info, op_pos, obj_ptr, new_val, new_pos, + mods, state, lib, op_info, op_pos, obj_ptr, root, new_val, + new_pos, )?; None } @@ -1269,7 +1271,7 @@ impl Engine { )?; let ((new_val, new_pos), (op_info, op_pos)) = new_val.unwrap(); self.eval_op_assignment( - mods, state, lib, op_info, op_pos, val, new_val, new_pos, + mods, state, lib, op_info, op_pos, val, root, new_val, new_pos, )?; Ok((Dynamic::UNIT, true)) } @@ -1298,7 +1300,7 @@ impl Engine { )?; let obj_ptr = (&mut orig_val).into(); self.eval_op_assignment( - mods, state, lib, op_info, op_pos, obj_ptr, new_val, new_pos, + mods, state, lib, op_info, op_pos, obj_ptr, root, new_val, new_pos, )?; new_val = orig_val; } @@ -1352,8 +1354,8 @@ impl Engine { let rhs_chain = rhs_chain.unwrap(); self.eval_dot_index_chain_helper( - mods, state, lib, this_ptr, &mut val, &x.rhs, idx_values, rhs_chain, - level, new_val, + mods, state, lib, this_ptr, &mut val, root, &x.rhs, idx_values, + rhs_chain, level, new_val, ) .map_err(|err| err.fill_position(*x_pos)) } @@ -1383,6 +1385,7 @@ impl Engine { lib, this_ptr, &mut val.into(), + root, &x.rhs, idx_values, rhs_chain, @@ -1425,7 +1428,7 @@ impl Engine { let target = &mut val.into(); self.eval_dot_index_chain_helper( - mods, state, lib, this_ptr, target, &x.rhs, idx_values, + mods, state, lib, this_ptr, target, root, &x.rhs, idx_values, rhs_chain, level, new_val, ) .map_err(|err| err.fill_position(*pos)) @@ -1474,21 +1477,16 @@ impl Engine { match lhs { // id.??? or id[???] - Expr::Variable(_, _var_pos, x) => { + Expr::Variable(_, var_pos, x) => { #[cfg(not(feature = "unchecked"))] - self.inc_operations(state, *_var_pos)?; + self.inc_operations(state, *var_pos)?; - let (target, pos) = - self.search_namespace(scope, mods, state, lib, this_ptr, lhs)?; - - // Constants cannot be modified - if target.is_read_only() && new_val.is_some() { - return EvalAltResult::ErrorAssignmentToConstant(x.2.to_string(), pos).into(); - } + let (target, _) = self.search_namespace(scope, mods, state, lib, this_ptr, lhs)?; let obj_ptr = &mut target.into(); + let root = (x.2.as_str(), *var_pos); self.eval_dot_index_chain_helper( - mods, state, lib, &mut None, obj_ptr, rhs, idx_values, chain_type, level, + mods, state, lib, &mut None, obj_ptr, root, rhs, idx_values, chain_type, level, new_val, ) .map(|(v, _)| v) @@ -1500,8 +1498,9 @@ impl Engine { expr => { let value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; let obj_ptr = &mut value.into(); + let root = ("", expr.position()); 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, root, rhs, idx_values, chain_type, level, new_val, ) .map(|(v, _)| v) @@ -1545,7 +1544,7 @@ impl Engine { }) .collect::, _>>()?; - x.constant_args + x.literal_args .iter() .inspect(|(_, pos)| arg_positions.push(*pos)) .for_each(|(v, _)| arg_values.push(v.clone())); @@ -1590,7 +1589,7 @@ impl Engine { }) .collect::, _>>()?; - x.constant_args + x.literal_args .iter() .inspect(|(_, pos)| arg_positions.push(*pos)) .for_each(|(v, _)| arg_values.push(v.clone())); @@ -1848,6 +1847,7 @@ impl Engine { Some(OpAssignment::new(TOKEN_OP_CONCAT)), pos, (&mut result).into(), + ("", Position::NONE), item, expr.position(), )?; @@ -1892,7 +1892,7 @@ impl Engine { namespace, hashes, args, - constant_args: c_args, + literal_args: c_args, .. } = x.as_ref(); let namespace = namespace.as_ref(); @@ -1910,7 +1910,7 @@ impl Engine { capture, hashes, args, - constant_args: c_args, + literal_args: c_args, .. } = x.as_ref(); self.make_function_call( @@ -2064,11 +2064,13 @@ impl Engine { op_info: Option, op_pos: Position, mut target: Target, + root: (&str, Position), mut new_value: Dynamic, new_value_pos: Position, ) -> Result<(), Box> { if target.is_read_only() { - unreachable!("LHS should not be read-only"); + // Assignment to constant variable + return EvalAltResult::ErrorAssignmentToConstant(root.0.to_string(), root.1).into(); } if let Some(OpAssignment { @@ -2167,26 +2169,18 @@ impl Engine { #[cfg(not(feature = "unchecked"))] self.inc_operations(state, pos)?; - if lhs_ptr.is_read_only() { - // Assignment to constant variable - EvalAltResult::ErrorAssignmentToConstant( - lhs_expr.get_variable_name(false).unwrap().to_string(), - pos, - ) - .into() - } else { - self.eval_op_assignment( - mods, - state, - lib, - op_info.clone(), - *op_pos, - lhs_ptr, - rhs_val, - rhs_expr.position(), - )?; - Ok(Dynamic::UNIT) - } + self.eval_op_assignment( + mods, + state, + lib, + op_info.clone(), + *op_pos, + lhs_ptr, + (lhs_expr.get_variable_name(false).unwrap(), pos), + rhs_val, + rhs_expr.position(), + )?; + Ok(Dynamic::UNIT) } // lhs op= rhs @@ -2454,7 +2448,7 @@ impl Engine { namespace, hashes, args, - constant_args: c_args, + literal_args: c_args, .. } = x.as_ref(); let namespace = namespace.as_ref(); @@ -2472,7 +2466,7 @@ impl Engine { capture, hashes, args, - constant_args: c_args, + literal_args: c_args, .. } = x.as_ref(); self.make_function_call( diff --git a/src/fn_call.rs b/src/fn_call.rs index e99a4a20..0c087b2e 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -1065,7 +1065,7 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, fn_name: &str, args_expr: &[Expr], - constant_args: &[(Dynamic, Position)], + literal_args: &[(Dynamic, Position)], mut hashes: FnCallHashes, pos: Position, capture_scope: bool, @@ -1074,8 +1074,8 @@ impl Engine { // Handle call() - Redirect function call let redirected; let mut args_expr = args_expr; - let mut constant_args = constant_args; - let mut total_args = args_expr.len() + constant_args.len(); + let mut literal_args = literal_args; + let mut total_args = args_expr.len() + literal_args.len(); let mut curry = StaticVec::new(); let mut name = fn_name; @@ -1083,7 +1083,7 @@ impl Engine { // Handle call() KEYWORD_FN_PTR_CALL if total_args >= 1 => { let (arg, arg_pos) = args_expr.get(0).map_or_else( - || Ok(constant_args[0].clone()), + || Ok(literal_args[0].clone()), |arg| { self.eval_expr(scope, mods, state, lib, this_ptr, arg, level) .map(|v| (v, arg.position())) @@ -1108,7 +1108,7 @@ impl Engine { if !args_expr.is_empty() { args_expr = &args_expr[1..]; } else { - constant_args = &constant_args[1..]; + literal_args = &literal_args[1..]; } total_args -= 1; @@ -1123,7 +1123,7 @@ impl Engine { // Handle Fn() KEYWORD_FN_PTR if total_args == 1 => { let (arg, arg_pos) = args_expr.get(0).map_or_else( - || Ok(constant_args[0].clone()), + || Ok(literal_args[0].clone()), |arg| { self.eval_expr(scope, mods, state, lib, this_ptr, arg, level) .map(|v| (v, arg.position())) @@ -1142,7 +1142,7 @@ impl Engine { // Handle curry() KEYWORD_FN_PTR_CURRY if total_args > 1 => { let (arg, arg_pos) = args_expr.get(0).map_or_else( - || Ok(constant_args[0].clone()), + || Ok(literal_args[0].clone()), |arg| { self.eval_expr(scope, mods, state, lib, this_ptr, arg, level) .map(|v| (v, arg.position())) @@ -1164,9 +1164,9 @@ impl Engine { self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) .map(|value| fn_curry.push(value)) })?; - fn_curry.extend(constant_args.iter().map(|(v, _)| v.clone())); + fn_curry.extend(literal_args.iter().map(|(v, _)| v.clone())); } else { - fn_curry.extend(constant_args.iter().skip(1).map(|(v, _)| v.clone())); + fn_curry.extend(literal_args.iter().skip(1).map(|(v, _)| v.clone())); } return Ok(FnPtr::new_unchecked(name, fn_curry).into()); @@ -1176,7 +1176,7 @@ impl Engine { #[cfg(not(feature = "no_closure"))] crate::engine::KEYWORD_IS_SHARED if total_args == 1 => { let arg = args_expr.get(0).map_or_else( - || Ok(constant_args[0].0.clone()), + || Ok(literal_args[0].0.clone()), |arg| self.eval_expr(scope, mods, state, lib, this_ptr, arg, level), )?; return Ok(arg.is_shared().into()); @@ -1191,7 +1191,7 @@ impl Engine { args_expr[0].position(), ) } else { - constant_args[0].clone() + literal_args[0].clone() }; let fn_name = arg @@ -1204,7 +1204,7 @@ impl Engine { args_expr[1].position(), ) } else { - constant_args[if args_expr.is_empty() { 1 } else { 0 }].clone() + literal_args[if args_expr.is_empty() { 1 } else { 0 }].clone() }; let num_params = arg @@ -1223,7 +1223,7 @@ impl Engine { // Handle is_def_var() KEYWORD_IS_DEF_VAR if total_args == 1 => { let (arg, arg_pos) = args_expr.get(0).map_or_else( - || Ok(constant_args[0].clone()), + || Ok(literal_args[0].clone()), |arg| { self.eval_expr(scope, mods, state, lib, this_ptr, arg, level) .map(|v| (v, arg.position())) @@ -1240,7 +1240,7 @@ impl Engine { // eval - only in function call style let prev_len = scope.len(); let (script, script_pos) = args_expr.get(0).map_or_else( - || Ok(constant_args[0].clone()), + || Ok(literal_args[0].clone()), |script_expr| { self.eval_expr(scope, mods, state, lib, this_ptr, script_expr, level) .map(|v| (v, script_expr.position())) @@ -1292,7 +1292,7 @@ impl Engine { None }; - if args_expr.is_empty() && constant_args.is_empty() && curry.is_empty() { + if args_expr.is_empty() && literal_args.is_empty() && curry.is_empty() { // No arguments args = Default::default(); } else { @@ -1308,7 +1308,7 @@ impl Engine { self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) .map(Dynamic::flatten) }) - .chain(constant_args.iter().map(|(v, _)| Ok(v.clone()))) + .chain(literal_args.iter().map(|(v, _)| Ok(v.clone()))) .collect::>()?; let (mut target, _pos) = @@ -1344,7 +1344,7 @@ impl Engine { self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) .map(Dynamic::flatten) }) - .chain(constant_args.iter().map(|(v, _)| Ok(v.clone()))) + .chain(literal_args.iter().map(|(v, _)| Ok(v.clone()))) .collect::>()?; args = curry.iter_mut().chain(arg_values.iter_mut()).collect(); @@ -1368,7 +1368,7 @@ impl Engine { namespace: Option<&NamespaceRef>, fn_name: &str, args_expr: &[Expr], - constant_args: &[(Dynamic, Position)], + literal_args: &[(Dynamic, Position)], hash: u64, pos: Position, level: usize, @@ -1378,7 +1378,7 @@ impl Engine { let mut first_arg_value = None; let mut args: StaticVec<_>; - if args_expr.is_empty() && constant_args.is_empty() { + if args_expr.is_empty() && literal_args.is_empty() { // No arguments args = Default::default(); } else { @@ -1399,7 +1399,7 @@ impl Engine { .map(Dynamic::flatten) } }) - .chain(constant_args.iter().map(|(v, _)| Ok(v.clone()))) + .chain(literal_args.iter().map(|(v, _)| Ok(v.clone()))) .collect::>()?; // Get target reference to first argument @@ -1432,7 +1432,7 @@ impl Engine { self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) .map(Dynamic::flatten) }) - .chain(constant_args.iter().map(|(v, _)| Ok(v.clone()))) + .chain(literal_args.iter().map(|(v, _)| Ok(v.clone()))) .collect::>()?; args = arg_values.iter_mut().collect(); diff --git a/src/fn_register.rs b/src/fn_register.rs index 3e752f36..fd0b5d5c 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -3,8 +3,10 @@ #![allow(non_snake_case)] use crate::dynamic::{DynamicWriteLock, Variant}; +use crate::engine::{FN_IDX_SET, FN_SET}; use crate::fn_native::{CallableFunction, FnAny, FnCallArgs, SendSync}; use crate::r#unsafe::unsafe_try_cast; +use crate::token::Position; use crate::{Dynamic, EvalAltResult, NativeCallContext}; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -97,7 +99,11 @@ macro_rules! def_register { #[cfg(feature = "metadata")] #[inline(always)] fn return_type() -> TypeId { TypeId::of::() } #[cfg(feature = "metadata")] #[inline(always)] fn return_type_name() -> &'static str { std::any::type_name::() } #[inline(always)] fn into_callable_function(self) -> CallableFunction { - CallableFunction::$abi(Box::new(move |_: NativeCallContext, args: &mut FnCallArgs| { + CallableFunction::$abi(Box::new(move |ctx: NativeCallContext, args: &mut FnCallArgs| { + if args.len() == 2 && args[0].is_read_only() && (ctx.fn_name().starts_with(FN_SET) || ctx.fn_name().starts_with(FN_IDX_SET)) { + return EvalAltResult::ErrorAssignmentToConstant(Default::default(), Position::NONE).into(); + } + // The arguments are assumed to be of the correct number and types! let mut _drain = args.iter_mut(); $($let $par = ($clone)(_drain.next().unwrap()); )* @@ -122,6 +128,10 @@ macro_rules! def_register { #[cfg(feature = "metadata")] #[inline(always)] fn return_type_name() -> &'static str { std::any::type_name::() } #[inline(always)] fn into_callable_function(self) -> CallableFunction { CallableFunction::$abi(Box::new(move |ctx: NativeCallContext, args: &mut FnCallArgs| { + if args.len() == 2 && args[0].is_read_only() && (ctx.fn_name().starts_with(FN_SET) || ctx.fn_name().starts_with(FN_IDX_SET)) { + return EvalAltResult::ErrorAssignmentToConstant(Default::default(), Position::NONE).into(); + } + // The arguments are assumed to be of the correct number and types! let mut _drain = args.iter_mut(); $($let $par = ($clone)(_drain.next().unwrap()); )* @@ -145,7 +155,11 @@ macro_rules! def_register { #[cfg(feature = "metadata")] #[inline(always)] fn return_type() -> TypeId { TypeId::of::>>() } #[cfg(feature = "metadata")] #[inline(always)] fn return_type_name() -> &'static str { std::any::type_name::>>() } #[inline(always)] fn into_callable_function(self) -> CallableFunction { - CallableFunction::$abi(Box::new(move |_: NativeCallContext, args: &mut FnCallArgs| { + CallableFunction::$abi(Box::new(move |ctx: NativeCallContext, args: &mut FnCallArgs| { + if args.len() == 2 && args[0].is_read_only() && (ctx.fn_name().starts_with(FN_SET) || ctx.fn_name().starts_with(FN_IDX_SET)) { + return EvalAltResult::ErrorAssignmentToConstant(Default::default(), Position::NONE).into(); + } + // The arguments are assumed to be of the correct number and types! let mut _drain = args.iter_mut(); $($let $par = ($clone)(_drain.next().unwrap()); )* @@ -167,6 +181,10 @@ macro_rules! def_register { #[cfg(feature = "metadata")] #[inline(always)] fn return_type_name() -> &'static str { std::any::type_name::>>() } #[inline(always)] fn into_callable_function(self) -> CallableFunction { CallableFunction::$abi(Box::new(move |ctx: NativeCallContext, args: &mut FnCallArgs| { + if args.len() == 2 && args[0].is_read_only() && (ctx.fn_name().starts_with(FN_SET) || ctx.fn_name().starts_with(FN_IDX_SET)) { + return EvalAltResult::ErrorAssignmentToConstant(Default::default(), Position::NONE).into(); + } + // The arguments are assumed to be of the correct number and types! let mut _drain = args.iter_mut(); $($let $par = ($clone)(_drain.next().unwrap()); )* diff --git a/src/packages/iter_basic.rs b/src/packages/iter_basic.rs index b7739151..30280c42 100644 --- a/src/packages/iter_basic.rs +++ b/src/packages/iter_basic.rs @@ -26,7 +26,7 @@ where if r == from { return EvalAltResult::ErrorInFunctionCall( "range".to_string(), - "".to_string(), + Default::default(), Box::new(EvalAltResult::ErrorArithmetic( "step value cannot be zero".to_string(), crate::Position::NONE, diff --git a/src/parser.rs b/src/parser.rs index 5cb722bc..c8a82deb 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1468,24 +1468,10 @@ fn make_assignment_stmt<'a>( Expr::Index(x, _) | Expr::Dot(x, _) => { match check_lvalue(&x.rhs, matches!(lhs, Expr::Dot(_, _))) { None => match &x.lhs { - // var[???] (non-indexed) = rhs, var.??? (non-indexed) = rhs - Expr::Variable(None, _, x) if x.0.is_none() => { + // var[???] = rhs, var.??? = rhs + Expr::Variable(_, _, _) => { Ok(Stmt::Assignment(Box::new((lhs, op_info, rhs)), op_pos)) } - // var[???] (indexed) = rhs, var.??? (indexed) = rhs - Expr::Variable(i, var_pos, x) => { - let (index, _, name) = x.as_ref(); - let index = i.map_or_else(|| index.unwrap().get(), |n| n.get() as usize); - match state.stack[state.stack.len() - index].1 { - AccessMode::ReadWrite => { - Ok(Stmt::Assignment(Box::new((lhs, op_info, rhs)), op_pos)) - } - // Constant values cannot be assigned to - AccessMode::ReadOnly => { - Err(PERR::AssignmentToConstant(name.to_string()).into_err(*var_pos)) - } - } - } // expr[???] = rhs, expr.??? = rhs expr => { Err(PERR::AssignmentToInvalidLHS("".to_string()).into_err(expr.position())) diff --git a/src/serde/de.rs b/src/serde/de.rs index c92cac19..d6e93d20 100644 --- a/src/serde/de.rs +++ b/src/serde/de.rs @@ -115,7 +115,7 @@ pub fn from_dynamic<'de, T: Deserialize<'de>>( impl Error for Box { fn custom(err: T) -> Self { - LexError::ImproperSymbol("".to_string(), err.to_string()) + LexError::ImproperSymbol(Default::default(), err.to_string()) .into_err(Position::NONE) .into() } diff --git a/tests/constants.rs b/tests/constants.rs index 8c3c95af..c64ecb1b 100644 --- a/tests/constants.rs +++ b/tests/constants.rs @@ -16,7 +16,7 @@ fn test_constant() -> Result<(), Box> { #[cfg(not(feature = "no_index"))] assert!(matches!( *engine.consume("const x = [1, 2, 3, 4, 5]; x[2] = 42;").expect_err("expects error"), - EvalAltResult::ErrorParsing(ParseErrorType::AssignmentToConstant(x), _) if x == "x" + EvalAltResult::ErrorAssignmentToConstant(x, _) if x == "x" )); Ok(()) @@ -45,17 +45,66 @@ fn test_constant_mut() -> Result<(), Box> { let mut engine = Engine::new(); + fn set_value(obj: &mut TestStruct, value: INT) { + obj.0 = value; + } + engine .register_type_with_name::("TestStruct") + .register_fn("new_ts", || TestStruct(123)) .register_get("value", |obj: &mut TestStruct| obj.0) - .register_fn("update_value", |obj: &mut TestStruct, value: INT| { - obj.0 = value - }); + .register_set("value", set_value) + .register_fn("update_value", set_value); + + assert_eq!( + engine.eval::( + " + const MY_NUMBER = new_ts(); + MY_NUMBER.update_value(42); + MY_NUMBER.value + ", + )?, + 42 + ); + + assert_eq!( + engine.eval::( + " + const MY_NUMBER = new_ts(); + update_value(MY_NUMBER, 42); + MY_NUMBER.value + ", + )?, + 123 + ); + + assert!(matches!( + *engine + .consume( + " + const MY_NUMBER = new_ts(); + MY_NUMBER.value = 42; + " + ) + .expect_err("should error"), + EvalAltResult::ErrorAssignmentToConstant(_, _) + )); let mut scope = Scope::new(); scope.push_constant("MY_NUMBER", TestStruct(123)); + assert_eq!( + engine.eval_with_scope::( + &mut scope, + " + update_value(MY_NUMBER, 42); + MY_NUMBER.value + ", + )?, + 123 + ); + assert_eq!( engine.eval_with_scope::( &mut scope, @@ -67,5 +116,12 @@ fn test_constant_mut() -> Result<(), Box> { 42 ); + assert!(matches!( + *engine + .consume_with_scope(&mut scope, "MY_NUMBER.value = 42;") + .expect_err("should error"), + EvalAltResult::ErrorAssignmentToConstant(_, _) + )); + Ok(()) } diff --git a/tests/plugins.rs b/tests/plugins.rs index 42aadcb7..9e5548f6 100644 --- a/tests/plugins.rs +++ b/tests/plugins.rs @@ -38,9 +38,10 @@ mod test { pub fn funky_add(x: INT, y: INT) -> INT { x / 2 + y * 2 } - #[rhai_fn(pure)] - pub fn no_effect(_array: &mut Array, _value: INT) { - // do nothing to array + #[rhai_fn(name = "no_effect", set = "no_effect", pure)] + pub fn no_effect(array: &mut Array, value: INT) { + // array is not modified + println!("Array = {:?}, Value = {}", array, value); } } } @@ -87,6 +88,7 @@ fn test_plugins_package() -> Result<(), Box> { { assert_eq!(engine.eval::("let a = [1, 2, 3]; a.foo")?, 1); engine.consume("const A = [1, 2, 3]; A.no_effect(42);")?; + engine.consume("const A = [1, 2, 3]; A.no_effect = 42;")?; assert!( matches!(*engine.consume("const A = [1, 2, 3]; A.test(42);").expect_err("should error"), diff --git a/tests/syntax.rs b/tests/syntax.rs index 2fa1dc0c..1b8857e9 100644 --- a/tests/syntax.rs +++ b/tests/syntax.rs @@ -128,7 +128,7 @@ fn test_custom_syntax_raw() -> Result<(), Box> { s => Err(ParseError( Box::new(ParseErrorType::BadInput(LexError::ImproperSymbol( s.to_string(), - "".to_string(), + Default::default(), ))), Position::NONE, )),