From 20cff87e148642e23f288cf882e980b7919bbe36 Mon Sep 17 00:00:00 2001 From: Ilya Lakhin Date: Sun, 9 Aug 2020 18:35:29 +0700 Subject: [PATCH 1/6] False-positive capturing prevention bug fix --- src/parser.rs | 5 ++++- tests/closures.rs | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/parser.rs b/src/parser.rs index cc0b91c8..a47f49e8 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -414,7 +414,8 @@ struct ParseState<'e> { /// Tracks a list of external variables (variables that are not explicitly declared in the scope). #[cfg(not(feature = "no_closure"))] externals: HashMap, - /// An indicator that disables variable capturing into externals one single time. + /// An indicator that disables variable capturing into externals one single time + /// up until the nearest consumed Identifier token. /// If set to false the next call to `access_var` will not capture the variable. /// All consequent calls to `access_var` will not be affected #[cfg(not(feature = "no_closure"))] @@ -1637,6 +1638,8 @@ fn parse_primary( // Function call Token::Identifier(s) if *next_token == Token::LeftParen || *next_token == Token::Bang => { + // Once the identifier consumed we must enable next variables capturing + state.allow_capture = true; Expr::Variable(Box::new(((s, settings.pos), None, 0, None))) } // Normal variable access diff --git a/tests/closures.rs b/tests/closures.rs index d7684540..66dad215 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -1,6 +1,7 @@ #![cfg(not(feature = "no_function"))] use rhai::{Dynamic, Engine, EvalAltResult, FnPtr, Module, RegisterFn, INT}; use std::any::TypeId; +use std::mem::take; #[test] fn test_fn_ptr_curry_call() -> Result<(), Box> { @@ -88,6 +89,29 @@ fn test_closures() -> Result<(), Box> { 42 ); + engine.register_raw_fn( + "custom_call", + &[TypeId::of::(), TypeId::of::()], + |engine: &Engine, module: &Module, args: &mut [&mut Dynamic]| { + let func = take(args[1]).cast::(); + + func.call_dynamic(engine, module, None, []) + }, + ); + + assert_eq!( + engine.eval::( + r#" + let a = 42.0; + let b = 0; + let f = || b.custom_call(|| a.to_int()); + + f.call() + "# + )?, + 42 + ); + Ok(()) } From 39ee74112c63ca7283e380b10cacd5cfc4de1f25 Mon Sep 17 00:00:00 2001 From: Ilya Lakhin Date: Sun, 9 Aug 2020 18:42:33 +0700 Subject: [PATCH 2/6] no-closure feature issue fixed --- src/parser.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/parser.rs b/src/parser.rs index a47f49e8..c16116a8 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1639,7 +1639,9 @@ fn parse_primary( // Function call Token::Identifier(s) if *next_token == Token::LeftParen || *next_token == Token::Bang => { // Once the identifier consumed we must enable next variables capturing - state.allow_capture = true; + #[cfg(not(feature = "no_closure"))] { + state.allow_capture = true; + } Expr::Variable(Box::new(((s, settings.pos), None, 0, None))) } // Normal variable access From d84ef1a0d11bc022a518bca5f9ac09e728bce631 Mon Sep 17 00:00:00 2001 From: Ilya Lakhin Date: Sun, 9 Aug 2020 18:55:01 +0700 Subject: [PATCH 3/6] CLosures test fix --- tests/closures.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/closures.rs b/tests/closures.rs index 66dad215..9d22fda3 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -102,9 +102,9 @@ fn test_closures() -> Result<(), Box> { assert_eq!( engine.eval::( r#" - let a = 42.0; + let a = 41; let b = 0; - let f = || b.custom_call(|| a.to_int()); + let f = || b.custom_call(|| a + 1); f.call() "# From 564d3bc3390a38b485bbce2d44ca76dcd9e510a6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 9 Aug 2020 22:12:50 +0800 Subject: [PATCH 4/6] Add more closure tests. --- src/parser.rs | 3 ++- tests/closures.rs | 52 +++++++++++++++++++++++++++++++++++++---------- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index c16116a8..b8bb9cd8 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1639,7 +1639,8 @@ fn parse_primary( // Function call Token::Identifier(s) if *next_token == Token::LeftParen || *next_token == Token::Bang => { // Once the identifier consumed we must enable next variables capturing - #[cfg(not(feature = "no_closure"))] { + #[cfg(not(feature = "no_closure"))] + { state.allow_capture = true; } Expr::Variable(Box::new(((s, settings.pos), None, 0, None))) diff --git a/tests/closures.rs b/tests/closures.rs index 9d22fda3..94b8a9ca 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -89,6 +89,36 @@ fn test_closures() -> Result<(), Box> { 42 ); + assert_eq!( + engine.eval::( + r#" + let a = 40; + let f = |x| { + let f = |x| { + let f = |x| plus_one(a) + x; + f.call(x) + }; + f.call(x) + }; + f.call(1) + "# + )?, + 42 + ); + + assert_eq!( + engine.eval::( + r#" + let a = 21; + let f = |x| a += x; + f.call(a); + a + "# + )?, + 42 + ); + + #[allow(deprecated)] engine.register_raw_fn( "custom_call", &[TypeId::of::(), TypeId::of::()], @@ -125,12 +155,12 @@ fn test_closures_data_race() -> Result<(), Box> { assert_eq!( engine.eval::( r#" - let a = 1; - let b = 40; - let foo = |x| { this += a + x }; - b.call(foo, 1); - b - "# + let a = 1; + let b = 40; + let foo = |x| { this += a + x }; + b.call(foo, 1); + b + "# )?, 42 ); @@ -139,11 +169,11 @@ fn test_closures_data_race() -> Result<(), Box> { *engine .eval::( r#" - let a = 20; - let foo = |x| { this += a + x }; - a.call(foo, 1); - a - "# + let a = 20; + let foo = |x| { this += a + x }; + a.call(foo, 1); + a + "# ) .expect_err("should error"), EvalAltResult::ErrorDataRace(_, _) From 2d4b85f67d952a1730202f01a177fc80e9f67e12 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 10 Aug 2020 12:03:22 +0800 Subject: [PATCH 5/6] Add cusotm syntax. --- doc/src/patterns/config.md | 51 +++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/doc/src/patterns/config.md b/doc/src/patterns/config.md index a1fcdf44..e2d0db54 100644 --- a/doc/src/patterns/config.md +++ b/doc/src/patterns/config.md @@ -46,7 +46,7 @@ struct Config { ### Make Shared Object ```rust -let config: Rc> = Rc::new(RefCell::(Default::default())); +let config: Rc> = Rc::new(RefCell::new(Default::default())); ``` ### Register Config API @@ -76,7 +76,17 @@ engine.register_fn("config_add", move |values: &mut Array| let cfg = config.clone(); engine.register_fn("config_add", move |key: String, value: bool| - cfg.borrow_mut().som_map.insert(key, value) + cfg.borrow_mut().some_map.insert(key, value) +); + +let cfg = config.clone(); +engine.register_fn("config_contains", move |value: String| + cfg.borrow().some_list.contains(&value) +); + +let cfg = config.clone(); +engine.register_fn("config_is_set", move |value: String| + cfg.borrow().some_map.get(&value).cloned().unwrap_or(false) ); ``` @@ -91,7 +101,10 @@ config_set_id("hello"); config_add("foo"); // add to list config_add("bar", true); // add to map -config_add("baz", false); // add to map + +if config_contains("hey") || config_is_set("hey") { + config_add("baz", false); // add to map +} ``` ### Load the Configuration @@ -103,3 +116,35 @@ let id = config_get_id(); id == "hello"; ``` + + +Consider a Custom Syntax +------------------------ + +This is probably one of the few scenarios where a [custom syntax] can be recommended. + +A properly-designed [custom syntax] can make the configuration file clean, simple to write, +easy to understand and quick to modify. + +For example, the above configuration example may be expressed by this custom syntax: + +```rust +------------------ +| my_config.rhai | +------------------ + +// Configure ID +id "hello"; + +// Add to list +list +"foo" + +// Add to map +map "bar" => true; + +if config contains "hey" || config is_set "hey" { + map "baz" => false; +} +``` + +Notice that `contains` and `is_set` may be implemented as a [custom operator]. From a5b4d61dff68e1a26fcccec2517ba66fd386dfcc Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 11 Aug 2020 13:46:09 +0800 Subject: [PATCH 6/6] Fix docs. --- doc/src/rust/register-raw.md | 4 ++-- src/module.rs | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/doc/src/rust/register-raw.md b/doc/src/rust/register-raw.md index b7069ce3..5a73cae7 100644 --- a/doc/src/rust/register-raw.md +++ b/doc/src/rust/register-raw.md @@ -171,9 +171,9 @@ to partition the slice: let (first, rest) = args.split_at_mut(1); // Mutable reference to the first parameter -let this_ptr: &mut Dynamic = &mut *first[0].write_lock::().unwrap(); +let this_ptr: &mut A = &mut *first[0].write_lock::().unwrap(); // Immutable reference to the second value parameter // This can be mutable but there is no point because the parameter is passed by value -let value_ref: &Dynamic = &*rest[0].read_lock::().unwrap(); +let value_ref: &B = &*rest[0].read_lock::().unwrap(); ``` diff --git a/src/module.rs b/src/module.rs index b3bd5598..dbb416a2 100644 --- a/src/module.rs +++ b/src/module.rs @@ -611,9 +611,9 @@ impl Module { ) -> u64 { let f = move |_: &Engine, _: &Module, args: &mut FnCallArgs| { let b = mem::take(args[1]).cast::(); - let mut a = args[0].write_lock::().unwrap(); + let a = &mut args[0].write_lock::().unwrap(); - func(&mut a, b).map(Dynamic::from) + func(a, b).map(Dynamic::from) }; let arg_types = [TypeId::of::(), TypeId::of::()]; self.set_fn(name, Public, &arg_types, Func::from_method(Box::new(f))) @@ -735,9 +735,9 @@ impl Module { let f = move |_: &Engine, _: &Module, args: &mut FnCallArgs| { let b = mem::take(args[1]).cast::(); let c = mem::take(args[2]).cast::(); - let mut a = args[0].write_lock::().unwrap(); + let a = &mut args[0].write_lock::().unwrap(); - func(&mut a, b, c).map(Dynamic::from) + func(a, b, c).map(Dynamic::from) }; let arg_types = [TypeId::of::(), TypeId::of::(), TypeId::of::()]; self.set_fn(name, Public, &arg_types, Func::from_method(Box::new(f))) @@ -769,9 +769,9 @@ impl Module { let f = move |_: &Engine, _: &Module, args: &mut FnCallArgs| { let b = mem::take(args[1]).cast::(); let c = mem::take(args[2]).cast::(); - let mut a = args[0].write_lock::().unwrap(); + let a = &mut args[0].write_lock::().unwrap(); - func(&mut a, b, c).map(Dynamic::from) + func(a, b, c).map(Dynamic::from) }; let arg_types = [TypeId::of::(), TypeId::of::(), TypeId::of::()]; self.set_fn( @@ -892,9 +892,9 @@ impl Module { let b = mem::take(args[1]).cast::(); let c = mem::take(args[2]).cast::(); let d = mem::take(args[3]).cast::(); - let mut a = args[0].write_lock::().unwrap(); + let a = &mut args[0].write_lock::().unwrap(); - func(&mut a, b, c, d).map(Dynamic::from) + func(a, b, c, d).map(Dynamic::from) }; let arg_types = [ TypeId::of::(),