From 17166f667acac79c852f2a6dca767d425dcf4f87 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 29 Jan 2024 16:26:17 +0800 Subject: [PATCH 01/11] Refine macro implementation. --- codegen/src/custom_type.rs | 379 ++++++++++++++++---------------- codegen/src/test/custom_type.rs | 4 - 2 files changed, 188 insertions(+), 195 deletions(-) diff --git a/codegen/src/custom_type.rs b/codegen/src/custom_type.rs index 1774f3706..9517ba740 100644 --- a/codegen/src/custom_type.rs +++ b/codegen/src/custom_type.rs @@ -1,26 +1,29 @@ use proc_macro2::{Span, TokenStream}; use quote::{quote, ToTokens}; use syn::{ - punctuated::Punctuated, spanned::Spanned, DeriveInput, Expr, Fields, MetaNameValue, Path, Token, + punctuated::Punctuated, spanned::Spanned, Data, DataStruct, DeriveInput, Expr, Field, Fields, + MetaNameValue, Path, Token, }; -const ATTR_ROOT: &str = "rhai_type"; -const ATTR_NAME: &str = "name"; -const ATTR_SKIP: &str = "skip"; -const ATTR_GET: &str = "get"; -const ATTR_GET_MUT: &str = "get_mut"; -const ATTR_SET: &str = "set"; -const ATTR_READONLY: &str = "readonly"; -const ATTR_EXTRA: &str = "extra"; +const ATTR: &str = "rhai_type"; + +const OPTION_NAME: &str = "name"; +const OPTION_SKIP: &str = "skip"; +const OPTION_GET: &str = "get"; +const OPTION_GET_MUT: &str = "get_mut"; +const OPTION_SET: &str = "set"; +const OPTION_READONLY: &str = "readonly"; +const OPTION_EXTRA: &str = "extra"; /// Derive the `CustomType` trait for a struct. pub fn derive_custom_type_impl(input: DeriveInput) -> TokenStream { let type_name = input.ident; - let mut pretty_print_name = quote! { stringify!(#type_name) }; + let mut display_name = quote! { stringify!(#type_name) }; + let mut field_accessors = Vec::new(); let mut extras = Vec::new(); let mut errors = Vec::new(); - for attr in input.attrs.iter().filter(|a| a.path().is_ident(ATTR_ROOT)) { + for attr in input.attrs.iter().filter(|a| a.path().is_ident(ATTR)) { let config_list: Result, _> = attr.parse_args_with(Punctuated::parse_terminated); @@ -29,14 +32,14 @@ pub fn derive_custom_type_impl(input: DeriveInput) -> TokenStream { for expr in list { match expr { // Key-value - syn::Expr::Assign(..) => { + Expr::Assign(..) => { let MetaNameValue { path, value, .. } = syn::parse2::(expr.to_token_stream()).unwrap(); - if path.is_ident(ATTR_NAME) { + if path.is_ident(OPTION_NAME) { // Type name - pretty_print_name = value.to_token_stream(); - } else if path.is_ident(ATTR_EXTRA) { + display_name = value.to_token_stream(); + } else if path.is_ident(OPTION_EXTRA) { match syn::parse2::(value.to_token_stream()) { Ok(path) => extras.push(path.to_token_stream()), Err(err) => errors.push(err.into_compile_error()), @@ -48,11 +51,11 @@ pub fn derive_custom_type_impl(input: DeriveInput) -> TokenStream { } } // skip - syn::Expr::Path(path) if path.path.is_ident(ATTR_SKIP) => { + Expr::Path(path) if path.path.is_ident(OPTION_SKIP) => { println!("SKIPPED"); } // any other identifier - syn::Expr::Path(path) if path.path.get_ident().is_some() => { + Expr::Path(path) if path.path.get_ident().is_some() => { let key = path.path.get_ident().unwrap().to_string(); let msg = format!("invalid option: '{}'", key); errors.push(syn::Error::new(path.span(), msg).into_compile_error()); @@ -69,208 +72,202 @@ pub fn derive_custom_type_impl(input: DeriveInput) -> TokenStream { } } - let accessors = match input.data { + match input.data { + // struct Foo { ... } + Data::Struct(DataStruct { + fields: Fields::Named(ref f), + .. + }) => scan_fields( + &f.named.iter().collect::>(), + &mut field_accessors, + &mut errors, + ), + + // struct Foo(...); + Data::Struct(DataStruct { + fields: Fields::Unnamed(ref f), + .. + }) => scan_fields( + &f.unnamed.iter().collect::>(), + &mut field_accessors, + &mut errors, + ), + // struct Foo; - syn::Data::Struct(syn::DataStruct { + Data::Struct(DataStruct { fields: Fields::Unit, .. - }) => quote! {}, + }) => (), - // struct Foo { ... } - syn::Data::Struct(syn::DataStruct { fields, .. }) => { - let fields = match fields { - Fields::Named(ref f) => f.named.iter(), - Fields::Unnamed(ref f) => f.unnamed.iter(), - Fields::Unit => unreachable!(), - }; + // enum ... + Data::Enum(_) => { + return syn::Error::new(Span::call_site(), "enums are not yet implemented") + .into_compile_error() + } - let iter = fields.enumerate().map(|(i, field)| { - let mut name = None; - let mut get_fn = None; - let mut get_mut_fn = None; - let mut set_fn = None; - let mut readonly = false; - let mut skip = false; + // union ... + Data::Union(_) => { + return syn::Error::new(Span::call_site(), "unions are not yet supported") + .into_compile_error() + } + }; - for attr in field.attrs.iter().filter(|a| a.path().is_ident(ATTR_ROOT)) { - let config_list: Result, _> = - attr.parse_args_with(Punctuated::parse_terminated); + quote! { + impl CustomType for #type_name { + fn build(mut builder: TypeBuilder) { + #(#errors)* + builder.with_name(#display_name); + #(#field_accessors)* + #(#extras(&mut builder);)* + } + } + } +} - let list = match config_list { - Ok(list) => list, - Err(err) => { - errors.push(err.into_compile_error()); - continue; - } - }; +fn scan_fields(fields: &[&Field], accessors: &mut Vec, errors: &mut Vec) { + for (i, &field) in fields.iter().enumerate() { + let mut map_name = None; + let mut get_fn = None; + let mut get_mut_fn = None; + let mut set_fn = None; + let mut readonly = false; + let mut skip = false; - for expr in list { - let ident = match expr { - // skip - syn::Expr::Path(path) if path.path.is_ident(ATTR_SKIP) => { - skip = true; + for attr in field.attrs.iter().filter(|a| a.path().is_ident(ATTR)) { + let options_list: Result, _> = + attr.parse_args_with(Punctuated::parse_terminated); - // `skip` cannot be used with any other attributes. - if get_fn.is_some() - || get_mut_fn.is_some() - || set_fn.is_some() - || name.is_some() - || readonly - { - let msg = - format!("cannot use '{ATTR_SKIP}' with other attributes"); - errors.push( - syn::Error::new(path.span(), msg).into_compile_error(), - ); - } + let options = match options_list { + Ok(list) => list, + Err(err) => { + errors.push(err.into_compile_error()); + continue; + } + }; - continue; - } - // readonly - syn::Expr::Path(path) if path.path.is_ident(ATTR_READONLY) => { - readonly = true; + for expr in options { + let ident = match expr { + // skip + Expr::Path(path) if path.path.is_ident(OPTION_SKIP) => { + skip = true; - if set_fn.is_some() { - let msg = - format!("cannot use '{ATTR_READONLY}' with '{ATTR_SET}'"); - errors.push( - syn::Error::new(path.path.span(), msg).into_compile_error(), - ); - } + // `skip` cannot be used with any other attributes. + if get_fn.is_some() + || get_mut_fn.is_some() + || set_fn.is_some() + || map_name.is_some() + || readonly + { + let msg = format!("cannot use '{OPTION_SKIP}' with other attributes"); + errors.push(syn::Error::new(path.span(), msg).into_compile_error()); + } - path.path.get_ident().unwrap().clone() - } - // Key-value - syn::Expr::Assign(..) => { - let MetaNameValue { path, value, .. } = - syn::parse2::(expr.to_token_stream()).unwrap(); + continue; + } + // readonly + Expr::Path(path) if path.path.is_ident(OPTION_READONLY) => { + readonly = true; - if path.is_ident(ATTR_NAME) { - // Type name - name = Some(value.to_token_stream()); - } else if path.is_ident(ATTR_GET) { - match syn::parse2::(value.to_token_stream()) { - Ok(path) => get_fn = Some(path.to_token_stream()), - Err(err) => errors.push(err.into_compile_error()), - } - } else if path.is_ident(ATTR_GET_MUT) { - match syn::parse2::(value.to_token_stream()) { - Ok(path) => get_mut_fn = Some(path.to_token_stream()), - Err(err) => errors.push(err.into_compile_error()), - } - } else if path.is_ident(ATTR_SET) { - match syn::parse2::(value.to_token_stream()) { - Ok(path) => set_fn = Some(path.to_token_stream()), - Err(err) => errors.push(err.into_compile_error()), - } - } else if path.is_ident(ATTR_SKIP) || path.is_ident(ATTR_READONLY) { - let key = path.get_ident().unwrap().to_string(); - let msg = format!("'{key}' cannot have value"); - errors.push( - syn::Error::new(path.span(), msg).into_compile_error(), - ); - continue; - } else { - let key = path.get_ident().unwrap().to_string(); - let msg = format!("invalid option: '{key}'"); - errors.push( - syn::Error::new(path.span(), msg).into_compile_error(), - ); - continue; - } + if set_fn.is_some() { + let msg = format!("cannot use '{OPTION_READONLY}' with '{OPTION_SET}'"); + errors + .push(syn::Error::new(path.path.span(), msg).into_compile_error()); + } - path.get_ident().unwrap().clone() + path.path.get_ident().unwrap().clone() + } + // Key-value + Expr::Assign(..) => { + let MetaNameValue { path, value, .. } = + syn::parse2::(expr.to_token_stream()).unwrap(); + + if path.is_ident(OPTION_NAME) { + // Type name + map_name = Some(value.to_token_stream()); + } else if path.is_ident(OPTION_GET) { + match syn::parse2::(value.to_token_stream()) { + Ok(path) => get_fn = Some(path.to_token_stream()), + Err(err) => errors.push(err.into_compile_error()), } - // any other identifier - syn::Expr::Path(path) if path.path.get_ident().is_some() => { - let key = path.path.get_ident().unwrap().to_string(); - let msg = format!("invalid option: '{key}'"); - errors.push(syn::Error::new(path.span(), msg).into_compile_error()); - continue; + } else if path.is_ident(OPTION_GET_MUT) { + match syn::parse2::(value.to_token_stream()) { + Ok(path) => get_mut_fn = Some(path.to_token_stream()), + Err(err) => errors.push(err.into_compile_error()), } - // Error - _ => { - errors.push( - syn::Error::new(expr.span(), "expecting identifier") - .into_compile_error(), - ); - continue; + } else if path.is_ident(OPTION_SET) { + match syn::parse2::(value.to_token_stream()) { + Ok(path) => set_fn = Some(path.to_token_stream()), + Err(err) => errors.push(err.into_compile_error()), } - }; - - if skip { - let msg = format!("cannot use '{ident}' with '{ATTR_SKIP}'"); - errors.push( - syn::Error::new(attr.path().span(), msg).into_compile_error(), - ); + } else if path.is_ident(OPTION_SKIP) || path.is_ident(OPTION_READONLY) { + let key = path.get_ident().unwrap().to_string(); + let msg = format!("'{key}' cannot have value"); + errors.push(syn::Error::new(path.span(), msg).into_compile_error()); + continue; + } else { + let key = path.get_ident().unwrap().to_string(); + let msg = format!("invalid option: '{key}'"); + errors.push(syn::Error::new(path.span(), msg).into_compile_error()); + continue; } - } - } - if !skip { - let field_name = if let Some(ref field_name) = field.ident { - quote! { #field_name } - } else { - if name.is_none() { - let map_name = format!("field{i}"); - name = Some(quote! { #map_name }); - } - let index = proc_macro2::Literal::usize_unsuffixed(i); - quote! { #index } - }; + path.get_ident().unwrap().clone() + } + // any other identifier + Expr::Path(path) if path.path.get_ident().is_some() => { + let key = path.path.get_ident().unwrap().to_string(); + let msg = format!("invalid option: '{key}'"); + errors.push(syn::Error::new(path.span(), msg).into_compile_error()); + continue; + } + // Error + _ => { + errors.push( + syn::Error::new(expr.span(), "expecting identifier") + .into_compile_error(), + ); + continue; + } + }; - generate_accessor_fns(field_name, name, get_fn, get_mut_fn, set_fn, readonly) - } else { - quote! {} + if skip { + let msg = format!("cannot use '{ident}' with '{OPTION_SKIP}'"); + errors.push(syn::Error::new(attr.path().span(), msg).into_compile_error()); } - }); - - quote! { #(#iter;)* } + } } - syn::Data::Enum(_) => { - return syn::Error::new(Span::call_site(), "enums are not yet implemented") - .into_compile_error() + // If skipped don't do anything. + if skip { + continue; } - syn::Data::Union(_) => { - return syn::Error::new(Span::call_site(), "unions are not yet supported") - .into_compile_error() - } - }; - quote! { - impl CustomType for #type_name { - fn build(mut builder: TypeBuilder) { - #(#errors;)* - builder.with_name(#pretty_print_name); - #accessors; - #(#extras(&mut builder);)* + // No field name - use field0, field1... + let field_name = if let Some(ref field_name) = field.ident { + quote! { #field_name } + } else { + if map_name.is_none() { + let name = format!("field{i}"); + map_name = Some(quote! { #name }); } - } - } -} + let index = proc_macro2::Literal::usize_unsuffixed(i); + quote! { #index } + }; -/// Generate a `TypeBuilder` accessor function. -fn generate_accessor_fns( - field: TokenStream, - name: Option, - get: Option, - get_mut: Option, - set: Option, - readonly: bool, -) -> proc_macro2::TokenStream { - let get = match (get_mut, get) { - (Some(func), _) => func, - (None, Some(func)) => quote! { |obj: &mut Self| #func(&*obj) }, - (None, None) => quote! { |obj: &mut Self| obj.#field.clone() }, - }; + // Override functions + let get = match (get_mut_fn, get_fn) { + (Some(func), _) => func, + (None, Some(func)) => quote! { |obj: &mut Self| #func(&*obj) }, + (None, None) => quote! { |obj: &mut Self| obj.#field_name.clone() }, + }; - let set = set.unwrap_or_else(|| quote! { |obj: &mut Self, val| obj.#field = val }); - let name = name.unwrap_or_else(|| quote! { stringify!(#field) }); + let set = set_fn.unwrap_or_else(|| quote! { |obj: &mut Self, val| obj.#field_name = val }); + let name = map_name.unwrap_or_else(|| quote! { stringify!(#field_name) }); - if readonly { - quote! { builder.with_get(#name, #get) } - } else { - quote! { builder.with_get_set(#name, #get, #set) } + accessors.push(if readonly { + quote! { builder.with_get(#name, #get); } + } else { + quote! { builder.with_get_set(#name, #get, #set); } + }); } } diff --git a/codegen/src/test/custom_type.rs b/codegen/src/test/custom_type.rs index db28a78c8..4853fc70d 100644 --- a/codegen/src/test/custom_type.rs +++ b/codegen/src/test/custom_type.rs @@ -26,7 +26,6 @@ mod custom_type_tests { impl CustomType for Bar { fn build(mut builder: TypeBuilder) { builder.with_name(stringify!(Bar)); - ; builder.with_get_set("field1", |obj: &mut Self| obj.1.clone(), |obj: &mut Self, val| obj.1 = val @@ -36,7 +35,6 @@ mod custom_type_tests { |obj: &mut Self| obj.3.clone(), |obj: &mut Self, val| obj.3 = val ); - ; } } }; @@ -70,7 +68,6 @@ mod custom_type_tests { impl CustomType for Foo { fn build(mut builder: TypeBuilder) { builder.with_name("MyFoo"); - ; builder.with_get_set(stringify!(bar), |obj: &mut Self| get_bar(&*obj), |obj: &mut Self, val| obj.bar = val @@ -80,7 +77,6 @@ mod custom_type_tests { |obj: &mut Self| obj.qux.clone(), Self::set_qux ); - ; Self::build_extra(&mut builder); } } From c7e1021655d9e9956f5a55ec612cad1457581099 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 30 Jan 2024 13:44:54 +0800 Subject: [PATCH 02/11] Non-volatile by default. --- CHANGELOG.md | 11 +- Cargo.toml | 2 +- src/api/register.rs | 19 ++-- src/func/register.rs | 10 +- src/module/mod.rs | 234 ++++++++++++++++++++++++++++--------------- 5 files changed, 176 insertions(+), 100 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f1a18e1c..6668a295c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,14 @@ Rhai Release Notes Version 1.17.0 ============== -In this version, fuzzing via [Google OSS-Fuzz](https://github.com/google/oss-fuzz) is used to flush -out hidden bugs and edge cases. This should result in higher code quality, better stability and -improved security. +Starting from this version, the official preferred method of registering an API for a custom type is +via the `#[derive(CustomType)]` macro. Of course the old API's are still available for types that +reside in external crates (and thus cannot implement `CustomType`). + +Starting from this version, fuzzing via [Google OSS-Fuzz](https://github.com/google/oss-fuzz) is +used to flush out hidden bugs and edge cases. This should result in higher code quality, better +stability and improved security. And indeed, a large number of bugs have been discovered from this +and fixed. Potentially breaking changes ---------------------------- diff --git a/Cargo.toml b/Cargo.toml index 615c78696..2c651d67a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -167,4 +167,4 @@ features = ["document-features", "metadata", "serde", "internals", "decimal", "d rustyline = { git = "https://github.com/schungx/rustyline", branch = "v13" } # Patch SmartString to resolve an UB issue. -smartstring = { git = "https://github.com/bodil/smartstring", ref = "refs/pull/34/head" } +#smartstring = { git = "https://github.com/bodil/smartstring", ref = "refs/pull/34/head" } diff --git a/src/api/register.rs b/src/api/register.rs index 9b25c0b39..9b35fb297 100644 --- a/src/api/register.rs +++ b/src/api/register.rs @@ -32,9 +32,11 @@ impl Engine { /// /// # Assumptions /// - /// * The function is assumed to be _pure_ unless it is a property setter or an index setter. + /// * **Accessibility**: The function namespace is [`FnNamespace::Global`]. /// - /// * The function is assumed to be _volatile_ -- i.e. it does not guarantee the same result for the same input(s). + /// * **Purity**: The function is assumed to be _pure_ unless it is a property setter or an index setter. + /// + /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). /// /// # Example /// @@ -73,8 +75,6 @@ impl Engine { name: impl AsRef + Into, func: FUNC, ) -> &mut Self { - let param_types = FUNC::param_types(); - #[cfg(feature = "metadata")] let mut param_type_names = FUNC::param_names() .iter() @@ -101,16 +101,15 @@ impl Engine { let is_pure = is_pure && (FUNC::num_params() != 2 || !fn_name.starts_with(crate::engine::FN_SET)); - let f = FuncRegistration::new(fn_name).with_namespace(FnNamespace::Global); + let f = FuncRegistration::new(fn_name) + .with_namespace(FnNamespace::Global) + .with_purity(is_pure) + .with_volatility(false); #[cfg(feature = "metadata")] let f = f.with_params_info(param_type_names); - f.set_into_module_raw( - self.global_namespace_mut(), - param_types, - func.into_callable_function(is_pure, true), - ); + f.set_into_module(self.global_namespace_mut(), func); self } diff --git a/src/func/register.rs b/src/func/register.rs index 7f6024059..f67452461 100644 --- a/src/func/register.rs +++ b/src/func/register.rs @@ -81,7 +81,7 @@ pub fn by_value(data: &mut Dynamic) -> T { pub trait RhaiNativeFunc { /// Convert this function into a [`RhaiFunc`]. #[must_use] - fn into_callable_function(self, is_pure: bool, is_volatile: bool) -> RhaiFunc; + fn into_rhai_function(self, is_pure: bool, is_volatile: bool) -> RhaiFunc; /// Get the type ID's of this function's parameters. #[must_use] fn param_types() -> [TypeId; N]; @@ -144,7 +144,7 @@ macro_rules! def_register { > RhaiNativeFunc<($($mark,)*), $n, false, RET, false> for FN { #[inline(always)] fn param_types() -> [TypeId;$n] { [$(TypeId::of::<$par>()),*] } #[cfg(feature = "metadata")] #[inline(always)] fn param_names() -> [&'static str;$n] { [$(type_name::<$param>()),*] } - #[inline(always)] fn into_callable_function(self, is_pure: bool, is_volatile: bool) -> RhaiFunc { + #[inline(always)] fn into_rhai_function(self, is_pure: bool, is_volatile: bool) -> RhaiFunc { RhaiFunc::$abi { func: Shared::new(move |_, args: &mut FnCallArgs| { // The arguments are assumed to be of the correct number and types! let mut drain = args.iter_mut(); @@ -166,7 +166,7 @@ macro_rules! def_register { > RhaiNativeFunc<($($mark,)*), $n, true, RET, false> for FN { #[inline(always)] fn param_types() -> [TypeId;$n] { [$(TypeId::of::<$par>()),*] } #[cfg(feature = "metadata")] #[inline(always)] fn param_names() -> [&'static str;$n] { [$(type_name::<$param>()),*] } - #[inline(always)] fn into_callable_function(self, is_pure: bool, is_volatile: bool) -> RhaiFunc { + #[inline(always)] fn into_rhai_function(self, is_pure: bool, is_volatile: bool) -> RhaiFunc { RhaiFunc::$abi { func: Shared::new(move |ctx: Option, args: &mut FnCallArgs| { let ctx = ctx.unwrap(); @@ -191,7 +191,7 @@ macro_rules! def_register { #[inline(always)] fn param_types() -> [TypeId;$n] { [$(TypeId::of::<$par>()),*] } #[cfg(feature = "metadata")] #[inline(always)] fn param_names() -> [&'static str;$n] { [$(type_name::<$param>()),*] } #[cfg(feature = "metadata")] #[inline(always)] fn return_type_name() -> &'static str { type_name::>() } - #[inline(always)] fn into_callable_function(self, is_pure: bool, is_volatile: bool) -> RhaiFunc { + #[inline(always)] fn into_rhai_function(self, is_pure: bool, is_volatile: bool) -> RhaiFunc { RhaiFunc::$abi { func: Shared::new(move |_, args: &mut FnCallArgs| { // The arguments are assumed to be of the correct number and types! let mut drain = args.iter_mut(); @@ -211,7 +211,7 @@ macro_rules! def_register { #[inline(always)] fn param_types() -> [TypeId;$n] { [$(TypeId::of::<$par>()),*] } #[cfg(feature = "metadata")] #[inline(always)] fn param_names() -> [&'static str;$n] { [$(type_name::<$param>()),*] } #[cfg(feature = "metadata")] #[inline(always)] fn return_type_name() -> &'static str { type_name::>() } - #[inline(always)] fn into_callable_function(self, is_pure: bool, is_volatile: bool) -> RhaiFunc { + #[inline(always)] fn into_rhai_function(self, is_pure: bool, is_volatile: bool) -> RhaiFunc { RhaiFunc::$abi { func: Shared::new(move |ctx: Option, args: &mut FnCallArgs| { let ctx = ctx.unwrap(); diff --git a/src/module/mod.rs b/src/module/mod.rs index 4a01c9e6e..2934d9831 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -167,7 +167,12 @@ pub fn calc_native_fn_hash<'a>( /// Type for fine-tuned module function registration. #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub struct FuncRegistration { + /// Function metadata. metadata: FuncMetadata, + /// Is the function pure? + purity: Option, + /// Is the function volatile? + volatility: Option, } impl FuncRegistration { @@ -203,6 +208,8 @@ impl FuncRegistration { #[cfg(feature = "metadata")] comments: <_>::default(), }, + purity: None, + volatility: None, } } /// Set the [namespace][`FnNamespace`] of the function. @@ -210,8 +217,31 @@ impl FuncRegistration { self.metadata.namespace = namespace; self } + /// Set whether the function is _pure_. + /// A pure function has no side effects. + pub fn with_purity(mut self, pure: bool) -> Self { + self.purity = Some(pure); + self + } + /// Set whether the function is _volatile_. + /// A volatile function does not guarantee the same result for the same input(s). + pub fn with_volatility(mut self, volatile: bool) -> Self { + self.volatility = Some(volatile); + self + } /// _(metadata)_ Set the function's parameter names and/or types. /// Exported under the `metadata` feature only. + /// + /// The input is a list of strings, each of which is either a parameter name or a parameter name/type pair. + /// + /// The _last_ string should be the _type_ of the return value. + /// + /// # Parameter Examples + /// + /// `"foo: &str"` <- parameter name = `foo`, type = `&str` + /// `"bar"` <- parameter name = `bar`, type unknown + /// `"_: i64"` <- parameter name unknown, type = `i64` + /// `"MyType"` <- parameter name unknown, type = `MyType` #[cfg(feature = "metadata")] pub fn with_params_info>(mut self, params: impl IntoIterator) -> Self { self.metadata.params_info = params.into_iter().map(|s| s.as_ref().into()).collect(); @@ -219,6 +249,22 @@ impl FuncRegistration { } /// _(metadata)_ Set the function's doc-comments. /// Exported under the `metadata` feature only. + /// + /// The input is a list of strings, each of which is either a block of single-line doc-comments + /// or a single block doc-comment. + /// + /// ## Comments + /// + /// Single-line doc-comments typically start with `///` and should be merged, with line-breaks, + /// into a single string without a final termination line-break. + /// + /// Block doc-comments typically start with `/**` and end with `*/` and should be kept in a + /// separate string. + /// + /// Leading white-spaces should be stripped, and each string should always start with the + /// corresponding doc-comment leader: `///` or `/**`. + /// + /// Each line in non-block doc-comments should start with `///`. #[cfg(feature = "metadata")] pub fn with_comments>(mut self, comments: impl IntoIterator) -> Self { self.metadata.comments = comments.into_iter().map(|s| s.as_ref().into()).collect(); @@ -228,9 +274,9 @@ impl FuncRegistration { /// /// # Assumptions /// - /// * The function is assumed to be _pure_ (so it can be called on constants) unless it is a property setter or an index setter. + /// * **Purity**: The function is assumed to be _pure_ (so it can be called on constants) unless it is a property setter or an index setter. /// - /// * The function is assumed to be _volatile_ -- i.e. it does not guarantee the same result for the same input(s). + /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). #[inline] pub fn set_into_module( self, @@ -241,19 +287,37 @@ impl FuncRegistration { R: Variant + Clone, FUNC: RhaiNativeFunc + SendSync + 'static, { - let is_pure = true; + let is_pure = self.purity.unwrap_or_else(|| { + // default to pure unless specified + let is_pure = true; + + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] + let is_pure = is_pure + && (FUNC::num_params() != 3 || self.metadata.name != crate::engine::FN_IDX_SET); + + #[cfg(not(feature = "no_object"))] + let is_pure = is_pure + && (FUNC::num_params() != 2 + || !self.metadata.name.starts_with(crate::engine::FN_SET)); + is_pure + }); + let is_volatile = self.volatility.unwrap_or(false); - #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] - let is_pure = - is_pure && (FUNC::num_params() != 3 || self.metadata.name != crate::engine::FN_IDX_SET); - #[cfg(not(feature = "no_object"))] - let is_pure = is_pure - && (FUNC::num_params() != 2 || !self.metadata.name.starts_with(crate::engine::FN_SET)); + let func = func.into_rhai_function(is_pure, is_volatile); + + // Clear flags + let mut reg = self; + reg.purity = None; + reg.volatility = None; - let func = func.into_callable_function(is_pure, true); - self.set_into_module_raw(module, FUNC::param_types(), func) + reg.set_into_module_raw(module, FUNC::param_types(), func) } /// Register the function into the specified [`Module`]. + /// + /// # WARNING - Low Level API + /// + /// This function is very low level. It takes a list of [`TypeId`][std::any::TypeId]'s + /// indicating the actual types of the parameters. #[inline] pub fn set_into_module_raw( self, @@ -261,6 +325,10 @@ impl FuncRegistration { arg_types: impl AsRef<[TypeId]>, func: RhaiFunc, ) -> &FuncMetadata { + // Make sure that conflicting flags should not be set. + debug_assert!(self.purity.is_none()); + debug_assert!(self.volatility.is_none()); + let mut f = self.metadata; f.num_params = arg_types.as_ref().len(); @@ -835,7 +903,7 @@ impl Module { /// let mut module = Module::new(); /// assert!(module.is_indexed()); /// - /// module.set_native_fn("foo", |x: &mut i64, y: i64| { *x = y; Ok(()) }); + /// module.set_native_fn("foo", |x: &mut i64, y: i64| *x = y); /// assert!(!module.is_indexed()); /// /// # #[cfg(not(feature = "no_module"))] @@ -1131,7 +1199,7 @@ impl Module { /// ``` /// # use rhai::Module; /// let mut module = Module::new(); - /// let hash = module.set_native_fn("calc", || Ok(42_i64)); + /// let hash = module.set_native_fn("calc", || 42_i64); /// assert!(module.contains_fn(hash)); /// ``` #[inline] @@ -1145,8 +1213,6 @@ impl Module { /// _(metadata)_ Update the metadata (parameter names/types, return type and doc-comments) of a registered function. /// Exported under the `metadata` feature only. /// - /// The [`u64`] hash is returned by the [`set_native_fn`][Module::set_native_fn] call. - /// /// # Deprecated /// /// This method is deprecated. @@ -1184,8 +1250,6 @@ impl Module { /// Update the namespace of a registered function. /// - /// The [`u64`] hash is returned by the [`set_native_fn`][Module::set_native_fn] call. - /// /// # Deprecated /// /// This method is deprecated. @@ -1223,6 +1287,11 @@ impl Module { } /// Set a native Rust function into the [`Module`] based on a [`FuncRegistration`]. + /// + /// # WARNING - Low Level API + /// + /// This function is very low level. It takes a list of [`TypeId`][std::any::TypeId]'s + /// indicating the actual types of the parameters. #[inline(always)] pub fn set_fn_raw_with_options( &mut self, @@ -1239,22 +1308,22 @@ impl Module { /// /// # Assumptions /// - /// * The function is assumed to be _pure_ unless it is a property setter or an index setter. + /// * **Accessibility**: The function namespace is [`FnNamespace::Internal`]. /// - /// * The function is assumed to be _volatile_ -- i.e. it does not guarantee the same result for the same input(s). + /// * **Purity**: The function is assumed to be _pure_ unless it is a property setter or an index setter. /// - /// * The function namespace is [`FnNamespace::Internal`]. + /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). /// - /// * No metadata for the function is registered. + /// * **Metadata**: No metadata for the function is registered. /// - /// To change this, use the [`FuncRegistration`] API instead. + /// To change these assumptions, use the [`FuncRegistration`] API instead. /// /// # Example /// /// ``` /// # use rhai::Module; /// let mut module = Module::new(); - /// let hash = module.set_native_fn("calc", || Ok(42_i64)); + /// let hash = module.set_native_fn("calc", |x: i64| 42 + x); /// assert!(module.contains_fn(hash)); /// ``` #[inline] @@ -1267,7 +1336,12 @@ impl Module { R: Variant + Clone, FUNC: RhaiNativeFunc + SendSync + 'static, { - FuncRegistration::new(name).set_into_module(self, func).hash + FuncRegistration::new(name) + .with_namespace(FnNamespace::Internal) + .with_purity(true) + .with_volatility(false) + .set_into_module(self, func) + .hash } /// Set a Rust getter function taking one mutable parameter, returning a [`u64`] hash key. @@ -1277,20 +1351,22 @@ impl Module { /// /// # Assumptions /// - /// * The function is assumed to be _volatile_ -- i.e. it does not guarantee the same result for the same input(s). + /// * **Accessibility**: The function namespace is [`FnNamespace::Global`]. + /// + /// * **Purity**: The function is assumed to be _pure_ (so it can be called on constants). /// - /// # Function Metadata + /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). /// - /// No metadata for the function is registered. + /// * **Metadata**: No metadata for the function is registered. /// - /// Use the [`FuncRegistration`] API instead to add metadata. + /// To change these assumptions, use the [`FuncRegistration`] API instead. /// /// # Example /// /// ``` /// # use rhai::Module; /// let mut module = Module::new(); - /// let hash = module.set_getter_fn("value", |x: &mut i64| { Ok(*x) }); + /// let hash = module.set_getter_fn("value", |x: &mut i64| *x); /// assert!(module.contains_fn(hash)); /// ``` #[cfg(not(feature = "no_object"))] @@ -1307,6 +1383,8 @@ impl Module { { FuncRegistration::new(crate::engine::make_getter(name.as_ref())) .with_namespace(FnNamespace::Global) + .with_purity(true) + .with_volatility(false) .set_into_module(self, func) .hash } @@ -1319,13 +1397,15 @@ impl Module { /// /// # Assumptions /// - /// * The function is assumed to be _volatile_ -- i.e. it does not guarantee the same result for the same input(s). + /// * **Accessibility**: The function namespace is [`FnNamespace::Global`]. /// - /// # Function Metadata + /// * **Purity**: The function is assumed to be _non-pure_ (so it cannot be called on constants). /// - /// No metadata for the function is registered. + /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). /// - /// Use the [`FuncRegistration`] API instead to add metadata. + /// * **Metadata**: No metadata for the function is registered. + /// + /// To change these assumptions, use the [`FuncRegistration`] API instead. /// /// # Example /// @@ -1333,10 +1413,9 @@ impl Module { /// use rhai::{Module, ImmutableString}; /// /// let mut module = Module::new(); - /// let hash = module.set_setter_fn("value", |x: &mut i64, y: ImmutableString| { - /// *x = y.len() as i64; - /// Ok(()) - /// }); + /// let hash = module.set_setter_fn("value", + /// |x: &mut i64, y: ImmutableString| *x = y.len() as i64 + /// ); /// assert!(module.contains_fn(hash)); /// ``` #[cfg(not(feature = "no_object"))] @@ -1353,6 +1432,8 @@ impl Module { { FuncRegistration::new(crate::engine::make_setter(name.as_ref())) .with_namespace(FnNamespace::Global) + .with_purity(false) + .with_volatility(false) .set_into_module(self, func) .hash } @@ -1364,11 +1445,7 @@ impl Module { /// /// If there are similar existing Rust functions, they are replaced. /// - /// # Function Metadata - /// - /// No metadata for the function is registered. - /// - /// Use the [`FuncRegistration`] API instead to add metadata. + /// To change these assumptions, use the [`FuncRegistration`] API instead. /// /// # Example /// @@ -1376,13 +1453,11 @@ impl Module { /// use rhai::{Module, ImmutableString}; /// /// let mut module = Module::new(); - /// let (hash_get, hash_set) = module.set_getter_setter_fn("value", - /// |x: &mut i64| { Ok(x.to_string().into()) }, - /// |x: &mut i64, y: ImmutableString| { - /// *x = y.len() as i64; - /// Ok(()) - /// } - /// ); + /// let (hash_get, hash_set) = + /// module.set_getter_setter_fn("value", + /// |x: &mut i64| x.to_string().into(), + /// |x: &mut i64, y: ImmutableString| *x = y.len() as i64 + /// ); /// assert!(module.contains_fn(hash_get)); /// assert!(module.contains_fn(hash_set)); /// ``` @@ -1413,18 +1488,20 @@ impl Module { /// /// # Assumptions /// - /// * The function is assumed to be _volatile_ -- i.e. it does not guarantee the same result for the same input(s). + /// * **Accessibility**: The function namespace is [`FnNamespace::Global`]. /// - /// # Panics + /// * **Purity**: The function is assumed to be _pure_ (so it can be called on constants). /// - /// Panics if the type is [`Array`][crate::Array] or [`Map`][crate::Map]. - /// Indexers for arrays, object maps and strings cannot be registered. + /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). + /// + /// * **Metadata**: No metadata for the function is registered. /// - /// # Function Metadata + /// To change these assumptions, use the [`FuncRegistration`] API instead. /// - /// No metadata for the function is registered. + /// # Panics /// - /// Use the [`FuncRegistration`] API instead to add metadata. + /// Panics if the type is [`Array`][crate::Array] or [`Map`][crate::Map]. + /// Indexers for arrays, object maps and strings cannot be registered. /// /// # Example /// @@ -1432,9 +1509,9 @@ impl Module { /// use rhai::{Module, ImmutableString}; /// /// let mut module = Module::new(); - /// let hash = module.set_indexer_get_fn(|x: &mut i64, y: ImmutableString| { - /// Ok(*x + y.len() as i64) - /// }); + /// let hash = module.set_indexer_get_fn( + /// |x: &mut i64, y: ImmutableString| *x + y.len() as i64 + /// ); /// assert!(module.contains_fn(hash)); /// ``` #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] @@ -1466,6 +1543,8 @@ impl Module { FuncRegistration::new(crate::engine::FN_IDX_GET) .with_namespace(FnNamespace::Global) + .with_purity(true) + .with_volatility(false) .set_into_module(self, func) .hash } @@ -1478,18 +1557,21 @@ impl Module { /// /// # Assumptions /// - /// * The function is assumed to be _volatile_ -- i.e. it does not guarantee the same result for the same input(s). + /// * **Accessibility**: The function namespace is [`FnNamespace::Global`]. + /// + /// * **Purity**: The function is assumed to be _non-pure_ (so it cannot be called on constants). + /// + /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). /// /// # Panics /// /// Panics if the type is [`Array`][crate::Array] or [`Map`][crate::Map]. /// Indexers for arrays, object maps and strings cannot be registered. /// - /// # Function Metadata - /// - /// No metadata for the function is registered. + /// # Panics /// - /// Use the [`FuncRegistration`] API instead to add metadata. + /// Panics if the type is [`Array`][crate::Array] or [`Map`][crate::Map]. + /// Indexers for arrays, object maps and strings cannot be registered. /// /// # Example /// @@ -1497,9 +1579,9 @@ impl Module { /// use rhai::{Module, ImmutableString}; /// /// let mut module = Module::new(); - /// let hash = module.set_indexer_set_fn(|x: &mut i64, y: ImmutableString, value: i64| { - /// *x = y.len() as i64 + value; Ok(()) - /// }); + /// let hash = module.set_indexer_set_fn(|x: &mut i64, y: ImmutableString, value: i64| + /// *x = y.len() as i64 + value + /// ); /// assert!(module.contains_fn(hash)); /// ``` #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] @@ -1531,6 +1613,8 @@ impl Module { FuncRegistration::new(crate::engine::FN_IDX_SET) .with_namespace(FnNamespace::Global) + .with_purity(false) + .with_volatility(false) .set_into_module(self, func) .hash } @@ -1548,12 +1632,6 @@ impl Module { /// Panics if the type is [`Array`][crate::Array] or [`Map`][crate::Map]. /// Indexers for arrays, object maps and strings cannot be registered. /// - /// # Function Metadata - /// - /// No metadata for the function is registered. - /// - /// Use the [`FuncRegistration`] API instead to add metadata. - /// /// # Example /// /// ``` @@ -1561,12 +1639,8 @@ impl Module { /// /// let mut module = Module::new(); /// let (hash_get, hash_set) = module.set_indexer_get_set_fn( - /// |x: &mut i64, y: ImmutableString| { - /// Ok(*x + y.len() as i64) - /// }, - /// |x: &mut i64, y: ImmutableString, value: i64| { - /// *x = y.len() as i64 + value; Ok(()) - /// } + /// |x: &mut i64, y: ImmutableString| *x + y.len() as i64, + /// |x: &mut i64, y: ImmutableString, value: i64| *x = y.len() as i64 + value /// ); /// assert!(module.contains_fn(hash_get)); /// assert!(module.contains_fn(hash_set)); @@ -1591,8 +1665,6 @@ impl Module { } /// Look up a native Rust function by hash. - /// - /// The [`u64`] hash is returned by the [`set_native_fn`][Module::set_native_fn] call. #[inline] #[must_use] pub(crate) fn get_fn(&self, hash_native: u64) -> Option<&RhaiFunc> { From b5ee5df383a462a7bb12861ca78e0bde26c10f21 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 30 Jan 2024 14:09:15 +0800 Subject: [PATCH 03/11] Fix tests. --- CHANGELOG.md | 26 +++++++++++++++----------- src/module/mod.rs | 46 +++++++++++++++++++++++++++++++--------------- 2 files changed, 46 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6668a295c..ef3a7b78f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,11 @@ Version 1.17.0 ============== Starting from this version, the official preferred method of registering an API for a custom type is -via the `#[derive(CustomType)]` macro. Of course the old API's are still available for types that -reside in external crates (and thus cannot implement `CustomType`). +via the `#[derive(CustomType)]` macro. The old API is still available for types that reside in +external crates (and thus cannot implement `CustomType`). + +Starting from this version, the new `FuncRegistration` API is preferred for registering native Rust +functions into a `Module`. The old API is still available but deprecated. Starting from this version, fuzzing via [Google OSS-Fuzz](https://github.com/google/oss-fuzz) is used to flush out hidden bugs and edge cases. This should result in higher code quality, better @@ -21,11 +24,21 @@ Potentially breaking changes * `EvalContext::new`, `FloatWrapper` and `ConditionalExpr` are now gated under `internals`. * Previously, Rhai follows [Unicode's definition for _whitespace_](https://en.wikipedia.org/wiki/Template:Whitespace_(Unicode)), which allows many exotic whitespace characters in scripts. Starting from this version, whitespace follows [WhatWG](https://infra.spec.whatwg.org/#ascii-whitespace)'s definition of five ASCII characters (TAB, SPACE, CR, LF and FF), which is the same as Rust. All other Unicode whitespace characters (not inside strings) are not considered whitespace by Rhai. If a script used to contain non-ASCII whitespace characters, it now fails to parse with a syntax error. +New features +------------ + +* `#[derive(CustomType)]` is now available, driven by procedural macros in `rhai_codegen`. +* A new `FuncRegistration` API is added to assist in registering native Rust functions into modules with various settings. Some of the original `Module::set_fn...` API is now deprecated. +* Functions defined in plugin modules can now be marked as `volatile` which prevents it from being optimized away even under `OptimizationLevel::Full`. +* Added `Engine::max_functions` and `Engine::set_max_functions` to limit the maximum number of functions allowed in a script. This is to guard against DOS attacks -- e.g. a simple closure `||` (two characters) is a function. When `max_function` is exceeded during script compilation, a new parse error, `ParseErrorType::TooManyFunctions`, is returned. +* `Engine::get_interned_string` is made public instead of gated under `internals`. + Deprecated API's ---------------- * `rhai::config::hashing::set_ahash_seed`, `rhai::config::hashing::get_ahash_seed` and the `RHAI_AHASH_SEED` environment variable are deprecated in favor of `rhai::config::hashing::set_hashing_seed`, `rhai::config::hashing::get_hashing_seed` and `RHAI_HASHING_SEED`. * `AST::clear_doc` is deprecated. +* Much of the `Module::update_XXX` API is deprecated in favor of using the `FuncRegistration` API. Fixes to bugs found via fuzzing ------------------------------- @@ -51,15 +64,6 @@ Other bug fixes * Arrays in object maps now serialize to JSON correctly via `to_json()` when the `serde` feature is not enabled. * `Engine::format_map_as_json` now serializes arrays correctly. -New features ------------- - -* `#[derive(CustomType)]` is now available, driven by procedural macros in `rhai_codegen`. -* A new `FuncRegistration` API is added to assist in registering native Rust functions into modules with various settings. Some of the original `Module::set_fn...` API is now deprecated. -* Functions defined in plugin modules can now be marked as `volatile` which prevents it from being optimized away even under `OptimizationLevel::Full`. -* Added `Engine::max_functions` and `Engine::set_max_functions` to limit the maximum number of functions allowed in a script. This is to guard against DOS attacks -- e.g. a simple closure `||` (two characters) is a function. When `max_function` is exceeded during script compilation, a new parse error, `ParseErrorType::TooManyFunctions`, is returned. -* `Engine::get_interned_string` is made public instead of gated under `internals`. - Enhancements ------------ diff --git a/src/module/mod.rs b/src/module/mod.rs index 2934d9831..31f2f99a8 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -903,7 +903,7 @@ impl Module { /// let mut module = Module::new(); /// assert!(module.is_indexed()); /// - /// module.set_native_fn("foo", |x: &mut i64, y: i64| *x = y); + /// module.set_native_fn("foo", |x: &mut i64, y: i64| { *x = y; Ok(()) }); /// assert!(!module.is_indexed()); /// /// # #[cfg(not(feature = "no_module"))] @@ -1199,7 +1199,7 @@ impl Module { /// ``` /// # use rhai::Module; /// let mut module = Module::new(); - /// let hash = module.set_native_fn("calc", || 42_i64); + /// let hash = module.set_native_fn("calc", |x: i64| Ok(42 + x)); /// assert!(module.contains_fn(hash)); /// ``` #[inline] @@ -1306,6 +1306,21 @@ impl Module { /// /// If there is a similar existing Rust function, it is replaced. /// + /// # Use `FuncRegistration` API + /// + /// It is recommended that the [`FuncRegistration`] API be used instead. + /// + /// Essentially, this method is a shortcut for: + /// + /// ```text + /// FuncRegistration::new(name) + /// .with_namespace(FnNamespace::Internal) + /// .with_purity(true) + /// .with_volatility(false) + /// .set_into_module(module, func) + /// .hash + /// ``` + /// /// # Assumptions /// /// * **Accessibility**: The function namespace is [`FnNamespace::Internal`]. @@ -1323,7 +1338,7 @@ impl Module { /// ``` /// # use rhai::Module; /// let mut module = Module::new(); - /// let hash = module.set_native_fn("calc", |x: i64| 42 + x); + /// let hash = module.set_native_fn("calc", |x: i64| Ok(42 + x)); /// assert!(module.contains_fn(hash)); /// ``` #[inline] @@ -1366,7 +1381,7 @@ impl Module { /// ``` /// # use rhai::Module; /// let mut module = Module::new(); - /// let hash = module.set_getter_fn("value", |x: &mut i64| *x); + /// let hash = module.set_getter_fn("value", |x: &mut i64| Ok(*x)); /// assert!(module.contains_fn(hash)); /// ``` #[cfg(not(feature = "no_object"))] @@ -1413,9 +1428,9 @@ impl Module { /// use rhai::{Module, ImmutableString}; /// /// let mut module = Module::new(); - /// let hash = module.set_setter_fn("value", - /// |x: &mut i64, y: ImmutableString| *x = y.len() as i64 - /// ); + /// let hash = module.set_setter_fn("value", |x: &mut i64, y: ImmutableString| { + /// *x = y.len() as i64; Ok(()) + /// }); /// assert!(module.contains_fn(hash)); /// ``` #[cfg(not(feature = "no_object"))] @@ -1455,8 +1470,8 @@ impl Module { /// let mut module = Module::new(); /// let (hash_get, hash_set) = /// module.set_getter_setter_fn("value", - /// |x: &mut i64| x.to_string().into(), - /// |x: &mut i64, y: ImmutableString| *x = y.len() as i64 + /// |x: &mut i64| Ok(x.to_string().into()), + /// |x: &mut i64, y: ImmutableString| { *x = y.len() as i64; Ok(()) } /// ); /// assert!(module.contains_fn(hash_get)); /// assert!(module.contains_fn(hash_set)); @@ -1510,7 +1525,7 @@ impl Module { /// /// let mut module = Module::new(); /// let hash = module.set_indexer_get_fn( - /// |x: &mut i64, y: ImmutableString| *x + y.len() as i64 + /// |x: &mut i64, y: ImmutableString| Ok(*x + y.len() as i64) /// ); /// assert!(module.contains_fn(hash)); /// ``` @@ -1579,9 +1594,10 @@ impl Module { /// use rhai::{Module, ImmutableString}; /// /// let mut module = Module::new(); - /// let hash = module.set_indexer_set_fn(|x: &mut i64, y: ImmutableString, value: i64| - /// *x = y.len() as i64 + value - /// ); + /// let hash = module.set_indexer_set_fn(|x: &mut i64, y: ImmutableString, value: i64| { + /// *x = y.len() as i64 + value; + /// Ok(()) + /// }); /// assert!(module.contains_fn(hash)); /// ``` #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] @@ -1639,8 +1655,8 @@ impl Module { /// /// let mut module = Module::new(); /// let (hash_get, hash_set) = module.set_indexer_get_set_fn( - /// |x: &mut i64, y: ImmutableString| *x + y.len() as i64, - /// |x: &mut i64, y: ImmutableString, value: i64| *x = y.len() as i64 + value + /// |x: &mut i64, y: ImmutableString| Ok(*x + y.len() as i64), + /// |x: &mut i64, y: ImmutableString, value: i64| { *x = y.len() as i64 + value; Ok(()) } /// ); /// assert!(module.contains_fn(hash_get)); /// assert!(module.contains_fn(hash_set)); From 9ab875bda3da7dd33e87439818e258e8220c8379 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 30 Jan 2024 15:41:26 +0800 Subject: [PATCH 04/11] Expand FuncRegistration API. --- src/api/mod.rs | 2 +- src/api/register.rs | 113 ++++++++------------------ src/ast/stmt.rs | 6 +- src/module/mod.rs | 146 ++++++++++++++++++++++++++++++++-- src/types/immutable_string.rs | 2 +- tests/optimizer.rs | 42 +++++++--- 6 files changed, 208 insertions(+), 103 deletions(-) diff --git a/src/api/mod.rs b/src/api/mod.rs index e797ef2f0..c4c44c7d5 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -44,7 +44,7 @@ pub mod default_limits { #[cfg(feature = "internals")] pub use super::limits::default_limits::*; - /// Maximum number of parameters in functions with [`Dynamic`] support. + /// Maximum number of parameters in functions with [`Dynamic`][crate::Dynamic] support. pub const MAX_DYNAMIC_PARAMETERS: usize = 16; } diff --git a/src/api/register.rs b/src/api/register.rs index 9b35fb297..5f66ebdbc 100644 --- a/src/api/register.rs +++ b/src/api/register.rs @@ -19,7 +19,7 @@ impl Engine { /// (which is the first module in `global_modules`). #[inline(always)] #[must_use] - fn global_namespace_mut(&mut self) -> &mut Module { + pub(crate) fn global_namespace_mut(&mut self) -> &mut Module { if self.global_modules.is_empty() { let mut global_namespace = Module::new(); global_namespace.flags |= ModuleFlags::INTERNAL; @@ -75,41 +75,31 @@ impl Engine { name: impl AsRef + Into, func: FUNC, ) -> &mut Self { - #[cfg(feature = "metadata")] - let mut param_type_names = FUNC::param_names() - .iter() - .map(|ty| format!("_: {}", self.format_type_name(ty))) - .collect::>(); - - #[cfg(feature = "metadata")] - if FUNC::return_type() != TypeId::of::<()>() { - param_type_names.push(self.format_type_name(FUNC::return_type_name()).into()); - } + let mut reg = FuncRegistration::new(name.into()) + .with_namespace(FnNamespace::Global) + .with_purity(true) + .with_volatility(false); #[cfg(feature = "metadata")] - let param_type_names = param_type_names - .iter() - .map(String::as_str) - .collect::>(); - - let fn_name = name.into(); - let is_pure = true; + { + let mut param_type_names = FUNC::param_names() + .iter() + .map(|ty| format!("_: {}", self.format_type_name(ty))) + .collect::>(); - #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] - let is_pure = is_pure && (FUNC::num_params() != 3 || fn_name != crate::engine::FN_IDX_SET); - #[cfg(not(feature = "no_object"))] - let is_pure = - is_pure && (FUNC::num_params() != 2 || !fn_name.starts_with(crate::engine::FN_SET)); + if FUNC::return_type() != TypeId::of::<()>() { + param_type_names.push(self.format_type_name(FUNC::return_type_name()).into()); + } - let f = FuncRegistration::new(fn_name) - .with_namespace(FnNamespace::Global) - .with_purity(is_pure) - .with_volatility(false); + let param_type_names = param_type_names + .iter() + .map(String::as_str) + .collect::>(); - #[cfg(feature = "metadata")] - let f = f.with_params_info(param_type_names); + reg = reg.with_params_info(param_type_names); + } - f.set_into_module(self.global_namespace_mut(), func); + reg.set_into_module(self.global_namespace_mut(), func); self } @@ -138,6 +128,15 @@ impl Engine { arg_types: impl AsRef<[TypeId]>, func: impl Fn(NativeCallContext, &mut FnCallArgs) -> RhaiResultOf + SendSync + 'static, ) -> &mut Self { + let name = name.into(); + let arg_types = arg_types.as_ref(); + let is_pure = true; + + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] + let is_pure = is_pure && (arg_types.len() != 3 || name != crate::engine::FN_IDX_SET); + #[cfg(not(feature = "no_object"))] + let is_pure = is_pure && (arg_types.len() != 2 || !name.starts_with(crate::engine::FN_SET)); + FuncRegistration::new(name) .with_namespace(FnNamespace::Global) .set_into_module_raw( @@ -150,7 +149,7 @@ impl Engine { }, ), has_context: true, - is_pure: false, + is_pure, is_volatile: true, }, ); @@ -498,7 +497,7 @@ impl Engine { /// # } /// ``` #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] - #[inline] + #[inline(always)] pub fn register_indexer_get< T: Variant + Clone, IDX: Variant + Clone, @@ -509,30 +508,6 @@ impl Engine { &mut self, get_fn: impl RhaiNativeFunc<(Mut, IDX), 2, X, R, F> + SendSync + 'static, ) -> &mut Self { - #[cfg(not(feature = "no_index"))] - assert!( - TypeId::of::() != TypeId::of::(), - "Cannot register indexer for arrays." - ); - - #[cfg(not(feature = "no_object"))] - assert!( - TypeId::of::() != TypeId::of::(), - "Cannot register indexer for object maps." - ); - - assert!( - TypeId::of::() != TypeId::of::() - && TypeId::of::() != TypeId::of::<&str>() - && TypeId::of::() != TypeId::of::(), - "Cannot register indexer for strings." - ); - - assert!( - TypeId::of::() != TypeId::of::(), - "Cannot register indexer for integers." - ); - self.register_fn(crate::engine::FN_IDX_GET, get_fn) } /// Register an index setter for a custom type with the [`Engine`]. @@ -585,7 +560,7 @@ impl Engine { /// # } /// ``` #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] - #[inline] + #[inline(always)] pub fn register_indexer_set< T: Variant + Clone, IDX: Variant + Clone, @@ -596,30 +571,6 @@ impl Engine { &mut self, set_fn: impl RhaiNativeFunc<(Mut, IDX, R), 3, X, (), F> + SendSync + 'static, ) -> &mut Self { - #[cfg(not(feature = "no_index"))] - assert!( - TypeId::of::() != TypeId::of::(), - "Cannot register indexer for arrays." - ); - - #[cfg(not(feature = "no_object"))] - assert!( - TypeId::of::() != TypeId::of::(), - "Cannot register indexer for object maps." - ); - - assert!( - TypeId::of::() != TypeId::of::() - && TypeId::of::() != TypeId::of::<&str>() - && TypeId::of::() != TypeId::of::(), - "Cannot register indexer for strings." - ); - - assert!( - TypeId::of::() != TypeId::of::(), - "Cannot register indexer for integers." - ); - self.register_fn(crate::engine::FN_IDX_SET, set_fn) } /// Short-hand for registering both index getter and setter functions for a custom type with the [`Engine`]. diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index c9eff5aa1..9ea3a1344 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -308,7 +308,7 @@ impl RangeCase { Self::InclusiveInt(..) => true, } } - /// Get the index to the [`ConditionalExpr`]. + /// Get the index to the list of expressions. #[inline(always)] #[must_use] pub const fn index(&self) -> usize { @@ -316,7 +316,7 @@ impl RangeCase { Self::ExclusiveInt(.., n) | Self::InclusiveInt(.., n) => *n, } } - /// Set the index to the [`ConditionalExpr`]. + /// Set the index to the list of expressions. #[inline(always)] pub fn set_index(&mut self, index: usize) { match self { @@ -333,7 +333,7 @@ pub type CaseBlocksList = smallvec::SmallVec<[usize; 2]>; pub struct SwitchCasesCollection { /// List of conditional expressions: LHS = condition, RHS = expression. pub expressions: FnArgsVec, - /// Dictionary mapping value hashes to [`ConditionalExpr`]'s. + /// Dictionary mapping value hashes to [`CaseBlocksList`]'s. pub cases: StraightHashMap, /// List of range cases. pub ranges: StaticVec, diff --git a/src/module/mod.rs b/src/module/mod.rs index 31f2f99a8..6775e6f5c 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -8,8 +8,8 @@ use crate::func::{ }; use crate::types::{dynamic::Variant, BloomFilterU64, CustomTypeInfo, CustomTypesCollection}; use crate::{ - calc_fn_hash, calc_fn_hash_full, Dynamic, FnArgsVec, Identifier, ImmutableString, RhaiResultOf, - Shared, SharedModule, SmartString, + calc_fn_hash, calc_fn_hash_full, Dynamic, Engine, FnArgsVec, Identifier, ImmutableString, + RhaiResultOf, Shared, SharedModule, SmartString, }; use bitflags::bitflags; #[cfg(feature = "no_std")] @@ -178,6 +178,16 @@ pub struct FuncRegistration { impl FuncRegistration { /// Create a new [`FuncRegistration`]. /// + /// # Defaults + /// + /// * **Accessibility**: The function namespace is [`FnNamespace::Internal`]. + /// + /// * **Purity**: The function is assumed to be _pure_ unless it is a property setter or an index setter. + /// + /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). + /// + /// * **Metadata**: No metadata for the function is registered. + /// /// ``` /// # use rhai::{Module, FuncRegistration, FnNamespace}; /// let mut module = Module::new(); @@ -212,6 +222,82 @@ impl FuncRegistration { volatility: None, } } + /// Create a new [`FuncRegistration`] for a property getter. + /// + /// Not available under `no_object`. + /// + /// # Defaults + /// + /// * **Accessibility**: The function namespace is [`FnNamespace::Global`]. + /// + /// * **Purity**: The function is assumed to be _pure_. + /// + /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). + /// + /// * **Metadata**: No metadata for the function is registered. + #[cfg(not(feature = "no_object"))] + #[inline(always)] + pub fn new_getter(prop: impl AsRef) -> Self { + Self::new(crate::engine::make_getter(prop.as_ref())).with_namespace(FnNamespace::Global) + } + /// Create a new [`FuncRegistration`] for a property setter. + /// + /// Not available under `no_object`. + /// + /// # Defaults + /// + /// * **Accessibility**: The function namespace is [`FnNamespace::Global`]. + /// + /// * **Purity**: The function is assumed to be _no-pure_. + /// + /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). + /// + /// * **Metadata**: No metadata for the function is registered. + #[cfg(not(feature = "no_object"))] + #[inline(always)] + pub fn new_setter(prop: impl AsRef) -> Self { + Self::new(crate::engine::make_setter(prop.as_ref())) + .with_namespace(FnNamespace::Global) + .with_purity(false) + } + /// Create a new [`FuncRegistration`] for an index getter. + /// + /// Not available under both `no_index` and `no_object`. + /// + /// # Defaults + /// + /// * **Accessibility**: The function namespace is [`FnNamespace::Global`]. + /// + /// * **Purity**: The function is assumed to be _pure_. + /// + /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). + /// + /// * **Metadata**: No metadata for the function is registered. + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] + #[inline(always)] + pub fn new_index_getter() -> Self { + Self::new(crate::engine::FN_IDX_GET).with_namespace(FnNamespace::Global) + } + /// Create a new [`FuncRegistration`] for an index setter. + /// + /// Not available under both `no_index` and `no_object`. + /// + /// # Defaults + /// + /// * **Accessibility**: The function namespace is [`FnNamespace::Global`]. + /// + /// * **Purity**: The function is assumed to be _no-pure_. + /// + /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). + /// + /// * **Metadata**: No metadata for the function is registered. + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] + #[inline(always)] + pub fn new_index_setter() -> Self { + Self::new(crate::engine::FN_IDX_SET) + .with_namespace(FnNamespace::Global) + .with_purity(false) + } /// Set the [namespace][`FnNamespace`] of the function. pub fn with_namespace(mut self, namespace: FnNamespace) -> Self { self.metadata.namespace = namespace; @@ -270,13 +356,21 @@ impl FuncRegistration { self.metadata.comments = comments.into_iter().map(|s| s.as_ref().into()).collect(); self } + /// Register the function into the specified [`Engine`]. + #[inline] + pub fn register_into_engine( + self, + engine: &mut Engine, + func: FUNC, + ) -> &FuncMetadata + where + R: Variant + Clone, + FUNC: RhaiNativeFunc + SendSync + 'static, + { + self.with_namespace(FnNamespace::Global) + .set_into_module(engine.global_namespace_mut(), func) + } /// Register the function into the specified [`Module`]. - /// - /// # Assumptions - /// - /// * **Purity**: The function is assumed to be _pure_ (so it can be called on constants) unless it is a property setter or an index setter. - /// - /// * **Volatility**: The function is assumed to be _non-volatile_ -- i.e. it guarantees the same result for the same input(s). #[inline] pub fn set_into_module( self, @@ -318,6 +412,14 @@ impl FuncRegistration { /// /// This function is very low level. It takes a list of [`TypeId`][std::any::TypeId]'s /// indicating the actual types of the parameters. + /// + /// # Panics + /// + /// Panics if the type of the first parameter is [`Array`][crate::Array], [`Map`][crate::Map], + /// [`String`], [`ImmutableString`][crate::ImmutableString], `&str` or [`INT`][crate::INT] and + /// the function name indicates that it is an index getter or setter. + /// + /// Indexers for arrays, object maps, strings and integers cannot be registered. #[inline] pub fn set_into_module_raw( self, @@ -334,6 +436,34 @@ impl FuncRegistration { f.num_params = arg_types.as_ref().len(); f.param_types.extend(arg_types.as_ref().iter().copied()); + if f.name == crate::engine::FN_IDX_GET || f.name == crate::engine::FN_IDX_SET { + if let Some(&type_id) = f.param_types.first() { + #[cfg(not(feature = "no_index"))] + assert!( + type_id != TypeId::of::(), + "Cannot register indexer for arrays." + ); + + #[cfg(not(feature = "no_object"))] + assert!( + type_id != TypeId::of::(), + "Cannot register indexer for object maps." + ); + + assert!( + type_id != TypeId::of::() + && type_id != TypeId::of::<&str>() + && type_id != TypeId::of::(), + "Cannot register indexer for strings." + ); + + assert!( + type_id != TypeId::of::(), + "Cannot register indexer for integers." + ); + } + } + let is_method = func.is_method(); f.param_types diff --git a/src/types/immutable_string.rs b/src/types/immutable_string.rs index 669ca56e0..0e53ac62c 100644 --- a/src/types/immutable_string.rs +++ b/src/types/immutable_string.rs @@ -754,7 +754,7 @@ impl ImmutableString { pub fn make_mut(&mut self) -> &mut SmartString { shared_make_mut(&mut self.0) } - /// Return a mutable reference to the [`SmartString`] wrapped by the [`ImmutableString`] + /// Return a mutable reference to the [` SmartString`] wrapped by the [`ImmutableString`] /// if there are no other outstanding references to it. #[inline(always)] pub fn get_mut(&mut self) -> Option<&mut SmartString> { diff --git a/tests/optimizer.rs b/tests/optimizer.rs index 7559608be..8aa27478d 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -1,5 +1,5 @@ #![cfg(not(feature = "no_optimize"))] -use rhai::{Engine, Module, OptimizationLevel, Scope, INT}; +use rhai::{CustomType, Engine, FuncRegistration, Module, OptimizationLevel, Scope, TypeBuilder, INT}; #[test] fn test_optimizer() { @@ -10,10 +10,10 @@ fn test_optimizer() { engine .eval::( " - const X = 0; - const X = 40 + 2 - 1 + 1; - X - " + const X = 0; + const X = 40 + 2 - 1 + 1; + X + " ) .unwrap(), 42 @@ -147,7 +147,7 @@ fn test_optimizer_re_optimize() { #[test] fn test_optimizer_full() { - #[derive(Debug, Clone)] + #[derive(Debug, Clone, CustomType)] struct TestStruct(INT); let mut engine = Engine::new(); @@ -170,16 +170,15 @@ fn test_optimizer_full() { ); engine - .register_type_with_name::("TestStruct") + .build_type::() .register_fn("ts", |n: INT| TestStruct(n)) - .register_fn("value", |ts: &mut TestStruct| ts.0) .register_fn("+", |ts1: &mut TestStruct, ts2: TestStruct| TestStruct(ts1.0 + ts2.0)); let ast = engine .compile( " const FOO = ts(40) + ts(2); - value(FOO) + field0(FOO) ", ) .unwrap(); @@ -193,3 +192,28 @@ fn test_optimizer_full() { assert_eq!(scope.get_value::("FOO").unwrap().0, 42); } + +#[test] +fn test_optimizer_volatile() { + let mut engine = Engine::new(); + + engine.set_optimization_level(OptimizationLevel::Full); + + FuncRegistration::new("foo").with_volatility(true).register_into_engine(&mut engine, |x: i64| x + 1); + + let ast = engine.compile("foo(42)").unwrap(); + + let text_ast = format!("{ast:?}"); + + // Make sure the call is not optimized away + assert!(text_ast.contains(r#"name: "foo""#)); + + FuncRegistration::new("foo").with_volatility(false).register_into_engine(&mut engine, |x: i64| x + 1); + + let ast = engine.compile("foo(42)").unwrap(); + + let text_ast = format!("{ast:?}"); + + // Make sure the call is optimized away + assert!(!text_ast.contains(r#"name: "foo""#)); +} From bdadfa46db966c91187a7683f1ad4917fc9f0dab Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 30 Jan 2024 16:32:59 +0800 Subject: [PATCH 05/11] Fix bug in Engine::gen_fn_signatures. --- CHANGELOG.md | 1 + src/api/definitions/mod.rs | 2 +- src/api/options.rs | 20 ++++++++++---------- src/api/register.rs | 2 +- src/ast/ast.rs | 6 +++--- src/ast/stmt.rs | 2 +- src/eval/chaining.rs | 8 ++++---- src/eval/stmt.rs | 12 ++++++------ src/module/mod.rs | 6 +++--- src/optimizer.rs | 20 ++++++++++---------- src/parser.rs | 22 +++++++++++----------- src/serde/metadata.rs | 2 +- 12 files changed, 52 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef3a7b78f..8dea010d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ Other bug fixes * Arrays in object maps now serialize to JSON correctly via `to_json()` when the `serde` feature is not enabled. * `Engine::format_map_as_json` now serializes arrays correctly. +* `Engine::gen_fn_signatures(false)` now properly skips functions in the standard library. Enhancements ------------ diff --git a/src/api/definitions/mod.rs b/src/api/definitions/mod.rs index 2d7391f08..da5147c1d 100644 --- a/src/api/definitions/mod.rs +++ b/src/api/definitions/mod.rs @@ -321,7 +321,7 @@ impl Definitions<'_> { self.engine .global_modules .iter() - .filter(|m| !m.flags.contains(exclude_flags)) + .filter(|&m| !m.flags.intersects(exclude_flags)) .enumerate() .for_each(|(i, m)| { if i > 0 { diff --git a/src/api/options.rs b/src/api/options.rs index 49c7d17e4..6317e54ee 100644 --- a/src/api/options.rs +++ b/src/api/options.rs @@ -68,7 +68,7 @@ impl Engine { #[inline(always)] #[must_use] pub const fn allow_if_expression(&self) -> bool { - self.options.contains(LangOptions::IF_EXPR) + self.options.intersects(LangOptions::IF_EXPR) } /// Set whether `if`-expression is allowed. #[inline(always)] @@ -81,7 +81,7 @@ impl Engine { #[inline(always)] #[must_use] pub const fn allow_switch_expression(&self) -> bool { - self.options.contains(LangOptions::SWITCH_EXPR) + self.options.intersects(LangOptions::SWITCH_EXPR) } /// Set whether `switch` expression is allowed. #[inline(always)] @@ -94,7 +94,7 @@ impl Engine { #[inline(always)] #[must_use] pub const fn allow_loop_expressions(&self) -> bool { - self.options.contains(LangOptions::LOOP_EXPR) + self.options.intersects(LangOptions::LOOP_EXPR) } /// Set whether loop expressions are allowed. #[inline(always)] @@ -107,7 +107,7 @@ impl Engine { #[inline(always)] #[must_use] pub const fn allow_statement_expression(&self) -> bool { - self.options.contains(LangOptions::STMT_EXPR) + self.options.intersects(LangOptions::STMT_EXPR) } /// Set whether statement-expression is allowed. #[inline(always)] @@ -123,7 +123,7 @@ impl Engine { #[inline(always)] #[must_use] pub const fn allow_anonymous_fn(&self) -> bool { - self.options.contains(LangOptions::ANON_FN) + self.options.intersects(LangOptions::ANON_FN) } /// Set whether anonymous function is allowed. /// @@ -139,7 +139,7 @@ impl Engine { #[inline(always)] #[must_use] pub const fn allow_looping(&self) -> bool { - self.options.contains(LangOptions::LOOPING) + self.options.intersects(LangOptions::LOOPING) } /// Set whether looping is allowed. #[inline(always)] @@ -152,7 +152,7 @@ impl Engine { #[inline(always)] #[must_use] pub const fn allow_shadowing(&self) -> bool { - self.options.contains(LangOptions::SHADOWING) + self.options.intersects(LangOptions::SHADOWING) } /// Set whether variables shadowing is allowed. #[inline(always)] @@ -165,7 +165,7 @@ impl Engine { #[inline(always)] #[must_use] pub const fn strict_variables(&self) -> bool { - self.options.contains(LangOptions::STRICT_VAR) + self.options.intersects(LangOptions::STRICT_VAR) } /// Set whether strict variables mode is enabled. #[inline(always)] @@ -182,7 +182,7 @@ impl Engine { #[must_use] pub const fn fail_on_invalid_map_property(&self) -> bool { self.options - .contains(LangOptions::FAIL_ON_INVALID_MAP_PROPERTY) + .intersects(LangOptions::FAIL_ON_INVALID_MAP_PROPERTY) } /// Set whether to raise error if an object map property does not exist. /// @@ -199,7 +199,7 @@ impl Engine { #[inline(always)] #[must_use] pub const fn fast_operators(&self) -> bool { - self.options.contains(LangOptions::FAST_OPS) + self.options.intersects(LangOptions::FAST_OPS) } /// Set whether fast operators mode is enabled. #[inline(always)] diff --git a/src/api/register.rs b/src/api/register.rs index 5f66ebdbc..3b02a41b3 100644 --- a/src/api/register.rs +++ b/src/api/register.rs @@ -772,7 +772,7 @@ impl Engine { self.global_modules .iter() .skip(1) - .filter(|m| !m.flags.contains(exclude_flags)) + .filter(|m| !m.flags.intersects(exclude_flags)) .flat_map(|m| m.gen_fn_signatures()), ); diff --git a/src/ast/ast.rs b/src/ast/ast.rs index 9a2444970..23c5736eb 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -807,12 +807,12 @@ impl AST { ) -> impl Iterator { self.statements().iter().filter_map(move |stmt| match stmt { Stmt::Var(x, options, ..) - if options.contains(ASTFlags::CONSTANT) && include_constants - || !options.contains(ASTFlags::CONSTANT) && include_variables => + if options.intersects(ASTFlags::CONSTANT) && include_constants + || !options.intersects(ASTFlags::CONSTANT) && include_variables => { let (name, expr, ..) = &**x; expr.get_literal_value() - .map(|value| (name.as_str(), options.contains(ASTFlags::CONSTANT), value)) + .map(|value| (name.as_str(), options.intersects(ASTFlags::CONSTANT), value)) } _ => None, }) diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index 9ea3a1344..f271e334e 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -867,7 +867,7 @@ impl Stmt { // Loops that exit can be pure because it can never be infinite. Self::While(x, ..) if matches!(x.expr, Expr::BoolConstant(false, ..)) => true, Self::Do(x, options, ..) if matches!(x.expr, Expr::BoolConstant(..)) => match x.expr { - Expr::BoolConstant(cond, ..) if cond == options.contains(ASTFlags::NEGATED) => { + Expr::BoolConstant(cond, ..) if cond == options.intersects(ASTFlags::NEGATED) => { x.body.iter().all(Self::is_pure) } _ => false, diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index b0771f5fc..9c73aa8d6 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -473,7 +473,7 @@ impl Engine { (Expr::Property(..), ChainType::Dotting) => (), (Expr::Index(x, ..) | Expr::Dot(x, ..), chain_type) - if !parent.options().contains(ASTFlags::BREAK) => + if !parent.options().intersects(ASTFlags::BREAK) => { let BinaryExpr { lhs, rhs, .. } = &**x; @@ -556,7 +556,7 @@ impl Engine { #[cfg(not(feature = "no_index"))] ChainType::Indexing => { // Check for existence with the null conditional operator - if parent.options().contains(ASTFlags::NEGATED) && target.as_ref().is_unit() { + if parent.options().intersects(ASTFlags::NEGATED) && target.as_ref().is_unit() { return Ok((Dynamic::UNIT, false)); } @@ -565,7 +565,7 @@ impl Engine { match (rhs, new_val) { // xxx[idx].expr... | xxx[idx][expr]... (Expr::Dot(x, ..) | Expr::Index(x, ..), new_val) - if !parent.options().contains(ASTFlags::BREAK) => + if !parent.options().intersects(ASTFlags::BREAK) => { #[cfg(feature = "debugging")] self.run_debugger(global, caches, _scope, this_ptr.as_deref_mut(), parent)?; @@ -688,7 +688,7 @@ impl Engine { #[cfg(not(feature = "no_object"))] ChainType::Dotting => { // Check for existence with the Elvis operator - if parent.options().contains(ASTFlags::NEGATED) && target.as_ref().is_unit() { + if parent.options().intersects(ASTFlags::NEGATED) && target.as_ref().is_unit() { return Ok((Dynamic::UNIT, false)); } diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index ca2e06798..c48934744 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -390,12 +390,12 @@ impl Engine { // Let/const statement let (var_name, expr, index) = &**x; - let access = if options.contains(ASTFlags::CONSTANT) { + let access = if options.intersects(ASTFlags::CONSTANT) { AccessMode::ReadOnly } else { AccessMode::ReadWrite }; - let export = options.contains(ASTFlags::EXPORTED); + let export = options.intersects(ASTFlags::EXPORTED); // Check variable definition filter if let Some(ref filter) = self.def_var_filter { @@ -632,7 +632,7 @@ impl Engine { // Do loop Stmt::Do(x, options, ..) => { let FlowControl { expr, body, .. } = &**x; - let is_while = !options.contains(ASTFlags::NEGATED); + let is_while = !options.intersects(ASTFlags::NEGATED); loop { if !body.is_empty() { @@ -773,7 +773,7 @@ impl Engine { // Continue/Break statement Stmt::BreakLoop(expr, options, pos) => { - let is_break = options.contains(ASTFlags::BREAK); + let is_break = options.intersects(ASTFlags::BREAK); let value = match expr { Some(ref expr) => self.eval_expr(global, caches, scope, this_ptr, expr)?, @@ -871,12 +871,12 @@ impl Engine { } // Throw value - Stmt::Return(Some(expr), options, pos) if options.contains(ASTFlags::BREAK) => self + Stmt::Return(Some(expr), options, pos) if options.intersects(ASTFlags::BREAK) => self .eval_expr(global, caches, scope, this_ptr, expr) .and_then(|v| Err(ERR::ErrorRuntime(v.flatten(), *pos).into())), // Empty throw - Stmt::Return(None, options, pos) if options.contains(ASTFlags::BREAK) => { + Stmt::Return(None, options, pos) if options.intersects(ASTFlags::BREAK) => { Err(ERR::ErrorRuntime(Dynamic::UNIT, *pos).into()) } diff --git a/src/module/mod.rs b/src/module/mod.rs index 6775e6f5c..3d479d225 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1003,7 +1003,7 @@ impl Module { #[inline] #[must_use] pub fn is_empty(&self) -> bool { - !self.flags.contains(ModuleFlags::INDEXED_GLOBAL_FUNCTIONS) + !self.flags.intersects(ModuleFlags::INDEXED_GLOBAL_FUNCTIONS) && self .functions .as_ref() @@ -1045,7 +1045,7 @@ impl Module { #[inline(always)] #[must_use] pub const fn is_indexed(&self) -> bool { - self.flags.contains(ModuleFlags::INDEXED) + self.flags.intersects(ModuleFlags::INDEXED) } /// _(metadata)_ Generate signatures for all the non-private functions in the [`Module`]. @@ -2365,7 +2365,7 @@ impl Module { #[inline(always)] #[must_use] pub const fn contains_indexed_global_functions(&self) -> bool { - self.flags.contains(ModuleFlags::INDEXED_GLOBAL_FUNCTIONS) + self.flags.intersects(ModuleFlags::INDEXED_GLOBAL_FUNCTIONS) } /// Scan through all the sub-modules in the [`Module`] and build a hash index of all diff --git a/src/optimizer.rs b/src/optimizer.rs index 4b0992c65..8dae63b74 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -223,7 +223,7 @@ fn optimize_stmt_block( Stmt::Var(x, options, ..) => { optimize_expr(&mut x.1, state, false); - let value = if options.contains(ASTFlags::CONSTANT) && x.1.is_constant() { + let value = if options.intersects(ASTFlags::CONSTANT) && x.1.is_constant() { // constant literal Some(Cow::Owned(x.1.get_literal_value().unwrap())) } else { @@ -282,7 +282,7 @@ fn optimize_stmt_block( match statements[..] { // { return; } -> {} [Stmt::Return(None, options, ..)] - if reduce_return && !options.contains(ASTFlags::BREAK) => + if reduce_return && !options.intersects(ASTFlags::BREAK) => { state.set_dirty(); statements.clear(); @@ -294,7 +294,7 @@ fn optimize_stmt_block( // { ...; return; } -> { ... } [.., ref last_stmt, Stmt::Return(None, options, ..)] if reduce_return - && !options.contains(ASTFlags::BREAK) + && !options.intersects(ASTFlags::BREAK) && !last_stmt.returns_value() => { state.set_dirty(); @@ -302,7 +302,7 @@ fn optimize_stmt_block( } // { ...; return val; } -> { ...; val } [.., Stmt::Return(ref mut expr, options, pos)] - if reduce_return && !options.contains(ASTFlags::BREAK) => + if reduce_return && !options.intersects(ASTFlags::BREAK) => { state.set_dirty(); *statements.last_mut().unwrap() = expr @@ -339,7 +339,7 @@ fn optimize_stmt_block( } // { ...; return; } -> { ... } [.., Stmt::Return(None, options, ..)] - if reduce_return && !options.contains(ASTFlags::BREAK) => + if reduce_return && !options.intersects(ASTFlags::BREAK) => { state.set_dirty(); statements.pop().unwrap(); @@ -347,7 +347,7 @@ fn optimize_stmt_block( // { ...; return pure_val; } -> { ... } [.., Stmt::Return(Some(ref expr), options, ..)] if reduce_return - && !options.contains(ASTFlags::BREAK) + && !options.intersects(ASTFlags::BREAK) && expr.is_pure() => { state.set_dirty(); @@ -771,7 +771,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b optimize_stmt_block(x.2.body.take_statements(), state, false, true, false); } // let id = expr; - Stmt::Var(x, options, ..) if !options.contains(ASTFlags::CONSTANT) => { + Stmt::Var(x, options, ..) if !options.intersects(ASTFlags::CONSTANT) => { optimize_expr(&mut x.1, state, false); } // import expr as var; @@ -911,7 +911,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { } // ()?.rhs #[cfg(not(feature = "no_object"))] - Expr::Dot(x, options, ..) if options.contains(ASTFlags::NEGATED) && matches!(x.lhs, Expr::Unit(..)) => { + Expr::Dot(x, options, ..) if options.intersects(ASTFlags::NEGATED) && matches!(x.lhs, Expr::Unit(..)) => { state.set_dirty(); *expr = x.lhs.take(); } @@ -953,7 +953,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { // ()?[rhs] #[cfg(not(feature = "no_index"))] - Expr::Index(x, options, ..) if options.contains(ASTFlags::NEGATED) && matches!(x.lhs, Expr::Unit(..)) => { + Expr::Index(x, options, ..) if options.intersects(ASTFlags::NEGATED) && matches!(x.lhs, Expr::Unit(..)) => { state.set_dirty(); *expr = x.lhs.take(); } @@ -1300,7 +1300,7 @@ impl Engine { if self .global_modules .iter() - .filter(|m| !m.flags.contains(ModuleFlags::STANDARD_LIB)) + .filter(|m| !m.flags.intersects(ModuleFlags::STANDARD_LIB)) .any(|m| m.contains_fn(hash)) { return true; diff --git a/src/parser.rs b/src/parser.rs index 0d6559427..b571a13e3 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -325,13 +325,13 @@ impl ParseSettings { #[inline(always)] #[must_use] pub const fn has_flag(&self, flag: ParseSettingFlags) -> bool { - self.flags.contains(flag) + self.flags.intersects(flag) } /// Is a particular language option on? #[inline(always)] #[must_use] pub const fn has_option(&self, option: LangOptions) -> bool { - self.options.contains(option) + self.options.intersects(option) } /// Create a new `ParseSettings` with one higher expression level. #[inline] @@ -595,7 +595,7 @@ fn optimize_combo_chain(expr: &mut Expr) { let mut tail = root.as_mut(); let mut tail_options = &mut root_options; - while !tail_options.contains(ASTFlags::BREAK) { + while !tail_options.intersects(ASTFlags::BREAK) { match tail.rhs { Expr::Index(ref mut x, ref mut options2, ..) => { tail = x.as_mut(); @@ -1663,7 +1663,7 @@ impl Engine { // Namespace qualification #[cfg(not(feature = "no_module"))] (token @ Token::DoubleColon, pos) => { - if options.contains(ChainingFlags::DISALLOW_NAMESPACES) { + if options.intersects(ChainingFlags::DISALLOW_NAMESPACES) { return Err(LexError::ImproperSymbol( token.literal_syntax().into(), String::new(), @@ -1681,7 +1681,7 @@ impl Engine { _ => { let (index, is_func) = state.access_var(&s, lib, settings.pos); - if !options.contains(ChainingFlags::PROPERTY) + if !options.intersects(ChainingFlags::PROPERTY) && !is_func && index.is_none() && settings.has_option(LangOptions::STRICT_VAR) @@ -1833,7 +1833,7 @@ impl Engine { // Disallowed module separator #[cfg(not(feature = "no_module"))] (_, token @ Token::DoubleColon) - if _options.contains(ChainingFlags::DISALLOW_NAMESPACES) => + if _options.intersects(ChainingFlags::DISALLOW_NAMESPACES) => { return Err(LexError::ImproperSymbol( token.literal_syntax().into(), @@ -2066,7 +2066,7 @@ impl Engine { match expr { Expr::Index(x, options, ..) | Expr::Dot(x, options, ..) if parent_is_dot => { match x.lhs { - Expr::Property(..) if !options.contains(ASTFlags::BREAK) => { + Expr::Property(..) if !options.intersects(ASTFlags::BREAK) => { check_lvalue(&x.rhs, matches!(expr, Expr::Dot(..))) } Expr::Property(..) => None, @@ -2076,7 +2076,7 @@ impl Engine { } Expr::Index(x, options, ..) | Expr::Dot(x, options, ..) => match x.lhs { Expr::Property(..) => unreachable!("unexpected Expr::Property in indexing"), - _ if !options.contains(ASTFlags::BREAK) => { + _ if !options.intersects(ASTFlags::BREAK) => { check_lvalue(&x.rhs, matches!(expr, Expr::Dot(..))) } _ => None, @@ -2124,7 +2124,7 @@ impl Engine { } // xxx[???]... = rhs, xxx.prop... = rhs Expr::Index(ref x, options, ..) | Expr::Dot(ref x, options, ..) => { - let valid_lvalue = if options.contains(ASTFlags::BREAK) { + let valid_lvalue = if options.intersects(ASTFlags::BREAK) { None } else { check_lvalue(&x.rhs, matches!(lhs, Expr::Dot(..))) @@ -2176,7 +2176,7 @@ impl Engine { match (lhs, rhs) { // lhs[...][...].rhs (Expr::Index(mut x, options, pos), rhs) - if !parent_options.contains(ASTFlags::BREAK) => + if !parent_options.intersects(ASTFlags::BREAK) => { let options = options | parent_options; x.rhs = Self::make_dot_expr(state, x.rhs, rhs, options, op_flags, op_pos)?; @@ -2777,7 +2777,7 @@ impl Engine { ) -> ParseResult { // do ... let mut settings = settings.level_up_with_position(eat_token(input, &Token::Do))?; - let orig_breakable = settings.flags.contains(ParseSettingFlags::BREAKABLE); + let orig_breakable = settings.has_flag(ParseSettingFlags::BREAKABLE); settings.flags |= ParseSettingFlags::BREAKABLE; // do { body } [while|until] guard diff --git a/src/serde/metadata.rs b/src/serde/metadata.rs index 3dfd3fbe5..3f950b1b8 100644 --- a/src/serde/metadata.rs +++ b/src/serde/metadata.rs @@ -249,7 +249,7 @@ pub fn gen_metadata_to_json( engine .global_modules .iter() - .filter(|m| !m.flags.contains(exclude_flags)) + .filter(|&m| !m.flags.intersects(exclude_flags)) .for_each(|m| { if !m.doc().is_empty() { if !global_doc.is_empty() { From 31b0760a47e8acc73a2c5ff6c7bc308332510c26 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 30 Jan 2024 17:53:23 +0800 Subject: [PATCH 06/11] Fix TypeBuilder. --- CHANGELOG.md | 1 + src/api/build_type.rs | 28 ++++++++-------------------- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8dea010d3..d4e8fe9ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ Other bug fixes * Arrays in object maps now serialize to JSON correctly via `to_json()` when the `serde` feature is not enabled. * `Engine::format_map_as_json` now serializes arrays correctly. * `Engine::gen_fn_signatures(false)` now properly skips functions in the standard library. +* `TypeBuilder::with_name` now properly sets the display-name of the type for use in generating metadata. Enhancements ------------ diff --git a/src/api/build_type.rs b/src/api/build_type.rs index 53ba2bdf1..62140fa6e 100644 --- a/src/api/build_type.rs +++ b/src/api/build_type.rs @@ -98,29 +98,27 @@ impl Engine { /// /// To define a pretty-print name, call [`with_name`][`TypeBuilder::with_name`], /// to use [`Engine::register_type_with_name`] instead. -pub struct TypeBuilder<'a, 's, T: Variant + Clone> { +pub struct TypeBuilder<'a, T: Variant + Clone> { engine: &'a mut Engine, - name: Option<&'s str>, _marker: PhantomData, } -impl<'a, T: Variant + Clone> TypeBuilder<'a, '_, T> { +impl<'a, T: Variant + Clone> TypeBuilder<'a, T> { /// Create a [`TypeBuilder`] linked to a particular [`Engine`] instance. #[inline(always)] fn new(engine: &'a mut Engine) -> Self { Self { engine, - name: None, _marker: PhantomData, } } } -impl<'s, T: Variant + Clone> TypeBuilder<'_, 's, T> { +impl<'s, T: Variant + Clone> TypeBuilder<'_, T> { /// Set a pretty-print name for the `type_of` function. #[inline(always)] - pub fn with_name(&mut self, name: &'s str) -> &mut Self { - self.name = Some(name.as_ref()); + pub fn with_name(&mut self, name: &str) -> &mut Self { + self.engine.register_type_with_name::(name); self } @@ -156,7 +154,7 @@ impl<'s, T: Variant + Clone> TypeBuilder<'_, 's, T> { } } -impl TypeBuilder<'_, '_, T> +impl TypeBuilder<'_, T> where T: Variant + Clone + IntoIterator, ::Item: Variant + Clone, @@ -171,7 +169,7 @@ where } #[cfg(not(feature = "no_object"))] -impl TypeBuilder<'_, '_, T> { +impl TypeBuilder<'_, T> { /// Register a getter function. /// /// The function signature must start with `&mut self` and not `&self`. @@ -224,7 +222,7 @@ impl TypeBuilder<'_, '_, T> { } #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] -impl TypeBuilder<'_, '_, T> { +impl TypeBuilder<'_, T> { /// Register an index getter. /// /// The function signature must start with `&mut self` and not `&self`. @@ -281,13 +279,3 @@ impl TypeBuilder<'_, '_, T> { self } } - -impl Drop for TypeBuilder<'_, '_, T> { - #[inline] - fn drop(&mut self) { - match self.name { - Some(ref name) => self.engine.register_type_with_name::(name), - None => self.engine.register_type::(), - }; - } -} From 83597328815eb2e1713113a00825cf41e7045c1a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 30 Jan 2024 18:09:07 +0800 Subject: [PATCH 07/11] Format function signatures. --- CHANGELOG.md | 2 + examples/custom_types_and_methods.rs | 23 +++++------ src/api/definitions/mod.rs | 2 +- src/api/deprecated.rs | 18 ++++++++- src/api/formatting.rs | 58 +++++++++++----------------- src/api/register.rs | 15 ++++--- src/module/mod.rs | 32 ++++++++++----- src/serde/metadata.rs | 8 ++-- 8 files changed, 88 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4e8fe9ef..9fe366a56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ Deprecated API's * `rhai::config::hashing::set_ahash_seed`, `rhai::config::hashing::get_ahash_seed` and the `RHAI_AHASH_SEED` environment variable are deprecated in favor of `rhai::config::hashing::set_hashing_seed`, `rhai::config::hashing::get_hashing_seed` and `RHAI_HASHING_SEED`. * `AST::clear_doc` is deprecated. * Much of the `Module::update_XXX` API is deprecated in favor of using the `FuncRegistration` API. +* `Module::gen_fn_signatures` is deprecated in favor of `Module::gen_fn_signatures_with_mapper`. Fixes to bugs found via fuzzing ------------------------------- @@ -75,6 +76,7 @@ Enhancements * `Dynamic::is_fnptr` is made a public API. * `Scope::get_value_ref` and `Scope::get_value_mut` are added. * `TypeBuilder::with_name` now takes any `&str` instead of just `&'static str`. +* `Engine::gen_fn_signatures` now formats the function signatures using pretty-print names of custom types. Version 1.16.3 diff --git a/examples/custom_types_and_methods.rs b/examples/custom_types_and_methods.rs index fecbccfec..4dd0f5618 100644 --- a/examples/custom_types_and_methods.rs +++ b/examples/custom_types_and_methods.rs @@ -5,11 +5,12 @@ fn main() { panic!("This example does not run under 'no_object'."); } -use rhai::{Engine, EvalAltResult}; +use rhai::{CustomType, Engine, EvalAltResult, TypeBuilder}; #[cfg(not(feature = "no_object"))] fn main() -> Result<(), Box> { - #[derive(Debug, Clone)] + #[derive(Debug, Clone, CustomType)] + #[rhai_type(extra = Self::build_extra)] struct TestStruct { x: i64, } @@ -24,22 +25,18 @@ fn main() -> Result<(), Box> { pub fn calculate(&mut self, data: i64) -> i64 { self.x * data } - pub fn get_x(&mut self) -> i64 { - self.x - } - pub fn set_x(&mut self, value: i64) { - self.x = value; + + fn build_extra(builder: &mut TypeBuilder) { + builder + .with_fn("new_ts", TestStruct::new) + .with_fn("update", TestStruct::update) + .with_fn("calc", TestStruct::calculate); } } let mut engine = Engine::new(); - engine - .register_type_with_name::("TestStruct") - .register_fn("new_ts", TestStruct::new) - .register_fn("update", TestStruct::update) - .register_fn("calc", TestStruct::calculate) - .register_get_set("x", TestStruct::get_x, TestStruct::set_x); + engine.build_type::(); #[cfg(feature = "metadata")] { diff --git a/src/api/definitions/mod.rs b/src/api/definitions/mod.rs index da5147c1d..a2c19d868 100644 --- a/src/api/definitions/mod.rs +++ b/src/api/definitions/mod.rs @@ -533,7 +533,7 @@ impl FuncMetadata { /// Associated generic types are also rewritten into regular generic type parameters. #[must_use] fn def_type_name<'a>(ty: &'a str, engine: &'a Engine) -> Cow<'a, str> { - let ty = engine.format_type_name(ty).replace("crate::", ""); + let ty = engine.format_param_type(ty).replace("crate::", ""); let ty = ty.strip_prefix("&mut").unwrap_or(&*ty).trim(); let ty = ty.split("::").last().unwrap(); diff --git a/src/api/deprecated.rs b/src/api/deprecated.rs index 17248b62c..554800213 100644 --- a/src/api/deprecated.rs +++ b/src/api/deprecated.rs @@ -551,7 +551,7 @@ impl Position { } #[allow(deprecated)] -impl TypeBuilder<'_, '_, T> { +impl TypeBuilder<'_, T> { /// Register a custom fallible function. /// /// # Deprecated @@ -788,6 +788,22 @@ impl Module { ) -> &mut Self { self.update_fn_metadata_with_comments(hash_fn, arg_names, [""; 0]) } + + /// _(metadata)_ Generate signatures for all the non-private functions in the [`Module`]. + /// Exported under the `metadata` feature only. + /// + /// # Deprecated + /// + /// This method is deprecated. + /// Use [`gen_fn_signatures_with_mapper`][`Module::gen_fn_signatures_with_mapper`] instead. + /// + /// This method will be removed in the next major version. + #[deprecated(since = "1.17.0", note = "use `gen_fn_signatures_with_mapper` instead")] + #[cfg(feature = "metadata")] + #[inline(always)] + pub fn gen_fn_signatures(&self) -> impl Iterator + '_ { + self.gen_fn_signatures_with_mapper(|s| s.into()) + } } #[cfg(not(feature = "no_index"))] diff --git a/src/api/formatting.rs b/src/api/formatting.rs index d06e34b55..0fd630735 100644 --- a/src/api/formatting.rs +++ b/src/api/formatting.rs @@ -17,6 +17,13 @@ use std::prelude::v1::*; pub fn map_std_type_name(name: &str, shorthands: bool) -> &str { let name = name.trim(); + match name { + "INT" => return type_name::(), + #[cfg(not(feature = "no_float"))] + "FLOAT" => return type_name::(), + _ => (), + } + if name == type_name::() { return if shorthands { "string" } else { "String" }; } @@ -111,7 +118,7 @@ pub fn map_std_type_name(name: &str, shorthands: bool) -> &str { .map_or(name, |s| map_std_type_name(s, shorthands)) } -/// Format a Rust type to be display-friendly. +/// Format a Rust parameter type to be display-friendly. /// /// * `rhai::` prefix is cleared. /// * `()` is cleared. @@ -119,7 +126,7 @@ pub fn map_std_type_name(name: &str, shorthands: bool) -> &str { /// * `INT` and `FLOAT` are expanded. /// * [`RhaiResult`][crate::RhaiResult] and [`RhaiResultOf`][crate::RhaiResultOf] are expanded. #[cfg(feature = "metadata")] -pub fn format_type(typ: &str, is_return_type: bool) -> std::borrow::Cow { +pub fn format_param_type_for_display(typ: &str, is_return_type: bool) -> std::borrow::Cow { const RESULT_TYPE: &str = "Result<"; const ERROR_TYPE: &str = ",Box>"; const RHAI_RESULT_TYPE: &str = "RhaiResult"; @@ -136,9 +143,9 @@ pub fn format_type(typ: &str, is_return_type: bool) -> std::borrow::Cow { let typ = typ.trim(); if let Some(x) = typ.strip_prefix("rhai::") { - return format_type(x, is_return_type); + return format_param_type_for_display(x, is_return_type); } else if let Some(x) = typ.strip_prefix("&mut ") { - let r = format_type(x, false); + let r = format_param_type_for_display(x, false); return if r == x { typ.into() } else { @@ -146,7 +153,7 @@ pub fn format_type(typ: &str, is_return_type: bool) -> std::borrow::Cow { }; } else if typ.contains(' ') { let typ = typ.replace(' ', ""); - let r = format_type(&typ, is_return_type); + let r = format_param_type_for_display(&typ, is_return_type); return r.into_owned().into(); } @@ -165,25 +172,25 @@ pub fn format_type(typ: &str, is_return_type: bool) -> std::borrow::Cow { ty if ty.starts_with(RHAI_RANGE_TYPE) && ty.ends_with('>') => { let inner = &ty[RHAI_RANGE_TYPE.len()..ty.len() - 1]; RHAI_RANGE_EXPAND - .replace("{}", format_type(inner, false).trim()) + .replace("{}", format_param_type_for_display(inner, false).trim()) .into() } ty if ty.starts_with(RHAI_INCLUSIVE_RANGE_TYPE) && ty.ends_with('>') => { let inner = &ty[RHAI_INCLUSIVE_RANGE_TYPE.len()..ty.len() - 1]; RHAI_INCLUSIVE_RANGE_EXPAND - .replace("{}", format_type(inner, false).trim()) + .replace("{}", format_param_type_for_display(inner, false).trim()) .into() } ty if ty.starts_with(RHAI_RESULT_OF_TYPE) && ty.ends_with('>') => { let inner = &ty[RHAI_RESULT_OF_TYPE.len()..ty.len() - 1]; RHAI_RESULT_OF_TYPE_EXPAND - .replace("{}", format_type(inner, false).trim()) + .replace("{}", format_param_type_for_display(inner, false).trim()) .into() } ty if ty.starts_with(RESULT_TYPE) && ty.ends_with(ERROR_TYPE) => { let inner = &ty[RESULT_TYPE.len()..ty.len() - ERROR_TYPE.len()]; RHAI_RESULT_OF_TYPE_EXPAND - .replace("{}", format_type(inner, false).trim()) + .replace("{}", format_param_type_for_display(inner, false).trim()) .into() } ty => ty.into(), @@ -195,10 +202,6 @@ impl Engine { /// /// If a type is registered via [`register_type_with_name`][Engine::register_type_with_name], /// the type name provided for the registration will be used. - /// - /// # Panics - /// - /// Panics if the type name is `&mut`. #[inline] #[must_use] pub fn map_type_name<'a>(&'a self, name: &'a str) -> &'a str { @@ -218,41 +221,24 @@ impl Engine { .unwrap_or_else(|| map_std_type_name(name, true)) } - /// Format a type name. + /// Format a Rust parameter type. /// /// If a type is registered via [`register_type_with_name`][Engine::register_type_with_name], /// the type name provided for the registration will be used. + /// + /// This method properly handles type names beginning with `&mut`. #[cfg(feature = "metadata")] #[inline] #[must_use] - pub(crate) fn format_type_name<'a>(&'a self, name: &'a str) -> std::borrow::Cow<'a, str> { + pub(crate) fn format_param_type<'a>(&'a self, name: &'a str) -> std::borrow::Cow<'a, str> { if let Some(x) = name.strip_prefix("&mut ") { - return match self.format_type_name(x) { + return match self.format_param_type(x) { r if r == x => name.into(), r => format!("&mut {r}").into(), }; } - self.global_modules - .iter() - .find_map(|m| m.get_custom_type_display_by_name(name)) - .or_else(|| { - #[cfg(not(feature = "no_module"))] - return self - .global_sub_modules - .values() - .find_map(|m| m.get_custom_type_display_by_name(name)); - - #[cfg(feature = "no_module")] - return None; - }) - .unwrap_or_else(|| match name { - "INT" => type_name::(), - #[cfg(not(feature = "no_float"))] - "FLOAT" => type_name::(), - _ => map_std_type_name(name, false), - }) - .into() + self.map_type_name(name).into() } /// Make a `Box<`[`EvalAltResult`][ERR::ErrorMismatchDataType]`>`. diff --git a/src/api/register.rs b/src/api/register.rs index 3b02a41b3..42882d8fa 100644 --- a/src/api/register.rs +++ b/src/api/register.rs @@ -84,11 +84,11 @@ impl Engine { { let mut param_type_names = FUNC::param_names() .iter() - .map(|ty| format!("_: {}", self.format_type_name(ty))) + .map(|ty| format!("_: {}", self.format_param_type(ty))) .collect::>(); if FUNC::return_type() != TypeId::of::<()>() { - param_type_names.push(self.format_type_name(FUNC::return_type_name()).into()); + param_type_names.push(self.format_param_type(FUNC::return_type_name()).into()); } let param_type_names = param_type_names @@ -754,12 +754,17 @@ impl Engine { let mut signatures = Vec::with_capacity(64); if let Some(global_namespace) = self.global_modules.first() { - signatures.extend(global_namespace.gen_fn_signatures()); + signatures.extend( + global_namespace.gen_fn_signatures_with_mapper(|s| self.format_param_type(s)), + ); } #[cfg(not(feature = "no_module"))] for (name, m) in &self.global_sub_modules { - signatures.extend(m.gen_fn_signatures().map(|f| format!("{name}::{f}"))); + signatures.extend( + m.gen_fn_signatures_with_mapper(|s| self.format_param_type(s)) + .map(|f| format!("{name}::{f}")), + ); } let exclude_flags = if include_packages { @@ -773,7 +778,7 @@ impl Engine { .iter() .skip(1) .filter(|m| !m.flags.intersects(exclude_flags)) - .flat_map(|m| m.gen_fn_signatures()), + .flat_map(|m| m.gen_fn_signatures_with_mapper(|s| self.format_param_type(s))), ); signatures diff --git a/src/module/mod.rs b/src/module/mod.rs index 3d479d225..860bc29e8 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1,7 +1,7 @@ //! Module defining external-loaded modules for Rhai. #[cfg(feature = "metadata")] -use crate::api::formatting::format_type; +use crate::api::formatting::format_param_type_for_display; use crate::ast::FnAccess; use crate::func::{ shared_take_or_clone, FnIterator, RhaiFunc, RhaiNativeFunc, SendSync, StraightHashMap, @@ -101,10 +101,13 @@ impl FuncMetadata { /// Exported under the `metadata` feature only. #[cfg(feature = "metadata")] #[must_use] - pub fn gen_signature(&self) -> String { + pub fn gen_signature<'a>( + &'a self, + type_mapper: impl Fn(&'a str) -> std::borrow::Cow<'a, str>, + ) -> String { let mut signature = format!("{}(", self.name); - let return_type = format_type(&self.return_type, true); + let return_type = format_param_type_for_display(&self.return_type, true); if self.params_info.is_empty() { for x in 0..self.num_params { @@ -120,12 +123,18 @@ impl FuncMetadata { .map(|param| { let mut segment = param.splitn(2, ':'); let name = match segment.next().unwrap().trim() { - "" => "_", + "" => "_".into(), s => s, }; - let result: std::borrow::Cow = segment.next().map_or_else( + let result: std::borrow::Cow<_> = segment.next().map_or_else( || name.into(), - |typ| format!("{name}: {}", format_type(typ, false)).into(), + |typ| { + format!( + "{name}: {}", + format_param_type_for_display(&type_mapper(typ), false) + ) + .into() + }, ); result }) @@ -601,7 +610,7 @@ impl fmt::Debug for Module { #[cfg(not(feature = "metadata"))] return _f.to_string(); #[cfg(feature = "metadata")] - return _m.gen_signature(); + return _m.gen_signature(|s| s.into()); }) .collect::>(), ) @@ -1052,14 +1061,17 @@ impl Module { /// Exported under the `metadata` feature only. #[cfg(feature = "metadata")] #[inline] - pub fn gen_fn_signatures(&self) -> impl Iterator + '_ { + pub fn gen_fn_signatures_with_mapper<'a>( + &'a self, + type_mapper: impl Fn(&'a str) -> std::borrow::Cow<'a, str> + 'a, + ) -> impl Iterator + 'a { self.iter_fn() - .map(|(_, f)| f) + .map(|(_, m)| m) .filter(|&f| match f.access { FnAccess::Public => true, FnAccess::Private => false, }) - .map(FuncMetadata::gen_signature) + .map(move |m| m.gen_signature(&type_mapper)) } /// Does a variable exist in the [`Module`]? diff --git a/src/serde/metadata.rs b/src/serde/metadata.rs index 3f950b1b8..23d079657 100644 --- a/src/serde/metadata.rs +++ b/src/serde/metadata.rs @@ -1,7 +1,7 @@ //! Serialization of functions metadata. #![cfg(feature = "metadata")] -use crate::api::formatting::format_type; +use crate::api::formatting::format_param_type_for_display; use crate::func::RhaiFunc; use crate::module::{calc_native_fn_hash, FuncMetadata, ModuleFlags}; use crate::types::custom_types::CustomTypeInfo; @@ -151,12 +151,12 @@ impl<'a> From<(&'a RhaiFunc, &'a FuncMetadata)> for FnMetadata<'a> { "_" => None, s => Some(s), }; - let typ = seg.next().map(|s| format_type(s, false)); + let typ = seg.next().map(|s| format_param_type_for_display(s, false)); FnParam { name, typ } }) .collect(), - return_type: format_type(&m.return_type, true), - signature: m.gen_signature().into(), + return_type: format_param_type_for_display(&m.return_type, true), + signature: m.gen_signature(|s| s.into()).into(), doc_comments: if f.is_script() { #[cfg(feature = "no_function")] unreachable!("script-defined functions should not exist under no_function"); From 2c7e179d2a23eace464042e951947c620b3bed4f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 30 Jan 2024 19:07:34 +0800 Subject: [PATCH 08/11] Fix tests. --- src/api/register.rs | 5 +-- src/module/mod.rs | 88 ++++++++++++++++++--------------------------- tests/optimizer.rs | 9 ++--- 3 files changed, 41 insertions(+), 61 deletions(-) diff --git a/src/api/register.rs b/src/api/register.rs index 42882d8fa..be335d2af 100644 --- a/src/api/register.rs +++ b/src/api/register.rs @@ -75,10 +75,7 @@ impl Engine { name: impl AsRef + Into, func: FUNC, ) -> &mut Self { - let mut reg = FuncRegistration::new(name.into()) - .with_namespace(FnNamespace::Global) - .with_purity(true) - .with_volatility(false); + let mut reg = FuncRegistration::new(name.into()).with_namespace(FnNamespace::Global); #[cfg(feature = "metadata")] { diff --git a/src/module/mod.rs b/src/module/mod.rs index 860bc29e8..918b9bfc9 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -445,7 +445,9 @@ impl FuncRegistration { f.num_params = arg_types.as_ref().len(); f.param_types.extend(arg_types.as_ref().iter().copied()); - if f.name == crate::engine::FN_IDX_GET || f.name == crate::engine::FN_IDX_SET { + if (f.name == crate::engine::FN_IDX_GET && f.num_params == 2) + || (f.name == crate::engine::FN_IDX_SET && f.num_params == 3) + { if let Some(&type_id) = f.param_types.first() { #[cfg(not(feature = "no_index"))] assert!( @@ -1657,18 +1659,25 @@ impl Module { /// /// # Panics /// - /// Panics if the type is [`Array`][crate::Array] or [`Map`][crate::Map]. - /// Indexers for arrays, object maps and strings cannot be registered. + /// Panics if the type is [`Array`][crate::Array], [`Map`][crate::Map], [`String`], + /// [`ImmutableString`][crate::ImmutableString], `&str` or [`INT`][crate::INT]. + /// + /// Indexers for arrays, object maps, strings and integers cannot be registered. /// /// # Example /// /// ``` /// use rhai::{Module, ImmutableString}; /// + /// #[derive(Clone)] + /// struct TestStruct(i64); + /// /// let mut module = Module::new(); + /// /// let hash = module.set_indexer_get_fn( - /// |x: &mut i64, y: ImmutableString| Ok(*x + y.len() as i64) + /// |x: &mut TestStruct, y: ImmutableString| Ok(x.0 + y.len() as i64) /// ); + /// /// assert!(module.contains_fn(hash)); /// ``` #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] @@ -1680,24 +1689,6 @@ impl Module { R: Variant + Clone, FUNC: RhaiNativeFunc<(Mut, B), 2, X, R, true> + SendSync + 'static, { - #[cfg(not(feature = "no_index"))] - assert!( - TypeId::of::() != TypeId::of::(), - "Cannot register indexer for arrays." - ); - #[cfg(not(feature = "no_object"))] - assert!( - TypeId::of::() != TypeId::of::(), - "Cannot register indexer for object maps." - ); - - assert!( - TypeId::of::() != TypeId::of::() - && TypeId::of::() != TypeId::of::<&str>() - && TypeId::of::() != TypeId::of::(), - "Cannot register indexer for strings." - ); - FuncRegistration::new(crate::engine::FN_IDX_GET) .with_namespace(FnNamespace::Global) .with_purity(true) @@ -1722,24 +1713,26 @@ impl Module { /// /// # Panics /// - /// Panics if the type is [`Array`][crate::Array] or [`Map`][crate::Map]. - /// Indexers for arrays, object maps and strings cannot be registered. - /// - /// # Panics + /// Panics if the type is [`Array`][crate::Array], [`Map`][crate::Map], [`String`], + /// [`ImmutableString`][crate::ImmutableString], `&str` or [`INT`][crate::INT]. /// - /// Panics if the type is [`Array`][crate::Array] or [`Map`][crate::Map]. - /// Indexers for arrays, object maps and strings cannot be registered. + /// Indexers for arrays, object maps, strings and integers cannot be registered. /// /// # Example /// /// ``` /// use rhai::{Module, ImmutableString}; /// + /// #[derive(Clone)] + /// struct TestStruct(i64); + /// /// let mut module = Module::new(); - /// let hash = module.set_indexer_set_fn(|x: &mut i64, y: ImmutableString, value: i64| { - /// *x = y.len() as i64 + value; + /// + /// let hash = module.set_indexer_set_fn(|x: &mut TestStruct, y: ImmutableString, value: i64| { + /// *x = TestStruct(y.len() as i64 + value); /// Ok(()) - /// }); + /// }); + /// /// assert!(module.contains_fn(hash)); /// ``` #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] @@ -1751,24 +1744,6 @@ impl Module { R: Variant + Clone, FUNC: RhaiNativeFunc<(Mut, B, R), 3, X, (), true> + SendSync + 'static, { - #[cfg(not(feature = "no_index"))] - assert!( - TypeId::of::() != TypeId::of::(), - "Cannot register indexer for arrays." - ); - #[cfg(not(feature = "no_object"))] - assert!( - TypeId::of::() != TypeId::of::(), - "Cannot register indexer for object maps." - ); - - assert!( - TypeId::of::() != TypeId::of::() - && TypeId::of::() != TypeId::of::<&str>() - && TypeId::of::() != TypeId::of::(), - "Cannot register indexer for strings." - ); - FuncRegistration::new(crate::engine::FN_IDX_SET) .with_namespace(FnNamespace::Global) .with_purity(false) @@ -1787,19 +1762,26 @@ impl Module { /// /// # Panics /// - /// Panics if the type is [`Array`][crate::Array] or [`Map`][crate::Map]. - /// Indexers for arrays, object maps and strings cannot be registered. + /// Panics if the type is [`Array`][crate::Array], [`Map`][crate::Map], [`String`], + /// [`ImmutableString`][crate::ImmutableString], `&str` or [`INT`][crate::INT]. + /// + /// Indexers for arrays, object maps, strings and integers cannot be registered. /// /// # Example /// /// ``` /// use rhai::{Module, ImmutableString}; /// + /// #[derive(Clone)] + /// struct TestStruct(i64); + /// /// let mut module = Module::new(); + /// /// let (hash_get, hash_set) = module.set_indexer_get_set_fn( - /// |x: &mut i64, y: ImmutableString| Ok(*x + y.len() as i64), - /// |x: &mut i64, y: ImmutableString, value: i64| { *x = y.len() as i64 + value; Ok(()) } + /// |x: &mut TestStruct, y: ImmutableString| Ok(x.0 + y.len() as i64), + /// |x: &mut TestStruct, y: ImmutableString, value: i64| { *x = TestStruct(y.len() as i64 + value); Ok(()) } /// ); + /// /// assert!(module.contains_fn(hash_get)); /// assert!(module.contains_fn(hash_set)); /// ``` diff --git a/tests/optimizer.rs b/tests/optimizer.rs index 8aa27478d..97accf07e 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -1,5 +1,5 @@ #![cfg(not(feature = "no_optimize"))] -use rhai::{CustomType, Engine, FuncRegistration, Module, OptimizationLevel, Scope, TypeBuilder, INT}; +use rhai::{Engine, FuncRegistration, Module, OptimizationLevel, Scope, INT}; #[test] fn test_optimizer() { @@ -147,7 +147,7 @@ fn test_optimizer_re_optimize() { #[test] fn test_optimizer_full() { - #[derive(Debug, Clone, CustomType)] + #[derive(Debug, Clone)] struct TestStruct(INT); let mut engine = Engine::new(); @@ -170,15 +170,16 @@ fn test_optimizer_full() { ); engine - .build_type::() + .register_type_with_name::("TestStruct") .register_fn("ts", |n: INT| TestStruct(n)) + .register_fn("value", |ts: &mut TestStruct| ts.0) .register_fn("+", |ts1: &mut TestStruct, ts2: TestStruct| TestStruct(ts1.0 + ts2.0)); let ast = engine .compile( " const FOO = ts(40) + ts(2); - field0(FOO) + value(FOO) ", ) .unwrap(); From db7466bae338ebf0db6b7f927d6ff4466615541e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 30 Jan 2024 19:24:20 +0800 Subject: [PATCH 09/11] Fix builds. --- src/api/register.rs | 8 ++++---- src/module/mod.rs | 1 + src/tokenizer.rs | 20 ++++++++++---------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/api/register.rs b/src/api/register.rs index be335d2af..998aad2e2 100644 --- a/src/api/register.rs +++ b/src/api/register.rs @@ -75,10 +75,10 @@ impl Engine { name: impl AsRef + Into, func: FUNC, ) -> &mut Self { - let mut reg = FuncRegistration::new(name.into()).with_namespace(FnNamespace::Global); + let reg = FuncRegistration::new(name.into()).with_namespace(FnNamespace::Global); #[cfg(feature = "metadata")] - { + let reg = { let mut param_type_names = FUNC::param_names() .iter() .map(|ty| format!("_: {}", self.format_param_type(ty))) @@ -93,8 +93,8 @@ impl Engine { .map(String::as_str) .collect::>(); - reg = reg.with_params_info(param_type_names); - } + reg.with_params_info(param_type_names) + }; reg.set_into_module(self.global_namespace_mut(), func); diff --git a/src/module/mod.rs b/src/module/mod.rs index 918b9bfc9..6d9d31a3d 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -445,6 +445,7 @@ impl FuncRegistration { f.num_params = arg_types.as_ref().len(); f.param_types.extend(arg_types.as_ref().iter().copied()); + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] if (f.name == crate::engine::FN_IDX_GET && f.num_params == 2) || (f.name == crate::engine::FN_IDX_SET && f.num_params == 3) { diff --git a/src/tokenizer.rs b/src/tokenizer.rs index 191fb6d12..b6ef01e8a 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -1591,8 +1591,8 @@ fn get_next_token_inner( let mut result = SmartString::new_const(); let mut radix_base: Option = None; let mut valid: fn(char) -> bool = is_numeric_digit; - let mut has_period = false; - let mut has_e = false; + let mut _has_period = false; + let mut _has_e = false; result.push(c); @@ -1606,7 +1606,7 @@ fn get_next_token_inner( stream.eat_next_and_advance(pos); } #[cfg(any(not(feature = "no_float"), feature = "decimal"))] - '.' if !has_period && radix_base.is_none() => { + '.' if !_has_period && radix_base.is_none() => { stream.get_next().unwrap(); // Check if followed by digits or something that cannot start a property name @@ -1615,7 +1615,7 @@ fn get_next_token_inner( Some('0'..='9') => { result.push('.'); pos.advance(); - has_period = true; + _has_period = true; } // _ - cannot follow a decimal point Some(NUMBER_SEPARATOR) => { @@ -1632,7 +1632,7 @@ fn get_next_token_inner( result.push('.'); pos.advance(); result.push('0'); - has_period = true; + _has_period = true; } // Not a floating-point number _ => { @@ -1642,7 +1642,7 @@ fn get_next_token_inner( } } #[cfg(not(feature = "no_float"))] - 'e' if !has_e && radix_base.is_none() => { + 'e' if !_has_e && radix_base.is_none() => { stream.get_next().unwrap(); // Check if followed by digits or +/- @@ -1651,8 +1651,8 @@ fn get_next_token_inner( Some('0'..='9') => { result.push('e'); pos.advance(); - has_e = true; - has_period = true; + _has_e = true; + _has_period = true; } // +/- after e - accept the e and the sign (no decimal points allowed) Some('+' | '-') => { @@ -1660,8 +1660,8 @@ fn get_next_token_inner( pos.advance(); result.push(stream.get_next().unwrap()); pos.advance(); - has_e = true; - has_period = true; + _has_e = true; + _has_period = true; } // Not a floating-point number _ => { From bcb20cd06aea1e3be8dca58f5fa327f3575ce5a3 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 30 Jan 2024 19:56:24 +0800 Subject: [PATCH 10/11] Fix test. --- tests/optimizer.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/optimizer.rs b/tests/optimizer.rs index 97accf07e..e51ced358 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -200,7 +200,7 @@ fn test_optimizer_volatile() { engine.set_optimization_level(OptimizationLevel::Full); - FuncRegistration::new("foo").with_volatility(true).register_into_engine(&mut engine, |x: i64| x + 1); + FuncRegistration::new("foo").with_volatility(true).register_into_engine(&mut engine, |x: INT| x + 1); let ast = engine.compile("foo(42)").unwrap(); @@ -209,11 +209,12 @@ fn test_optimizer_volatile() { // Make sure the call is not optimized away assert!(text_ast.contains(r#"name: "foo""#)); - FuncRegistration::new("foo").with_volatility(false).register_into_engine(&mut engine, |x: i64| x + 1); + FuncRegistration::new("foo").with_volatility(false).register_into_engine(&mut engine, |x: INT| x + 1); let ast = engine.compile("foo(42)").unwrap(); let text_ast = format!("{ast:?}"); + println!("{text_ast:#?}"); // Make sure the call is optimized away assert!(!text_ast.contains(r#"name: "foo""#)); From 11e9cc7b6097262e915e8b2b63efeb06ea862856 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 30 Jan 2024 22:51:53 +0800 Subject: [PATCH 11/11] Rewrite iterators using FuncRegistration. --- src/lib.rs | 1 - src/packages/iter_basic.rs | 535 +++++++++++++++-------------------- src/packages/string_basic.rs | 4 +- src/packages/string_more.rs | 44 +-- 4 files changed, 253 insertions(+), 331 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 44cbfb53d..0d8e70ffa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -226,7 +226,6 @@ use once_cell::sync::OnceCell; #[cfg(not(feature = "std"))] use once_cell::race::OnceBox as OnceCell; -#[allow(deprecated)] pub use api::build_type::{CustomType, TypeBuilder}; #[cfg(not(feature = "no_custom_syntax"))] pub use api::custom_syntax::Expression; diff --git a/src/packages/iter_basic.rs b/src/packages/iter_basic.rs index a4f0417ae..e6f81f853 100644 --- a/src/packages/iter_basic.rs +++ b/src/packages/iter_basic.rs @@ -1,6 +1,7 @@ use crate::eval::calc_index; use crate::module::ModuleFlags; use crate::plugin::*; +use crate::FuncRegistration; use crate::{ def_package, ExclusiveRange, InclusiveRange, RhaiResultOf, ERR, INT, INT_BITS, MAX_USIZE_INT, }; @@ -241,15 +242,15 @@ macro_rules! reg_range { ($lib:ident | $x:expr => $( $y:ty ),*) => { $( $lib.set_iterator::>(); - let _hash = $lib.set_native_fn($x, |from: $y, to: $y| Ok(from..to)); + + let f = FuncRegistration::new($x); #[cfg(feature = "metadata")] - #[allow(deprecated)] - $lib.update_fn_metadata_with_comments(_hash, [ - concat!("from: ", stringify!($y)), - concat!("to: ", stringify!($y)), - concat!("Iterator<", stringify!($y), ">"), - ], ["\ + let f = f.with_params_info([ + concat!("from: ", stringify!($y)), + concat!("to: ", stringify!($y)), + concat!("Iterator<", stringify!($y), ">"), + ]).with_comments(["\ /// Return an iterator over the exclusive range of `from..to`.\n\ /// The value `to` is never included.\n\ ///\n\ @@ -263,6 +264,8 @@ macro_rules! reg_range { /// ```" ]); + f.set_into_module($lib, |from: $y, to: $y| from..to); + $lib.set_iterator::>(); )* }; @@ -275,16 +278,16 @@ macro_rules! reg_range { ($lib:ident | step ( $add:ident ) $x:expr => $( $y:ty ),*) => { $( $lib.set_iterator::>(); - let _hash = $lib.set_native_fn($x, |from: $y, to: $y, step: $y| StepRange::new(from, to, step, $add)); + + let f = FuncRegistration::new($x); #[cfg(feature = "metadata")] - #[allow(deprecated)] - $lib.update_fn_metadata_with_comments(_hash, [ - concat!("from: ", stringify!($y)), - concat!("to: ", stringify!($y)), - concat!("step: ", stringify!($y)), - concat!("Iterator<", stringify!($y), ">") - ], ["\ + let f = f.with_params_info([ + concat!("from: ", stringify!($y)), + concat!("to: ", stringify!($y)), + concat!("step: ", stringify!($y)), + concat!("Iterator<", stringify!($y), ">") + ]).with_comments(["\ /// Return an iterator over the exclusive range of `from..to`, each iteration increasing by `step`.\n\ /// The value `to` is never included.\n\ ///\n\ @@ -307,15 +310,16 @@ macro_rules! reg_range { /// ```" ]); - let _hash = $lib.set_native_fn($x, |range: std::ops::Range<$y>, step: $y| StepRange::new(range.start, range.end, step, $add)); + f.set_into_module($lib, |from: $y, to: $y, step: $y| StepRange::new(from, to, step, $add)); + + let f = FuncRegistration::new($x); #[cfg(feature = "metadata")] - #[allow(deprecated)] - $lib.update_fn_metadata_with_comments(_hash, [ - concat!("range: Range<", stringify!($y), ">"), - concat!("step: ", stringify!($y)), - concat!("Iterator<", stringify!($y), ">") - ], ["\ + let f = f.with_params_info([ + concat!("range: Range<", stringify!($y), ">"), + concat!("step: ", stringify!($y)), + concat!("Iterator<", stringify!($y), ">") + ]).with_comments(["\ /// Return an iterator over an exclusive range, each iteration increasing by `step`.\n\ ///\n\ /// If `range` is reversed and `step` < 0, iteration goes backwards.\n\ @@ -336,6 +340,8 @@ macro_rules! reg_range { /// }\n\ /// ```" ]); + + f.set_into_module($lib, |range: std::ops::Range<$y>, step: $y| StepRange::new(range.start, range.end, step, $add)); )* }; } @@ -376,297 +382,214 @@ def_package! { // Register string iterator lib.set_iterator::(); - #[cfg(feature = "metadata")] - let (range_type, range_inclusive_type) = ( - format!("range: Range<{}>", type_name::()), - format!("range: RangeInclusive<{}>", type_name::()), - ); - - let _hash = lib.set_native_fn("chars", |string, range: ExclusiveRange| { - let from = INT::max(range.start, 0); - let to = INT::max(range.end, from); - Ok(CharsStream::new(string, from, to - from)) - }); - #[cfg(feature = "metadata")] - #[allow(deprecated)] - lib.update_fn_metadata_with_comments( - _hash, - ["string: &str", &range_type, "Iterator"], - [ - "/// Return an iterator over an exclusive range of characters in the string.", - "///", - "/// # Example", - "///", - "/// ```rhai", - r#"/// for ch in "hello, world!".chars(2..5) {"#, - "/// print(ch);", - "/// }", - "/// ```" - ] - ); - - let _hash = lib.set_native_fn("chars", |string, range: InclusiveRange| { - let from = INT::max(*range.start(), 0); - let to = INT::max(*range.end(), from - 1); - Ok(CharsStream::new(string, from, to-from + 1)) - }); - #[cfg(feature = "metadata")] - #[allow(deprecated)] - lib.update_fn_metadata_with_comments( - _hash, - ["string: &str", &range_inclusive_type, "Iterator"], - [ - "/// Return an iterator over an inclusive range of characters in the string.", - "///", - "/// # Example", - "///", - "/// ```rhai", - r#"/// for ch in "hello, world!".chars(2..=6) {"#, - "/// print(ch);", - "/// }", - "/// ```" - ] - ); - - let _hash = lib.set_native_fn("chars", |string, from, len| Ok(CharsStream::new(string, from, len))); - #[cfg(feature = "metadata")] - #[allow(deprecated)] - lib.update_fn_metadata_with_comments( - _hash, - ["string: &str", "start: INT", "len: INT", "Iterator"], - [ - "/// Return an iterator over a portion of characters in the string.", - "///", - "/// * If `start` < 0, position counts from the end of the string (`-1` is the last character).", - "/// * If `start` < -length of string, position counts from the beginning of the string.", - "/// * If `start` ≥ length of string, an empty iterator is returned.", - "/// * If `len` ≤ 0, an empty iterator is returned.", - "/// * If `start` position + `len` ≥ length of string, all characters of the string after the `start` position are iterated.", - "///", - "/// # Example", - "///", - "/// ```rhai", - r#"/// for ch in "hello, world!".chars(2, 4) {"#, - "/// print(ch);", - "/// }", - "/// ```" - ] - ); - - let _hash = lib.set_native_fn("chars", |string, from| Ok(CharsStream::new(string, from, INT::MAX))); - #[cfg(feature = "metadata")] - #[allow(deprecated)] - lib.update_fn_metadata_with_comments( - _hash, - ["string: &str", "from: INT", "Iterator"], - [ - "/// Return an iterator over the characters in the string starting from the `start` position.", - "///", - "/// * If `start` < 0, position counts from the end of the string (`-1` is the last character).", - "/// * If `start` < -length of string, position counts from the beginning of the string.", - "/// * If `start` ≥ length of string, an empty iterator is returned.", - "///", - "/// # Example", - "///", - "/// ```rhai", - r#"/// for ch in "hello, world!".chars(2) {"#, - "/// print(ch);", - "/// }", - "/// ```" - ] - ); - - let _hash = lib.set_native_fn("chars", |string| Ok(CharsStream::new(string, 0, INT::MAX))); - #[cfg(feature = "metadata")] - #[allow(deprecated)] - lib.update_fn_metadata_with_comments( - _hash, - ["string: &str", "Iterator"], - [ - "/// Return an iterator over the characters in the string.", - "///", - "/// # Example", - "///", - "/// ```rhai", - r#"/// for ch in "hello, world!".chars() {"#, - "/// print(ch);", - "/// }", - "/// ```" - ] - ); - - #[cfg(not(feature = "no_object"))] - { - let _hash = lib.set_getter_fn("chars", |string: &mut ImmutableString| Ok(CharsStream::new(string, 0, INT::MAX))); - #[cfg(feature = "metadata")] - #[allow(deprecated)] - lib.update_fn_metadata_with_comments( - _hash, - ["string: &mut ImmutableString", "Iterator"], - [ - "/// Return an iterator over all the characters in the string.", - "///", - "/// # Example", - "///", - "/// ```rhai", - r#"/// for ch in "hello, world!".chars {"#, - "/// print(ch);", - "/// }", - "/// ```" - ] - ); - } - // Register bit-field iterator lib.set_iterator::(); - let _hash = lib.set_native_fn("bits", |value, range: ExclusiveRange| { - let from = INT::max(range.start, 0); - let to = INT::max(range.end, from); - BitRange::new(value, from, to - from) - }); - #[cfg(feature = "metadata")] - #[allow(deprecated)] - lib.update_fn_metadata_with_comments( - _hash, - ["value: INT", &range_type, "Iterator"], - [ - "/// Return an iterator over an exclusive range of bits in the number.", - "///", - "/// # Example", - "///", - "/// ```rhai", - "/// let x = 123456;", - "///", - "/// for bit in x.bits(10..24) {", - "/// print(bit);", - "/// }", - "/// ```" - ] - ); - - let _hash = lib.set_native_fn("bits", |value, range: InclusiveRange| { - let from = INT::max(*range.start(), 0); - let to = INT::max(*range.end(), from - 1); - BitRange::new(value, from, to - from + 1) - }); - #[cfg(feature = "metadata")] - #[allow(deprecated)] - lib.update_fn_metadata_with_comments( - _hash, - ["value: INT", &range_inclusive_type, "Iterator"], - [ - "/// Return an iterator over an inclusive range of bits in the number.", - "///", - "/// # Example", - "///", - "/// ```rhai", - "/// let x = 123456;", - "///", - "/// for bit in x.bits(10..=23) {", - "/// print(bit);", - "/// }", - "/// ```" - ] - ); - - let _hash = lib.set_native_fn("bits", BitRange::new); - #[cfg(feature = "metadata")] - #[allow(deprecated)] - lib.update_fn_metadata_with_comments( - _hash, - ["value: INT", "from: INT", "len: INT", "Iterator"], - [ - "/// Return an iterator over a portion of bits in the number.", - "///", - "/// * If `start` < 0, position counts from the MSB (Most Significant Bit)>.", - "/// * If `len` ≤ 0, an empty iterator is returned.", - "/// * If `start` position + `len` ≥ length of string, all bits of the number after the `start` position are iterated.", - "///", - "/// # Example", - "///", - "/// ```rhai", - "/// let x = 123456;", - "///", - "/// for bit in x.bits(10, 8) {", - "/// print(bit);", - "/// }", - "/// ```" - ] - ); - - let _hash = lib.set_native_fn("bits", |value, from| BitRange::new(value, from, INT::MAX)); - #[cfg(feature = "metadata")] - #[allow(deprecated)] - lib.update_fn_metadata_with_comments( - _hash, - ["value: INT", "from: INT", "Iterator"], - [ - "/// Return an iterator over the bits in the number starting from the specified `start` position.", - "///", - "/// If `start` < 0, position counts from the MSB (Most Significant Bit)>.", - "///", - "/// # Example", - "///", - "/// ```rhai", - "/// let x = 123456;", - "///", - "/// for bit in x.bits(10) {", - "/// print(bit);", - "/// }", - "/// ```" - ] - ); - - let _hash = lib.set_native_fn("bits", |value| BitRange::new(value, 0, INT::MAX) ); - #[cfg(feature = "metadata")] - #[allow(deprecated)] - lib.update_fn_metadata_with_comments( - _hash, - ["value: INT", "Iterator"], - [ - "/// Return an iterator over all the bits in the number.", - "///", - "/// # Example", - "///", - "/// ```rhai", - "/// let x = 123456;", - "///", - "/// for bit in x.bits() {", - "/// print(bit);", - "/// }", - "/// ```" - ] - ); - - #[cfg(not(feature = "no_object"))] - { - let _hash = lib.set_getter_fn("bits", |value: &mut INT| BitRange::new(*value, 0, INT::MAX) ); - #[cfg(feature = "metadata")] - #[allow(deprecated)] - lib.update_fn_metadata_with_comments( - _hash, - ["value: &mut INT", "Iterator"], - [ - "/// Return an iterator over all the bits in the number.", - "///", - "/// # Example", - "///", - "/// ```rhai", - "/// let x = 123456;", - "///", - "/// for bit in x.bits {", - "/// print(bit);", - "/// }", - "/// ```" - ] - ); - } - + // Register iterator functions + combine_with_exported_module!(lib, "iterator", iterator_functions); combine_with_exported_module!(lib, "range", range_functions); } } +#[export_module] +mod iterator_functions { + /// Return an iterator over an exclusive range of characters in the string. + /// + /// # Example + /// + /// ```rhai + /// for ch in "hello, world!".chars(2..5) { + /// print(ch); + /// } + /// ``` + #[rhai_fn(name = "chars")] + pub fn chars_from_exclusive_range(string: &str, range: ExclusiveRange) -> CharsStream { + let from = INT::max(range.start, 0); + let to = INT::max(range.end, from); + CharsStream::new(string, from, to - from) + } + /// Return an iterator over an inclusive range of characters in the string. + /// + /// # Example + /// + /// ```rhai + /// for ch in "hello, world!".chars(2..=6) { + /// print(ch); + /// } + /// ``` + #[rhai_fn(name = "chars")] + pub fn chars_from_inclusive_range(string: &str, range: InclusiveRange) -> CharsStream { + let from = INT::max(*range.start(), 0); + let to = INT::max(*range.end(), from - 1); + CharsStream::new(string, from, to - from + 1) + } + /// Return an iterator over a portion of characters in the string. + /// + /// * If `start` < 0, position counts from the end of the string (`-1` is the last character). + /// * If `start` < -length of string, position counts from the beginning of the string. + /// * If `start` ≥ length of string, an empty iterator is returned. + /// * If `len` ≤ 0, an empty iterator is returned. + /// * If `start` position + `len` ≥ length of string, all characters of the string after the `start` position are iterated. + /// + /// # Example + /// + /// ```rhai + /// for ch in "hello, world!".chars(2, 4) { + /// print(ch); + /// } + /// ``` + #[rhai_fn(name = "chars")] + pub fn chars_from_start_len(string: &str, start: INT, len: INT) -> CharsStream { + CharsStream::new(string, start, len) + } + /// Return an iterator over the characters in the string starting from the `start` position. + /// + /// * If `start` < 0, position counts from the end of the string (`-1` is the last character). + /// * If `start` < -length of string, position counts from the beginning of the string. + /// * If `start` ≥ length of string, an empty iterator is returned. + /// + /// # Example + /// + /// ```rhai + /// for ch in "hello, world!".chars(2) { + /// print(ch); + /// } + /// ``` + #[rhai_fn(name = "chars")] + pub fn chars_from_start(string: &str, start: INT) -> CharsStream { + CharsStream::new(string, start, INT::MAX) + } + /// Return an iterator over the characters in the string. + /// + /// # Example + /// + /// ```rhai + /// for ch in "hello, world!".chars() { + /// print(ch); + /// } + /// ``` + #[rhai_fn(name = "chars")] + pub fn chars(string: &str) -> CharsStream { + CharsStream::new(string, 0, INT::MAX) + } + /// Return an iterator over all the characters in the string. + /// + /// # Example + /// + /// ```rhai + /// for ch in "hello, world!".chars {" + /// print(ch); + /// } + /// ``` + #[cfg(not(feature = "no_object"))] + #[rhai_fn(get = "chars")] + pub fn get_chars(string: &str) -> CharsStream { + CharsStream::new(string, 0, INT::MAX) + } + + /// Return an iterator over an exclusive range of bits in the number. + /// + /// # Example + /// + /// ```rhai + /// let x = 123456; + /// + /// for bit in x.bits(10..24) { + /// print(bit); + /// } + /// ``` + #[rhai_fn(name = "bits", return_raw)] + pub fn bits_from_exclusive_range(value: INT, range: ExclusiveRange) -> RhaiResultOf { + let from = INT::max(range.start, 0); + let to = INT::max(range.end, from); + BitRange::new(value, from, to - from) + } + /// Return an iterator over an inclusive range of bits in the number. + /// + /// # Example + /// + /// ```rhai + /// let x = 123456; + /// + /// for bit in x.bits(10..=23) { + /// print(bit); + /// } + /// ``` + #[rhai_fn(name = "bits", return_raw)] + pub fn bits_from_inclusive_range(value: INT, range: InclusiveRange) -> RhaiResultOf { + let from = INT::max(*range.start(), 0); + let to = INT::max(*range.end(), from - 1); + BitRange::new(value, from, to - from + 1) + } + /// Return an iterator over a portion of bits in the number. + /// + /// * If `start` < 0, position counts from the MSB (Most Significant Bit)>. + /// * If `len` ≤ 0, an empty iterator is returned. + /// * If `start` position + `len` ≥ length of string, all bits of the number after the `start` position are iterated. + /// + /// # Example + /// + /// ```rhai + /// let x = 123456; + /// + /// for bit in x.bits(10, 8) { + /// print(bit); + /// } + /// ``` + #[rhai_fn(name = "bits", return_raw)] + pub fn bits_from_start_and_len(value: INT, from: INT, len: INT) -> RhaiResultOf { + BitRange::new(value, from, len) + } + /// Return an iterator over the bits in the number starting from the specified `start` position. + /// + /// If `start` < 0, position counts from the MSB (Most Significant Bit)>. + /// + /// # Example + /// + /// ```rhai + /// let x = 123456; + /// + /// for bit in x.bits(10) { + /// print(bit); + /// } + /// ``` + #[rhai_fn(name = "bits", return_raw)] + pub fn bits_from_start(value: INT, from: INT) -> RhaiResultOf { + BitRange::new(value, from, INT::MAX) + } + /// Return an iterator over all the bits in the number. + /// + /// # Example + /// + /// ```rhai + /// let x = 123456; + /// + /// for bit in x.bits() { + /// print(bit); + /// } + /// ``` + #[rhai_fn(name = "bits", return_raw)] + pub fn bits(value: INT) -> RhaiResultOf { + BitRange::new(value, 0, INT::MAX) + } + /// Return an iterator over all the bits in the number. + /// + /// # Example + /// + /// ```rhai + /// let x = 123456; + /// + /// for bit in x.bits { + /// print(bit); + /// } + /// ``` + #[cfg(not(feature = "no_object"))] + #[rhai_fn(get = "bits", return_raw)] + pub fn get_bits(value: INT) -> RhaiResultOf { + BitRange::new(value, 0, INT::MAX) + } +} + #[export_module] mod range_functions { /// Return the start of the exclusive range. diff --git a/src/packages/string_basic.rs b/src/packages/string_basic.rs index 30fcacea5..d7a34f514 100644 --- a/src/packages/string_basic.rs +++ b/src/packages/string_basic.rs @@ -100,8 +100,8 @@ mod print_debug_functions { string } /// Convert the string into debug format. - #[rhai_fn(name = "debug", name = "to_debug", pure)] - pub fn debug_string(string: &mut ImmutableString) -> ImmutableString { + #[rhai_fn(name = "debug", name = "to_debug")] + pub fn debug_string(string: &str) -> ImmutableString { let mut buf = SmartString::new_const(); write!(&mut buf, "{string:?}").unwrap(); buf.into() diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index 1884c1f2b..e06dc3f95 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -66,16 +66,19 @@ mod string_functions { // The following are needed in order to override the generic versions with `Dynamic` parameters. - #[rhai_fn(name = "+", pure)] - pub fn add_append_str( - string1: &mut ImmutableString, - string2: ImmutableString, - ) -> ImmutableString { - &*string1 + string2 + #[rhai_fn(name = "+")] + pub fn add_append_str(string1: &str, string2: &str) -> ImmutableString { + let mut buf = SmartString::new_const(); + buf.push_str(string1); + buf.push_str(string2); + buf.into() } - #[rhai_fn(name = "+", pure)] - pub fn add_append_char(string: &mut ImmutableString, character: char) -> ImmutableString { - &*string + character + #[rhai_fn(name = "+")] + pub fn add_append_char(string: &str, character: char) -> ImmutableString { + let mut buf = SmartString::new_const(); + buf.push_str(string); + buf.push(character); + buf.into() } #[rhai_fn(name = "+")] pub fn add_prepend_char(character: char, string: &str) -> ImmutableString { @@ -112,22 +115,21 @@ mod string_functions { pub mod blob_functions { use crate::Blob; - #[rhai_fn(name = "+", pure)] - pub fn add_append(string: &mut ImmutableString, utf8: Blob) -> ImmutableString { + #[rhai_fn(name = "+")] + pub fn add_append(string: ImmutableString, utf8: Blob) -> ImmutableString { if utf8.is_empty() { - return string.clone(); + return string; } let s = String::from_utf8_lossy(&utf8); if string.is_empty() { match s { - std::borrow::Cow::Borrowed(_) => String::from_utf8(utf8).unwrap(), - std::borrow::Cow::Owned(_) => s.into_owned(), + std::borrow::Cow::Borrowed(_) => String::from_utf8(utf8).unwrap().into(), + std::borrow::Cow::Owned(_) => s.into_owned().into(), } - .into() } else { - let mut x = >::as_ref(string).clone(); + let mut x = string.into_owned(); x.push_str(s.as_ref()); x.into() } @@ -383,10 +385,9 @@ mod string_functions { /// /// print(text); // prints "hello, world!" /// ``` - #[rhai_fn(pure)] - pub fn to_upper(string: &mut ImmutableString) -> ImmutableString { + pub fn to_upper(string: ImmutableString) -> ImmutableString { if string.chars().all(char::is_uppercase) { - return string.clone(); + return string; } string.to_uppercase().into() @@ -420,10 +421,9 @@ mod string_functions { /// /// print(text); // prints "HELLO, WORLD!" /// ``` - #[rhai_fn(pure)] - pub fn to_lower(string: &mut ImmutableString) -> ImmutableString { + pub fn to_lower(string: ImmutableString) -> ImmutableString { if string.is_empty() || string.chars().all(char::is_lowercase) { - return string.clone(); + return string; } string.to_lowercase().into()