diff --git a/CHANGELOG.md b/CHANGELOG.md index 86567a04..3015f82d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,14 @@ Enhancements Version 1.0.6 ============= +Bug fixes +--------- + +* Eliminate unnecessary property write-back when accessed via a getter since property getters are assumed to be _pure_. + +Enhancements +------------ + * `MultiInputsStream`, `ParseState`, `TokenIterator`, `IdentifierBuilder` and `AccessMode` are exported under the `internals` feature. diff --git a/src/engine.rs b/src/engine.rs index a25bb6fc..d0be3696 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1506,6 +1506,7 @@ impl Engine { } _ => Err(err), }, + // Assume getters are always pure |(v, _)| Ok((v, false)), ) } @@ -1559,7 +1560,9 @@ impl Engine { let hash_set = FnCallHashes::from_native(*hash_set); let mut arg_values = [target.as_mut(), &mut Default::default()]; let args = &mut arg_values[..1]; - let (mut val, updated) = self + + // Assume getters are always pure + let (mut val, _) = self .exec_fn_call( mods, state, lib, getter, hash_get, args, is_ref_mut, true, *pos, None, level, @@ -1603,7 +1606,7 @@ impl Engine { .map_err(|err| err.fill_position(*x_pos))?; // Feed the value back via a setter just in case it has been updated - if updated || may_be_changed { + if may_be_changed { // Re-use args because the first &mut parameter will not be consumed let mut arg_values = [target.as_mut(), val]; let args = &mut arg_values; diff --git a/tests/get_set.rs b/tests/get_set.rs index ab0894a9..cdaa1e68 100644 --- a/tests/get_set.rs +++ b/tests/get_set.rs @@ -1,6 +1,6 @@ #![cfg(not(feature = "no_object"))] -use rhai::{Engine, EvalAltResult, INT}; +use rhai::{Engine, EvalAltResult, Scope, INT}; #[test] fn test_get_set() -> Result<(), Box> { @@ -70,7 +70,7 @@ fn test_get_set() -> Result<(), Box> { } #[test] -fn test_get_set_chain() -> Result<(), Box> { +fn test_get_set_chain_with_write_back() -> Result<(), Box> { #[derive(Clone)] struct TestChild { x: INT, @@ -121,9 +121,10 @@ fn test_get_set_chain() -> Result<(), Box> { engine.register_fn("new_tp", TestParent::new); + assert_eq!(engine.eval::("let a = new_tp(); a.child.x")?, 1); assert_eq!( - engine.eval::("let a = new_tp(); a.child.x = 500; a.child.x")?, - 500 + engine.eval::("let a = new_tp(); a.child.x = 42; a.child.x")?, + 42 ); assert_eq!( @@ -166,3 +167,48 @@ fn test_get_set_op_assignment() -> Result<(), Box> { Ok(()) } + +#[test] +fn test_get_set_chain_without_write_back() -> Result<(), Box> { + #[derive(Debug, Clone)] + struct Outer { + pub inner: Inner, + } + + #[derive(Debug, Clone)] + struct Inner { + pub value: INT, + } + + let mut engine = Engine::new(); + let mut scope = Scope::new(); + + scope.push( + "outer", + Outer { + inner: Inner { value: 42 }, + }, + ); + + engine + .register_type::() + .register_get_set( + "value", + |t: &mut Inner| t.value, + |_: &mut Inner, new: INT| panic!("Inner::value setter called with {}", new), + ) + .register_type::() + .register_get_set( + "inner", + |t: &mut Outer| t.inner.clone(), + |_: &mut Outer, new: Inner| panic!("Outer::inner setter called with {:?}", new), + ); + + assert_eq!( + engine.eval_with_scope::(&mut scope, "outer.inner.value")?, + 42 + ); + engine.consume_with_scope(&mut scope, "print(outer.inner.value)")?; + + Ok(()) +}