Merge pull request #184 from schungx/master

Support String parameters in functions.

README.md

@@ -18,8 +18,8 @@ Supported targets and builds
 * WebAssembly (WASM)
 * `no-std`
 
-Features
---------
+Standard Features
+----------------
 
 * Easy-to-use language similar to JavaScript+Rust with dynamic typing.
 * Tight integration with native Rust [functions](https://schungx.github.io/rhai/rust/functions.html) and [types]([#custom-types-and-methods](https://schungx.github.io/rhai/rust/custom.html)), including [getters/setters](https://schungx.github.io/rhai/rust/getters-setters.html), [methods](https://schungx.github.io/rhai/rust/custom.html) and [indexers](https://schungx.github.io/rhai/rust/indexers.html).
@@ -30,20 +30,30 @@ Features
 * Relatively little `unsafe` code (yes there are some for performance reasons, and most `unsafe` code is limited to
   one single source file, all with names starting with `"unsafe_"`).
 * Re-entrant scripting engine can be made `Send + Sync` (via the `sync` feature).
-* Sand-boxed - the scripting engine, if declared immutable, cannot mutate the containing environment unless explicitly permitted (e.g. via a `RefCell`).
-* 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.
 * [Function overloading](https://schungx.github.io/rhai/language/overload.html).
 * [Operator overloading](https://schungx.github.io/rhai/rust/operators.html).
-* Support for use as a [DSL](https://schungx.github.io/rhai/engine/dsl.html) - [disabling keywords/operators](https://schungx.github.io/rhai/engine/disable.html), [custom operators](https://schungx.github.io/rhai/engine/custom-op.html) and extending the language with [custom syntax](https://schungx.github.io/rhai/engine/custom-syntax.html).
 * Dynamic dispatch via [function pointers](https://schungx.github.io/rhai/language/fn-ptr.html).
 * Some support for [object-oriented programming (OOP)](https://schungx.github.io/rhai/language/oop.html).
 * Organize code base with dynamically-loadable [modules](https://schungx.github.io/rhai/language/modules.html).
 * Serialization/deserialization support via [serde](https://crates.io/crates/serde) (requires the `serde` feature).
-* Surgically disable keywords and operators to restrict the language.
 * Scripts are [optimized](https://schungx.github.io/rhai/engine/optimize.html) (useful for template-based machine-generated scripts) for repeated evaluations.
 * Support for [minimal builds](https://schungx.github.io/rhai/start/builds/minimal.html) by excluding unneeded language [features](https://schungx.github.io/rhai/start/features.html).
 
+Protection Against Attacks
+-------------------------
+
+* Sand-boxed - the scripting engine, if declared immutable, cannot mutate the containing environment unless explicitly permitted (e.g. via a `RefCell`).
+* 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.
+
+For Those Who Actually Want Their Own Language
+---------------------------------------------
+
+* Use as a [DSL](https://schungx.github.io/rhai/engine/dsl.html).
+* Define [custom operators](https://schungx.github.io/rhai/engine/custom-op.html).
+* Restrict the language by surgically [disabling keywords and operators](https://schungx.github.io/rhai/engine/disable.html).
+* Extend the language with [custom syntax](https://schungx.github.io/rhai/engine/custom-syntax.html).
+
 Documentation
 -------------
 

RELEASES.md

@@ -7,8 +7,9 @@ Version 0.17.0
 This version adds:
 
 * [`serde`](https://crates.io/crates/serde) support for working with `Dynamic` values (particularly _object maps_).
-* Ability to surgically disable keywords and/or operators in the language.
-* Ability to define custom operators (which must be valid identifiers).
+* Surgically disable keywords and/or operators in the language.
+* Define custom operators.
+* Extend the language via custom syntax.
 * Low-level API to register functions.
 
 Bug fixes
@@ -22,7 +23,6 @@ Breaking changes
 * `EvalAltResult::ErrorMismatchOutputType` has an extra argument containing the name of the requested type.
 * `Engine::call_fn_dynamic` take an extra argument, allowing a `Dynamic` value to be bound to the `this` pointer.
 * Precedence of the `%` (modulo) operator is lowered to below `<<` ad `>>`. This is to handle the case of `x << 3 % 10`.
-* Many configuration/setting API's now returns `&mut Self` so that the calls can be chained. This should not affect most code.
 
 New features
 ------------
@@ -38,7 +38,8 @@ New features
 * `FnPtr` is exposed as the function pointer type.
 * `rhai::module_resolvers::ModuleResolversCollection` added to try a list of module resolvers.
 * It is now possible to mutate the first argument of a module-qualified function call when the argument is a simple variable (but not a module constant).
-
+* Many configuration/setting API's now returns `&mut Self` so that the calls can be chained.
+* `String` parameters in functions are supported (but inefficiently).
 
 Version 0.16.1
 ==============

doc/src/engine/call-fn.md

@@ -91,5 +91,5 @@ let result = engine.call_fn_dynamic(
                         [ 41_i64.into() ]
              )?;
 
-assert_eq!(value.as_int().unwrap(), 42);
+assert_eq!(value.as_int()?, 42);
 ```

doc/src/engine/custom-op.md

@@ -21,7 +21,7 @@ let mut engine = Engine::new();
 // (i.e. between +|- and *|/)
 // Also register the implementation of the customer operator as a function
 engine
-    .register_custom_operator("foo", 160).unwrap()
+    .register_custom_operator("foo", 160)?
     .register_fn("foo", |x: i64, y: i64| (x * y) - (x + y));
 
 // The custom operator can be used in expressions
@@ -72,7 +72,7 @@ _Unary_ custom operators are not supported.
 
 ```rust
 engine
-    .register_custom_operator("foo", 160).unwrap()
+    .register_custom_operator("foo", 160)?
     .register_fn("foo", |x: i64| x * x);
 
 engine.eval::<i64>("1 + 2 * 3 foo 4 - 5 / 6")?; // error: function 'foo (i64, i64)' not found

doc/src/language/dynamic.md

@@ -66,7 +66,7 @@ item.is::<i64>() == true;                       // 'is' returns whether a 'Dynam
 let value = item.cast::<i64>();                 // if the element is 'i64', this succeeds; otherwise it panics
 let value: i64 = item.cast();                   // type can also be inferred
 
-let value = item.try_cast::<i64>().unwrap();    // 'try_cast' does not panic when the cast fails, but returns 'None'
+let value = item.try_cast::<i64>()?;            // 'try_cast' does not panic when the cast fails, but returns 'None'
 ```
 
 Type Name

doc/src/language/method.md

@@ -3,6 +3,10 @@ Call Method as Function
 
 {{#include ../links.md}}
 
+
+First `&mut` Reference Parameter
+-------------------------------
+
 Property [getters/setters] and [methods][custom types] in a Rust custom type registered with the [`Engine`] can be called
 just like a regular function.  In fact, like Rust, property getters/setters and object methods
 are registered as regular [functions] in Rhai that take a first `&mut` parameter.
@@ -31,3 +35,23 @@ update(array[0]);   // <- 'array[0]' is an expression returning a calculated val
 
 array[0].update();  // <- call in method-call style will update 'a'
 ```
+
+
+Encouraged Usage
+----------------
+
+Using a `&mut` first parameter is highly encouraged when using types that are expensive to clone,
+even when the intention is not to mutate that argument, because it avoids cloning that argument value.
+
+For primary types that are cheap to clone, including `ImmutableString`, this is not necessary.
+
+
+Avoid `&mut ImmutableString`
+---------------------------
+
+`ImmutableString`, Rhai internal [string] type, is an exception.
+
+`ImmutableString` is cheap to clone, but expensive to take a mutable reference (because the underlying
+string must be cloned to make a private copy).
+
+Therefore, avoid using `&mut ImmutableString` unless the intention is to mutate it.

doc/src/language/strings-chars.md

@@ -6,20 +6,25 @@ Strings and Characters
 String in Rhai contain any text sequence of valid Unicode characters.
 Internally strings are stored in UTF-8 encoding.
 
-Strings can be built up from other strings and types via the `+` operator (provided by the [`MoreStringPackage`][packages]
-but excluded if using a [raw `Engine`]). This is particularly useful when printing output.
+Strings can be built up from other strings and types via the `+` operator
+(provided by the [`MoreStringPackage`][packages] but excluded if using a [raw `Engine`]).
+This is particularly useful when printing output.
 
 [`type_of()`] a string returns `"string"`.
 
 The maximum allowed length of a string can be controlled via `Engine::set_max_string_size`
 (see [maximum length of strings]).
 
+
 The `ImmutableString` Type
 -------------------------
 
 All strings in Rhai are implemented as `ImmutableString` (see [standard types]).
 
-`ImmutableString` should be used in place of the standard Rust type `String` when registering functions.
+`ImmutableString` should be used in place of the standard Rust type `String` when registering functions
+because using `String` is very inefficient (the `String` must always be cloned).
+
+A alternative is to use `&str` which maps straight to `ImmutableString`.
 
 
 String and Character Literals
@@ -59,13 +64,13 @@ Unicode characters.
 
 Individual characters within a Rhai string can also be replaced just as if the string is an array of Unicode characters.
 
-In Rhai, there is also no separate concepts of `String` and `&str` as in Rust.
+In Rhai, there are also no separate concepts of `String` and `&str` as in Rust.
 
 
 Immutable Strings
 ----------------
 
-Rhai strings are _immutable_ and can be shared.
+Rhai use _immutable_ strings (type `ImmutableString`) and can be shared.
 
 Modifying a Rhai string actually causes it first to be cloned, and then the modification made to the copy.
 

doc/src/rust/getters-setters.md

@@ -5,6 +5,8 @@ Custom Type Getters and Setters
 
 A custom type can also expose members by registering `get` and/or `set` functions.
 
+Getters and setters each take a `&mut` reference to the first parameter.
+
 ```rust
 #[derive(Clone)]
 struct TestStruct {

doc/src/rust/indexers.md

@@ -7,6 +7,8 @@ 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.
 
+Like getters and setters, indexers take a `&mut` reference to the first parameter.
+
 Indexers are disabled when the [`no_index`] feature is used.
 
 For efficiency reasons, indexers **cannot** be used to overload (i.e. override) built-in indexing operations for
@@ -33,12 +35,10 @@ impl TestStruct {
 
 let mut engine = Engine::new();
 
-engine.register_type::<TestStruct>();
-
-engine.register_fn("new_ts", TestStruct::new);
-
-// Shorthand: engine.register_indexer_get_set(TestStruct::get_field, TestStruct::set_field);
 engine
+    .register_type::<TestStruct>()
+    .register_fn("new_ts", TestStruct::new)
+    // Shorthand: .register_indexer_get_set(TestStruct::get_field, TestStruct::set_field);
     .register_indexer_get(TestStruct::get_field)
     .register_indexer_set(TestStruct::set_field);
 

doc/src/rust/strings.md

@@ -3,11 +3,19 @@
 
 {{#include ../links.md}}
 
-Rust functions accepting parameters of `String` should use `&str` instead because it maps directly to [`ImmutableString`]
-which is the type that Rhai uses to represent [strings] internally.
+
+`&str` Maps to `ImmutableString`
+-------------------------------
+
+Rust functions accepting parameters of `String` should use `&str` instead because it maps directly to
+[`ImmutableString`][string] which is the type that Rhai uses to represent [strings] internally.
+
+The parameter type `String` is discouraged because it involves converting an [`ImmutableString`] into a `String`.
+Using `ImmutableString` or `&str` is much more efficient.
+A common mistake made by novice Rhai users is to register functions with `String` parameters.
 
 ```rust
-fn get_len1(s: String) -> i64 { s.len() as i64 }            // <- Rhai will not find this function
+fn get_len1(s: String) -> i64 { s.len() as i64 }            // <- Rhai finds this function, but very inefficient
 fn get_len2(s: &str) -> i64 { s.len() as i64 }              // <- Rhai finds this function fine
 fn get_len3(s: ImmutableString) -> i64 { s.len() as i64 }   // <- the above is equivalent to this
 
@@ -20,3 +28,23 @@ let len = engine.eval::<i64>("x.len1()")?;                  // error: function '
 let len = engine.eval::<i64>("x.len2()")?;                  // works fine
 let len = engine.eval::<i64>("x.len3()")?;                  // works fine
 ```
+
+
+Avoid `&mut ImmutableString`
+---------------------------
+
+Rhai functions can take a first `&mut` parameter.  Usually this is a good idea because it avoids
+cloning of the argument (except for primary types where cloning is cheap), so its use is encouraged
+even though there is no intention to ever mutate that argument.
+
+`ImmutableString` is an exception to this rule.  While `ImmutableString` is cheap to clone (only
+incrementing a reference count), taking a mutable reference to it involves making a private clone
+of the underlying string because Rhai has no way to find out whether that parameter will be mutated.
+
+If the `ImmutableString` is not shared by any other variables, then Rhai just returns a mutable
+reference to it since nobody else is watching! Otherwise a private copy is made first,
+because other reference holders will not expect the `ImmutableString` to ever change
+(it is supposed to be _immutable_).
+
+Therefore, avoid using `&mut ImmutableString` as the first parameter of a function unless you really
+intend to mutate that string.  Use `ImmutableString` instead.

doc/src/start/builds/performance.md

@@ -29,3 +29,16 @@ Turning on [`no_float`], and [`only_i32`] makes the key [`Dynamic`] data type on
 while normally it can be up to 16 bytes (e.g. on x86/x64 CPU's) in order to hold an `i64` or `f64`.
 
 Making [`Dynamic`] small helps performance due to better cache efficiency.
+
+
+Use `ImmutableString`
+--------------------
+
+Internally, Rhai uses _immutable_ [strings] instead of the Rust `String` type.  This is mainly to avoid excessive
+cloning when passing function arguments.
+
+The encapsulated immutable string type is `ImmutableString`.  It is cheap to clone (just an `Rc` or `Arc` reference
+count increment depending on the [`sync`] feature).
+
+Therefore, functions taking `String` parameters should use `ImmutableString` or `&str` (which maps to `ImmutableString`)
+for the best performance with Rhai.

doc/src/start/examples/rust.md

@@ -10,7 +10,7 @@ A number of examples can be found in the `examples` folder:
 | [`arrays_and_structs`](https://github.com/jonathandturner/rhai/tree/master/examples/arrays_and_structs.rs)             | Shows how to register a custom Rust type and using [arrays] on it.                                                                            |
 | [`custom_types_and_methods`](https://github.com/jonathandturner/rhai/tree/master/examples/custom_types_and_methods.rs) | Shows how to register a custom Rust type and methods for it.                                                                                  |
 | [`hello`](https://github.com/jonathandturner/rhai/tree/master/examples/hello.rs)                                       | Simple example that evaluates an expression and prints the result.                                                                            |
-| [`no_std`](https://github.com/jonathandturner/rhai/tree/master/examples/no_std.rs)                                     | Example to test out `no-std` builds.                                                                                                          |
+| [`no_std`](https://github.com/jonathandturner/rhai/tree/master/examples/no_std.rs)                                     | Example to test out `no-std` builds.</br>The [`no_std`] feature is required to build in `no-std`.                                             |
 | [`reuse_scope`](https://github.com/jonathandturner/rhai/tree/master/examples/reuse_scope.rs)                           | Evaluates two pieces of code in separate runs, but using a common [`Scope`].                                                                  |
 | [`rhai_runner`](https://github.com/jonathandturner/rhai/tree/master/examples/rhai_runner.rs)                           | Runs each filename passed to it as a Rhai script.                                                                                             |
 | [`serde`](https://github.com/jonathandturner/rhai/tree/master/examples/serde.rs)                                       | Example to serialize and deserialize Rust types with [`serde`](https://crates.io/crates/serde).<br/>The [`serde`] feature is required to run. |

src/engine.rs

@@ -855,7 +855,7 @@ impl Engine {
                 fn_name,
                 args.iter()
                     .map(|name| if name.is::<ImmutableString>() {
-                        "&str | ImmutableString"
+                        "&str | ImmutableString | String"
                     } else {
                         self.map_type_name((*name).type_name())
                     })

src/fn_register.rs

@@ -6,6 +6,7 @@ use crate::engine::Engine;
 use crate::fn_native::{CallableFunction, FnAny, FnCallArgs, SendSync};
 use crate::module::Module;
 use crate::parser::FnAccess;
+use crate::r#unsafe::unsafe_cast_box;
 use crate::result::EvalAltResult;
 use crate::utils::ImmutableString;
 
@@ -105,6 +106,9 @@ pub fn by_value<T: Variant + Clone>(data: &mut Dynamic) -> T {
         let ref_str = data.as_str().unwrap();
         let ref_T = unsafe { mem::transmute::<_, &T>(&ref_str) };
         ref_T.clone()
+    } else if TypeId::of::<T>() == TypeId::of::<String>() {
+        // If T is String, data must be ImmutableString, so map directly to it
+        *unsafe_cast_box(Box::new(data.as_str().unwrap().to_string())).unwrap()
     } else {
         // We consume the argument and then replace it with () - the argument is not supposed to be used again.
         // This way, we avoid having to clone the argument again, because it is already a clone when passed here.
@@ -154,13 +158,15 @@ pub fn map_result(
     data
 }
 
-/// Remap `&str` to `ImmutableString`.
+/// Remap `&str` | `String` to `ImmutableString`.
 #[inline(always)]
 fn map_type_id<T: 'static>() -> TypeId {
     let id = TypeId::of::<T>();
 
     if id == TypeId::of::<&str>() {
         TypeId::of::<ImmutableString>()
+    } else if id == TypeId::of::<String>() {
+        TypeId::of::<ImmutableString>()
     } else {
         id
     }

tests/decrement.rs

@@ -8,7 +8,7 @@ fn test_decrement() -> Result<(), Box<EvalAltResult>> {
 
     assert!(matches!(
         *engine.eval::<String>(r#"let s = "test"; s -= "ing"; s"#).expect_err("expects error"),
-        EvalAltResult::ErrorFunctionNotFound(err, _) if err == "- (&str | ImmutableString, &str | ImmutableString)"
+        EvalAltResult::ErrorFunctionNotFound(err, _) if err == "- (&str | ImmutableString | String, &str | ImmutableString | String)"
     ));
 
     Ok(())

tests/string.rs

@@ -173,17 +173,16 @@ fn test_string_fn() -> Result<(), Box<EvalAltResult>> {
         "foo"
     );
 
-    engine.register_fn("foo1", |s: &str| s.len() as INT);
-    engine.register_fn("foo2", |s: ImmutableString| s.len() as INT);
-    engine.register_fn("foo3", |s: String| s.len() as INT);
+    engine
+        .register_fn("foo1", |s: &str| s.len() as INT)
+        .register_fn("foo2", |s: ImmutableString| s.len() as INT)
+        .register_fn("foo3", |s: String| s.len() as INT)
+        .register_fn("foo4", |s: &mut ImmutableString| s.len() as INT);
 
     assert_eq!(engine.eval::<INT>(r#"foo1("hello")"#)?, 5);
     assert_eq!(engine.eval::<INT>(r#"foo2("hello")"#)?, 5);
-
-    assert!(matches!(
-        *engine.eval::<INT>(r#"foo3("hello")"#).expect_err("should error"),
-        EvalAltResult::ErrorFunctionNotFound(err, _) if err == "foo3 (&str | ImmutableString)"
-    ));
+    assert_eq!(engine.eval::<INT>(r#"foo3("hello")"#)?, 5);
+    assert_eq!(engine.eval::<INT>(r#"foo4("hello")"#)?, 5);
 
     Ok(())
 }