From e942ef358c7be0ed5372e4dd3607ddb8b6ca658f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 7 Jun 2020 17:54:33 +0800 Subject: [PATCH 1/3] Transparently convert &str to ImmutableString for register_fn. --- README.md | 44 ++++++++++++++++++++++++++++++++++++++------ RELEASES.md | 1 + src/engine.rs | 10 ++++++++++ src/fn_register.rs | 30 +++++++++++++++++++++++++----- tests/string.rs | 21 ++++++++++++++++++++- 5 files changed, 94 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 39539e54..4fb86ec8 100644 --- a/README.md +++ b/README.md @@ -684,13 +684,18 @@ 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`]. ```rust -use rhai::{Dynamic, Engine, EvalAltResult}; +use rhai::{Dynamic, Engine, EvalAltResult, ImmutableString}; use rhai::RegisterFn; // use 'RegisterFn' trait for 'register_fn' use rhai::RegisterResultFn; // use 'RegisterResultFn' trait for 'register_result_fn' -// Normal function that returns any value type -fn add(x: i64, y: i64) -> i64 { - x + y +// Normal function that returns a standard type +// Remember to use 'ImmutableString' and not 'String' +fn add_len(x: i64, s: ImmutableString) -> i64 { + x + s.len() +} +// Alternatively, '&str' maps directly to 'ImmutableString' +fn add_len_str(x: i64, s: &str) -> i64 { + x + s.len() } // Function that returns a 'Dynamic' value - must return a 'Result' @@ -702,9 +707,14 @@ fn main() -> Result<(), Box> { let engine = Engine::new(); - engine.register_fn("add", add); + engine.register_fn("add", add_len); + engine.register_fn("add_str", add_len_str); - let result = engine.eval::("add(40, 2)")?; + let result = engine.eval::(r#"add(40, "xx")"#)?; + + println!("Answer: {}", result); // prints 42 + + let result = engine.eval::(r#"add_str(40, "xx")"#)?; println!("Answer: {}", result); // prints 42 @@ -735,6 +745,25 @@ i.e. different functions can have the same name as long as their parameters are and/or different number. New definitions _overwrite_ previous definitions of the same name and same number/types of parameters. +### `String` parameters + +Functions accepting a parameter of `String` should use `&str` instead because it maps directly to `ImmutableString` +which is the type that Rhai uses to represent strings internally. + +```rust +fn get_len1(s: String) -> i64 { s.len() as i64 } // <- Rhai will not find this function +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 + +engine.register_fn("len1", get_len1); +engine.register_fn("len2", get_len2); +engine.register_fn("len3", get_len3); + +let len = engine.eval::("x.len1()")?; // error: function 'len1 (string)' not found +let len = engine.eval::("x.len2()")?; // works fine +let len = engine.eval::("x.len3()")?; // works fine +``` + Generic functions ----------------- @@ -1388,6 +1417,9 @@ Strings and Chars [strings]: #strings-and-chars [char]: #strings-and-chars +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. + String and character literals follow C-style formatting, with support for Unicode ('`\u`_xxxx_' or '`\U`_xxxxxxxx_') and hex ('`\x`_xx_') escape sequences. diff --git a/RELEASES.md b/RELEASES.md index 58303958..3622167a 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -15,6 +15,7 @@ New features ------------ * Indexers are now split into getters ans setters (which now support updates). The API is split into `Engine::register_indexer_get` and `Engine::register_indexer_set` with `Engine::register_indexer_get_set` being a shorthand. Similarly, `Module::set_indexer_get_fn` and `Module::set_indexer_set_fn` are added. +* `Engine:register_fn` and `Engine:register_result_fn` accepts functions that take parameters of type `&str` (immutable string slice), which maps directly to `ImmutableString`. This is to avoid needing wrappers for functions taking string parameters. Version 0.15.0 diff --git a/src/engine.rs b/src/engine.rs index c10a89d4..9be3c04c 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -906,6 +906,9 @@ impl Engine { let mut idx_val = idx_values.pop(); if is_index { + #[cfg(feature = "no_index")] + unreachable!(); + let pos = rhs.position(); match rhs { @@ -1292,6 +1295,7 @@ impl Engine { } } + #[cfg(not(feature = "no_index"))] _ => { let fn_name = FUNC_INDEXER_GET; let type_name = self.map_type_name(val.type_name()); @@ -1305,6 +1309,12 @@ impl Engine { )) }) } + + #[cfg(feature = "no_index")] + _ => Err(Box::new(EvalAltResult::ErrorIndexingType( + self.map_type_name(val.type_name()).into(), + Position::none(), + ))), } } diff --git a/src/fn_register.rs b/src/fn_register.rs index cb208d12..1384f666 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -7,6 +7,7 @@ use crate::engine::Engine; use crate::fn_native::{CallableFunction, FnAny, FnCallArgs}; use crate::parser::FnAccess; use crate::result::EvalAltResult; +use crate::utils::ImmutableString; use crate::stdlib::{any::TypeId, boxed::Box, mem}; @@ -99,9 +100,16 @@ pub fn by_ref(data: &mut Dynamic) -> &mut T { /// Dereference into value. #[inline(always)] pub fn by_value(data: &mut Dynamic) -> T { - // 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. - mem::take(data).cast::() + if TypeId::of::() == TypeId::of::<&str>() { + // &str parameters are mapped to the underlying ImmutableString + let r = data.as_str().unwrap(); + let x = unsafe { mem::transmute::<_, &T>(&r) }; + x.clone() + } 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. + mem::take(data).cast::() + } } /// This macro creates a closure wrapping a registered function. @@ -146,6 +154,18 @@ pub fn map_result( data } +/// Remap `&str` to `ImmutableString`. +#[inline(always)] +fn map_type_id() -> TypeId { + let id = TypeId::of::(); + + if id == TypeId::of::<&str>() { + TypeId::of::() + } else { + id + } +} + macro_rules! def_register { () => { def_register!(imp from_pure :); @@ -170,7 +190,7 @@ macro_rules! def_register { { fn register_fn(&mut self, name: &str, f: FN) { self.global_module.set_fn(name, FnAccess::Public, - &[$(TypeId::of::<$par>()),*], + &[$(map_type_id::<$par>()),*], CallableFunction::$abi(make_func!(f : map_dynamic ; $($par => $clone),*)) ); } @@ -187,7 +207,7 @@ macro_rules! def_register { { fn register_result_fn(&mut self, name: &str, f: FN) { self.global_module.set_fn(name, FnAccess::Public, - &[$(TypeId::of::<$par>()),*], + &[$(map_type_id::<$par>()),*], CallableFunction::$abi(make_func!(f : map_result ; $($par => $clone),*)) ); } diff --git a/tests/string.rs b/tests/string.rs index 995bd76e..27854629 100644 --- a/tests/string.rs +++ b/tests/string.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, INT}; +use rhai::{Engine, EvalAltResult, ImmutableString, RegisterFn, INT}; #[test] fn test_string() -> Result<(), Box> { @@ -154,3 +154,22 @@ fn test_string_substring() -> Result<(), Box> { Ok(()) } + +#[test] +fn test_string_fn() -> Result<(), Box> { + let mut engine = Engine::new(); + + 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); + + assert_eq!(engine.eval::(r#"foo1("hello")"#)?, 5); + assert_eq!(engine.eval::(r#"foo2("hello")"#)?, 5); + + assert!(matches!( + *engine.eval::(r#"foo3("hello")"#).expect_err("should error"), + EvalAltResult::ErrorFunctionNotFound(ref x, _) if x == "foo3 (string)" + )); + + Ok(()) +} From 5fb4b04cb015d8ae973ce740b128cea85bc63ee7 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 8 Jun 2020 10:26:12 +0800 Subject: [PATCH 2/3] Put type on transmute call. --- src/utils.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index ccd3d6be..4a8cd3c8 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -190,8 +190,9 @@ impl Clone for StaticVec { if self.is_fixed_storage() { for x in 0..self.len { - let item: &T = unsafe { mem::transmute(self.list.get(x).unwrap()) }; - value.list[x] = MaybeUninit::new(item.clone()); + let item = self.list.get(x).unwrap(); + let item_value = unsafe { mem::transmute::<_, &T>(item) }; + value.list[x] = MaybeUninit::new(item_value.clone()); } } else { value.more = self.more.clone(); @@ -424,7 +425,7 @@ impl StaticVec { panic!("index OOB in StaticVec"); } - let list: &[T; MAX_STATIC_VEC] = unsafe { mem::transmute(&self.list) }; + let list = unsafe { mem::transmute::<_, &[T; MAX_STATIC_VEC]>(&self.list) }; if self.is_fixed_storage() { list.get(index).unwrap() @@ -442,7 +443,7 @@ impl StaticVec { panic!("index OOB in StaticVec"); } - let list: &mut [T; MAX_STATIC_VEC] = unsafe { mem::transmute(&mut self.list) }; + let list = unsafe { mem::transmute::<_, &mut [T; MAX_STATIC_VEC]>(&mut self.list) }; if self.is_fixed_storage() { list.get_mut(index).unwrap() @@ -452,7 +453,7 @@ impl StaticVec { } /// Get an iterator to entries in the `StaticVec`. pub fn iter(&self) -> impl Iterator { - let list: &[T; MAX_STATIC_VEC] = unsafe { mem::transmute(&self.list) }; + let list = unsafe { mem::transmute::<_, &[T; MAX_STATIC_VEC]>(&self.list) }; if self.is_fixed_storage() { list[..self.len].iter() @@ -462,7 +463,7 @@ impl StaticVec { } /// Get a mutable iterator to entries in the `StaticVec`. pub fn iter_mut(&mut self) -> impl Iterator { - let list: &mut [T; MAX_STATIC_VEC] = unsafe { mem::transmute(&mut self.list) }; + let list = unsafe { mem::transmute::<_, &mut [T; MAX_STATIC_VEC]>(&mut self.list) }; if self.is_fixed_storage() { list[..self.len].iter_mut() @@ -549,7 +550,7 @@ impl fmt::Debug for StaticVec { impl AsRef<[T]> for StaticVec { fn as_ref(&self) -> &[T] { - let list: &[T; MAX_STATIC_VEC] = unsafe { mem::transmute(&self.list) }; + let list = unsafe { mem::transmute::<_, &[T; MAX_STATIC_VEC]>(&self.list) }; if self.is_fixed_storage() { &list[..self.len] @@ -561,7 +562,7 @@ impl AsRef<[T]> for StaticVec { impl AsMut<[T]> for StaticVec { fn as_mut(&mut self) -> &mut [T] { - let list: &mut [T; MAX_STATIC_VEC] = unsafe { mem::transmute(&mut self.list) }; + let list = unsafe { mem::transmute::<_, &mut [T; MAX_STATIC_VEC]>(&mut self.list) }; if self.is_fixed_storage() { &mut list[..self.len] From ead366aac80b51298aea655d699a84f7fc8aeff9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 8 Jun 2020 10:26:32 +0800 Subject: [PATCH 3/3] Better String parameter error message. --- src/engine.rs | 6 +++++- src/fn_register.rs | 8 ++++---- tests/string.rs | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 9be3c04c..4ad31b36 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -708,7 +708,11 @@ impl Engine { "{} ({})", fn_name, args.iter() - .map(|name| self.map_type_name(name.type_name())) + .map(|name| if name.is::() { + "&str | ImmutableString" + } else { + self.map_type_name(name.type_name()) + }) .collect::>() .join(", ") ), diff --git a/src/fn_register.rs b/src/fn_register.rs index 1384f666..1285c119 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -101,10 +101,10 @@ pub fn by_ref(data: &mut Dynamic) -> &mut T { #[inline(always)] pub fn by_value(data: &mut Dynamic) -> T { if TypeId::of::() == TypeId::of::<&str>() { - // &str parameters are mapped to the underlying ImmutableString - let r = data.as_str().unwrap(); - let x = unsafe { mem::transmute::<_, &T>(&r) }; - x.clone() + // If T is &str, data must be ImmutableString, so map directly to it + let ref_str = data.as_str().unwrap(); + let ref_T = unsafe { mem::transmute::<_, &T>(&ref_str) }; + ref_T.clone() } 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. diff --git a/tests/string.rs b/tests/string.rs index 27854629..c93278de 100644 --- a/tests/string.rs +++ b/tests/string.rs @@ -168,7 +168,7 @@ fn test_string_fn() -> Result<(), Box> { assert!(matches!( *engine.eval::(r#"foo3("hello")"#).expect_err("should error"), - EvalAltResult::ErrorFunctionNotFound(ref x, _) if x == "foo3 (string)" + EvalAltResult::ErrorFunctionNotFound(ref x, _) if x == "foo3 (&str | ImmutableString)" )); Ok(())