From b089d5b8f4ba071be612f51871f9a4d2401de3ea Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 3 Apr 2021 11:12:35 +0800 Subject: [PATCH] Fix bug in property setter op-assignment. --- CHANGELOG.md | 5 + src/engine.rs | 24 +++- src/module/resolvers/stat.rs | 2 +- tests/functions.rs | 238 +++-------------------------------- tests/get_set.rs | 33 +++++ tests/internal_fn.rs | 210 ++++++++++++++++++++++++++++++- tests/native.rs | 20 ++- 7 files changed, 306 insertions(+), 226 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6192021..1222f2f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ Rhai Release Notes Version 0.19.16 =============== +Bug fixes +--------- + +* Property setter op-assignments now work properly. + Breaking changes ---------------- diff --git a/src/engine.rs b/src/engine.rs index fa8d8713..15ed6f36 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1205,12 +1205,28 @@ impl Engine { Ok((val.take_or_clone(), false)) } - // xxx.id = ??? + // xxx.id op= ??? Expr::Property(x) if new_val.is_some() => { - let (_, (setter, hash_set), Ident { pos, .. }) = x.as_ref(); + let ((getter, hash_get), (setter, hash_set), Ident { pos, .. }) = + x.as_ref(); + let ((mut new_val, new_pos), (op_info, op_pos)) = new_val.unwrap(); + + if op_info.is_some() { + let hash = FnCallHash::from_native(*hash_get); + let mut args = [target.as_mut()]; + let (mut orig_val, _) = self.exec_fn_call( + mods, state, lib, getter, hash, &mut args, is_ref, true, *pos, + None, level, + )?; + let obj_ptr = (&mut orig_val).into(); + self.eval_op_assignment( + mods, state, lib, op_info, op_pos, obj_ptr, new_val, new_pos, + )?; + new_val = orig_val; + } + let hash = FnCallHash::from_native(*hash_set); - let mut new_val = new_val; - let mut args = [target_val, &mut (new_val.as_mut().unwrap().0).0]; + let mut args = [target.as_mut(), &mut new_val]; self.exec_fn_call( mods, state, lib, setter, hash, &mut args, is_ref, true, *pos, None, level, diff --git a/src/module/resolvers/stat.rs b/src/module/resolvers/stat.rs index 8a697e20..11c99c76 100644 --- a/src/module/resolvers/stat.rs +++ b/src/module/resolvers/stat.rs @@ -1,4 +1,4 @@ -use crate::stdlib::{boxed::Box, collections::BTreeMap, ops::AddAssign, string::String}; +use crate::stdlib::{boxed::Box, collections::BTreeMap, ops::AddAssign}; use crate::{Engine, EvalAltResult, Identifier, Module, ModuleResolver, Position, Shared}; /// A static [module][Module] resolution service that serves [modules][Module] added into it. diff --git a/tests/functions.rs b/tests/functions.rs index 5cbdcf3f..d74444ec 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -1,84 +1,35 @@ #![cfg(not(feature = "no_function"))] -use rhai::{Engine, EvalAltResult, FnNamespace, Module, NativeCallContext, ParseErrorType, INT}; +use rhai::{Engine, EvalAltResult, FnNamespace, Module, ParseErrorType, Shared, INT}; #[test] -fn test_functions() -> Result<(), Box> { - let engine = Engine::new(); +fn test_functions_trait_object() -> Result<(), Box> { + trait TestTrait { + fn greet(&self) -> INT; + } - assert_eq!(engine.eval::("fn add(x, n) { x + n } add(40, 2)")?, 42); + #[derive(Debug, Clone)] + struct ABC(INT); - assert_eq!( - engine.eval::("fn add(x, n,) { x + n } add(40, 2,)")?, - 42 - ); + impl TestTrait for ABC { + fn greet(&self) -> INT { + self.0 + } + } - assert_eq!( - engine.eval::("fn add(x, n) { x + n } let a = 40; add(a, 2); a")?, - 40 - ); + type MySharedTestTrait = Shared; - #[cfg(not(feature = "no_object"))] - assert_eq!( - engine.eval::("fn add(n) { this + n } let x = 40; x.add(2)")?, - 42 - ); - - #[cfg(not(feature = "no_object"))] - assert_eq!( - engine.eval::("fn add(n) { this += n; } let x = 40; x.add(2); x")?, - 42 - ); - - assert_eq!(engine.eval::("fn mul2(x) { x * 2 } mul2(21)")?, 42); - - assert_eq!( - engine.eval::("fn mul2(x) { x *= 2 } let a = 21; mul2(a); a")?, - 21 - ); - - #[cfg(not(feature = "no_object"))] - assert_eq!( - engine.eval::("fn mul2() { this * 2 } let x = 21; x.mul2()")?, - 42 - ); - - #[cfg(not(feature = "no_object"))] - assert_eq!( - engine.eval::("fn mul2() { this *= 2; } let x = 21; x.mul2(); x")?, - 42 - ); - - Ok(()) -} - -#[cfg(not(feature = "no_module"))] -#[cfg(not(feature = "unchecked"))] -#[test] -fn test_functions_context() -> Result<(), Box> { let mut engine = Engine::new(); - engine.set_max_modules(40); - engine.register_fn("test", |context: NativeCallContext, x: INT| { - context.engine().max_modules() as INT + x - }); + engine + .register_type_with_name::("MySharedTestTrait") + .register_fn("new_ts", || Shared::new(ABC(42)) as MySharedTestTrait) + .register_fn("greet", |x: MySharedTestTrait| x.greet()); - assert_eq!(engine.eval::("test(2)")?, 42); - - Ok(()) -} - -#[test] -fn test_functions_params() -> Result<(), Box> { - let engine = Engine::new(); - - // Expect duplicated parameters error assert_eq!( - *engine - .compile("fn hello(x, x) { x }") - .expect_err("should be error") - .0, - ParseErrorType::FnDuplicatedParam("hello".to_string(), "x".to_string()) + engine.eval::("type_of(new_ts())")?, + "MySharedTestTrait" ); + assert_eq!(engine.eval::("let x = new_ts(); greet(x)")?, 42); Ok(()) } @@ -110,152 +61,3 @@ fn test_functions_namespaces() -> Result<(), Box> { Ok(()) } - -#[test] -fn test_function_pointers() -> Result<(), Box> { - let engine = Engine::new(); - - assert_eq!(engine.eval::(r#"type_of(Fn("abc"))"#)?, "Fn"); - - assert_eq!( - engine.eval::( - r#" - fn foo(x) { 40 + x } - - let f = Fn("foo"); - call(f, 2) - "# - )?, - 42 - ); - - #[cfg(not(feature = "no_object"))] - assert_eq!( - engine.eval::( - r#" - fn foo(x) { 40 + x } - - let fn_name = "f"; - fn_name += "oo"; - - let f = Fn(fn_name); - f.call(2) - "# - )?, - 42 - ); - - #[cfg(not(feature = "no_object"))] - assert!(matches!( - *engine.eval::(r#"let f = Fn("abc"); f.call(0)"#).expect_err("should error"), - EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with("abc (") - )); - - #[cfg(not(feature = "no_object"))] - assert_eq!( - engine.eval::( - r#" - fn foo(x) { 40 + x } - - let x = #{ action: Fn("foo") }; - x.action.call(2) - "# - )?, - 42 - ); - - #[cfg(not(feature = "no_object"))] - assert_eq!( - engine.eval::( - r#" - fn foo(x) { this.data += x; } - - let x = #{ data: 40, action: Fn("foo") }; - x.action(2); - x.data - "# - )?, - 42 - ); - - Ok(()) -} - -#[test] -#[cfg(not(feature = "no_closure"))] -fn test_function_captures() -> Result<(), Box> { - let engine = Engine::new(); - - assert_eq!( - engine.eval::( - r#" - fn foo(y) { x += y; x } - - let x = 41; - let y = 999; - - foo!(1) + x - "# - )?, - 83 - ); - - assert!(engine - .eval::( - r#" - fn foo(y) { x += y; x } - - let x = 41; - let y = 999; - - foo(1) + x - "# - ) - .is_err()); - - #[cfg(not(feature = "no_object"))] - assert!(matches!( - *engine - .compile( - r#" - fn foo() { this += x; } - - let x = 41; - let y = 999; - - y.foo!(); - "# - ) - .expect_err("should error") - .0, - ParseErrorType::MalformedCapture(_) - )); - - Ok(()) -} - -#[test] -fn test_function_is_def() -> Result<(), Box> { - let engine = Engine::new(); - - assert!(engine.eval::( - r#" - fn foo(x) { x + 1 } - is_def_fn("foo", 1) - "# - )?); - assert!(!engine.eval::( - r#" - fn foo(x) { x + 1 } - is_def_fn("bar", 1) - "# - )?); - assert!(!engine.eval::( - r#" - fn foo(x) { x + 1 } - is_def_fn("foo", 0) - "# - )?); - - Ok(()) -} diff --git a/tests/get_set.rs b/tests/get_set.rs index 79acc67d..a55f5854 100644 --- a/tests/get_set.rs +++ b/tests/get_set.rs @@ -128,3 +128,36 @@ fn test_get_set_chain() -> Result<(), Box> { Ok(()) } + +#[test] +fn test_get_set_op_assignment() -> Result<(), Box> { + #[derive(Clone, Debug, Eq, PartialEq)] + struct Num(INT); + + impl Num { + fn get(&mut self) -> INT { + self.0 + } + fn set(&mut self, x: INT) { + self.0 = x; + } + } + + let mut engine = Engine::new(); + + engine + .register_type::() + .register_fn("new_ts", || Num(40)) + .register_get_set("v", Num::get, Num::set); + + assert_eq!( + engine.eval::("let a = new_ts(); a.v = a.v + 2; a")?, + Num(42) + ); + assert_eq!( + engine.eval::("let a = new_ts(); a.v += 2; a")?, + Num(42) + ); + + Ok(()) +} diff --git a/tests/internal_fn.rs b/tests/internal_fn.rs index 70a94a7c..b7bee8b6 100644 --- a/tests/internal_fn.rs +++ b/tests/internal_fn.rs @@ -18,11 +18,54 @@ fn test_internal_fn() -> Result<(), Box> { assert_eq!(engine.eval::("fn bob() { return 4; 5 } bob()")?, 4); + assert_eq!(engine.eval::("fn add(x, n) { x + n } add(40, 2)")?, 42); + + assert_eq!( + engine.eval::("fn add(x, n,) { x + n } add(40, 2,)")?, + 42 + ); + + assert_eq!( + engine.eval::("fn add(x, n) { x + n } let a = 40; add(a, 2); a")?, + 40 + ); + + #[cfg(not(feature = "no_object"))] + assert_eq!( + engine.eval::("fn add(n) { this + n } let x = 40; x.add(2)")?, + 42 + ); + + #[cfg(not(feature = "no_object"))] + assert_eq!( + engine.eval::("fn add(n) { this += n; } let x = 40; x.add(2); x")?, + 42 + ); + + assert_eq!(engine.eval::("fn mul2(x) { x * 2 } mul2(21)")?, 42); + + assert_eq!( + engine.eval::("fn mul2(x) { x *= 2 } let a = 21; mul2(a); a")?, + 21 + ); + + #[cfg(not(feature = "no_object"))] + assert_eq!( + engine.eval::("fn mul2() { this * 2 } let x = 21; x.mul2()")?, + 42 + ); + + #[cfg(not(feature = "no_object"))] + assert_eq!( + engine.eval::("fn mul2() { this *= 2; } let x = 21; x.mul2(); x")?, + 42 + ); + Ok(()) } #[test] -fn test_big_internal_fn() -> Result<(), Box> { +fn test_internal_fn_big() -> Result<(), Box> { let engine = Engine::new(); assert_eq!( @@ -73,3 +116,168 @@ fn test_internal_fn_overloading() -> Result<(), Box> { Ok(()) } + +#[test] +fn test_internal_fn_params() -> Result<(), Box> { + let engine = Engine::new(); + + // Expect duplicated parameters error + assert_eq!( + *engine + .compile("fn hello(x, x) { x }") + .expect_err("should be error") + .0, + ParseErrorType::FnDuplicatedParam("hello".to_string(), "x".to_string()) + ); + + Ok(()) +} + +#[test] +fn test_function_pointers() -> Result<(), Box> { + let engine = Engine::new(); + + assert_eq!(engine.eval::(r#"type_of(Fn("abc"))"#)?, "Fn"); + + assert_eq!( + engine.eval::( + r#" + fn foo(x) { 40 + x } + + let f = Fn("foo"); + call(f, 2) + "# + )?, + 42 + ); + + #[cfg(not(feature = "no_object"))] + assert_eq!( + engine.eval::( + r#" + fn foo(x) { 40 + x } + + let fn_name = "f"; + fn_name += "oo"; + + let f = Fn(fn_name); + f.call(2) + "# + )?, + 42 + ); + + #[cfg(not(feature = "no_object"))] + assert!(matches!( + *engine.eval::(r#"let f = Fn("abc"); f.call(0)"#).expect_err("should error"), + EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with("abc (") + )); + + #[cfg(not(feature = "no_object"))] + assert_eq!( + engine.eval::( + r#" + fn foo(x) { 40 + x } + + let x = #{ action: Fn("foo") }; + x.action.call(2) + "# + )?, + 42 + ); + + #[cfg(not(feature = "no_object"))] + assert_eq!( + engine.eval::( + r#" + fn foo(x) { this.data += x; } + + let x = #{ data: 40, action: Fn("foo") }; + x.action(2); + x.data + "# + )?, + 42 + ); + + Ok(()) +} + +#[test] +#[cfg(not(feature = "no_closure"))] +fn test_internal_fn_captures() -> Result<(), Box> { + let engine = Engine::new(); + + assert_eq!( + engine.eval::( + r#" + fn foo(y) { x += y; x } + + let x = 41; + let y = 999; + + foo!(1) + x + "# + )?, + 83 + ); + + assert!(engine + .eval::( + r#" + fn foo(y) { x += y; x } + + let x = 41; + let y = 999; + + foo(1) + x + "# + ) + .is_err()); + + #[cfg(not(feature = "no_object"))] + assert!(matches!( + *engine + .compile( + r#" + fn foo() { this += x; } + + let x = 41; + let y = 999; + + y.foo!(); + "# + ) + .expect_err("should error") + .0, + ParseErrorType::MalformedCapture(_) + )); + + Ok(()) +} + +#[test] +fn test_internal_fn_is_def() -> Result<(), Box> { + let engine = Engine::new(); + + assert!(engine.eval::( + r#" + fn foo(x) { x + 1 } + is_def_fn("foo", 1) + "# + )?); + assert!(!engine.eval::( + r#" + fn foo(x) { x + 1 } + is_def_fn("bar", 1) + "# + )?); + assert!(!engine.eval::( + r#" + fn foo(x) { x + 1 } + is_def_fn("foo", 0) + "# + )?); + + Ok(()) +} diff --git a/tests/native.rs b/tests/native.rs index 209c4a57..2112f6f7 100644 --- a/tests/native.rs +++ b/tests/native.rs @@ -1,8 +1,24 @@ use rhai::{Dynamic, Engine, EvalAltResult, NativeCallContext, INT}; use std::any::TypeId; +#[cfg(not(feature = "no_module"))] +#[cfg(not(feature = "unchecked"))] #[test] fn test_native_context() -> Result<(), Box> { + let mut engine = Engine::new(); + + engine.set_max_modules(40); + engine.register_fn("test", |context: NativeCallContext, x: INT| { + context.engine().max_modules() as INT + x + }); + + assert_eq!(engine.eval::("test(2)")?, 42); + + Ok(()) +} + +#[test] +fn test_native_context_fn_name() -> Result<(), Box> { fn add_double( context: NativeCallContext, args: &mut [&mut Dynamic], @@ -21,14 +37,14 @@ fn test_native_context() -> Result<(), Box> { add_double, ) .register_raw_fn( - "adbl", + "append_x2", &[TypeId::of::(), TypeId::of::()], add_double, ); assert_eq!(engine.eval::("add_double(40, 1)")?, "add_double_42"); - assert_eq!(engine.eval::("adbl(40, 1)")?, "adbl_42"); + assert_eq!(engine.eval::("append_x2(40, 1)")?, "append_x2_42"); Ok(()) }