From c4fe1782df69e8e247eb0bf30d513ccbe0a76889 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 24 Mar 2021 13:17:52 +0800 Subject: [PATCH] Streamline string handling. --- codegen/src/lib.rs | 2 +- examples/strings.rs | 2 +- src/ast.rs | 4 +-- src/dynamic.rs | 2 +- src/engine.rs | 4 +-- src/engine_api.rs | 64 ++++++++++++++++++++++++++++-------------- src/engine_settings.rs | 12 ++++---- src/fn_call.rs | 16 +++++------ src/fn_native.rs | 4 ++- src/module/mod.rs | 49 +++++++++++++++++--------------- src/parser.rs | 17 ++++------- src/utils.rs | 35 ++++++++++++++++++++--- 12 files changed, 129 insertions(+), 82 deletions(-) diff --git a/codegen/src/lib.rs b/codegen/src/lib.rs index c39e0860..209b8227 100644 --- a/codegen/src/lib.rs +++ b/codegen/src/lib.rs @@ -293,7 +293,7 @@ pub fn register_exported_fn(args: proc_macro::TokenStream) -> proc_macro::TokenS Ok((engine_expr, export_name, rust_mod_path)) => { let gen_mod_path = crate::register::generated_module_path(&rust_mod_path); proc_macro::TokenStream::from(quote! { - #engine_expr.register_result_fn(&(#export_name), #gen_mod_path::dynamic_result_fn); + #engine_expr.register_result_fn(#export_name, #gen_mod_path::dynamic_result_fn); }) } Err(e) => e.to_compile_error().into(), diff --git a/examples/strings.rs b/examples/strings.rs index f10fac94..ba5749ec 100644 --- a/examples/strings.rs +++ b/examples/strings.rs @@ -21,7 +21,7 @@ fn count_string_bytes(s: &str) -> INT { /// This version uses `ImmutableString` and `&str`. fn find_substring(s: ImmutableString, sub: &str) -> INT { - s.as_str().find(sub).map(|x| x as INT).unwrap_or(-1) + s.find(sub).map(|x| x as INT).unwrap_or(-1) } fn main() -> Result<(), Box> { diff --git a/src/ast.rs b/src/ast.rs index 17c981e0..4c04160f 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -6,7 +6,7 @@ use crate::module::NamespaceRef; use crate::stdlib::{ borrow::Cow, boxed::Box, - collections::BTreeMap, + collections::{BTreeMap, BTreeSet}, fmt, hash::Hash, num::NonZeroUsize, @@ -58,7 +58,7 @@ pub struct ScriptFnDef { pub params: StaticVec, /// Access to external variables. #[cfg(not(feature = "no_closure"))] - pub externals: StaticVec, + pub externals: BTreeSet, /// Function doc-comments (if any). pub comments: StaticVec, } diff --git a/src/dynamic.rs b/src/dynamic.rs index 014a0ed9..6c3f9dbf 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -1349,7 +1349,7 @@ impl Dynamic { } if TypeId::of::() == TypeId::of::() { return match &self.0 { - Union::Str(value, _) => ::downcast_ref::(value.as_ref()), + Union::Str(value, _) => ::downcast_ref::(value.as_ref() as &String), _ => None, }; } diff --git a/src/engine.rs b/src/engine.rs index cab9bb17..8652ef25 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -75,7 +75,7 @@ impl Imports { .iter() .enumerate() .rev() - .find_map(|(i, key)| if key.as_str() == name { Some(i) } else { None }) + .find_map(|(i, key)| if *key == name { Some(i) } else { None }) } /// Push an imported [modules][Module] onto the stack. #[inline(always)] @@ -996,7 +996,7 @@ impl Engine { }; // Check if the variable is `this` - if name.as_str() == KEYWORD_THIS { + if *name == KEYWORD_THIS { return if let Some(val) = this_ptr { Ok(((*val).into(), *pos)) } else { diff --git a/src/engine_api.rs b/src/engine_api.rs index 0a86a6b9..c13bbb0d 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -13,8 +13,8 @@ use crate::stdlib::{ vec::Vec, }; use crate::{ - scope::Scope, Dynamic, Engine, EvalAltResult, FnAccess, FnNamespace, Module, NativeCallContext, - ParseError, Position, RhaiResult, Shared, StaticVec, AST, + scope::Scope, Dynamic, Engine, EvalAltResult, FnAccess, FnNamespace, ImmutableString, Module, + NativeCallContext, ParseError, Position, RhaiResult, Shared, StaticVec, AST, }; #[cfg(not(feature = "no_index"))] @@ -52,7 +52,11 @@ impl Engine { /// # } /// ``` #[inline] - pub fn register_fn(&mut self, name: &str, func: F) -> &mut Self + pub fn register_fn( + &mut self, + name: impl AsRef + Into, + func: F, + ) -> &mut Self where F: RegisterNativeFunction, { @@ -102,7 +106,11 @@ impl Engine { /// .expect_err("expecting division by zero error!"); /// ``` #[inline] - pub fn register_result_fn(&mut self, name: &str, func: F) -> &mut Self + pub fn register_result_fn( + &mut self, + name: impl AsRef + Into, + func: F, + ) -> &mut Self where F: RegisterNativeFunction>>, { @@ -144,7 +152,7 @@ impl Engine { #[inline(always)] pub fn register_raw_fn( &mut self, - name: &str, + name: impl AsRef + Into, arg_types: &[TypeId], func: impl Fn(NativeCallContext, &mut FnCallArgs) -> Result> + SendSync @@ -878,25 +886,29 @@ impl Engine { /// # } /// ``` #[cfg(not(feature = "no_module"))] - pub fn register_static_module(&mut self, name: &str, module: Shared) -> &mut Self { + pub fn register_static_module( + &mut self, + name: impl AsRef + Into, + module: Shared, + ) -> &mut Self { fn register_static_module_raw( root: &mut crate::stdlib::collections::BTreeMap>, - name: &str, + name: impl AsRef + Into, module: Shared, ) { let separator = crate::token::Token::DoubleColon.syntax(); - if !name.contains(separator.as_ref()) { + if !name.as_ref().contains(separator.as_ref()) { if !module.is_indexed() { // Index the module (making a clone copy if necessary) if it is not indexed let mut module = crate::fn_native::shared_take_or_clone(module); module.build_index(); - root.insert(name.trim().into(), module.into()); + root.insert(name.into(), module.into()); } else { - root.insert(name.trim().into(), module); + root.insert(name.into(), module); } } else { - let mut iter = name.splitn(2, separator.as_ref()); + let mut iter = name.as_ref().splitn(2, separator.as_ref()); let sub_module = iter.next().unwrap().trim(); let remainder = iter.next().unwrap().trim(); @@ -915,7 +927,7 @@ impl Engine { } } - register_static_module_raw(&mut self.global_sub_modules, name.as_ref(), module); + register_static_module_raw(&mut self.global_sub_modules, name, module); self } @@ -927,7 +939,11 @@ impl Engine { #[cfg(not(feature = "no_module"))] #[inline(always)] #[deprecated = "use `register_static_module` instead"] - pub fn register_module(&mut self, name: &str, module: impl Into>) -> &mut Self { + pub fn register_module( + &mut self, + name: impl AsRef + Into, + module: impl Into>, + ) -> &mut Self { self.register_static_module(name, module.into()) } /// Compile a string into an [`AST`], which can be used later for evaluation. @@ -1013,7 +1029,6 @@ impl Engine { fn_native::shared_take_or_clone, module::resolvers::StaticModuleResolver, stdlib::collections::BTreeSet, - ImmutableString, }; fn collect_imports( @@ -1271,9 +1286,14 @@ impl Engine { /// # } /// ``` #[cfg(not(feature = "no_object"))] - pub fn parse_json(&self, json: &str, has_null: bool) -> Result> { + pub fn parse_json( + &self, + json: impl AsRef, + has_null: bool, + ) -> Result> { use crate::token::Token; + let json = json.as_ref(); let mut scope = Default::default(); // Trims the JSON string and add a '#' in front @@ -1765,7 +1785,7 @@ impl Engine { &self, scope: &mut Scope, ast: &AST, - name: &str, + name: impl AsRef, args: impl crate::fn_args::FuncArgs, ) -> Result> { let mut arg_values: crate::StaticVec<_> = Default::default(); @@ -1844,7 +1864,7 @@ impl Engine { scope: &mut Scope, ast: &AST, eval_ast: bool, - name: &str, + name: impl AsRef, mut this_ptr: Option<&mut Dynamic>, mut arg_values: impl AsMut<[Dynamic]>, ) -> RhaiResult { @@ -1867,7 +1887,7 @@ impl Engine { scope: &mut Scope, ast: &AST, eval_ast: bool, - name: &str, + name: impl AsRef, this_ptr: &mut Option<&mut Dynamic>, args: &mut FnCallArgs, ) -> RhaiResult { @@ -1881,12 +1901,14 @@ impl Engine { let fn_def = ast .lib() - .get_script_fn(name, args.len()) - .ok_or_else(|| EvalAltResult::ErrorFunctionNotFound(name.into(), Position::NONE))?; + .get_script_fn(name.as_ref(), args.len()) + .ok_or_else(|| { + EvalAltResult::ErrorFunctionNotFound(name.as_ref().into(), Position::NONE) + })?; // Check for data race. #[cfg(not(feature = "no_closure"))] - crate::fn_call::ensure_no_data_race(name, args, false)?; + crate::fn_call::ensure_no_data_race(name.as_ref(), args, false)?; self.call_script_fn( scope, diff --git a/src/engine_settings.rs b/src/engine_settings.rs index 1bf32f1a..b666714e 100644 --- a/src/engine_settings.rs +++ b/src/engine_settings.rs @@ -236,7 +236,7 @@ impl Engine { /// # } /// ``` #[inline(always)] - pub fn disable_symbol(&mut self, symbol: &str) -> &mut Self { + pub fn disable_symbol(&mut self, symbol: impl Into) -> &mut Self { self.disabled_symbols.insert(symbol.into()); self } @@ -270,7 +270,7 @@ impl Engine { /// ``` pub fn register_custom_operator( &mut self, - keyword: &str, + keyword: impl AsRef + Into, precedence: u8, ) -> Result<&mut Self, String> { let precedence = Precedence::new(precedence); @@ -279,25 +279,25 @@ impl Engine { return Err("precedence cannot be zero".into()); } - match Token::lookup_from_syntax(keyword) { + match Token::lookup_from_syntax(keyword.as_ref()) { // Standard identifiers, reserved keywords and custom keywords are OK None | Some(Token::Reserved(_)) | Some(Token::Custom(_)) => (), // Active standard keywords cannot be made custom // Disabled keywords are OK Some(token) if token.is_keyword() => { if !self.disabled_symbols.contains(token.syntax().as_ref()) { - return Err(format!("'{}' is a reserved keyword", keyword).into()); + return Err(format!("'{}' is a reserved keyword", keyword.as_ref()).into()); } } // Active standard symbols cannot be made custom Some(token) if token.is_symbol() => { if !self.disabled_symbols.contains(token.syntax().as_ref()) { - return Err(format!("'{}' is a reserved operator", keyword).into()); + return Err(format!("'{}' is a reserved operator", keyword.as_ref()).into()); } } // Active standard symbols cannot be made custom Some(token) if !self.disabled_symbols.contains(token.syntax().as_ref()) => { - return Err(format!("'{}' is a reserved symbol", keyword).into()) + return Err(format!("'{}' is a reserved symbol", keyword.as_ref()).into()) } // Disabled symbols are OK Some(_) => (), diff --git a/src/fn_call.rs b/src/fn_call.rs index f55562ee..18cc509c 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -475,9 +475,9 @@ impl Engine { fn_def .lib .as_ref() - .and_then(|m| m.id()) - .unwrap_or_else(|| state.source.as_ref().map_or_else(|| "", |s| s.as_str())) - .to_string(), + .and_then(|m| m.id().map(|id| id.to_string())) + .or_else(|| state.source.as_ref().map(|s| s.to_string())) + .unwrap_or_default(), err, pos, ) @@ -651,14 +651,14 @@ impl Engine { crate::engine::KEYWORD_IS_DEF_FN if args.len() == 2 && args[0].is::() && args[1].is::() => { - let fn_name = args[0].read_lock::().unwrap(); + let fn_name = &*args[0].read_lock::().unwrap(); let num_params = args[1].as_int().unwrap(); return Ok(( if num_params < 0 { Dynamic::FALSE } else { - let hash_script = calc_fn_hash(empty(), &fn_name, num_params as usize); + let hash_script = calc_fn_hash(empty(), fn_name, num_params as usize); self.has_script_fn(Some(mods), state, lib, hash_script) .into() }, @@ -737,7 +737,7 @@ impl Engine { if !func.externals.is_empty() { captured .into_iter() - .filter(|(name, _, _)| func.externals.iter().any(|ex| ex == name)) + .filter(|(name, _, _)| func.externals.contains(name.as_ref())) .for_each(|(name, value, _)| { // Consume the scope values. scope.push_dynamic(name, value); @@ -1219,8 +1219,8 @@ impl Engine { state .source .as_ref() - .map_or_else(|| "", |s| s.as_str()) - .to_string(), + .map(|s| s.to_string()) + .unwrap_or_default(), err, pos, )) diff --git a/src/fn_native.rs b/src/fn_native.rs index 76943e02..e893550b 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -184,10 +184,12 @@ impl<'a> NativeCallContext<'a> { #[inline(always)] pub fn call_fn_dynamic_raw( &self, - fn_name: &str, + fn_name: impl AsRef, is_method: bool, args: &mut [&mut Dynamic], ) -> RhaiResult { + let fn_name = fn_name.as_ref(); + let hash = if is_method { FnCallHash::from_script_and_native( calc_fn_hash(empty(), fn_name, args.len() - 1), diff --git a/src/module/mod.rs b/src/module/mod.rs index abe0e51d..770a9fe7 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -7,7 +7,7 @@ use crate::fn_register::RegisterNativeFunction; use crate::stdlib::{ any::TypeId, boxed::Box, - collections::{BTreeMap, BTreeSet}, + collections::BTreeMap, fmt, format, iter::empty, num::NonZeroUsize, @@ -16,6 +16,7 @@ use crate::stdlib::{ vec::Vec, }; use crate::token::Token; +use crate::utils::StringInterner; use crate::{ calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, EvalAltResult, ImmutableString, NativeCallContext, Position, Shared, StaticVec, @@ -114,7 +115,7 @@ impl FuncInfo { #[inline(always)] fn calc_native_fn_hash<'a>( modules: impl Iterator, - fn_name: &str, + fn_name: impl AsRef, params: &[TypeId], ) -> u64 { let hash_script = calc_fn_hash(modules, fn_name, params.len()); @@ -148,7 +149,7 @@ pub struct Module { /// Does the [`Module`] contain indexed functions that have been exposed to the global namespace? contains_indexed_global_functions: bool, /// Interned strings - interned_strings: BTreeSet, + interned_strings: StringInterner, } impl Default for Module { @@ -361,15 +362,6 @@ impl Module { self.indexed } - /// Return an interned string. - fn get_interned_string(&mut self, s: &str) -> ImmutableString { - self.interned_strings.get(s).cloned().unwrap_or_else(|| { - let s: ImmutableString = s.into(); - self.interned_strings.insert(s.clone()); - s - }) - } - /// Generate signatures for all the non-private functions in the [`Module`]. #[inline(always)] pub fn gen_fn_signatures(&self) -> impl Iterator + '_ { @@ -625,7 +617,7 @@ impl Module { pub fn update_fn_metadata(&mut self, hash_fn: u64, arg_names: &[&str]) -> &mut Self { let param_names = arg_names .iter() - .map(|&name| self.get_interned_string(name)) + .map(|&name| self.interned_strings.get(name)) .collect(); if let Some(f) = self.functions.get_mut(&hash_fn) { @@ -657,7 +649,7 @@ impl Module { #[inline] pub fn set_fn( &mut self, - name: &str, + name: impl AsRef + Into, namespace: FnNamespace, access: FnAccess, arg_names: Option<&[&str]>, @@ -690,11 +682,11 @@ impl Module { let hash_fn = calc_native_fn_hash(empty(), &name, ¶m_types); - let name = self.get_interned_string(name); + let name = self.interned_strings.get(name); let param_names = arg_names .iter() .flat_map(|p| p.iter()) - .map(|&name| self.get_interned_string(name)) + .map(|&arg| self.interned_strings.get(arg)) .collect(); self.functions.insert( @@ -783,16 +775,21 @@ impl Module { /// assert!(module.contains_fn(hash)); /// ``` #[inline(always)] - pub fn set_raw_fn( + pub fn set_raw_fn( &mut self, - name: &str, + name: N, namespace: FnNamespace, access: FnAccess, arg_types: &[TypeId], - func: impl Fn(NativeCallContext, &mut FnCallArgs) -> Result> + func: F, + ) -> u64 + where + N: AsRef + Into, + T: Variant + Clone, + F: Fn(NativeCallContext, &mut FnCallArgs) -> Result> + SendSync + 'static, - ) -> u64 { + { let f = move |ctx: NativeCallContext, args: &mut FnCallArgs| func(ctx, args).map(Dynamic::from); @@ -830,8 +827,9 @@ impl Module { /// assert!(module.contains_fn(hash)); /// ``` #[inline(always)] - pub fn set_native_fn(&mut self, name: &str, func: F) -> u64 + pub fn set_native_fn(&mut self, name: N, func: F) -> u64 where + N: AsRef + Into, T: Variant + Clone, F: RegisterNativeFunction>>, { @@ -1079,11 +1077,16 @@ impl Module { /// ``` #[cfg(not(feature = "no_index"))] #[inline(always)] - pub fn set_indexer_get_set_fn( + pub fn set_indexer_get_set_fn( &mut self, get_fn: impl Fn(&mut A, B) -> Result> + SendSync + 'static, set_fn: impl Fn(&mut A, B, T) -> Result<(), Box> + SendSync + 'static, - ) -> (u64, u64) { + ) -> (u64, u64) + where + A: Variant + Clone, + B: Variant + Clone, + T: Variant + Clone, + { ( self.set_indexer_get_fn(get_fn), self.set_indexer_set_fn(set_fn), diff --git a/src/parser.rs b/src/parser.rs index 34f066b2..fa5aa424 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -22,7 +22,7 @@ use crate::stdlib::{ }; use crate::syntax::{CustomSyntax, MARKER_BLOCK, MARKER_EXPR, MARKER_IDENT}; use crate::token::{is_keyword_function, is_valid_identifier, Token, TokenStream}; -use crate::utils::get_hasher; +use crate::utils::{get_hasher, StringInterner}; use crate::{ calc_fn_hash, Dynamic, Engine, ImmutableString, LexError, ParseError, ParseErrorType, Position, Scope, Shared, StaticVec, AST, @@ -44,7 +44,7 @@ struct ParseState<'e> { /// Reference to the scripting [`Engine`]. engine: &'e Engine, /// Interned strings. - interned_strings: BTreeMap, + interned_strings: StringInterner, /// Encapsulates a local stack with variable names to simulate an actual runtime scope. stack: Vec<(ImmutableString, AccessMode)>, /// Size of the local variables stack upon entry of the current block scope. @@ -160,24 +160,17 @@ impl<'e> ParseState<'e> { .iter() .rev() .enumerate() - .find(|(_, n)| **n == name) + .find(|&(_, n)| *n == name) .and_then(|(i, _)| NonZeroUsize::new(i + 1)) } /// Get an interned string, creating one if it is not yet interned. + #[inline(always)] pub fn get_interned_string( &mut self, text: impl AsRef + Into, ) -> ImmutableString { - #[allow(clippy::map_entry)] - if !self.interned_strings.contains_key(text.as_ref()) { - let value = text.into(); - self.interned_strings - .insert(value.clone().into(), value.clone()); - value - } else { - self.interned_strings.get(text.as_ref()).unwrap().clone() - } + self.interned_strings.get(text) } } diff --git a/src/utils.rs b/src/utils.rs index 6be824bf..60fa7e0f 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -6,6 +6,7 @@ use crate::stdlib::{ borrow::Borrow, boxed::Box, cmp::Ordering, + collections::BTreeSet, fmt, fmt::{Debug, Display}, hash::{BuildHasher, Hash, Hasher}, @@ -70,7 +71,11 @@ pub fn get_hasher() -> ahash::AHasher { /// /// The first module name is skipped. Hashing starts from the _second_ module in the chain. #[inline(always)] -pub fn calc_fn_hash<'a>(modules: impl Iterator, fn_name: &str, num: usize) -> u64 { +pub fn calc_fn_hash<'a>( + modules: impl Iterator, + fn_name: impl AsRef, + num: usize, +) -> u64 { let s = &mut get_hasher(); // We always skip the first module @@ -80,7 +85,7 @@ pub fn calc_fn_hash<'a>(modules: impl Iterator, fn_name: &str, n .skip(1) .for_each(|m| m.hash(s)); len.hash(s); - fn_name.hash(s); + fn_name.as_ref().hash(s); num.hash(s); s.finish() } @@ -137,7 +142,7 @@ pub(crate) fn combine_hashes(a: u64, b: u64) -> u64 { /// assert_ne!(s2.as_str(), s.as_str()); /// assert_eq!(s, "hello, world!"); /// ``` -#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Default)] +#[derive(Clone, Eq, Ord, Hash, Default)] pub struct ImmutableString(Shared); impl Deref for ImmutableString { @@ -156,6 +161,13 @@ impl AsRef for ImmutableString { } } +impl AsRef for ImmutableString { + #[inline(always)] + fn as_ref(&self) -> &str { + &self.0 + } +} + impl Borrow for ImmutableString { #[inline(always)] fn borrow(&self) -> &String { @@ -559,7 +571,6 @@ impl PartialEq for String { } impl> PartialOrd for ImmutableString { - #[inline(always)] fn partial_cmp(&self, other: &S) -> Option { self.as_str().partial_cmp(other.as_ref()) } @@ -594,3 +605,19 @@ impl ImmutableString { shared_make_mut(&mut self.0) } } + +/// A collection of interned strings. +#[derive(Debug, Clone, Default, Hash)] +pub struct StringInterner(BTreeSet); + +impl StringInterner { + /// Get an interned string, creating one if it is not yet interned. + #[inline(always)] + pub fn get(&mut self, text: impl AsRef + Into) -> ImmutableString { + self.0.get(text.as_ref()).cloned().unwrap_or_else(|| { + let s = text.into(); + self.0.insert(s.clone()); + s + }) + } +}