From d56585c8771ceb3cd115268ba4e6cf021ce71188 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 27 Nov 2021 14:24:36 +0800 Subject: [PATCH] Remove no_smartstring feature. --- CHANGELOG.md | 5 +++ Cargo.toml | 3 -- src/ast.rs | 11 ++++--- src/engine.rs | 45 +++++++++++++------------ src/func/call.rs | 40 +++++++++++----------- src/lib.rs | 17 ---------- src/module/mod.rs | 62 +++++++++++++++++++++-------------- src/parser.rs | 33 +++++++------------ src/tests.rs | 9 +---- src/types/fn_ptr.rs | 1 - src/types/immutable_string.rs | 3 -- 11 files changed, 105 insertions(+), 124 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82227921..9842d2d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,11 @@ Enhancements * Added `into_array` and `into_typed_array` for `Dynamic`. +Deprecated API's +---------------- + +* The internal `no_smartstring` feature is removed since `SmartString` now supports `no-std`. + Version 1.2.1 ============= diff --git a/Cargo.toml b/Cargo.toml index 0055ed88..97461d7f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,9 +48,6 @@ no_std = ["no-std-compat", "num-traits/libm", "core-error", "libm", "ahash/compi wasm-bindgen = ["instant/wasm-bindgen"] stdweb = ["instant/stdweb"] -# internal feature flags - volatile -no_smartstring = [] # do not use SmartString - [profile.release] lto = "fat" codegen-units = 1 diff --git a/src/ast.rs b/src/ast.rs index bca05835..8f74ebcd 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -228,7 +228,7 @@ impl AST { pub fn empty() -> Self { Self { source: None, - body: StmtBlock::empty(Position::NONE), + body: StmtBlock::NONE, functions: Module::new().into(), #[cfg(not(feature = "no_module"))] resolver: None, @@ -401,7 +401,7 @@ impl AST { functions.merge_filtered(&self.functions, &filter); Self { source: self.source.clone(), - body: StmtBlock::empty(Position::NONE), + body: StmtBlock::NONE, functions: functions.into(), #[cfg(not(feature = "no_module"))] resolver: self.resolver.clone(), @@ -597,7 +597,7 @@ impl AST { } (false, true) => body.clone(), (true, false) => other.body.clone(), - (true, true) => StmtBlock::empty(Position::NONE), + (true, true) => StmtBlock::NONE, }; let source = other.source.clone().or_else(|| self.source.clone()); @@ -744,7 +744,7 @@ impl AST { /// Clear all statements in the [`AST`], leaving only function definitions. #[inline(always)] pub fn clear_statements(&mut self) -> &mut Self { - self.body = StmtBlock::empty(Position::NONE); + self.body = StmtBlock::NONE; self } /// Extract all top-level literal constant and/or variable definitions. @@ -987,6 +987,9 @@ impl ASTNode<'_> { pub struct StmtBlock(StaticVec, Position); impl StmtBlock { + /// A [`StmtBlock`] that does not exist. + pub const NONE: Self = Self::empty(Position::NONE); + /// Create a new [`StmtBlock`]. #[must_use] pub fn new(statements: impl IntoIterator, pos: Position) -> Self { diff --git a/src/engine.rs b/src/engine.rs index f35509e2..70253581 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -797,6 +797,12 @@ impl EvalState { pub const fn is_global(&self) -> bool { self.scope_level == 0 } + /// Get the number of function resolution cache(s) in the stack. + #[inline(always)] + #[must_use] + pub fn fn_resolution_caches_len(&self) -> usize { + self.fn_resolution_caches.len() + } /// Get a mutable reference to the current function resolution cache. #[inline] #[must_use] @@ -813,14 +819,10 @@ impl EvalState { pub fn push_fn_resolution_cache(&mut self) { self.fn_resolution_caches.push(BTreeMap::new()); } - /// Remove the current function resolution cache from the stack and make the last one current. - /// - /// # Panics - /// - /// Panics if there is no more function resolution cache in the stack. + /// Rewind the function resolution caches stack to a particular size. #[inline(always)] - pub fn pop_fn_resolution_cache(&mut self) { - self.fn_resolution_caches.pop().expect("not empty"); + pub fn rewind_fn_resolution_caches(&mut self, len: usize) { + self.fn_resolution_caches.truncate(len); } } @@ -2444,7 +2446,7 @@ impl Engine { lib: &[&Module], this_ptr: &mut Option<&mut Dynamic>, statements: &[Stmt], - restore_prev_state: bool, + restore_orig_state: bool, rewind_scope: bool, level: usize, ) -> RhaiResult { @@ -2452,10 +2454,10 @@ impl Engine { return Ok(Dynamic::UNIT); } - let mut _extra_fn_resolution_cache = false; - let prev_always_search_scope = state.always_search_scope; - let prev_scope_len = scope.len(); - let prev_mods_len = mods.len(); + let orig_always_search_scope = state.always_search_scope; + let orig_scope_len = scope.len(); + let orig_mods_len = mods.len(); + let orig_fn_resolution_caches_len = state.fn_resolution_caches_len(); if rewind_scope { state.scope_level += 1; @@ -2475,15 +2477,14 @@ impl Engine { .skip(_mods_len) .any(|(_, m)| m.contains_indexed_global_functions()) { - if _extra_fn_resolution_cache { + if state.fn_resolution_caches_len() > orig_fn_resolution_caches_len { // When new module is imported with global functions and there is already // a new cache, clear it - notice that this is expensive as all function // resolutions must start again state.fn_resolution_cache_mut().clear(); - } else if restore_prev_state { + } else if restore_orig_state { // When new module is imported with global functions, push a new cache state.push_fn_resolution_cache(); - _extra_fn_resolution_cache = true; } else { // When the block is to be evaluated in-place, just clear the current cache state.fn_resolution_cache_mut().clear(); @@ -2494,21 +2495,19 @@ impl Engine { Ok(r) }); - if _extra_fn_resolution_cache { - // If imports list is modified, pop the functions lookup cache - state.pop_fn_resolution_cache(); - } + // If imports list is modified, pop the functions lookup cache + state.rewind_fn_resolution_caches(orig_fn_resolution_caches_len); if rewind_scope { - scope.rewind(prev_scope_len); + scope.rewind(orig_scope_len); state.scope_level -= 1; } - if restore_prev_state { - mods.truncate(prev_mods_len); + if restore_orig_state { + mods.truncate(orig_mods_len); // The impact of new local variables goes away at the end of a block // because any new variables introduced will go out of scope - state.always_search_scope = prev_always_search_scope; + state.always_search_scope = orig_always_search_scope; } result diff --git a/src/func/call.rs b/src/func/call.rs index f8eacb96..8a54a187 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -530,8 +530,8 @@ impl Engine { return Err(EvalAltResult::ErrorStackOverflow(pos).into()); } - let prev_scope_len = scope.len(); - let prev_mods_len = mods.len(); + let orig_scope_len = scope.len(); + let orig_mods_len = mods.len(); // Put arguments into scope as variables // Actually consume the arguments instead of cloning them @@ -549,14 +549,19 @@ impl Engine { // Merge in encapsulated environment, if any let mut lib_merged = StaticVec::with_capacity(lib.len() + 1); + let orig_fn_resolution_caches_len = state.fn_resolution_caches_len(); - let (unified, is_unified) = if let Some(ref env_lib) = fn_def.lib { - state.push_fn_resolution_cache(); - lib_merged.push(env_lib.as_ref()); - lib_merged.extend(lib.iter().cloned()); - (lib_merged.as_ref(), true) + let lib = if let Some(ref env_lib) = fn_def.lib { + if env_lib.is_empty() { + lib + } else { + state.push_fn_resolution_cache(); + lib_merged.push(env_lib.as_ref()); + lib_merged.extend(lib.iter().cloned()); + lib_merged.as_ref() + } } else { - (lib, false) + lib }; #[cfg(not(feature = "no_module"))] @@ -574,7 +579,7 @@ impl Engine { scope, mods, state, - unified, + lib, this_ptr, body, true, @@ -605,17 +610,14 @@ impl Engine { // Remove all local variables if rewind_scope { - scope.rewind(prev_scope_len); + scope.rewind(orig_scope_len); } else if !args.is_empty() { // Remove arguments only, leaving new variables in the scope - scope.remove_range(prev_scope_len, args.len()) + scope.remove_range(orig_scope_len, args.len()) } - mods.truncate(prev_mods_len); - - if is_unified { - state.pop_fn_resolution_cache(); - } + mods.truncate(orig_mods_len); + state.rewind_fn_resolution_caches(orig_fn_resolution_caches_len); result } @@ -879,7 +881,7 @@ impl Engine { )?; // If new functions are defined within the eval string, it is an error - if ast.lib().count().0 != 0 { + if !ast.lib().is_empty() { return Err(ParseErrorType::WrongFnDefinition.into()); } @@ -1225,7 +1227,7 @@ impl Engine { // Handle eval() KEYWORD_EVAL if total_args == 1 => { // eval - only in function call style - let prev_len = scope.len(); + let orig_scope_len = scope.len(); let (value, pos) = self.get_arg_value( scope, mods, state, lib, this_ptr, level, &a_expr[0], constants, )?; @@ -1237,7 +1239,7 @@ impl Engine { // IMPORTANT! If the eval defines new variables in the current scope, // all variable offsets from this point on will be mis-aligned. - if scope.len() != prev_len { + if scope.len() != orig_scope_len { state.always_search_scope = true; } diff --git a/src/lib.rs b/src/lib.rs index 3f12bc68..4aef3c70 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -129,27 +129,14 @@ pub use types::{ /// An identifier in Rhai. [`SmartString`](https://crates.io/crates/smartstring) is used because most /// identifiers are ASCII and short, fewer than 23 characters, so they can be stored inline. #[cfg(not(feature = "internals"))] -#[cfg(not(feature = "no_smartstring"))] pub(crate) type Identifier = SmartString; -/// An identifier in Rhai. -#[cfg(not(feature = "internals"))] -#[cfg(feature = "no_smartstring")] -pub(crate) type Identifier = ImmutableString; - /// An identifier in Rhai. [`SmartString`](https://crates.io/crates/smartstring) is used because most /// identifiers are ASCII and short, fewer than 23 characters, so they can be stored inline. #[cfg(feature = "internals")] -#[cfg(not(feature = "no_smartstring"))] #[deprecated = "this type is volatile and may change"] pub type Identifier = SmartString; -/// An identifier in Rhai. -#[cfg(feature = "internals")] -#[cfg(feature = "no_smartstring")] -#[deprecated = "this type is volatile and may change"] -pub type Identifier = ImmutableString; - /// Alias to [`Rc`][std::rc::Rc] or [`Arc`][std::sync::Arc] depending on the `sync` feature flag. pub use func::Shared; @@ -314,12 +301,8 @@ type StaticVec = smallvec::SmallVec<[T; 3]>; #[cfg(feature = "internals")] pub type StaticVec = smallvec::SmallVec<[T; 3]>; -#[cfg(not(feature = "no_smartstring"))] pub(crate) type SmartString = smartstring::SmartString; -#[cfg(feature = "no_smartstring")] -pub(crate) type SmartString = String; - // Compiler guards against mutually-exclusive feature flags #[cfg(feature = "no_float")] diff --git a/src/module/mod.rs b/src/module/mod.rs index d9da37a3..d5bd568f 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -324,7 +324,9 @@ impl Module { #[inline] #[must_use] pub fn is_empty(&self) -> bool { - self.functions.is_empty() + self.indexed + && !self.contains_indexed_global_functions + && self.functions.is_empty() && self.all_functions.is_empty() && self.variables.is_empty() && self.all_variables.is_empty() @@ -502,10 +504,14 @@ impl Module { name: &str, num_params: usize, ) -> Option<&Shared> { - self.functions - .values() - .find(|f| f.params == num_params && f.name == name) - .and_then(|f| f.func.get_script_fn_def()) + if self.functions.is_empty() { + None + } else { + self.functions + .values() + .find(|f| f.params == num_params && f.name == name) + .and_then(|f| f.func.get_script_fn_def()) + } } /// Get a mutable reference to the underlying [`BTreeMap`] of sub-modules. @@ -1150,6 +1156,7 @@ impl Module { self.all_type_iterators.clear(); self.indexed = false; self.contains_indexed_global_functions = false; + self.identifiers += other.identifiers; self } @@ -1169,6 +1176,7 @@ impl Module { self.all_type_iterators.clear(); self.indexed = false; self.contains_indexed_global_functions = false; + self.identifiers += other.identifiers; self } @@ -1197,6 +1205,7 @@ impl Module { self.all_type_iterators.clear(); self.indexed = false; self.contains_indexed_global_functions = false; + self.identifiers.merge(&other.identifiers); self } @@ -1246,6 +1255,7 @@ impl Module { self.all_type_iterators.clear(); self.indexed = false; self.contains_indexed_global_functions = false; + self.identifiers.merge(&other.identifiers); self } @@ -1454,26 +1464,28 @@ impl Module { // Non-private functions defined become module functions #[cfg(not(feature = "no_function"))] - ast.lib() - .functions - .values() - .filter(|f| match f.access { - FnAccess::Public => true, - FnAccess::Private => false, - }) - .filter(|f| f.func.is_script()) - .for_each(|f| { - // Encapsulate AST environment - let mut func = f - .func - .get_script_fn_def() - .expect("scripted function") - .as_ref() - .clone(); - func.lib = Some(ast.shared_lib()); - func.mods = func_mods.clone(); - module.set_script_fn(func); - }); + if !ast.lib().functions.is_empty() { + ast.lib() + .functions + .values() + .filter(|f| match f.access { + FnAccess::Public => true, + FnAccess::Private => false, + }) + .filter(|f| f.func.is_script()) + .for_each(|f| { + // Encapsulate AST environment + let mut func = f + .func + .get_script_fn_def() + .expect("scripted function") + .as_ref() + .clone(); + func.lib = Some(ast.shared_lib()); + func.mods = func_mods.clone(); + module.set_script_fn(func); + }); + } if let Some(s) = ast.source_raw() { module.set_id(s.clone()); diff --git a/src/parser.rs b/src/parser.rs index f6bd9259..2d5c6c19 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -23,6 +23,7 @@ use std::{ collections::BTreeMap, hash::{Hash, Hasher}, num::{NonZeroU8, NonZeroUsize}, + ops::AddAssign, }; #[cfg(not(feature = "no_float"))] @@ -51,39 +52,29 @@ const NEVER_ENDS: &str = "`TokenStream` never ends"; /// collection of strings and returns shared instances, only creating a new string when it is not /// yet interned. #[derive(Debug, Clone, Hash)] -pub struct IdentifierBuilder( - #[cfg(feature = "no_smartstring")] std::collections::BTreeSet, -); +pub struct IdentifierBuilder(); impl IdentifierBuilder { - /// Create a new IdentifierBuilder. - #[cfg(not(feature = "no_smartstring"))] + /// Create a new [`IdentifierBuilder`]. #[inline] #[must_use] pub const fn new() -> Self { Self() } - /// Create a new IdentifierBuilder. - #[cfg(feature = "no_smartstring")] - #[inline] - #[must_use] - pub fn new() -> Self { - Self(std::collections::BTreeSet::new()) - } /// Get an identifier from a text string. #[inline] #[must_use] pub fn get(&mut self, text: impl AsRef + Into) -> Identifier { - #[cfg(not(feature = "no_smartstring"))] - return text.into(); - - #[cfg(feature = "no_smartstring")] - return self.0.get(text.as_ref()).cloned().unwrap_or_else(|| { - let s: Identifier = text.into(); - self.0.insert(s.clone()); - s - }); + text.into() } + /// Merge another [`IdentifierBuilder`] into this. + #[inline(always)] + pub fn merge(&mut self, _other: &Self) {} +} + +impl AddAssign for IdentifierBuilder { + #[inline(always)] + fn add_assign(&mut self, _rhs: Self) {} } /// _(internals)_ A type that encapsulates the current state of the parser. diff --git a/src/tests.rs b/src/tests.rs index de960d48..50e82788 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -21,14 +21,7 @@ fn check_struct_sizes() { assert_eq!(size_of::>(), 16); assert_eq!(size_of::(), 32); assert_eq!(size_of::>(), 32); - assert_eq!( - size_of::(), - if cfg!(feature = "no_smartstring") { - 64 - } else { - 80 - } - ); + assert_eq!(size_of::(), 80); assert_eq!(size_of::(), 464); assert_eq!(size_of::(), 56); assert_eq!( diff --git a/src/types/fn_ptr.rs b/src/types/fn_ptr.rs index fd4e01fe..8710010c 100644 --- a/src/types/fn_ptr.rs +++ b/src/types/fn_ptr.rs @@ -153,7 +153,6 @@ impl TryFrom for FnPtr { } } -#[cfg(not(feature = "no_smartstring"))] impl TryFrom for FnPtr { type Error = Box; diff --git a/src/types/immutable_string.rs b/src/types/immutable_string.rs index be5d4c91..822223df 100644 --- a/src/types/immutable_string.rs +++ b/src/types/immutable_string.rs @@ -115,14 +115,12 @@ impl From for ImmutableString { Self(value.into()) } } -#[cfg(not(feature = "no_smartstring"))] impl From<&SmartString> for ImmutableString { #[inline(always)] fn from(value: &SmartString) -> Self { Self(value.clone().into()) } } -#[cfg(not(feature = "no_smartstring"))] impl From for ImmutableString { #[inline(always)] fn from(value: SmartString) -> Self { @@ -180,7 +178,6 @@ impl<'a> FromIterator for ImmutableString { } } -#[cfg(not(feature = "no_smartstring"))] impl<'a> FromIterator for ImmutableString { #[inline] fn from_iter>(iter: T) -> Self {