From 5d908a1153b2092efbb3609a447af6abc815b99c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 7 Aug 2020 09:25:52 +0800 Subject: [PATCH 01/11] Set fail-fast to false. --- .github/workflows/build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8c8b19eb..c5295c74 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -38,6 +38,7 @@ jobs: - {toolchain: stable, os: macos-latest, experimental: false, flags: ""} - {toolchain: beta, os: ubuntu-latest, experimental: false, flags: ""} - {toolchain: nightly, os: ubuntu-latest, experimental: true, flags: ""} + fail-fast: false steps: - name: Checkout uses: actions/checkout@v2 From 5e6d5e8e8016b91cdb40fd93ad1a6cd3c1195542 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 7 Aug 2020 11:10:38 +0800 Subject: [PATCH 02/11] Expand getter/setter/indexer API. --- RELEASES.md | 5 ++ src/api.rs | 208 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 212 insertions(+), 1 deletion(-) diff --git a/RELEASES.md b/RELEASES.md index 87cb5bdb..fcf498cf 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -4,6 +4,11 @@ Rhai Release Notes Version 0.19.0 ============== +New features +------------ + +* Adds `Engine::register_get_result`, `Engine::register_set_result`, `Engine::register_indexer_get_result`, `Engine::register_indexer_set_result` API. + Version 0.18.1 ============== diff --git a/src/api.rs b/src/api.rs index 34af07a1..8487370f 100644 --- a/src/api.rs +++ b/src/api.rs @@ -18,7 +18,7 @@ use crate::engine::{FN_IDX_GET, FN_IDX_SET}; #[cfg(not(feature = "no_object"))] use crate::{ engine::{make_getter, make_setter, Map}, - fn_register::RegisterFn, + fn_register::{RegisterFn, RegisterResultFn}, token::Token, }; @@ -224,6 +224,54 @@ impl Engine { self.register_fn(&make_getter(name), callback) } + /// Register a getter function for a member of a registered type with the `Engine`. + /// Returns `Result>`. + /// + /// The function signature must start with `&mut self` and not `&self`. + /// + /// # Example + /// + /// ``` + /// use rhai::{Engine, Dynamic, EvalAltResult, RegisterFn}; + /// + /// #[derive(Clone)] + /// struct TestStruct { + /// field: i64 + /// } + /// + /// impl TestStruct { + /// fn new() -> Self { TestStruct { field: 1 } } + /// + /// // Even a getter must start with `&mut self` and not `&self`. + /// fn get_field(&mut self) -> Result> { + /// Ok(self.field.into()) + /// } + /// } + /// + /// # fn main() -> Result<(), Box> { + /// let mut engine = Engine::new(); + /// + /// // Register the custom type. + /// engine.register_type::(); + /// + /// engine.register_fn("new_ts", TestStruct::new); + /// + /// // Register a getter on a property (notice it doesn't have to be the same name). + /// engine.register_get_result("xyz", TestStruct::get_field); + /// + /// assert_eq!(engine.eval::("let a = new_ts(); a.xyz")?, 1); + /// # Ok(()) + /// # } + /// ``` + #[cfg(not(feature = "no_object"))] + pub fn register_get_result( + &mut self, + name: &str, + callback: impl Fn(&mut T) -> Result> + SendSync + 'static, + ) -> &mut Self { + self.register_result_fn(&make_getter(name), callback) + } + /// Register a setter function for a member of a registered type with the `Engine`. /// /// # Example @@ -273,6 +321,59 @@ impl Engine { self.register_fn(&make_setter(name), callback) } + /// Register a setter function for a member of a registered type with the `Engine`. + /// Returns `Result>`. + /// + /// # Example + /// + /// ``` + /// use rhai::{Engine, Dynamic, EvalAltResult, RegisterFn}; + /// + /// #[derive(Debug, Clone, Eq, PartialEq)] + /// struct TestStruct { + /// field: i64 + /// } + /// + /// impl TestStruct { + /// fn new() -> Self { TestStruct { field: 1 } } + /// fn set_field(&mut self, new_val: i64) -> Result> { + /// self.field = new_val; + /// Ok(().into()) + /// } + /// } + /// + /// # fn main() -> Result<(), Box> { + /// let mut engine = Engine::new(); + /// + /// // Register the custom type. + /// engine.register_type::(); + /// + /// engine.register_fn("new_ts", TestStruct::new); + /// + /// // Register a setter on a property (notice it doesn't have to be the same name) + /// engine.register_set_result("xyz", TestStruct::set_field); + /// + /// // Notice that, with a getter, there is no way to get the property value + /// assert_eq!( + /// engine.eval::("let a = new_ts(); a.xyz = 42; a")?, + /// TestStruct { field: 42 } + /// ); + /// # Ok(()) + /// # } + /// ``` + #[cfg(not(feature = "no_object"))] + pub fn register_set_result( + &mut self, + name: &str, + callback: impl Fn(&mut T, U) -> Result> + SendSync + 'static, + ) -> &mut Self + where + T: Variant + Clone, + U: Variant + Clone, + { + self.register_result_fn(&make_setter(name), callback) + } + /// Shorthand for registering both getter and setter functions /// of a registered type with the `Engine`. /// @@ -375,6 +476,58 @@ impl Engine { self.register_fn(FN_IDX_GET, callback) } + /// Register an index getter for a registered type with the `Engine`. + /// Returns `Result>`. + /// + /// The function signature must start with `&mut self` and not `&self`. + /// + /// # Example + /// + /// ``` + /// use rhai::{Engine, Dynamic, EvalAltResult, RegisterFn}; + /// + /// #[derive(Clone)] + /// struct TestStruct { + /// fields: Vec + /// } + /// + /// impl TestStruct { + /// fn new() -> Self { TestStruct { fields: vec![1, 2, 3, 4, 5] } } + /// + /// // Even a getter must start with `&mut self` and not `&self`. + /// fn get_field(&mut self, index: i64) -> Result> { + /// Ok(self.fields[index as usize].into()) + /// } + /// } + /// + /// # fn main() -> Result<(), Box> { + /// let mut engine = Engine::new(); + /// + /// // Register the custom type. + /// engine.register_type::(); + /// + /// engine.register_fn("new_ts", TestStruct::new); + /// + /// // Register an indexer. + /// engine.register_indexer_get_result(TestStruct::get_field); + /// + /// assert_eq!(engine.eval::("let a = new_ts(); a[2]")?, 3); + /// # Ok(()) + /// # } + /// ``` + #[cfg(not(feature = "no_object"))] + #[cfg(not(feature = "no_index"))] + pub fn register_indexer_get_result( + &mut self, + callback: impl Fn(&mut T, X) -> Result> + SendSync + 'static, + ) -> &mut Self + where + T: Variant + Clone, + X: Variant + Clone, + { + self.register_result_fn(FN_IDX_GET, callback) + } + /// Register an index setter for a registered type with the `Engine`. /// /// # Example @@ -424,6 +577,59 @@ impl Engine { self.register_fn(FN_IDX_SET, callback) } + /// Register an index setter for a registered type with the `Engine`. + /// Returns `Result>`. + /// + /// # Example + /// + /// ``` + /// use rhai::{Engine, Dynamic, EvalAltResult, RegisterFn}; + /// + /// #[derive(Clone)] + /// struct TestStruct { + /// fields: Vec + /// } + /// + /// impl TestStruct { + /// fn new() -> Self { TestStruct { fields: vec![1, 2, 3, 4, 5] } } + /// fn set_field(&mut self, index: i64, value: i64) -> Result> { + /// self.fields[index as usize] = value; + /// Ok(().into()) + /// } + /// } + /// + /// # fn main() -> Result<(), Box> { + /// let mut engine = Engine::new(); + /// + /// // Register the custom type. + /// engine.register_type::(); + /// + /// engine.register_fn("new_ts", TestStruct::new); + /// + /// // Register an indexer. + /// engine.register_indexer_set_result(TestStruct::set_field); + /// + /// assert_eq!( + /// engine.eval::("let a = new_ts(); a[2] = 42; a")?.fields[2], + /// 42 + /// ); + /// # Ok(()) + /// # } + /// ``` + #[cfg(not(feature = "no_object"))] + #[cfg(not(feature = "no_index"))] + pub fn register_indexer_set_result( + &mut self, + callback: impl Fn(&mut T, X, U) -> Result> + SendSync + 'static, + ) -> &mut Self + where + T: Variant + Clone, + U: Variant + Clone, + X: Variant + Clone, + { + self.register_result_fn(FN_IDX_SET, callback) + } + /// Shorthand for register both index getter and setter functions for a registered type with the `Engine`. /// /// # Example From 0b21d80641acefa1d8ee8c6e6bda200c76e85e46 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 7 Aug 2020 11:44:15 +0800 Subject: [PATCH 03/11] Add patterns section. --- doc/src/SUMMARY.md | 8 +- doc/src/advanced.md | 4 +- doc/src/language/object-maps-oop.md | 15 +-- doc/src/links.md | 2 +- doc/src/patterns/config.md | 103 +++++++++++++++++++ doc/src/patterns/control.md | 117 ++++++++++++++++++++++ doc/src/patterns/index.md | 9 ++ doc/src/{language => patterns}/oop.md | 0 doc/src/patterns/singleton.md | 136 ++++++++++++++++++++++++++ 9 files changed, 382 insertions(+), 12 deletions(-) create mode 100644 doc/src/patterns/config.md create mode 100644 doc/src/patterns/control.md create mode 100644 doc/src/patterns/index.md rename doc/src/{language => patterns}/oop.md (100%) create mode 100644 doc/src/patterns/singleton.md diff --git a/doc/src/SUMMARY.md b/doc/src/SUMMARY.md index 3b23b321..a705b015 100644 --- a/doc/src/SUMMARY.md +++ b/doc/src/SUMMARY.md @@ -100,8 +100,12 @@ The Rhai Scripting Language 8. [Maximum Call Stack Depth](safety/max-call-stack.md) 9. [Maximum Statement Depth](safety/max-stmt-depth.md) 7. [Advanced Topics](advanced.md) - 1. [Capture Scope for Function Call](language/fn-capture.md) - 2. [Object-Oriented Programming (OOP)](language/oop.md) + 1. [Advanced Patterns](patterns/index.md) + 1. [Loadable Configuration](patterns/config.md) + 2. [Control Layer](patterns/control.md) + 3. [Singleton Command](patterns/singleton.md) + 4. [Object-Oriented Programming (OOP)](patterns/oop.md) + 2. [Capture Scope for Function Call](language/fn-capture.md) 3. [Serialization/Deserialization of `Dynamic` with `serde`](rust/serde.md) 4. [Script Optimization](engine/optimize/index.md) 1. [Optimization Levels](engine/optimize/optimize-levels.md) diff --git a/doc/src/advanced.md b/doc/src/advanced.md index 6b79fe70..0b9e170e 100644 --- a/doc/src/advanced.md +++ b/doc/src/advanced.md @@ -5,9 +5,9 @@ Advanced Topics This section covers advanced features such as: -* [Capture the calling scope]({{rootUrl}}/language/fn-capture.md) in a function call. +* [Advanced patterns]({{rootUrl}}/patterns/index.md) in using Rhai. -* Simulated [Object Oriented Programming (OOP)][OOP]. +* [Capture the calling scope]({{rootUrl}}/language/fn-capture.md) in a function call. * [`serde`] integration. diff --git a/doc/src/language/object-maps-oop.md b/doc/src/language/object-maps-oop.md index c93f6847..0c2b840d 100644 --- a/doc/src/language/object-maps-oop.md +++ b/doc/src/language/object-maps-oop.md @@ -10,24 +10,23 @@ If an [object map]'s property holds a [function pointer], the property can simpl a normal method in method-call syntax. This is a _short-hand_ to avoid the more verbose syntax of using the `call` function keyword. -When a property holding a [function pointer] is called like a method, what happens next depends -on whether the target function is a native Rust function or a script-defined function. +When a property holding a [function pointer] (which incudes [closures]) is called like a method, +what happens next depends on whether the target function is a native Rust function or +a script-defined function. -If it is a registered native Rust method function, then it is called directly. +If it is a registered native Rust method function, it is called directly. If it is a script-defined function, the `this` variable within the function body is bound to the [object map] before the function is called. There is no way to simulate this behavior via a normal function-call syntax because all scripted function arguments are passed by value. ```rust -fn do_action(x) { this.data += x; } // 'this' binds to the object when called - let obj = #{ data: 40, - action: Fn("do_action") // 'action' holds a function pointer to 'do_action' + action: || this.data += x // 'action' holds a function pointer which is a closure }; -obj.action(2); // Calls 'do_action' with `this` bound to 'obj' +obj.action(2); // Calls the function pointer with `this` bound to 'obj' obj.call(obj.action, 2); // The above de-sugars to this @@ -36,5 +35,7 @@ obj.data == 42; // To achieve the above with normal function pointer call will fail. fn do_action(map, x) { map.data += x; } // 'map' is a copy +obj.action = Fn("do_action"); + obj.action.call(obj, 2); // 'obj' is passed as a copy by value ``` diff --git a/doc/src/links.md b/doc/src/links.md index 11b16796..d2df1d5b 100644 --- a/doc/src/links.md +++ b/doc/src/links.md @@ -97,7 +97,7 @@ [`eval`]: {{rootUrl}}/language/eval.md -[OOP]: {{rootUrl}}/language/oop.md +[OOP]: {{rootUrl}}/patterns/oop.md [DSL]: {{rootUrl}}/engine/dsl.md [maximum statement depth]: {{rootUrl}}/safety/max-stmt-depth.md diff --git a/doc/src/patterns/config.md b/doc/src/patterns/config.md new file mode 100644 index 00000000..640191fa --- /dev/null +++ b/doc/src/patterns/config.md @@ -0,0 +1,103 @@ +Loadable Configuration +====================== + +{{#include ../links.md}} + + +Usage Scenario +-------------- + +* A system where settings and configurations are complex and logic-driven. + +* Where it is not possible to configure said system via standard configuration file formats such as `TOML` or `YAML`. + +* The system configuration is complex enough that it requires a full programming language. Essentially _configuration by code_. + +* Yet the configurations must be flexible, late-bound and dynamically loadable, just like a configuration file. + + +Key Concepts +------------ + +* Leverage the loadable [modules] of Rhai. The [`no_module`] feature must not be on. + +* Expose the configuration API. Use separate scripts to configure that API. Dynamically load scripts via the `import` statement. + +* Since Rhai is _sand-boxed_, it cannot mutate the environment. To modify the external configuration object via an API, it must be wrapped in a `RefCell` (or `RwLock`/`Mutex` for [`sync`]) and shared to the [`Engine`]. + + +Implementation +-------------- + +### Configuration Type + +```rust +#[derive(Debug, Clone, Default)] +struct Config { + pub id: String; + pub some_field: i64; + pub some_list: Vec; + pub some_map: HashMap; +} +``` + +### Make Shared Object + +```rust +let config: Rc> = Rc::new(RefCell::(Default::default())); +``` + +### Register Config API + +```rust +// Notice 'move' is used to move the shared configuration object into the closure. +let cfg = config.clone(); +engine.register_fn("config_set_id", move |id: String| *cfg.borrow_mut().id = id); + +let cfg = config.clone(); +engine.register_fn("config_get_id", move || cfg.borrow().id.clone()); + +let cfg = config.clone(); +engine.register_fn("config_set", move |value: i64| *cfg.borrow_mut().some_field = value); + +// Remember Rhai functions can be overloaded when designing the API. + +let cfg = config.clone(); +engine.register_fn("config_add", move |value: String| + cfg.borrow_mut().some_list.push(value) +); + +let cfg = config.clone(); +engine.register_fn("config_add", move |values: &mut Array| + cfg.borrow_mut().some_list.extend(values.into_iter().map(|v| v.to_string())) +); + +let cfg = config.clone(); +engine.register_fn("config_add", move |key: String, value: bool| + cfg.borrow_mut().som_map.insert(key, value) +); +``` + +### Configuration Script + +```rust +------------------ +| my_config.rhai | +------------------ + +config_set_id("hello"); + +config_add("foo"); // add to list +config_add("bar", true); // add to map +config_add("baz", false); // add to map +``` + +### Load the Configuration + +```rust +import "my_config"; // run configuration script without creating a module + +let id = config_get_id(); + +id == "hello"; +``` diff --git a/doc/src/patterns/control.md b/doc/src/patterns/control.md new file mode 100644 index 00000000..2f5aa961 --- /dev/null +++ b/doc/src/patterns/control.md @@ -0,0 +1,117 @@ +Scriptable Control Layer +======================== + +{{#include ../links.md}} + + +Usage Scenario +-------------- + +* A system provides core functionalities, but no driving logic. + +* The driving logic must be dynamic and hot-loadable. + +* A script is used to drive the system and provide control intelligence. + + +Key Concepts +------------ + +* Expose a Control API. + +* Since Rhai is _sand-boxed_, it cannot mutate the environment. To perform external actions via an API, the actual system must be wrapped in a `RefCell` (or `RwLock`/`Mutex` for [`sync`]) and shared to the [`Engine`]. + + +Implementation +-------------- + +### Functional API + +Assume that a system provides the following functional API: + +```rust +struct EnergizerBunny; + +impl EnergizerBunny { + pub fn new () -> Self { ... } + pub fn go (&mut self) { ... } + pub fn stop (&mut self) { ... } + pub fn is_going (&self) { ... } + pub fn get_speed (&self) -> i64 { ... } + pub fn set_speed (&mut self, speed: i64) { ... } +} +``` + +### Wrap API in Shared Object + +```rust +let bunny: Rc> = Rc::new(RefCell::(EnergizerBunny::new())); +``` + +### Register Control API + +```rust +// Notice 'move' is used to move the shared API object into the closure. +let b = bunny.clone(); +engine.register_fn("bunny_power", move |on: bool| { + if on { + if b.borrow().is_going() { + println!("Still going..."); + } else { + b.borrow_mut().go(); + } + } else { + if b.borrow().is_going() { + b.borrow_mut().stop(); + } else { + println!("Already out of battery!"); + } + } +}); + +let b = bunny.clone(); +engine.register_fn("bunny_is_going", move || b.borrow().is_going()); + +let b = bunny.clone(); +engine.register_fn("bunny_get_speed", move || + if b.borrow().is_going() { b.borrow().get_speed() } else { 0 } +); + +let b = bunny.clone(); +engine.register_result_fn("bunny_set_speed", move |speed: i64| + if speed <= 0 { + return Err("Speed must be positive!".into()); + } else if speed > 100 { + return Err("Bunny will be going too fast!".into()); + } + + if b.borrow().is_going() { + b.borrow_mut().set_speed(speed) + } else { + return Err("Bunny is not yet going!".into()); + } + + Ok(().into()) +); +``` + +### Use the API + +```rust +if !bunny_is_going() { bunny_power(true); } + +if bunny_get_speed() > 50 { bunny_set_speed(50); } +``` + + +Caveat +------ + +Although this usage pattern appears a perfect fit for _game_ logic, avoid writing the +_entire game_ in Rhai. Performance will not be acceptable. + +Implement as much functionalities of the game engine in Rust as possible. +Rhai integrates well with Rust so this is usually not a hinderance. + +Lift as much out of Rhai as possible. +Use Rhai only for the logic that _must_ be dynamic or hot-loadable. diff --git a/doc/src/patterns/index.md b/doc/src/patterns/index.md new file mode 100644 index 00000000..522d3403 --- /dev/null +++ b/doc/src/patterns/index.md @@ -0,0 +1,9 @@ +Advanced Patterns +================= + +{{#include ../links.md}} + + +Use Rhai in different scenarios other than simply evaluating a user script. + +These patterns are useful when Rhai needs to affect/control the external environment. diff --git a/doc/src/language/oop.md b/doc/src/patterns/oop.md similarity index 100% rename from doc/src/language/oop.md rename to doc/src/patterns/oop.md diff --git a/doc/src/patterns/singleton.md b/doc/src/patterns/singleton.md new file mode 100644 index 00000000..7172e745 --- /dev/null +++ b/doc/src/patterns/singleton.md @@ -0,0 +1,136 @@ +Singleton Command Objects +======================== + +{{#include ../links.md}} + + +Usage Scenario +-------------- + +* A system provides core functionalities, but no driving logic. + +* The driving logic must be dynamic and hot-loadable. + +* A script is used to drive the system and provide control intelligence. + +* The API is multiplexed, meaning that it can act on multiple system-provided entities, or + +* The API lends itself readily to an object-oriented (OO) representation. + + +Key Concepts +------------ + +* Expose a Command type with an API. + +* Since Rhai is _sand-boxed_, it cannot mutate the environment. To perform external actions via an API, the command object type must be wrapped in a `RefCell` (or `RwLock`/`Mutex` for [`sync`]) and shared to the [`Engine`]. + +* Load each command object into a custom [`Scope`] as constant variables. + +* Control each command object in script via the constants. + + +Implementation +-------------- + +### Functional API + +Assume the following command object type: + +```rust +struct EnergizerBunny { ... } + +impl EnergizerBunny { + pub fn new () -> Self { ... } + pub fn go (&mut self) { ... } + pub fn stop (&mut self) { ... } + pub fn is_going (&self) { ... } + pub fn get_speed (&self) -> i64 { ... } + pub fn set_speed (&mut self, speed: i64) { ... } + pub fn turn (&mut self, left_turn: bool) { ... } +} +``` + +### Wrap Command Object Type as Shared + +```rust +let SharedBunnyType = Rc>; +``` + +### Register the Custom Type + +```rust +engine.register_type_with_name::("EnergizerBunny"); +``` + +### Register Methods and Getters/Setters + +```rust +engine + .register_get_set("power", + |bunny: &mut SharedBunnyType| bunny.borrow().is_going(), + |bunny: &mut SharedBunnyType, on: bool| { + if on { + if bunny.borrow().is_going() { + println!("Still going..."); + } else { + bunny.borrow_mut().go(); + } + } else { + if bunny.borrow().is_going() { + bunny.borrow_mut().stop(); + } else { + println!("Already out of battery!"); + } + } + } + ).register_get("speed", |bunny: &mut SharedBunnyType| { + if bunny.borrow().is_going() { + bunny.borrow().get_speed() + } else { + 0 + } + }).register_set_result("speed", |bunny: &mut SharedBunnyType, speed: i64| { + if speed <= 0 { + Err("Speed must be positive!".into()) + } else if speed > 100 { + Err("Bunny will be going too fast!".into()) + } else if !bunny.borrow().is_going() { + Err("Bunny is not yet going!".into()) + } else { + b.borrow_mut().set_speed(speed); + Ok(().into()) + } + }).register_fn("turn_left", |bunny: &mut SharedBunnyType| { + if bunny.borrow().is_going() { + bunny.borrow_mut().turn(true); + } + }).register_fn("turn_right", |bunny: &mut SharedBunnyType| { + if bunny.borrow().is_going() { + bunny.borrow_mut().turn(false); + } + }); +``` + +### Push Constant Command Object into Custom Scope + +```rust +let bunny: SharedBunnyType = Rc::new(RefCell::(EnergizerBunny::new())); + +let mut scope = Scope::new(); +scope.push_constant("BUNNY", bunny.clone()); + +engine.consume_with_scope(&mut scope, script)?; +``` + +### Use the Command API in Script + +```rust +// Access the command object via constant variable 'BUNNY'. + +if !BUNNY.power { BUNNY.power = true; } + +if BUNNY.speed > 50 { BUNNY.speed = 50; } + +BUNNY.turn_left(); +``` From c86a97960118651ca74fc2b5888bb006c6b34f70 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 7 Aug 2020 18:40:31 +0800 Subject: [PATCH 04/11] Refine docs. --- README.md | 2 +- doc/src/SUMMARY.md | 8 ++++---- doc/src/about/features.md | 2 +- doc/src/language/arrays.md | 10 ++++++++-- doc/src/language/object-maps.md | 31 +++++++++++++++++++++++-------- doc/src/links.md | 3 +++ doc/src/patterns/config.md | 8 +++++--- doc/src/patterns/control.md | 16 +++++++++++++++- doc/src/patterns/index.md | 4 +--- doc/src/patterns/oop.md | 29 ++++++++++++++--------------- doc/src/patterns/singleton.md | 22 ++++++++++++++++++---- doc/src/rust/functions.md | 8 ++++++-- doc/src/rust/indexers.md | 4 +++- doc/src/rust/serde.md | 2 +- doc/src/safety/sandbox.md | 30 ++++++++++++++++++++++++------ examples/serde.rs | 2 +- 16 files changed, 128 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 338a1309..bfd3fc0c 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ Standard features Protection against attacks -------------------------- -* Sand-boxed - the scripting engine, if declared immutable, cannot mutate the containing environment unless explicitly permitted (e.g. via a `RefCell`). +* Sand-boxed - the scripting engine, if declared immutable, cannot mutate the containing environment unless [explicitly permitted](https://schungx.github.io/rhai/patterns/control.html). * Rugged - protected against malicious attacks (such as [stack-overflow](https://schungx.github.io/rhai/safety/max-call-stack.html), [over-sized data](https://schungx.github.io/rhai/safety/max-string-size.html), and [runaway scripts](https://schungx.github.io/rhai/safety/max-operations.html) etc.) that may come from untrusted third-party user-land scripts. * Track script evaluation [progress](https://schungx.github.io/rhai/safety/progress.html) and manually terminate a script run. diff --git a/doc/src/SUMMARY.md b/doc/src/SUMMARY.md index a705b015..2ef05a0f 100644 --- a/doc/src/SUMMARY.md +++ b/doc/src/SUMMARY.md @@ -101,10 +101,10 @@ The Rhai Scripting Language 9. [Maximum Statement Depth](safety/max-stmt-depth.md) 7. [Advanced Topics](advanced.md) 1. [Advanced Patterns](patterns/index.md) - 1. [Loadable Configuration](patterns/config.md) - 2. [Control Layer](patterns/control.md) - 3. [Singleton Command](patterns/singleton.md) - 4. [Object-Oriented Programming (OOP)](patterns/oop.md) + 1. [Object-Oriented Programming (OOP)](patterns/oop.md) + 2. [Loadable Configuration](patterns/config.md) + 3. [Control Layer](patterns/control.md) + 4. [Singleton Command](patterns/singleton.md) 2. [Capture Scope for Function Call](language/fn-capture.md) 3. [Serialization/Deserialization of `Dynamic` with `serde`](rust/serde.md) 4. [Script Optimization](engine/optimize/index.md) diff --git a/doc/src/about/features.md b/doc/src/about/features.md index 6c2e3965..d0624c2b 100644 --- a/doc/src/about/features.md +++ b/doc/src/about/features.md @@ -46,7 +46,7 @@ Safe * Relatively little `unsafe` code (yes there are some for performance reasons). -* Sand-boxed - the scripting [`Engine`], if declared immutable, cannot mutate the containing environment unless explicitly permitted (e.g. via a `RefCell`). +* Sand-boxed - the scripting [`Engine`], if declared immutable, cannot mutate the containing environment unless [explicitly permitted]({{rootUrl}}/patterns/control.md). Rugged ------ diff --git a/doc/src/language/arrays.md b/doc/src/language/arrays.md index 3afc09b6..69f24103 100644 --- a/doc/src/language/arrays.md +++ b/doc/src/language/arrays.md @@ -3,9 +3,15 @@ Arrays {{#include ../links.md}} -Arrays are first-class citizens in Rhai. Like C, arrays are accessed with zero-based, non-negative integer indices. +Arrays are first-class citizens in Rhai. Like C, arrays are accessed with zero-based, non-negative integer indices: -Array literals are built within square brackets '`[`' ... '`]`' and separated by commas '`,`'. +> _array_ `[` _index_ `]` + +Array literals are built within square brackets '`[`' ... '`]`' and separated by commas '`,`': + +> `[` _value_ `,` _value_ `,` `...` `,` _value_ `]` +> +> `[` _value_ `,` _value_ `,` `...` `,` _value_ `,` `]` `// trailing comma is OK` All elements stored in an array are [`Dynamic`], and the array can freely grow or shrink with elements added or removed. diff --git a/doc/src/language/object-maps.md b/doc/src/language/object-maps.md index 27220134..c2c0d20e 100644 --- a/doc/src/language/object-maps.md +++ b/doc/src/language/object-maps.md @@ -19,21 +19,36 @@ Object Map Literals ------------------ Object map literals are built within braces '`#{`' ... '`}`' (_name_ `:` _value_ syntax similar to Rust) -and separated by commas '`,`'. The property _name_ can be a simple variable name following the same +and separated by commas '`,`': + +> `#{` _property_ `:` _value_ `,` `...` `,` _property_ `:` _value_ `}` +> +> `#{` _property_ `:` _value_ `,` `...` `,` _property_ `:` _value_ `,` `}` `// trailing comma is OK` + +The property _name_ can be a simple variable name following the same naming rules as [variables], or an arbitrary [string] literal. Access Properties ----------------- +----------------- -Property values can be accessed via the _dot_ notation (_object_ `.` _property_) -or _index_ notation (_object_ `[` _property_ `]`). +### Dot Notation -The dot notation allows only property names that follow the same naming rules as [variables]. +The _dot notation_ allows only property names that follow the same naming rules as [variables]. -The index notation allows setting/getting properties of arbitrary names (even the empty [string]). +> _object_ `.` _property_ -**Important:** Trying to read a non-existent property returns [`()`] instead of causing an error. +### Index Notation + +The _index notation_ allows setting/getting properties of arbitrary names (even the empty [string]). + +> _object_ `[` _property_ `]` + +### Non-Existence + +Trying to read a non-existing property returns [`()`] instead of causing an error. + +This is similar to JavaScript where accessing a non-existing property returns `undefined`. Built-in Functions @@ -89,7 +104,7 @@ let foo = #{ a:1, b:2, c:3 }["a"]; foo == 1; fn abc() { - #{ a:1, b:2, c:3 } // a function returning an object map + ##{ a:1, b:2, c:3 } // a function returning an object map } let foo = abc().b; diff --git a/doc/src/links.md b/doc/src/links.md index d2df1d5b..ee7f05e4 100644 --- a/doc/src/links.md +++ b/doc/src/links.md @@ -76,6 +76,8 @@ [function]: {{rootUrl}}/language/functions.md [functions]: {{rootUrl}}/language/functions.md +[function overloading]: {{rootUrl}}/rust/functions.md#function-overloading +[fallible functions]: {{rootUrl}}/rust/fallible.md [function pointer]: {{rootUrl}}/language/fn-ptr.md [function pointers]: {{rootUrl}}/language/fn-ptr.md [currying]: {{rootUrl}}/language/fn-curry.md @@ -100,6 +102,7 @@ [OOP]: {{rootUrl}}/patterns/oop.md [DSL]: {{rootUrl}}/engine/dsl.md +[sand-boxed]: {{rootUrl}}/safety/sandbox.md [maximum statement depth]: {{rootUrl}}/safety/max-stmt-depth.md [maximum call stack depth]: {{rootUrl}}/safety/max-call-stack.md [maximum number of operations]: {{rootUrl}}/safety/max-operations.md diff --git a/doc/src/patterns/config.md b/doc/src/patterns/config.md index 640191fa..a1fcdf44 100644 --- a/doc/src/patterns/config.md +++ b/doc/src/patterns/config.md @@ -9,11 +9,11 @@ Usage Scenario * A system where settings and configurations are complex and logic-driven. -* Where it is not possible to configure said system via standard configuration file formats such as `TOML` or `YAML`. +* Where said system is too complex to configure via standard configuration file formats such as `JSON`, `TOML` or `YAML`. -* The system configuration is complex enough that it requires a full programming language. Essentially _configuration by code_. +* The system is complex enough to require a full programming language to configure. Essentially _configuration by code_. -* Yet the configurations must be flexible, late-bound and dynamically loadable, just like a configuration file. +* Yet the configuration must be flexible, late-bound and dynamically loadable, just like a configuration file. Key Concepts @@ -23,6 +23,8 @@ Key Concepts * Expose the configuration API. Use separate scripts to configure that API. Dynamically load scripts via the `import` statement. +* Leverage [function overloading] to simplify the API design. + * Since Rhai is _sand-boxed_, it cannot mutate the environment. To modify the external configuration object via an API, it must be wrapped in a `RefCell` (or `RwLock`/`Mutex` for [`sync`]) and shared to the [`Engine`]. diff --git a/doc/src/patterns/control.md b/doc/src/patterns/control.md index 2f5aa961..af6021e6 100644 --- a/doc/src/patterns/control.md +++ b/doc/src/patterns/control.md @@ -19,12 +19,26 @@ Key Concepts * Expose a Control API. -* Since Rhai is _sand-boxed_, it cannot mutate the environment. To perform external actions via an API, the actual system must be wrapped in a `RefCell` (or `RwLock`/`Mutex` for [`sync`]) and shared to the [`Engine`]. +* Leverage [function overloading] to simplify the API design. + +* Since Rhai is _[sand-boxed]_, it cannot mutate the environment. To perform external actions via an API, the actual system must be wrapped in a `RefCell` (or `RwLock`/`Mutex` for [`sync`]) and shared to the [`Engine`]. Implementation -------------- +There are two broad ways for Rhai to control an external system, both of which involve +wrapping the system in a shared, interior-mutated object. + +This is one way which does not involve exposing the data structures of the external system, +but only through exposing an abstract API primarily made up of functions. + +Use this when the API is relatively simple and clean, and the number of functions is small enough. + +For a complex API involving lots of functions, or an API that is object-based, +use the [Singleton Command Object]({{rootUrl}}/patterns/singleton.md) pattern instead. + + ### Functional API Assume that a system provides the following functional API: diff --git a/doc/src/patterns/index.md b/doc/src/patterns/index.md index 522d3403..b0e5d633 100644 --- a/doc/src/patterns/index.md +++ b/doc/src/patterns/index.md @@ -4,6 +4,4 @@ Advanced Patterns {{#include ../links.md}} -Use Rhai in different scenarios other than simply evaluating a user script. - -These patterns are useful when Rhai needs to affect/control the external environment. +Leverage the full power and flexibility of Rhai in advanced scenarios. diff --git a/doc/src/patterns/oop.md b/doc/src/patterns/oop.md index 017c4932..823df31e 100644 --- a/doc/src/patterns/oop.md +++ b/doc/src/patterns/oop.md @@ -18,17 +18,17 @@ Rhai's [object maps] has [special support for OOP]({{rootUrl}}/language/object-m | [Object map] properties that hold [function pointers] | methods | When a property of an [object map] is called like a method function, and if it happens to hold -a valid [function pointer] (perhaps defined via an [anonymous function]), then the call will be -dispatched to the actual function with `this` binding to the [object map] itself. +a valid [function pointer] (perhaps defined via an [anonymous function] or more commonly as a [closure]), +then the call will be dispatched to the actual function with `this` binding to the [object map] itself. Use Anonymous Functions to Define Methods ---------------------------------------- -[Anonymous functions] defined as values for [object map] properties take on a syntactic shape -that resembles very closely that of class methods in an OOP language. +[Anonymous functions] or [closures] defined as values for [object map] properties take on +a syntactic shape that resembles very closely that of class methods in an OOP language. -Anonymous functions can also _capture_ variables from the defining environment, which is a very +Closures also _[capture][automatic currying]_ variables from the defining environment, which is a very common OOP pattern. Capturing is accomplished via a feature called _[automatic currying]_ and can be turned off via the [`no_closure`] feature. @@ -40,23 +40,22 @@ Examples let factor = 1; // Define the object -let obj = - #{ - data: 0, - increment: |x| this.data += x, // 'this' binds to 'obj' - update: |x| this.data = x * factor, // 'this' binds to 'obj', 'factor' is captured - action: || print(this.data) // 'this' binds to 'obj' - }; +let obj = #{ + data: 0, // object field + increment: |x| this.data += x, // 'this' binds to 'obj' + update: |x| this.data = x * factor, // 'this' binds to 'obj', 'factor' is captured + action: || print(this.data) // 'this' binds to 'obj' + }; // Use the object obj.increment(1); -obj.action(); // prints 1 +obj.action(); // prints 1 obj.update(42); -obj.action(); // prints 42 +obj.action(); // prints 42 factor = 2; obj.update(42); -obj.action(); // prints 84 +obj.action(); // prints 84 ``` diff --git a/doc/src/patterns/singleton.md b/doc/src/patterns/singleton.md index 7172e745..c38df1d5 100644 --- a/doc/src/patterns/singleton.md +++ b/doc/src/patterns/singleton.md @@ -1,5 +1,5 @@ -Singleton Command Objects -======================== +Singleton Command Object +======================= {{#include ../links.md}} @@ -21,9 +21,11 @@ Usage Scenario Key Concepts ------------ -* Expose a Command type with an API. +* Expose a Command type with an API. The [`no_object`] feature must not be on. -* Since Rhai is _sand-boxed_, it cannot mutate the environment. To perform external actions via an API, the command object type must be wrapped in a `RefCell` (or `RwLock`/`Mutex` for [`sync`]) and shared to the [`Engine`]. +* Leverage [function overloading] to simplify the API design. + +* Since Rhai is _[sand-boxed]_, it cannot mutate the environment. To perform external actions via an API, the command object type must be wrapped in a `RefCell` (or `RwLock`/`Mutex` for [`sync`]) and shared to the [`Engine`]. * Load each command object into a custom [`Scope`] as constant variables. @@ -33,6 +35,18 @@ Key Concepts Implementation -------------- +There are two broad ways for Rhai to control an external system, both of which involve +wrapping the system in a shared, interior-mutated object. + +This is the other way which involves directly exposing the data structures of the external system +as a name singleton object in the scripting space. + +Use this when the API is complex and clearly object-based. + +For a relatively simple API that is action-based and not object-based, +use the [Control Layer]({{rootUrl}}/patterns/control.md) pattern instead. + + ### Functional API Assume the following command object type: diff --git a/doc/src/rust/functions.md b/doc/src/rust/functions.md index 8d8666cf..99e9a485 100644 --- a/doc/src/rust/functions.md +++ b/doc/src/rust/functions.md @@ -7,7 +7,7 @@ Rhai's scripting engine is very lightweight. It gets most of its abilities from To call these functions, they need to be _registered_ with the [`Engine`] using `Engine::register_fn` (in the `RegisterFn` trait) and `Engine::register_result_fn` (in the `RegisterResultFn` trait, -see [fallible functions]({{rootUrl}}/rust/fallible.md)). +see [fallible functions]). ```rust use rhai::{Dynamic, Engine, EvalAltResult, ImmutableString}; @@ -62,8 +62,12 @@ let x = (42_i64).into(); // 'into()' works for standard t let y = Dynamic::from("hello!".to_string()); // remember &str is not supported by Rhai ``` + +Function Overloading +-------------------- + Functions registered with the [`Engine`] can be _overloaded_ as long as the _signature_ is unique, i.e. different functions can have the same name as long as their parameters are of different types -and/or different number. +or different number. New definitions _overwrite_ previous definitions of the same name and same number/types of parameters. diff --git a/doc/src/rust/indexers.md b/doc/src/rust/indexers.md index d5905b44..94a6b5f1 100644 --- a/doc/src/rust/indexers.md +++ b/doc/src/rust/indexers.md @@ -5,7 +5,9 @@ Custom Type Indexers A custom type can also expose an _indexer_ by registering an indexer function. -A custom type with an indexer function defined can use the bracket '`[]`' notation to get a property value. +A custom type with an indexer function defined can use the bracket notation to get a property value: + +> _object_ `[` _index_ `]` Like getters and setters, indexers take a `&mut` reference to the first parameter. diff --git a/doc/src/rust/serde.md b/doc/src/rust/serde.md index de59dc34..c28eccfd 100644 --- a/doc/src/rust/serde.md +++ b/doc/src/rust/serde.md @@ -91,7 +91,7 @@ struct MyStruct { let engine = Engine::new(); let result: Dynamic = engine.eval(r#" - #{ + ##{ a: 42, b: [ "hello", "world" ], c: true, diff --git a/doc/src/safety/sandbox.md b/doc/src/safety/sandbox.md index 61d8469d..a85a5f9e 100644 --- a/doc/src/safety/sandbox.md +++ b/doc/src/safety/sandbox.md @@ -5,13 +5,31 @@ Sand-Boxing - Block Access to External Data Rhai is _sand-boxed_ so a script can never read from outside its own environment. -Furthermore, an [`Engine`] created non-`mut` cannot mutate any state outside of itself; -so it is highly recommended that [`Engine`]'s are created immutable as much as possible. +Furthermore, an [`Engine`] created non-`mut` cannot mutate any state, including itself +(and therefore it is also _re-entrant_). + +It is highly recommended that [`Engine`]'s be created immutable as much as possible. ```rust -let mut engine = Engine::new(); // create mutable 'Engine' +// Use the fluent API to configure an 'Engine' and then keep an immutable instance. +let engine = Engine::new() + .register_get("field", get_field) + .register_set("field", set_field) + .register_fn("do_work", action); -engine.register_get("add", add); // configure 'engine' - -let engine = engine; // shadow the variable so that 'engine' is now immutable +// 'engine' is immutable... ``` + + +Using Rhai to Control External Environment +----------------------------------------- + +How does a _sand-boxed_, immutable [`Engine`] control the external environment? +This is necessary in order to use Rhai as a _dynamic control layer_ over a Rust core system. + +There are two general patterns, both involving wrapping the external system +in a shared, interior-mutated object (e.g. `Rc>`): + +* [Control Layer]({{rootUrl}}/patterns/control.md) pattern. + +* [Singleton Command Object]({{rootUrl}}/patterns/singleton.md) pattern. diff --git a/examples/serde.rs b/examples/serde.rs index 3cb68459..52c39b51 100644 --- a/examples/serde.rs +++ b/examples/serde.rs @@ -56,7 +56,7 @@ mod example { let result: Dynamic = engine .eval( r#" - #{ + ##{ a: 42, b: [ "hello", "world" ], c: true, From 7b258ac4107fbf9a2c4543e462333a429ed15038 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 8 Aug 2020 11:46:30 +0800 Subject: [PATCH 05/11] Add more inlining. --- src/any.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/any.rs b/src/any.rs index 318b97d1..7ea7436c 100644 --- a/src/any.rs +++ b/src/any.rs @@ -134,6 +134,7 @@ impl Variant for T { impl dyn Variant { /// Is this `Variant` a specific type? + #[inline(always)] pub fn is(&self) -> bool { TypeId::of::() == self.type_id() } @@ -250,6 +251,7 @@ impl<'d, T: Variant + Clone> DerefMut for DynamicWriteLock<'d, T> { impl Dynamic { /// Does this `Dynamic` hold a variant data type /// instead of one of the support system primitive types? + #[inline(always)] pub fn is_variant(&self) -> bool { match self.0 { Union::Variant(_) => true, @@ -259,6 +261,7 @@ impl Dynamic { /// Does this `Dynamic` hold a shared data type /// instead of one of the supported system primitive types? + #[inline(always)] pub fn is_shared(&self) -> bool { match self.0 { #[cfg(not(feature = "no_closure"))] @@ -271,6 +274,7 @@ impl Dynamic { /// /// If the `Dynamic` is a Shared variant checking is performed on /// top of it's internal value. + #[inline(always)] pub fn is(&self) -> bool { let mut target_type_id = TypeId::of::(); @@ -301,7 +305,9 @@ impl Dynamic { #[cfg(not(feature = "no_object"))] Union::Map(_) => TypeId::of::(), Union::FnPtr(_) => TypeId::of::(), + Union::Variant(value) => (***value).type_id(), + #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] Union::Shared(cell) => (*cell.borrow()).type_id(), @@ -335,6 +341,7 @@ impl Dynamic { #[cfg(not(feature = "no_std"))] Union::Variant(value) if value.is::() => "timestamp", Union::Variant(value) => (***value).type_name(), + #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] Union::Shared(cell) => cell @@ -468,7 +475,9 @@ impl Clone for Dynamic { #[cfg(not(feature = "no_object"))] Union::Map(ref value) => Self(Union::Map(value.clone())), Union::FnPtr(ref value) => Self(Union::FnPtr(value.clone())), + Union::Variant(ref value) => (***value).clone_into_dynamic(), + #[cfg(not(feature = "no_closure"))] Union::Shared(ref cell) => Self(Union::Shared(cell.clone())), } @@ -476,6 +485,7 @@ impl Clone for Dynamic { } impl Default for Dynamic { + #[inline(always)] fn default() -> Self { Self(Union::Unit(())) } @@ -1043,6 +1053,7 @@ impl Dynamic { /// Cast the `Dynamic` as the system integer type `INT` and return it. /// Returns the name of the actual type if the cast fails. + #[inline(always)] pub fn as_int(&self) -> Result { match self.0 { Union::Int(n) => Ok(n), @@ -1055,6 +1066,7 @@ impl Dynamic { /// Cast the `Dynamic` as the system floating-point type `FLOAT` and return it. /// Returns the name of the actual type if the cast fails. #[cfg(not(feature = "no_float"))] + #[inline(always)] pub fn as_float(&self) -> Result { match self.0 { Union::Float(n) => Ok(n), @@ -1066,6 +1078,7 @@ impl Dynamic { /// Cast the `Dynamic` as a `bool` and return it. /// Returns the name of the actual type if the cast fails. + #[inline(always)] pub fn as_bool(&self) -> Result { match self.0 { Union::Bool(b) => Ok(b), @@ -1077,6 +1090,7 @@ impl Dynamic { /// Cast the `Dynamic` as a `char` and return it. /// Returns the name of the actual type if the cast fails. + #[inline(always)] pub fn as_char(&self) -> Result { match self.0 { Union::Char(n) => Ok(n), @@ -1090,6 +1104,7 @@ impl Dynamic { /// Returns the name of the actual type if the cast fails. /// /// Cast is failing if `self` is Shared Dynamic + #[inline(always)] pub fn as_str(&self) -> Result<&str, &'static str> { match &self.0 { Union::Str(s) => Ok(s), @@ -1100,6 +1115,7 @@ impl Dynamic { /// Convert the `Dynamic` into `String` and return it. /// Returns the name of the actual type if the cast fails. + #[inline(always)] pub fn take_string(self) -> Result { self.take_immutable_string() .map(ImmutableString::into_owned) @@ -1138,38 +1154,45 @@ impl Dynamic { } impl From<()> for Dynamic { + #[inline(always)] fn from(value: ()) -> Self { Self(Union::Unit(value)) } } impl From for Dynamic { + #[inline(always)] fn from(value: bool) -> Self { Self(Union::Bool(value)) } } impl From for Dynamic { + #[inline(always)] fn from(value: INT) -> Self { Self(Union::Int(value)) } } #[cfg(not(feature = "no_float"))] impl From for Dynamic { + #[inline(always)] fn from(value: FLOAT) -> Self { Self(Union::Float(value)) } } impl From for Dynamic { + #[inline(always)] fn from(value: char) -> Self { Self(Union::Char(value)) } } impl> From for Dynamic { + #[inline(always)] fn from(value: S) -> Self { Self(Union::Str(value.into())) } } #[cfg(not(feature = "no_index"))] impl From> for Dynamic { + #[inline(always)] fn from(value: Vec) -> Self { Self(Union::Array(Box::new( value.into_iter().map(Dynamic::from).collect(), @@ -1178,6 +1201,7 @@ impl From> for Dynamic { } #[cfg(not(feature = "no_index"))] impl From<&[T]> for Dynamic { + #[inline(always)] fn from(value: &[T]) -> Self { Self(Union::Array(Box::new( value.iter().cloned().map(Dynamic::from).collect(), @@ -1186,6 +1210,7 @@ impl From<&[T]> for Dynamic { } #[cfg(not(feature = "no_object"))] impl, T: Variant + Clone> From> for Dynamic { + #[inline(always)] fn from(value: HashMap) -> Self { Self(Union::Map(Box::new( value @@ -1196,11 +1221,13 @@ impl, T: Variant + Clone> From> for Dynam } } impl From for Dynamic { + #[inline(always)] fn from(value: FnPtr) -> Self { Self(Union::FnPtr(Box::new(value))) } } impl From> for Dynamic { + #[inline(always)] fn from(value: Box) -> Self { Self(Union::FnPtr(value)) } From f016655414235dab84f5dc8a3119a47b9cb474a9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 8 Aug 2020 16:03:14 +0800 Subject: [PATCH 06/11] Simply test error matching. --- tests/call_fn.rs | 14 ++++++------ tests/data_size.rs | 53 +++++++++++++++++++++++++++++----------------- tests/functions.rs | 14 +++++++----- tests/looping.rs | 24 +++++++++++++-------- tests/stack.rs | 27 +++++++++++++---------- 5 files changed, 80 insertions(+), 52 deletions(-) diff --git a/tests/call_fn.rs b/tests/call_fn.rs index a22f7b6d..a7a5d514 100644 --- a/tests/call_fn.rs +++ b/tests/call_fn.rs @@ -1,7 +1,6 @@ #![cfg(not(feature = "no_function"))] use rhai::{ - Dynamic, Engine, EvalAltResult, FnPtr, Func, Module, ParseError, ParseErrorType, RegisterFn, - Scope, INT, + Dynamic, Engine, EvalAltResult, FnPtr, Func, Module, ParseErrorType, RegisterFn, Scope, INT, }; use std::any::TypeId; @@ -10,12 +9,13 @@ fn test_fn() -> Result<(), Box> { let engine = Engine::new(); // Expect duplicated parameters error - assert!(matches!( - engine + assert_eq!( + *engine .compile("fn hello(x, x) { x }") - .expect_err("should be error"), - ParseError(x, _) if *x == ParseErrorType::FnDuplicatedParam("hello".to_string(), "x".to_string()) - )); + .expect_err("should be error") + .0, + ParseErrorType::FnDuplicatedParam("hello".to_string(), "x".to_string()) + ); Ok(()) } diff --git a/tests/data_size.rs b/tests/data_size.rs index 33257cfd..ca91fb5c 100644 --- a/tests/data_size.rs +++ b/tests/data_size.rs @@ -1,5 +1,5 @@ #![cfg(not(feature = "unchecked"))] -use rhai::{Engine, EvalAltResult, ParseError, ParseErrorType}; +use rhai::{Engine, EvalAltResult, ParseErrorType}; #[cfg(not(feature = "no_index"))] use rhai::Array; @@ -12,15 +12,21 @@ fn test_max_string_size() -> Result<(), Box> { let mut engine = Engine::new(); engine.set_max_string_size(10); - assert!(matches!( - engine.compile(r#"let x = "hello, world!";"#).expect_err("should error"), - ParseError(x, _) if *x == ParseErrorType::LiteralTooLarge("Length of string literal".to_string(), 10) - )); + assert_eq!( + *engine + .compile(r#"let x = "hello, world!";"#) + .expect_err("should error") + .0, + ParseErrorType::LiteralTooLarge("Length of string literal".to_string(), 10) + ); - assert!(matches!( - engine.compile(r#"let x = "朝に紅顔、暮に白骨";"#).expect_err("should error"), - ParseError(x, _) if *x == ParseErrorType::LiteralTooLarge("Length of string literal".to_string(), 10) - )); + assert_eq!( + *engine + .compile(r#"let x = "朝に紅顔、暮に白骨";"#) + .expect_err("should error") + .0, + ParseErrorType::LiteralTooLarge("Length of string literal".to_string(), 10) + ); assert!(matches!( *engine @@ -74,12 +80,13 @@ fn test_max_array_size() -> Result<(), Box> { #[cfg(not(feature = "no_object"))] engine.set_max_map_size(10); - assert!(matches!( - engine + assert_eq!( + *engine .compile("let x = [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15];") - .expect_err("should error"), - ParseError(x, _) if *x == ParseErrorType::LiteralTooLarge("Size of array literal".to_string(), 10) - )); + .expect_err("should error") + .0, + ParseErrorType::LiteralTooLarge("Size of array literal".to_string(), 10) + ); assert!(matches!( *engine @@ -186,12 +193,18 @@ fn test_max_map_size() -> Result<(), Box> { #[cfg(not(feature = "no_index"))] engine.set_max_array_size(10); - assert!(matches!( - engine - .compile("let x = #{a:1,b:2,c:3,d:4,e:5,f:6,g:7,h:8,i:9,j:10,k:11,l:12,m:13,n:14,o:15};") - .expect_err("should error"), - ParseError(x, _) if *x == ParseErrorType::LiteralTooLarge("Number of properties in object map literal".to_string(), 10) - )); + assert_eq!( + *engine + .compile( + "let x = #{a:1,b:2,c:3,d:4,e:5,f:6,g:7,h:8,i:9,j:10,k:11,l:12,m:13,n:14,o:15};" + ) + .expect_err("should error") + .0, + ParseErrorType::LiteralTooLarge( + "Number of properties in object map literal".to_string(), + 10 + ) + ); assert!(matches!( *engine diff --git a/tests/functions.rs b/tests/functions.rs index 58ef8d99..296f0ddd 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -1,5 +1,6 @@ #![cfg(not(feature = "no_function"))] -use rhai::{Engine, EvalAltResult, ParseError, ParseErrorType, INT}; +use rhai::{Engine, EvalAltResult, ParseErrorType, INT}; +use std::io::Read; #[test] fn test_functions() -> Result<(), Box> { @@ -155,8 +156,9 @@ fn test_function_captures() -> Result<(), Box> { #[cfg(not(feature = "no_object"))] assert!(matches!( - engine.compile( - r#" + *engine + .compile( + r#" fn foo() { this += x; } let x = 41; @@ -164,8 +166,10 @@ fn test_function_captures() -> Result<(), Box> { y.foo!(); "# - ).expect_err("should error"), - ParseError(err, _) if matches!(*err, ParseErrorType::MalformedCapture(_)) + ) + .expect_err("should error") + .0, + ParseErrorType::MalformedCapture(_) )); Ok(()) diff --git a/tests/looping.rs b/tests/looping.rs index 951f8cd7..23f87bb0 100644 --- a/tests/looping.rs +++ b/tests/looping.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, ParseError, ParseErrorType, INT}; +use rhai::{Engine, EvalAltResult, ParseErrorType, INT}; #[test] fn test_loop() -> Result<(), Box> { @@ -26,15 +26,21 @@ fn test_loop() -> Result<(), Box> { 21 ); - assert!(matches!( - engine.compile("let x = 0; break;").expect_err("should error"), - ParseError(x, _) if *x == ParseErrorType::LoopBreak - )); + assert_eq!( + *engine + .compile("let x = 0; break;") + .expect_err("should error") + .0, + ParseErrorType::LoopBreak + ); - assert!(matches!( - engine.compile("let x = 0; if x > 0 { continue; }").expect_err("should error"), - ParseError(x, _) if *x == ParseErrorType::LoopBreak - )); + assert_eq!( + *engine + .compile("let x = 0; if x > 0 { continue; }") + .expect_err("should error") + .0, + ParseErrorType::LoopBreak + ); Ok(()) } diff --git a/tests/stack.rs b/tests/stack.rs index 47202b47..b7ff9a57 100644 --- a/tests/stack.rs +++ b/tests/stack.rs @@ -1,5 +1,5 @@ #![cfg(not(feature = "unchecked"))] -use rhai::{Engine, EvalAltResult, ParseError, ParseErrorType, INT}; +use rhai::{Engine, EvalAltResult, ParseErrorType, INT}; #[test] #[cfg(not(feature = "no_function"))] @@ -33,12 +33,12 @@ fn test_stack_overflow_fn_calls() -> Result<(), Box> { fn test_stack_overflow_parsing() -> Result<(), Box> { let mut engine = Engine::new(); - assert!(matches!( - engine.compile(r" + assert_eq!( + *engine.compile(r" let a = (1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+1)))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))) - ").expect_err("should error"), - ParseError(x, _) if *x == ParseErrorType::ExprTooDeep - )); + ").expect_err("should error").0, + ParseErrorType::ExprTooDeep + ); engine.set_max_expr_depths(100, 6); @@ -58,8 +58,10 @@ fn test_stack_overflow_parsing() -> Result<(), Box> { ", )?; - assert!(matches!( - engine.compile(r" + assert_eq!( + *engine + .compile( + r" 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 0 + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 0 + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 0 + @@ -70,9 +72,12 @@ fn test_stack_overflow_parsing() -> Result<(), Box> { 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 0 + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 0 + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 - ").expect_err("should error"), - ParseError(x, _) if *x == ParseErrorType::ExprTooDeep - )); + " + ) + .expect_err("should error") + .0, + ParseErrorType::ExprTooDeep + ); #[cfg(not(feature = "no_function"))] engine.compile("fn abc(x) { x + 1 }")?; From 5a1a141ce367d4f78f9cb0b8fe9d1d02fe84f87e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 8 Aug 2020 16:24:10 +0800 Subject: [PATCH 07/11] Improve shared value treatment. --- .gitignore | 4 +-- src/any.rs | 53 +++++++++++++++++++++++++++------- src/engine.rs | 75 +++++++++++++++++++++++++++--------------------- src/fn_call.rs | 4 +-- src/fn_native.rs | 29 ++++++------------- src/scope.rs | 4 +-- 6 files changed, 100 insertions(+), 69 deletions(-) diff --git a/.gitignore b/.gitignore index e884ce0f..bdd52cb9 100644 --- a/.gitignore +++ b/.gitignore @@ -3,5 +3,5 @@ Cargo.lock .vscode/ .cargo/ doc/book/ -before -after +before* +after* diff --git a/src/any.rs b/src/any.rs index 7ea7436c..5d47465d 100644 --- a/src/any.rs +++ b/src/any.rs @@ -5,7 +5,7 @@ use crate::parser::{ImmutableString, INT}; use crate::r#unsafe::{unsafe_cast_box, unsafe_try_cast}; #[cfg(not(feature = "no_closure"))] -use crate::fn_native::SharedMut; +use crate::fn_native::{shared_try_take, Shared}; #[cfg(not(feature = "no_float"))] use crate::parser::FLOAT; @@ -159,9 +159,15 @@ pub enum Union { #[cfg(not(feature = "no_object"))] Map(Box), FnPtr(Box), + Variant(Box>), + #[cfg(not(feature = "no_closure"))] - Shared(SharedMut), + #[cfg(not(feature = "sync"))] + Shared(Shared>), + #[cfg(not(feature = "no_closure"))] + #[cfg(feature = "sync")] + Shared(Shared>), } /// Underlying `Variant` read guard for `Dynamic`. @@ -176,6 +182,7 @@ pub struct DynamicReadLock<'d, T: Variant + Clone>(DynamicReadLockInner<'d, T>); enum DynamicReadLockInner<'d, T: Variant + Clone> { /// A simple reference to a non-shared value. Reference(&'d T), + /// A read guard to a shared `RefCell`. #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] @@ -212,6 +219,7 @@ pub struct DynamicWriteLock<'d, T: Variant + Clone>(DynamicWriteLockInner<'d, T> enum DynamicWriteLockInner<'d, T: Variant + Clone> { /// A simple mutable reference to a non-shared value. Reference(&'d mut T), + /// A write guard to a shared `RefCell`. #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] @@ -406,6 +414,7 @@ impl fmt::Display for Dynamic { #[cfg(not(feature = "no_std"))] Union::Variant(value) if value.is::() => f.write_str(""), Union::Variant(value) => f.write_str((*value).type_name()), + #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] Union::Shared(cell) => { @@ -444,6 +453,7 @@ impl fmt::Debug for Dynamic { #[cfg(not(feature = "no_std"))] Union::Variant(value) if value.is::() => write!(f, ""), Union::Variant(value) => write!(f, "{}", (*value).type_name()), + #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "sync"))] Union::Shared(cell) => { @@ -779,25 +789,46 @@ impl Dynamic { self.try_cast::().unwrap() } - /// Get a copy of the `Dynamic` as a specific type. + /// Dereference the `Dynamic`. /// - /// If the `Dynamic` is not a shared value, it returns a cloned copy of the value. + /// If the `Dynamic` is not a shared value, it returns a cloned copy. /// - /// If the `Dynamic` is a shared value, it returns a cloned copy of the shared value. - /// - /// Returns `None` if the cast fails. + /// If the `Dynamic` is a shared value, it a cloned copy of the shared value. #[inline(always)] - pub fn clone_inner_data(self) -> Option { + pub fn get_inner_clone(&self) -> Self { + match &self.0 { + #[cfg(not(feature = "no_closure"))] + Union::Shared(cell) => { + #[cfg(not(feature = "sync"))] + return cell.borrow().clone(); + + #[cfg(feature = "sync")] + return cell.read().unwrap().clone(); + } + _ => self.clone(), + } + } + + /// Flatten the `Dynamic`. + /// + /// If the `Dynamic` is not a shared value, it returns itself. + /// + /// If the `Dynamic` is a shared value, it returns the shared value if there are + /// no outstanding references, or a cloned copy. + #[inline(always)] + pub fn flatten(self) -> Self { match self.0 { #[cfg(not(feature = "no_closure"))] Union::Shared(cell) => { #[cfg(not(feature = "sync"))] - return Some(cell.borrow().downcast_ref::().unwrap().clone()); + return shared_try_take(cell) + .map_or_else(|c| c.borrow().clone(), RefCell::into_inner); #[cfg(feature = "sync")] - return Some(cell.read().unwrap().downcast_ref::().unwrap().clone()); + return shared_try_take(cell) + .map_or_else(|c| c.read().unwrap().clone(), RwLock::into_inner); } - _ => self.try_cast(), + _ => self, } } diff --git a/src/engine.rs b/src/engine.rs index 9a771ac6..5c4fdaee 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -147,6 +147,7 @@ pub enum Target<'a> { #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] impl Target<'_> { /// Is the `Target` a reference pointing to other data? + #[inline(always)] pub fn is_ref(&self) -> bool { match self { Self::Ref(_) => true, @@ -159,6 +160,7 @@ impl Target<'_> { } } /// Is the `Target` an owned value? + #[inline(always)] pub fn is_value(&self) -> bool { match self { Self::Ref(_) => false, @@ -171,6 +173,7 @@ impl Target<'_> { } } /// Is the `Target` a shared value? + #[inline(always)] pub fn is_shared(&self) -> bool { match self { Self::Ref(r) => r.is_shared(), @@ -184,6 +187,7 @@ impl Target<'_> { } /// Is the `Target` a specific type? #[allow(dead_code)] + #[inline(always)] pub fn is(&self) -> bool { match self { Target::Ref(r) => r.is::(), @@ -196,6 +200,7 @@ impl Target<'_> { } } /// Get the value of the `Target` as a `Dynamic`, cloning a referenced value if necessary. + #[inline(always)] pub fn clone_into_dynamic(self) -> Dynamic { match self { Self::Ref(r) => r.clone(), // Referenced value is cloned @@ -208,6 +213,7 @@ impl Target<'_> { } } /// Get a mutable reference from the `Target`. + #[inline(always)] pub fn as_mut(&mut self) -> &mut Dynamic { match self { Self::Ref(r) => *r, @@ -258,6 +264,7 @@ impl Target<'_> { #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] impl<'a> From<&'a mut Dynamic> for Target<'a> { + #[inline(always)] fn from(value: &'a mut Dynamic) -> Self { #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "no_object"))] @@ -273,6 +280,7 @@ impl<'a> From<&'a mut Dynamic> for Target<'a> { #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] impl> From for Target<'_> { + #[inline(always)] fn from(value: T) -> Self { Self::Value(value.into()) } @@ -301,6 +309,7 @@ pub struct State { impl State { /// Create a new `State`. + #[inline(always)] pub fn new() -> Self { Default::default() } @@ -407,6 +416,7 @@ pub struct Engine { } impl fmt::Debug for Engine { + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.id.as_ref() { Some(id) => write!(f, "Engine({})", id), @@ -689,7 +699,7 @@ impl Engine { idx_values: &mut StaticVec, chain_type: ChainType, level: usize, - mut _new_val: Option, + new_val: Option, ) -> Result<(Dynamic, bool), Box> { if chain_type == ChainType::None { panic!(); @@ -722,12 +732,12 @@ impl Engine { self.eval_dot_index_chain_helper( state, lib, this_ptr, obj_ptr, expr, idx_values, next_chain, level, - _new_val, + new_val, ) .map_err(|err| err.new_position(*pos)) } // xxx[rhs] = new_val - _ if _new_val.is_some() => { + _ if new_val.is_some() => { let mut idx_val2 = idx_val.clone(); // `call_setter` is introduced to bypass double mutable borrowing of target @@ -737,7 +747,7 @@ impl Engine { // Indexed value is a reference - update directly Ok(ref mut obj_ptr) => { obj_ptr - .set_value(_new_val.unwrap()) + .set_value(new_val.unwrap()) .map_err(|err| err.new_position(rhs.position()))?; None @@ -747,7 +757,7 @@ impl Engine { #[cfg(not(feature = "no_index"))] EvalAltResult::ErrorIndexingType(_, _) => { // Raise error if there is no index getter nor setter - Some(_new_val.unwrap()) + Some(new_val.unwrap()) } // Any other error - return err => return Err(Box::new(err)), @@ -799,13 +809,13 @@ impl Engine { // xxx.module::fn_name(...) - syntax error Expr::FnCall(_) => unreachable!(), // {xxx:map}.id = ??? - Expr::Property(x) if target.is::() && _new_val.is_some() => { + Expr::Property(x) if target.is::() && new_val.is_some() => { let ((prop, _, _), pos) = x.as_ref(); let index = prop.clone().into(); let mut val = self .get_indexed_mut(state, lib, target, index, *pos, true, false, level)?; - val.set_value(_new_val.unwrap()) + val.set_value(new_val.unwrap()) .map_err(|err| err.new_position(rhs.position()))?; Ok((Default::default(), true)) } @@ -820,9 +830,10 @@ impl Engine { Ok((val.clone_into_dynamic(), false)) } // xxx.id = ??? - Expr::Property(x) if _new_val.is_some() => { + Expr::Property(x) if new_val.is_some() => { let ((_, _, setter), pos) = x.as_ref(); - let mut args = [target.as_mut(), _new_val.as_mut().unwrap()]; + let mut new_val = new_val; + let mut args = [target.as_mut(), new_val.as_mut().unwrap()]; self.exec_fn_call( state, lib, setter, 0, &mut args, is_ref, true, false, None, None, level, @@ -872,7 +883,7 @@ impl Engine { self.eval_dot_index_chain_helper( state, lib, this_ptr, &mut val, expr, idx_values, next_chain, level, - _new_val, + new_val, ) .map_err(|err| err.new_position(*pos)) } @@ -905,7 +916,7 @@ impl Engine { idx_values, next_chain, level, - _new_val, + new_val, ) .map_err(|err| err.new_position(*pos))?; @@ -944,7 +955,7 @@ impl Engine { self.eval_dot_index_chain_helper( state, lib, this_ptr, target, expr, idx_values, next_chain, - level, _new_val, + level, new_val, ) .map_err(|err| err.new_position(*pos)) } @@ -1114,7 +1125,7 @@ impl Engine { state: &mut State, _lib: &Module, target: &'a mut Target, - mut _idx: Dynamic, + idx: Dynamic, idx_pos: Position, _create: bool, _indexers: bool, @@ -1132,7 +1143,7 @@ impl Engine { #[cfg(not(feature = "no_index"))] Dynamic(Union::Array(arr)) => { // val_array[idx] - let index = _idx + let index = idx .as_int() .map_err(|_| EvalAltResult::ErrorNumericIndexExpr(idx_pos))?; @@ -1153,13 +1164,13 @@ impl Engine { Dynamic(Union::Map(map)) => { // val_map[idx] Ok(if _create { - let index = _idx + let index = idx .take_immutable_string() .map_err(|_| EvalAltResult::ErrorStringIndexExpr(idx_pos))?; map.entry(index).or_insert(Default::default()).into() } else { - let index = _idx + let index = idx .read_lock::() .ok_or_else(|| EvalAltResult::ErrorStringIndexExpr(idx_pos))?; @@ -1173,7 +1184,7 @@ impl Engine { Dynamic(Union::Str(s)) => { // val_string[idx] let chars_len = s.chars().count(); - let index = _idx + let index = idx .as_int() .map_err(|_| EvalAltResult::ErrorNumericIndexExpr(idx_pos))?; @@ -1192,7 +1203,8 @@ impl Engine { #[cfg(not(feature = "no_index"))] _ if _indexers => { let type_name = val.type_name(); - let args = &mut [val, &mut _idx]; + let mut idx = idx; + let args = &mut [val, &mut idx]; self.exec_fn_call( state, _lib, FN_IDX_GET, 0, args, is_ref, true, false, None, None, _level, ) @@ -1331,11 +1343,11 @@ impl Engine { )), // Normal assignment ScopeEntryType::Normal if op.is_empty() => { - let rhs_val = rhs_val.clone_inner_data().unwrap(); + let value = rhs_val.flatten(); if cfg!(not(feature = "no_closure")) && lhs_ptr.is_shared() { - *lhs_ptr.write_lock::().unwrap() = rhs_val; + *lhs_ptr.write_lock::().unwrap() = value; } else { - *lhs_ptr = rhs_val; + *lhs_ptr = value; } Ok(Default::default()) } @@ -1378,7 +1390,7 @@ impl Engine { ) .map_err(|err| err.new_position(*op_pos))?; - let value = value.clone_inner_data().unwrap(); + let value = value.flatten(); if cfg!(not(feature = "no_closure")) && lhs_ptr.is_shared() { *lhs_ptr.write_lock::().unwrap() = value; } else { @@ -1674,14 +1686,14 @@ impl Engine { let index = scope.len() - 1; state.scope_level += 1; - for loop_var in func(iter_type) { - let for_var = scope.get_mut(index).0; - let value = loop_var.clone_inner_data().unwrap(); + for iter_value in func(iter_type) { + let (loop_var, _) = scope.get_mut(index); - if cfg!(not(feature = "no_closure")) && for_var.is_shared() { - *for_var.write_lock().unwrap() = value; + let value = iter_value.flatten(); + if cfg!(not(feature = "no_closure")) && loop_var.is_shared() { + *loop_var.write_lock().unwrap() = value; } else { - *for_var = value; + *loop_var = value; } self.inc_operations(state) @@ -1750,8 +1762,7 @@ impl Engine { let expr = expr.as_ref().unwrap(); let val = self .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? - .clone_inner_data() - .unwrap(); + .flatten(); let var_name = unsafe_cast_var_name_to_lifetime(var_name, &state); scope.push_dynamic_value(var_name, ScopeEntryType::Normal, val, false); Ok(Default::default()) @@ -1769,8 +1780,7 @@ impl Engine { let ((var_name, _), expr, _) = x.as_ref(); let val = self .eval_expr(scope, mods, state, lib, this_ptr, &expr, level)? - .clone_inner_data() - .unwrap(); + .flatten(); let var_name = unsafe_cast_var_name_to_lifetime(var_name, &state); scope.push_dynamic_value(var_name, ScopeEntryType::Constant, val, true); Ok(Default::default()) @@ -2000,6 +2010,7 @@ impl Engine { } /// Map a type_name into a pretty-print name + #[inline(always)] pub(crate) fn map_type_name<'a>(&'a self, name: &'a str) -> &'a str { self.type_names .as_ref() diff --git a/src/fn_call.rs b/src/fn_call.rs index 20557748..87cc4e2b 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -925,11 +925,11 @@ impl Engine { self.inc_operations(state) .map_err(|err| err.new_position(pos))?; - // Turn it into a method call only if the object is not shared args = if target.is_shared() { - arg_values.insert(0, target.clone().clone_inner_data().unwrap()); + arg_values.insert(0, target.get_inner_clone()); arg_values.iter_mut().collect() } else { + // Turn it into a method call only if the object is not shared is_ref = true; once(target).chain(arg_values.iter_mut()).collect() }; diff --git a/src/fn_native.rs b/src/fn_native.rs index cc3ed0dc..a32e7d8b 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -21,13 +21,6 @@ use crate::stdlib::rc::Rc; #[cfg(feature = "sync")] use crate::stdlib::sync::Arc; -#[cfg(not(feature = "no_closure"))] -#[cfg(not(feature = "sync"))] -use crate::stdlib::cell::RefCell; -#[cfg(not(feature = "no_closure"))] -#[cfg(feature = "sync")] -use crate::stdlib::sync::RwLock; - /// Trait that maps to `Send + Sync` only under the `sync` feature. #[cfg(feature = "sync")] pub trait SendSync: Send + Sync {} @@ -49,15 +42,6 @@ pub type Shared = Rc; #[cfg(feature = "sync")] pub type Shared = Arc; -/// Mutable reference-counted container (read-write lock) -#[cfg(not(feature = "no_closure"))] -#[cfg(not(feature = "sync"))] -pub type SharedMut = Shared>; -/// Mutable reference-counted container (read-write lock) -#[cfg(not(feature = "no_closure"))] -#[cfg(feature = "sync")] -pub type SharedMut = Shared>; - /// Consume a `Shared` resource and return a mutable reference to the wrapped value. /// If the resource is shared (i.e. has other outstanding references), a cloned copy is used. pub fn shared_make_mut(value: &mut Shared) -> &mut T { @@ -67,16 +51,21 @@ pub fn shared_make_mut(value: &mut Shared) -> &mut T { return Arc::make_mut(value); } +/// Consume a `Shared` resource if is unique (i.e. not shared). +pub fn shared_try_take(value: Shared) -> Result> { + #[cfg(not(feature = "sync"))] + return Rc::try_unwrap(value); + #[cfg(feature = "sync")] + return Arc::try_unwrap(value); +} + /// Consume a `Shared` resource, assuming that it is unique (i.e. not shared). /// /// # Panics /// /// Panics if the resource is shared (i.e. has other outstanding references). pub fn shared_take(value: Shared) -> T { - #[cfg(not(feature = "sync"))] - return Rc::try_unwrap(value).map_err(|_| ()).unwrap(); - #[cfg(feature = "sync")] - return Arc::try_unwrap(value).map_err(|_| ()).unwrap(); + shared_try_take(value).map_err(|_| ()).unwrap() } pub type FnCallArgs<'a> = [&'a mut Dynamic]; diff --git a/src/scope.rs b/src/scope.rs index 61f6fa17..afc55e72 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -338,7 +338,7 @@ impl<'a> Scope<'a> { /// ``` pub fn get_value(&self, name: &str) -> Option { self.get_entry(name) - .and_then(|Entry { value, .. }| value.clone().clone_inner_data::()) + .and_then(|Entry { value, .. }| value.get_inner_clone().try_cast()) } /// Update the value of the named entry. @@ -441,7 +441,7 @@ impl<'a> Scope<'a> { /// ``` pub fn iter(&self) -> impl Iterator { self.iter_raw() - .map(|(name, value)| (name, value.clone().clone_inner_data().unwrap())) + .map(|(name, value)| (name, value.get_inner_clone())) } /// Get an iterator to entries in the Scope. From f68c5a699db404c643f61f484003825ea1c37d4c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 8 Aug 2020 17:04:21 +0800 Subject: [PATCH 08/11] Fix sync feature. --- src/any.rs | 6 +++--- src/fn_call.rs | 2 +- src/fn_native.rs | 4 ++-- src/scope.rs | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/any.rs b/src/any.rs index 5d47465d..c154a682 100644 --- a/src/any.rs +++ b/src/any.rs @@ -789,13 +789,13 @@ impl Dynamic { self.try_cast::().unwrap() } - /// Dereference the `Dynamic`. + /// Flatten the `Dynamic` and clone it. /// /// If the `Dynamic` is not a shared value, it returns a cloned copy. /// /// If the `Dynamic` is a shared value, it a cloned copy of the shared value. #[inline(always)] - pub fn get_inner_clone(&self) -> Self { + pub fn flatten_clone(&self) -> Self { match &self.0 { #[cfg(not(feature = "no_closure"))] Union::Shared(cell) => { @@ -826,7 +826,7 @@ impl Dynamic { #[cfg(feature = "sync")] return shared_try_take(cell) - .map_or_else(|c| c.read().unwrap().clone(), RwLock::into_inner); + .map_or_else(|c| c.read().unwrap().clone(), |v| v.into_inner().unwrap()); } _ => self, } diff --git a/src/fn_call.rs b/src/fn_call.rs index 87cc4e2b..7d5e313a 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -926,7 +926,7 @@ impl Engine { .map_err(|err| err.new_position(pos))?; args = if target.is_shared() { - arg_values.insert(0, target.get_inner_clone()); + arg_values.insert(0, target.flatten_clone()); arg_values.iter_mut().collect() } else { // Turn it into a method call only if the object is not shared diff --git a/src/fn_native.rs b/src/fn_native.rs index a32e7d8b..3b8e040a 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -52,7 +52,7 @@ pub fn shared_make_mut(value: &mut Shared) -> &mut T { } /// Consume a `Shared` resource if is unique (i.e. not shared). -pub fn shared_try_take(value: Shared) -> Result> { +pub fn shared_try_take(value: Shared) -> Result> { #[cfg(not(feature = "sync"))] return Rc::try_unwrap(value); #[cfg(feature = "sync")] @@ -64,7 +64,7 @@ pub fn shared_try_take(value: Shared) -> Result> { /// # Panics /// /// Panics if the resource is shared (i.e. has other outstanding references). -pub fn shared_take(value: Shared) -> T { +pub fn shared_take(value: Shared) -> T { shared_try_take(value).map_err(|_| ()).unwrap() } diff --git a/src/scope.rs b/src/scope.rs index afc55e72..1e489f0a 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -338,7 +338,7 @@ impl<'a> Scope<'a> { /// ``` pub fn get_value(&self, name: &str) -> Option { self.get_entry(name) - .and_then(|Entry { value, .. }| value.get_inner_clone().try_cast()) + .and_then(|Entry { value, .. }| value.flatten_clone().try_cast()) } /// Update the value of the named entry. @@ -441,7 +441,7 @@ impl<'a> Scope<'a> { /// ``` pub fn iter(&self) -> impl Iterator { self.iter_raw() - .map(|(name, value)| (name, value.get_inner_clone())) + .map(|(name, value)| (name, value.flatten_clone())) } /// Get an iterator to entries in the Scope. From 45d021c7efa7ae90ba26d1b83923919903a52400 Mon Sep 17 00:00:00 2001 From: Ilya Lakhin Date: Sat, 8 Aug 2020 17:55:58 +0700 Subject: [PATCH 09/11] Function names capturing as external variables bug --- src/parser.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/parser.rs b/src/parser.rs index 8b3bf3ea..8c9c2940 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1635,6 +1635,11 @@ fn parse_primary( Token::CharConstant(c) => Expr::CharConstant(Box::new((c, settings.pos))), Token::StringConstant(s) => Expr::StringConstant(Box::new((s.into(), settings.pos))), Token::Identifier(s) => { + // prevents capturing of the function call + #[cfg(not(feature = "no_closure"))] + if *next_token == Token::LeftParen || *next_token == Token::Bang { + state.allow_capture = false; + } let index = state.access_var(&s, settings.pos); Expr::Variable(Box::new(((s, settings.pos), None, 0, index))) } From 9c6584240f32c3d4c3574cf89754fca008148895 Mon Sep 17 00:00:00 2001 From: Ilya Lakhin Date: Sat, 8 Aug 2020 19:09:18 +0700 Subject: [PATCH 10/11] Unit test for registered functions in anon function context --- tests/closures.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/closures.rs b/tests/closures.rs index 3fc1cbd5..bbc91653 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -38,7 +38,7 @@ fn test_fn_ptr_curry_call() -> Result<(), Box> { #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "no_object"))] fn test_closures() -> Result<(), Box> { - let engine = Engine::new(); + let mut engine = Engine::new(); assert_eq!( engine.eval::( @@ -77,6 +77,23 @@ fn test_closures() -> Result<(), Box> { "# )?); + let mut module = Module::new(); + + module.set_fn_1("plus_one", |x: INT| Ok(x + 1)); + + engine.load_package(module); + + assert_eq!( + engine.eval::( + r#" + let a = 41; + let f = || plus_one(a); + f.call() + "# + )?, + 42 + ); + Ok(()) } From da3cce58d3e8ae8f962df7e4de76180cd0f1f9f9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 8 Aug 2020 22:59:05 +0800 Subject: [PATCH 11/11] Minor refactor. --- src/parser.rs | 11 ++++++----- tests/closures.rs | 16 +++++----------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 8c9c2940..cc0b91c8 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1634,12 +1634,13 @@ fn parse_primary( Token::FloatConstant(x) => Expr::FloatConstant(Box::new(FloatWrapper(x, settings.pos))), Token::CharConstant(c) => Expr::CharConstant(Box::new((c, settings.pos))), Token::StringConstant(s) => Expr::StringConstant(Box::new((s.into(), settings.pos))), + + // Function call + Token::Identifier(s) if *next_token == Token::LeftParen || *next_token == Token::Bang => { + Expr::Variable(Box::new(((s, settings.pos), None, 0, None))) + } + // Normal variable access Token::Identifier(s) => { - // prevents capturing of the function call - #[cfg(not(feature = "no_closure"))] - if *next_token == Token::LeftParen || *next_token == Token::Bang { - state.allow_capture = false; - } let index = state.access_var(&s, settings.pos); Expr::Variable(Box::new(((s, settings.pos), None, 0, index))) } diff --git a/tests/closures.rs b/tests/closures.rs index bbc91653..d7684540 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -1,12 +1,13 @@ #![cfg(not(feature = "no_function"))] -use rhai::{Dynamic, Engine, EvalAltResult, FnPtr, Module, INT}; +use rhai::{Dynamic, Engine, EvalAltResult, FnPtr, Module, RegisterFn, INT}; use std::any::TypeId; #[test] fn test_fn_ptr_curry_call() -> Result<(), Box> { - let mut module = Module::new(); + let mut engine = Engine::new(); - module.set_raw_fn( + #[allow(deprecated)] + engine.register_raw_fn( "call_with_arg", &[TypeId::of::(), TypeId::of::()], |engine: &Engine, lib: &Module, args: &mut [&mut Dynamic]| { @@ -15,9 +16,6 @@ fn test_fn_ptr_curry_call() -> Result<(), Box> { }, ); - let mut engine = Engine::new(); - engine.load_package(module); - #[cfg(not(feature = "no_object"))] assert_eq!( engine.eval::( @@ -77,11 +75,7 @@ fn test_closures() -> Result<(), Box> { "# )?); - let mut module = Module::new(); - - module.set_fn_1("plus_one", |x: INT| Ok(x + 1)); - - engine.load_package(module); + engine.register_fn("plus_one", |x: INT| x + 1); assert_eq!( engine.eval::(