From 2c94f956e53e30f21603eecccbbb511fde3be8d0 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 22 Mar 2023 21:38:55 +0800 Subject: [PATCH 01/17] Fix error message. --- src/parser.rs | 56 +++++++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 207a53a6..4209b6b5 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3612,31 +3612,39 @@ impl Engine { // Parse type for `this` pointer #[cfg(not(feature = "no_object"))] - let ((token, pos), this_type) = match token { - Token::StringConstant(s) if input.peek().expect(NEVER_ENDS).0 == Token::Period => { - eat_token(input, Token::Period); - let s = match s.as_str() { - "int" => state.get_interned_string(std::any::type_name::()), - #[cfg(not(feature = "no_float"))] - "float" => state.get_interned_string(std::any::type_name::()), - _ => state.get_interned_string(*s), - }; - (input.next().expect(NEVER_ENDS), Some(s)) + let ((token, pos), this_type) = { + let (next_token, next_pos) = input.peek().expect(NEVER_ENDS); + + match token { + Token::StringConstant(s) if next_token == &Token::Period => { + eat_token(input, Token::Period); + let s = match s.as_str() { + "int" => state.get_interned_string(std::any::type_name::()), + #[cfg(not(feature = "no_float"))] + "float" => state.get_interned_string(std::any::type_name::()), + _ => state.get_interned_string(*s), + }; + (input.next().expect(NEVER_ENDS), Some(s)) + } + Token::StringConstant(..) => { + return Err(PERR::MissingToken( + Token::Period.into(), + "after the type name for 'this'".into(), + ) + .into_err(*next_pos)) + } + Token::Identifier(s) if next_token == &Token::Period => { + eat_token(input, Token::Period); + let s = match s.as_str() { + "int" => state.get_interned_string(std::any::type_name::()), + #[cfg(not(feature = "no_float"))] + "float" => state.get_interned_string(std::any::type_name::()), + _ => state.get_interned_string(*s), + }; + (input.next().expect(NEVER_ENDS), Some(s)) + } + _ => ((token, pos), None), } - Token::StringConstant(..) => { - return Err(PERR::MissingSymbol(".".to_string()).into_err(pos)) - } - Token::Identifier(s) if input.peek().expect(NEVER_ENDS).0 == Token::Period => { - eat_token(input, Token::Period); - let s = match s.as_str() { - "int" => state.get_interned_string(std::any::type_name::()), - #[cfg(not(feature = "no_float"))] - "float" => state.get_interned_string(std::any::type_name::()), - _ => state.get_interned_string(*s), - }; - (input.next().expect(NEVER_ENDS), Some(s)) - } - _ => ((token, pos), None), }; let name = match token.into_function_name_for_override() { From 3c7cd8e278b4ab3b5e63a139dd38601a9acbac9b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 23 Mar 2023 09:12:48 +0800 Subject: [PATCH 02/17] Use debug_assert. --- src/eval/chaining.rs | 3 ++- src/eval/expr.rs | 5 +---- src/eval/stmt.rs | 10 ++++------ src/func/call.rs | 13 ++++++------- src/func/script.rs | 2 +- src/parser.rs | 2 +- src/tokenizer.rs | 8 ++++---- 7 files changed, 19 insertions(+), 24 deletions(-) diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index a6184061..cda741a5 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -397,9 +397,10 @@ impl Engine { match lhs { // id.??? or id[???] Expr::Variable(.., var_pos) => { + self.track_operation(global, *var_pos)?; + #[cfg(feature = "debugging")] self.run_debugger(global, caches, scope, this_ptr.as_deref_mut(), lhs)?; - self.track_operation(global, *var_pos)?; let target = &mut self.search_namespace(global, caches, scope, this_ptr, lhs)?; diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 540eb7a7..e728249e 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -236,8 +236,7 @@ impl Engine { mut this_ptr: Option<&mut Dynamic>, expr: &Expr, ) -> RhaiResult { - // Coded this way for better branch prediction. - // Popular branches are lifted out of the `match` statement into their own branches. + self.track_operation(global, expr.position())?; #[cfg(feature = "debugging")] let reset = @@ -245,8 +244,6 @@ impl Engine { #[cfg(feature = "debugging")] auto_restore! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } - self.track_operation(global, expr.position())?; - match expr { // Constants Expr::IntegerConstant(x, ..) => Ok((*x).into()), diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 2410d036..2fdd5cec 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -267,14 +267,14 @@ impl Engine { stmt: &Stmt, rewind_scope: bool, ) -> RhaiResult { + self.track_operation(global, stmt.position())?; + #[cfg(feature = "debugging")] let reset = self.run_debugger_with_reset(global, caches, scope, this_ptr.as_deref_mut(), stmt)?; #[cfg(feature = "debugging")] auto_restore! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } - self.track_operation(global, stmt.position())?; - match stmt { // No-op Stmt::Noop(..) => Ok(Dynamic::UNIT), @@ -307,6 +307,8 @@ impl Engine { .eval_expr(global, caches, scope, this_ptr.as_deref_mut(), rhs)? .flatten(); + self.track_operation(global, lhs.position())?; + let mut target = self.search_namespace(global, caches, scope, this_ptr, lhs)?; let is_temp_result = !target.is_ref(); @@ -326,8 +328,6 @@ impl Engine { .into()); } - self.track_operation(global, lhs.position())?; - self.eval_op_assignment(global, caches, op_info, lhs, &mut target, rhs_val)?; } else { #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] @@ -715,8 +715,6 @@ impl Engine { *scope.get_mut_by_index(index).write_lock().unwrap() = value; // Run block - self.track_operation(global, body.position())?; - if body.is_empty() { continue; } diff --git a/src/func/call.rs b/src/func/call.rs index e5fa0507..e834b750 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -488,7 +488,7 @@ impl Engine { // index getter function not found? #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] crate::engine::FN_IDX_GET => { - assert!(args.len() == 2); + debug_assert_eq!(args.len(), 2); let t0 = self.map_type_name(args[0].type_name()); let t1 = self.map_type_name(args[1].type_name()); @@ -499,7 +499,7 @@ impl Engine { // index setter function not found? #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] crate::engine::FN_IDX_SET => { - assert!(args.len() == 3); + debug_assert_eq!(args.len(), 3); let t0 = self.map_type_name(args[0].type_name()); let t1 = self.map_type_name(args[1].type_name()); @@ -511,7 +511,7 @@ impl Engine { // Getter function not found? #[cfg(not(feature = "no_object"))] _ if name.starts_with(crate::engine::FN_GET) => { - assert!(args.len() == 1); + debug_assert_eq!(args.len(), 1); let prop = &name[crate::engine::FN_GET.len()..]; let t0 = self.map_type_name(args[0].type_name()); @@ -528,7 +528,7 @@ impl Engine { // Setter function not found? #[cfg(not(feature = "no_object"))] _ if name.starts_with(crate::engine::FN_SET) => { - assert!(args.len() == 2); + debug_assert_eq!(args.len(), 2); let prop = &name[crate::engine::FN_SET.len()..]; let t0 = self.map_type_name(args[0].type_name()); @@ -1365,10 +1365,11 @@ impl Engine { // Get target reference to first argument let first_arg = &args_expr[0]; - let target = self.search_scope_only(global, caches, scope, this_ptr, first_arg)?; self.track_operation(global, first_arg.position())?; + let target = self.search_scope_only(global, caches, scope, this_ptr, first_arg)?; + #[cfg(not(feature = "no_closure"))] let target_is_shared = target.is_shared(); #[cfg(feature = "no_closure")] @@ -1433,8 +1434,6 @@ impl Engine { }), ); - self.track_operation(global, pos)?; - if let Some(f) = module.get_qualified_fn(hash_qualified_fn) { func = Some(f); break; diff --git a/src/func/script.rs b/src/func/script.rs index 4d119cfc..d385f389 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -35,7 +35,7 @@ impl Engine { rewind_scope: bool, pos: Position, ) -> RhaiResult { - assert_eq!(fn_def.params.len(), args.len()); + debug_assert_eq!(fn_def.params.len(), args.len()); self.track_operation(global, pos)?; diff --git a/src/parser.rs b/src/parser.rs index 4209b6b5..64f5731e 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3929,7 +3929,7 @@ impl Engine { let expr = self.parse_expr(&mut input, state, &mut functions, settings)?; #[cfg(feature = "no_function")] - assert!(functions.is_empty()); + debug_assert!(functions.is_empty()); match input.peek().expect(NEVER_ENDS) { (Token::EOF, ..) => (), diff --git a/src/tokenizer.rs b/src/tokenizer.rs index 27d0cf6b..d82075fe 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -1219,12 +1219,12 @@ pub fn parse_string_literal( ch } None if verbatim => { - assert_eq!(escape, "", "verbatim strings should not have any escapes"); + debug_assert_eq!(escape, "", "verbatim strings should not have any escapes"); pos.advance(); break; } None if allow_line_continuation && !escape.is_empty() => { - assert_eq!(escape, "\\", "unexpected escape {escape} at end of line"); + debug_assert_eq!(escape, "\\", "unexpected escape {} at end of line", escape); pos.advance(); break; } @@ -1343,14 +1343,14 @@ pub fn parse_string_literal( // Verbatim '\n' if verbatim => { - assert_eq!(escape, "", "verbatim strings should not have any escapes"); + debug_assert_eq!(escape, "", "verbatim strings should not have any escapes"); pos.new_line(); result.push(next_char); } // Line continuation '\n' if allow_line_continuation && !escape.is_empty() => { - assert_eq!(escape, "\\", "unexpected escape {escape} at end of line"); + debug_assert_eq!(escape, "\\", "unexpected escape {} at end of line", escape); escape.clear(); pos.new_line(); From 3d06ddc6e27fb04eea620b941ebec8e691a00e0c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 23 Mar 2023 09:38:54 +0800 Subject: [PATCH 03/17] Fix for loop operations. --- src/eval/stmt.rs | 80 +++++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 2fdd5cec..18ffb037 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -686,51 +686,53 @@ impl Engine { let mut result = Dynamic::UNIT; - for (x, iter_value) in iter_func(iter_obj).enumerate() { - // Increment counter - if let Some(counter_index) = counter_index { - // As the variable increments from 0, this should always work - // since any overflow will first be caught below. - let index_value = x as INT; + if body.is_empty() { + for _ in iter_func(iter_obj) { + self.track_operation(global, body.position())?; + } + } else { + for (x, iter_value) in iter_func(iter_obj).enumerate() { + // Increment counter + if let Some(counter_index) = counter_index { + // As the variable increments from 0, this should always work + // since any overflow will first be caught below. + let index_value = x as INT; - #[cfg(not(feature = "unchecked"))] - #[allow(clippy::absurd_extreme_comparisons)] - if index_value > crate::MAX_USIZE_INT { - return Err(ERR::ErrorArithmetic( - format!("for-loop counter overflow: {x}"), - counter.pos, - ) - .into()); + #[cfg(not(feature = "unchecked"))] + #[allow(clippy::absurd_extreme_comparisons)] + if index_value > crate::MAX_USIZE_INT { + return Err(ERR::ErrorArithmetic( + format!("for-loop counter overflow: {x}"), + counter.pos, + ) + .into()); + } + + *scope.get_mut_by_index(counter_index).write_lock().unwrap() = + Dynamic::from_int(index_value); } - *scope.get_mut_by_index(counter_index).write_lock().unwrap() = - Dynamic::from_int(index_value); - } + // Set loop value + let value = iter_value + .map_err(|err| err.fill_position(expr.position()))? + .flatten(); - // Set loop value - let value = iter_value - .map_err(|err| err.fill_position(expr.position()))? - .flatten(); + *scope.get_mut_by_index(index).write_lock().unwrap() = value; - *scope.get_mut_by_index(index).write_lock().unwrap() = value; + // Run block + let this_ptr = this_ptr.as_deref_mut(); - // Run block - if body.is_empty() { - continue; - } - - let this_ptr = this_ptr.as_deref_mut(); - - match self.eval_stmt_block(global, caches, scope, this_ptr, body, true) { - Ok(_) => (), - Err(err) => match *err { - ERR::LoopBreak(false, ..) => (), - ERR::LoopBreak(true, value, ..) => { - result = value; - break; - } - _ => return Err(err), - }, + match self.eval_stmt_block(global, caches, scope, this_ptr, body, true) { + Ok(_) => (), + Err(err) => match *err { + ERR::LoopBreak(false, ..) => (), + ERR::LoopBreak(true, value, ..) => { + result = value; + break; + } + _ => return Err(err), + }, + } } } From 2e724b804e0c5fca08c25118d19ac98e97346df7 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 23 Mar 2023 13:37:10 +0800 Subject: [PATCH 04/17] Remove indirection. --- src/api/custom_syntax.rs | 10 +++++----- src/api/definitions/mod.rs | 4 ++-- src/api/formatting.rs | 4 ++-- src/api/mod.rs | 6 +++--- src/api/register.rs | 2 +- src/engine.rs | 10 ++++------ src/eval/stmt.rs | 2 +- src/func/call.rs | 4 ++-- src/func/script.rs | 2 +- src/module/mod.rs | 36 ++++++++++++++++++------------------ src/optimizer.rs | 2 +- src/packages/lang_core.rs | 2 +- src/parser.rs | 16 ++++++++-------- src/serde/metadata.rs | 2 +- src/tokenizer.rs | 12 ++++++------ 15 files changed, 56 insertions(+), 58 deletions(-) diff --git a/src/api/custom_syntax.rs b/src/api/custom_syntax.rs index acd01195..e5dfb56c 100644 --- a/src/api/custom_syntax.rs +++ b/src/api/custom_syntax.rs @@ -261,12 +261,12 @@ impl Engine { // Make it a custom keyword/symbol if it is disabled or reserved if (self .disabled_symbols - .as_deref() + .as_ref() .map_or(false, |m| m.contains(s)) || token.as_ref().map_or(false, Token::is_reserved)) && !self .custom_keywords - .as_deref() + .as_ref() .map_or(false, |m| m.contains_key(s)) { self.custom_keywords @@ -281,7 +281,7 @@ impl Engine { && token.as_ref().map_or(false, Token::is_standard_keyword) && !self .disabled_symbols - .as_deref() + .as_ref() .map_or(false, |m| m.contains(s)) => { return Err(LexError::ImproperSymbol( @@ -301,12 +301,12 @@ impl Engine { // Make it a custom keyword/symbol if it is disabled or reserved if self .disabled_symbols - .as_deref() + .as_ref() .map_or(false, |m| m.contains(s)) || (token.as_ref().map_or(false, Token::is_reserved) && !self .custom_keywords - .as_deref() + .as_ref() .map_or(false, |m| m.contains_key(s))) { self.custom_keywords diff --git a/src/api/definitions/mod.rs b/src/api/definitions/mod.rs index a34a5ac5..69224b27 100644 --- a/src/api/definitions/mod.rs +++ b/src/api/definitions/mod.rs @@ -372,7 +372,7 @@ impl Definitions<'_> { let mut m = self .engine .global_sub_modules - .as_deref() + .as_ref() .into_iter() .flatten() .map(move |(name, module)| { @@ -461,7 +461,7 @@ impl Module { || def .engine .custom_keywords - .as_deref() + .as_ref() .map_or(false, |m| m.contains_key(f.metadata.name.as_str())); f.write_definition(writer, def, operator)?; diff --git a/src/api/formatting.rs b/src/api/formatting.rs index 07441f49..7fb7a376 100644 --- a/src/api/formatting.rs +++ b/src/api/formatting.rs @@ -208,7 +208,7 @@ impl Engine { #[cfg(not(feature = "no_module"))] return self .global_sub_modules - .as_deref() + .as_ref() .into_iter() .flatten() .find_map(|(_, m)| m.get_custom_type(name)); @@ -243,7 +243,7 @@ impl Engine { #[cfg(not(feature = "no_module"))] return self .global_sub_modules - .as_deref() + .as_ref() .into_iter() .flatten() .find_map(|(_, m)| m.get_custom_type(name)); diff --git a/src/api/mod.rs b/src/api/mod.rs index a2cf21ce..21238f5a 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -171,7 +171,7 @@ impl Engine { Some(token) if token.is_standard_keyword() => { if !self .disabled_symbols - .as_deref() + .as_ref() .map_or(false, |m| m.contains(token.literal_syntax())) { return Err(format!("'{keyword}' is a reserved keyword")); @@ -181,7 +181,7 @@ impl Engine { Some(token) if token.is_standard_symbol() => { if !self .disabled_symbols - .as_deref() + .as_ref() .map_or(false, |m| m.contains(token.literal_syntax())) { return Err(format!("'{keyword}' is a reserved operator")); @@ -191,7 +191,7 @@ impl Engine { Some(token) if !self .disabled_symbols - .as_deref() + .as_ref() .map_or(false, |m| m.contains(token.literal_syntax())) => { return Err(format!("'{keyword}' is a reserved symbol")) diff --git a/src/api/register.rs b/src/api/register.rs index 43c43a72..8f4d4833 100644 --- a/src/api/register.rs +++ b/src/api/register.rs @@ -786,7 +786,7 @@ impl Engine { } #[cfg(not(feature = "no_module"))] - for (name, m) in self.global_sub_modules.as_deref().into_iter().flatten() { + for (name, m) in self.global_sub_modules.as_ref().into_iter().flatten() { signatures.extend(m.gen_fn_signatures().map(|f| format!("{name}::{f}"))); } diff --git a/src/engine.rs b/src/engine.rs index 1e35d21f..59516a82 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -96,8 +96,7 @@ pub struct Engine { pub(crate) global_modules: StaticVec, /// A collection of all sub-modules directly loaded into the Engine. #[cfg(not(feature = "no_module"))] - pub(crate) global_sub_modules: - Option>>, + pub(crate) global_sub_modules: Option>, /// A module resolution service. #[cfg(not(feature = "no_module"))] @@ -107,15 +106,14 @@ pub struct Engine { pub(crate) interned_strings: Option>>, /// A set of symbols to disable. - pub(crate) disabled_symbols: Option>>, + pub(crate) disabled_symbols: Option>, /// A map containing custom keywords and precedence to recognize. #[cfg(not(feature = "no_custom_syntax"))] - pub(crate) custom_keywords: - Option>>>, + pub(crate) custom_keywords: Option>>, /// Custom syntax. #[cfg(not(feature = "no_custom_syntax"))] pub(crate) custom_syntax: Option< - Box>>, + std::collections::BTreeMap>, >, /// Callback closure for filtering variable definition. pub(crate) def_var_filter: Option>, diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 18ffb037..021417d7 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -664,7 +664,7 @@ impl Engine { .or_else(|| global.get_iter(iter_type)) .or_else(|| { self.global_sub_modules - .as_deref() + .as_ref() .into_iter() .flatten() .find_map(|(_, m)| m.get_qualified_iter(iter_type)) diff --git a/src/func/call.rs b/src/func/call.rs index e834b750..61d54818 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -206,7 +206,7 @@ impl Engine { .or_else(|| _global.get_qualified_fn(hash, true)) .or_else(|| { self.global_sub_modules - .as_deref() + .as_ref() .into_iter() .flatten() .filter(|(_, m)| m.contains_indexed_global_functions()) @@ -248,7 +248,7 @@ impl Engine { #[cfg(not(feature = "no_module"))] let is_dynamic = is_dynamic || _global.may_contain_dynamic_fn(hash_base) - || self.global_sub_modules.as_deref().map_or(false, |m| { + || self.global_sub_modules.as_ref().map_or(false, |m| { m.values().any(|m| m.may_contain_dynamic_fn(hash_base)) }); diff --git a/src/func/script.rs b/src/func/script.rs index d385f389..98551fe5 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -219,7 +219,7 @@ impl Engine { // Then check imported modules global.contains_qualified_fn(hash_script) // Then check sub-modules - || self.global_sub_modules.as_deref().map_or(false, |m| { + || self.global_sub_modules.as_ref().map_or(false, |m| { m.values().any(|m| m.contains_qualified_fn(hash_script)) }); diff --git a/src/module/mod.rs b/src/module/mod.rs index 84b27c4b..26dbca16 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -195,22 +195,22 @@ pub struct Module { /// Custom types. custom_types: Option>, /// Sub-modules. - modules: Option>>, + modules: Option>, /// [`Module`] variables. - variables: Option>>, + variables: Option>, /// Flattened collection of all [`Module`] variables, including those in sub-modules. - all_variables: Option>>, + all_variables: Option>, /// Functions (both native Rust and scripted). functions: Option>, /// Flattened collection of all functions, native Rust and scripted. /// including those in sub-modules. - all_functions: Option>>, + all_functions: Option>, /// Bloom filter on native Rust functions (in scripted hash format) that contain [`Dynamic`] parameters. dynamic_functions_filter: Option>, /// Iterator functions, keyed by the type producing the iterator. - type_iterators: Option>>>, + type_iterators: Option>>, /// Flattened collection of iterator functions, including those in sub-modules. - all_type_iterators: Option>>>, + all_type_iterators: Option>>, /// Flags. pub(crate) flags: ModuleFlags, } @@ -234,7 +234,7 @@ impl fmt::Debug for Module { "modules", &self .modules - .as_deref() + .as_ref() .into_iter() .flat_map(BTreeMap::keys) .map(SmartString::as_str) @@ -561,23 +561,23 @@ impl Module { .functions .as_ref() .map_or(true, StraightHashMap::is_empty) - && self.variables.as_deref().map_or(true, BTreeMap::is_empty) - && self.modules.as_deref().map_or(true, BTreeMap::is_empty) + && self.variables.as_ref().map_or(true, BTreeMap::is_empty) + && self.modules.as_ref().map_or(true, BTreeMap::is_empty) && self .type_iterators - .as_deref() + .as_ref() .map_or(true, BTreeMap::is_empty) && self .all_functions - .as_deref() + .as_ref() .map_or(true, StraightHashMap::is_empty) && self .all_variables - .as_deref() + .as_ref() .map_or(true, StraightHashMap::is_empty) && self .all_type_iterators - .as_deref() + .as_ref() .map_or(true, BTreeMap::is_empty) } @@ -1979,9 +1979,9 @@ impl Module { #[must_use] pub fn count(&self) -> (usize, usize, usize) { ( - self.variables.as_deref().map_or(0, BTreeMap::len), + self.variables.as_ref().map_or(0, BTreeMap::len), self.functions.as_ref().map_or(0, StraightHashMap::len), - self.type_iterators.as_deref().map_or(0, BTreeMap::len), + self.type_iterators.as_ref().map_or(0, BTreeMap::len), ) } @@ -1989,7 +1989,7 @@ impl Module { #[inline] pub fn iter_sub_modules(&self) -> impl Iterator { self.modules - .as_deref() + .as_ref() .into_iter() .flatten() .map(|(k, m)| (k.as_str(), m)) @@ -1999,7 +1999,7 @@ impl Module { #[inline] pub fn iter_var(&self) -> impl Iterator { self.variables - .as_deref() + .as_ref() .into_iter() .flatten() .map(|(k, v)| (k.as_str(), v)) @@ -2392,7 +2392,7 @@ impl Module { if !self.is_indexed() { let mut path = Vec::with_capacity(4); - let mut variables = new_hash_map(self.variables.as_deref().map_or(0, BTreeMap::len)); + let mut variables = new_hash_map(self.variables.as_ref().map_or(0, BTreeMap::len)); let mut functions = new_hash_map(self.functions.as_ref().map_or(0, StraightHashMap::len)); let mut type_iterators = BTreeMap::new(); diff --git a/src/optimizer.rs b/src/optimizer.rs index dd8166c0..ea8f00d2 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -1262,7 +1262,7 @@ impl Engine { #[cfg(not(feature = "no_module"))] if self .global_sub_modules - .as_deref() + .as_ref() .into_iter() .flatten() .any(|(_, m)| m.contains_qualified_fn(hash)) diff --git a/src/packages/lang_core.rs b/src/packages/lang_core.rs index c7041387..26815f43 100644 --- a/src/packages/lang_core.rs +++ b/src/packages/lang_core.rs @@ -273,7 +273,7 @@ fn collect_fn_metadata( #[cfg(not(feature = "no_module"))] ctx.engine() .global_sub_modules - .as_deref() + .as_ref() .into_iter() .flatten() .flat_map(|(_, m)| m.iter_script_fn()) diff --git a/src/parser.rs b/src/parser.rs index 64f5731e..37c13265 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -601,7 +601,7 @@ impl Engine { .any(|m| m.as_str() == root) && !self .global_sub_modules - .as_deref() + .as_ref() .map_or(false, |m| m.contains_key(root)) { return Err( @@ -676,7 +676,7 @@ impl Engine { .any(|m| m.as_str() == root) && !self .global_sub_modules - .as_deref() + .as_ref() .map_or(false, |m| m.contains_key(root)) { return Err( @@ -1577,12 +1577,12 @@ impl Engine { Token::Custom(key) | Token::Reserved(key) | Token::Identifier(key) if self .custom_syntax - .as_deref() + .as_ref() .map_or(false, |m| m.contains_key(&**key)) => { let (key, syntax) = self .custom_syntax - .as_deref() + .as_ref() .and_then(|m| m.get_key_value(&**key)) .unwrap(); let (.., pos) = input.next().expect(NEVER_ENDS); @@ -1888,7 +1888,7 @@ impl Engine { .any(|m| m.as_str() == root) && !self .global_sub_modules - .as_deref() + .as_ref() .map_or(false, |m| m.contains_key(root)) { return Err( @@ -2303,7 +2303,7 @@ impl Engine { #[cfg(not(feature = "no_custom_syntax"))] Token::Custom(c) => self .custom_keywords - .as_deref() + .as_ref() .and_then(|m| m.get(&**c)) .copied() .ok_or_else(|| PERR::Reserved(c.to_string()).into_err(*current_pos))?, @@ -2329,7 +2329,7 @@ impl Engine { #[cfg(not(feature = "no_custom_syntax"))] Token::Custom(c) => self .custom_keywords - .as_deref() + .as_ref() .and_then(|m| m.get(&**c)) .copied() .ok_or_else(|| PERR::Reserved(c.to_string()).into_err(*next_pos))?, @@ -2434,7 +2434,7 @@ impl Engine { Token::Custom(s) if self .custom_keywords - .as_deref() + .as_ref() .and_then(|m| m.get(s.as_str())) .map_or(false, Option::is_some) => { diff --git a/src/serde/metadata.rs b/src/serde/metadata.rs index 719298eb..4eec9053 100644 --- a/src/serde/metadata.rs +++ b/src/serde/metadata.rs @@ -182,7 +182,7 @@ pub fn gen_metadata_to_json( let mut global = ModuleMetadata::new(); #[cfg(not(feature = "no_module"))] - for (name, m) in engine.global_sub_modules.as_deref().into_iter().flatten() { + for (name, m) in engine.global_sub_modules.as_ref().into_iter().flatten() { global.modules.insert(name, m.as_ref().into()); } diff --git a/src/tokenizer.rs b/src/tokenizer.rs index d82075fe..60d13808 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -2450,7 +2450,7 @@ impl<'a> Iterator for TokenIterator<'a> { Some((Token::Reserved(s), pos)) => (match (s.as_str(), #[cfg(not(feature = "no_custom_syntax"))] - self.engine.custom_keywords.as_deref().map_or(false, |m| m.contains_key(&*s)), + self.engine.custom_keywords.as_ref().map_or(false, |m| m.contains_key(&*s)), #[cfg(feature = "no_custom_syntax")] false ) @@ -2487,7 +2487,7 @@ impl<'a> Iterator for TokenIterator<'a> { #[cfg(feature = "no_custom_syntax")] (.., true) => unreachable!("no custom operators"), // Reserved keyword that is not custom and disabled. - (token, false) if self.engine.disabled_symbols.as_deref().map_or(false,|m| m.contains(token)) => { + (token, false) if self.engine.disabled_symbols.as_ref().map_or(false,|m| m.contains(token)) => { let msg = format!("reserved {} '{token}' is disabled", if is_valid_identifier(token) { "keyword"} else {"symbol"}); Token::LexError(LERR::ImproperSymbol(s.to_string(), msg).into()) }, @@ -2496,13 +2496,13 @@ impl<'a> Iterator for TokenIterator<'a> { }, pos), // Custom keyword #[cfg(not(feature = "no_custom_syntax"))] - Some((Token::Identifier(s), pos)) if self.engine.custom_keywords.as_deref().map_or(false,|m| m.contains_key(&*s)) => { + Some((Token::Identifier(s), pos)) if self.engine.custom_keywords.as_ref().map_or(false,|m| m.contains_key(&*s)) => { (Token::Custom(s), pos) } // Custom keyword/symbol - must be disabled #[cfg(not(feature = "no_custom_syntax"))] - Some((token, pos)) if token.is_literal() && self.engine.custom_keywords.as_deref().map_or(false,|m| m.contains_key(token.literal_syntax())) => { - if self.engine.disabled_symbols.as_deref().map_or(false,|m| m.contains(token.literal_syntax())) { + Some((token, pos)) if token.is_literal() && self.engine.custom_keywords.as_ref().map_or(false,|m| m.contains_key(token.literal_syntax())) => { + if self.engine.disabled_symbols.as_ref().map_or(false,|m| m.contains(token.literal_syntax())) { // Disabled standard keyword/symbol (Token::Custom(Box::new(token.literal_syntax().into())), pos) } else { @@ -2511,7 +2511,7 @@ impl<'a> Iterator for TokenIterator<'a> { } } // Disabled symbol - Some((token, pos)) if token.is_literal() && self.engine.disabled_symbols.as_deref().map_or(false,|m| m.contains(token.literal_syntax())) => { + Some((token, pos)) if token.is_literal() && self.engine.disabled_symbols.as_ref().map_or(false,|m| m.contains(token.literal_syntax())) => { (Token::Reserved(Box::new(token.literal_syntax().into())), pos) } // Normal symbol From 892ed82f2eeb3baa3a5247bdfb87435895f8615b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 23 Mar 2023 17:11:42 +0800 Subject: [PATCH 05/17] Add script example. --- scripts/function_decl5.rhai | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 scripts/function_decl5.rhai diff --git a/scripts/function_decl5.rhai b/scripts/function_decl5.rhai new file mode 100644 index 00000000..65a4c111 --- /dev/null +++ b/scripts/function_decl5.rhai @@ -0,0 +1,35 @@ +//! This script defines multiple versions of the same function +//! for use as method with different data types. + +// For strings +fn string.calc(x) { + this.len + x +} +// For integers +fn int.calc(x) { + this * x +} +// For booleans +fn bool.calc(x) { + if this { x } else { 0} +} +// For arrays +fn array.calc(x) { + this.len + x +} +// For object maps +fn map.calc(x) { + this[x] +} +// Catch-all +fn calc(x) { + `${this}: ${x}` +} + +print("hello".calc(42)); // 47 +print(42.calc(42)); // 1764 +print(true.calc(42)); // 42 +print(false.calc(42)); // 0 +print([1,2,3].calc(42)); // 45 +print(#{"a": 1, "b": 2}.calc("b")); // 2 +print('x'.calc(42)); // x: 42 From ce355aa55323ab0071a7a34a38408dc18336f336 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 25 Mar 2023 17:24:05 +0800 Subject: [PATCH 06/17] More slots for boxed arrays. --- src/ast/expr.rs | 4 ++-- src/parser.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 2f96af59..1896f0df 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -276,9 +276,9 @@ pub enum Expr { /// [String][ImmutableString] constant. StringConstant(ImmutableString, Position), /// An interpolated [string][ImmutableString]. - InterpolatedString(Box>, Position), + InterpolatedString(Box>, Position), /// [ expr, ... ] - Array(Box>, Position), + Array(Box>, Position), /// #{ name:expr, ... } Map( Box<(StaticVec<(Ident, Expr)>, BTreeMap)>, diff --git a/src/parser.rs b/src/parser.rs index 37c13265..d7518138 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -908,7 +908,7 @@ impl Engine { let mut settings = settings; settings.pos = eat_token(input, Token::LeftBracket); - let mut array = StaticVec::new_const(); + let mut array = FnArgsVec::new_const(); loop { const MISSING_RBRACKET: &str = "to end this array literal"; @@ -1501,7 +1501,7 @@ impl Engine { // Interpolated string Token::InterpolatedString(..) => { - let mut segments = StaticVec::::new(); + let mut segments = FnArgsVec::new_const(); let settings = settings.level_up()?; match input.next().expect(NEVER_ENDS) { From 534b7bbab3c2ecc725b246bae2f3db5352293eae Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 25 Mar 2023 21:13:34 +0800 Subject: [PATCH 07/17] Rename auto_restore! to defer! --- src/api/call_fn.rs | 2 +- src/api/eval.rs | 2 +- src/{restore.rs => defer.rs} | 38 ++++++++++++++++++------------------ src/eval/chaining.rs | 16 +++++++-------- src/eval/expr.rs | 2 +- src/eval/stmt.rs | 14 ++++++------- src/func/call.rs | 14 ++++++------- src/lib.rs | 4 ++-- 8 files changed, 46 insertions(+), 46 deletions(-) rename src/{restore.rs => defer.rs} (65%) diff --git a/src/api/call_fn.rs b/src/api/call_fn.rs index 07ea0c99..120c4f4c 100644 --- a/src/api/call_fn.rs +++ b/src/api/call_fn.rs @@ -233,7 +233,7 @@ impl Engine { let rewind_scope = options.rewind_scope; let result = if options.eval_ast && !statements.is_empty() { - auto_restore! { + defer! { scope if rewind_scope => rewind; let orig_scope_len = scope.len(); } diff --git a/src/api/eval.rs b/src/api/eval.rs index 2c93b182..60a9b816 100644 --- a/src/api/eval.rs +++ b/src/api/eval.rs @@ -242,7 +242,7 @@ impl Engine { ast.resolver().cloned(), ); - auto_restore! { global => move |g| { + defer! { global => move |g| { #[cfg(not(feature = "no_module"))] { g.embedded_module_resolver = orig_embedded_module_resolver; diff --git a/src/restore.rs b/src/defer.rs similarity index 65% rename from src/restore.rs rename to src/defer.rs index d369f6bc..f3947430 100644 --- a/src/restore.rs +++ b/src/defer.rs @@ -5,56 +5,56 @@ use std::ops::{Deref, DerefMut}; use std::prelude::v1::*; /// Automatically restore state at the end of the scope. -macro_rules! auto_restore { +macro_rules! defer { (let $temp:ident = $var:ident . $prop:ident; $code:stmt) => { - auto_restore!(let $temp = $var.$prop; $code => move |v| v.$prop = $temp); + defer!(let $temp = $var.$prop; $code => move |v| v.$prop = $temp); }; (let $temp:ident = $var:ident . $prop:ident; $code:stmt => $restore:expr) => { let $temp = $var.$prop; $code - auto_restore!($var => $restore); + defer!($var => $restore); }; ($var:ident => $restore:ident; let $temp:ident = $save:expr;) => { - auto_restore!($var => $restore; let $temp = $save; {}); + defer!($var => $restore; let $temp = $save; {}); }; ($var:ident if $guard:expr => $restore:ident; let $temp:ident = $save:expr;) => { - auto_restore!($var if $guard => $restore; let $temp = $save; {}); + defer!($var if $guard => $restore; let $temp = $save; {}); }; ($var:ident => $restore:ident; let $temp:ident = $save:expr; $code:stmt) => { let $temp = $save; $code - auto_restore!($var => move |v| { v.$restore($temp); }); + defer!($var => move |v| { v.$restore($temp); }); }; ($var:ident if $guard:expr => $restore:ident; let $temp:ident = $save:expr; $code:stmt) => { let $temp = $save; $code - auto_restore!($var if $guard => move |v| { v.$restore($temp); }); + defer!($var if $guard => move |v| { v.$restore($temp); }); }; ($var:ident => $restore:expr) => { - auto_restore!($var = $var => $restore); + defer!($var = $var => $restore); }; ($var:ident = $value:expr => $restore:expr) => { - let $var = &mut *crate::RestoreOnDrop::lock($value, $restore); + let $var = &mut *crate::Deferred::lock($value, $restore); }; ($var:ident if Some($guard:ident) => $restore:expr) => { - auto_restore!($var = ($var) if Some($guard) => $restore); + defer!($var = ($var) if Some($guard) => $restore); }; ($var:ident = ( $value:expr ) if Some($guard:ident) => $restore:expr) => { let mut __rx__; let $var = if let Some($guard) = $guard { - __rx__ = crate::RestoreOnDrop::lock($value, $restore); + __rx__ = crate::Deferred::lock($value, $restore); &mut *__rx__ } else { &mut *$value }; }; ($var:ident if $guard:expr => $restore:expr) => { - auto_restore!($var = ($var) if $guard => $restore); + defer!($var = ($var) if $guard => $restore); }; ($var:ident = ( $value:expr ) if $guard:expr => $restore:expr) => { let mut __rx__; let $var = if $guard { - __rx__ = crate::RestoreOnDrop::lock($value, $restore); + __rx__ = crate::Deferred::lock($value, $restore); &mut *__rx__ } else { &mut *$value @@ -64,13 +64,13 @@ macro_rules! auto_restore { /// Run custom restoration logic upon the end of scope. #[must_use] -pub struct RestoreOnDrop<'a, T: ?Sized, R: FnOnce(&mut T)> { +pub struct Deferred<'a, T: ?Sized, R: FnOnce(&mut T)> { value: &'a mut T, restore: Option, } -impl<'a, T: ?Sized, R: FnOnce(&mut T)> RestoreOnDrop<'a, T, R> { - /// Create a new [`RestoreOnDrop`] that locks a mutable reference and runs restoration logic at +impl<'a, T: ?Sized, R: FnOnce(&mut T)> Deferred<'a, T, R> { + /// Create a new [`Deferred`] that locks a mutable reference and runs restoration logic at /// the end of scope. /// /// Beware that the end of scope means the end of its lifetime, not necessarily waiting until @@ -84,14 +84,14 @@ impl<'a, T: ?Sized, R: FnOnce(&mut T)> RestoreOnDrop<'a, T, R> { } } -impl<'a, T: ?Sized, R: FnOnce(&mut T)> Drop for RestoreOnDrop<'a, T, R> { +impl<'a, T: ?Sized, R: FnOnce(&mut T)> Drop for Deferred<'a, T, R> { #[inline(always)] fn drop(&mut self) { self.restore.take().unwrap()(self.value); } } -impl<'a, T: ?Sized, R: FnOnce(&mut T)> Deref for RestoreOnDrop<'a, T, R> { +impl<'a, T: ?Sized, R: FnOnce(&mut T)> Deref for Deferred<'a, T, R> { type Target = T; #[inline(always)] @@ -100,7 +100,7 @@ impl<'a, T: ?Sized, R: FnOnce(&mut T)> Deref for RestoreOnDrop<'a, T, R> { } } -impl<'a, T: ?Sized, R: FnOnce(&mut T)> DerefMut for RestoreOnDrop<'a, T, R> { +impl<'a, T: ?Sized, R: FnOnce(&mut T)> DerefMut for Deferred<'a, T, R> { #[inline(always)] fn deref_mut(&mut self) -> &mut Self::Target { self.value diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index cda741a5..43570317 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -67,7 +67,7 @@ impl Engine { idx: &mut Dynamic, pos: Position, ) -> RhaiResultOf { - auto_restore! { let orig_level = global.level; global.level += 1 } + defer! { let orig_level = global.level; global.level += 1 } let hash = hash_idx().0; let args = &mut [target, idx]; @@ -88,7 +88,7 @@ impl Engine { is_ref_mut: bool, pos: Position, ) -> RhaiResultOf<(Dynamic, bool)> { - auto_restore! { let orig_level = global.level; global.level += 1 } + defer! { let orig_level = global.level; global.level += 1 } let hash = hash_idx().1; let args = &mut [target, idx, new_val]; @@ -689,14 +689,14 @@ impl Engine { let reset = self.run_debugger_with_reset(global, caches, scope, this_ptr, rhs)?; #[cfg(feature = "debugging")] - auto_restore! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } + defer! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } let crate::ast::FnCallExpr { name, hashes, args, .. } = &**x; // Truncate the index values upon exit - auto_restore! { idx_values => truncate; let offset = idx_values.len() - args.len(); } + defer! { idx_values => truncate; let offset = idx_values.len() - args.len(); } let call_args = &mut idx_values[offset..]; let arg1_pos = args.get(0).map_or(Position::NONE, Expr::position); @@ -858,14 +858,14 @@ impl Engine { let reset = self .run_debugger_with_reset(global, caches, scope, _tp, _node)?; #[cfg(feature = "debugging")] - auto_restore! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } + defer! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } let crate::ast::FnCallExpr { name, hashes, args, .. } = &**x; // Truncate the index values upon exit - auto_restore! { idx_values => truncate; let offset = idx_values.len() - args.len(); } + defer! { idx_values => truncate; let offset = idx_values.len() - args.len(); } let call_args = &mut idx_values[offset..]; let arg1_pos = args.get(0).map_or(Position::NONE, Expr::position); @@ -978,14 +978,14 @@ impl Engine { global, caches, scope, _tp, _node, )?; #[cfg(feature = "debugging")] - auto_restore! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } + defer! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } let crate::ast::FnCallExpr { name, hashes, args, .. } = &**f; // Truncate the index values upon exit - auto_restore! { idx_values => truncate; let offset = idx_values.len() - args.len(); } + defer! { idx_values => truncate; let offset = idx_values.len() - args.len(); } let call_args = &mut idx_values[offset..]; let pos1 = args.get(0).map_or(Position::NONE, Expr::position); diff --git a/src/eval/expr.rs b/src/eval/expr.rs index e728249e..f8af1f35 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -242,7 +242,7 @@ impl Engine { let reset = self.run_debugger_with_reset(global, caches, scope, this_ptr.as_deref_mut(), expr)?; #[cfg(feature = "debugging")] - auto_restore! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } + defer! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } match expr { // Constants diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 021417d7..5d090b76 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -43,7 +43,7 @@ impl Engine { } // Restore scope at end of block if necessary - auto_restore! { scope if restore_orig_state => rewind; let orig_scope_len = scope.len(); } + defer! { scope if restore_orig_state => rewind; let orig_scope_len = scope.len(); } // Restore global state at end of block if necessary let orig_always_search_scope = global.always_search_scope; @@ -54,7 +54,7 @@ impl Engine { global.scope_level += 1; } - auto_restore! { global if restore_orig_state => move |g| { + defer! { global if restore_orig_state => move |g| { g.scope_level -= 1; #[cfg(not(feature = "no_module"))] @@ -66,7 +66,7 @@ impl Engine { }} // Pop new function resolution caches at end of block - auto_restore! { + defer! { caches => rewind_fn_resolution_caches; let orig_fn_resolution_caches_len = caches.fn_resolution_caches_len(); } @@ -209,7 +209,7 @@ impl Engine { get_builtin_op_assignment_fn(op_x, &*lock_guard, &new_val) { // We may not need to bump the level because built-in's do not need it. - //auto_restore! { let orig_level = global.level; global.level += 1 } + //defer! { let orig_level = global.level; global.level += 1 } let args = &mut [&mut *lock_guard, &mut new_val]; let context = need_context @@ -273,7 +273,7 @@ impl Engine { let reset = self.run_debugger_with_reset(global, caches, scope, this_ptr.as_deref_mut(), stmt)?; #[cfg(feature = "debugging")] - auto_restore! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } + defer! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } match stmt { // No-op @@ -673,7 +673,7 @@ impl Engine { let iter_func = iter_func.ok_or_else(|| ERR::ErrorFor(expr.start_position()))?; // Restore scope at end of statement - auto_restore! { scope => rewind; let orig_scope_len = scope.len(); } + defer! { scope => rewind; let orig_scope_len = scope.len(); } // Add the loop variables let counter_index = (!counter.is_empty()).then(|| { @@ -808,7 +808,7 @@ impl Engine { }; // Restore scope at end of block - auto_restore! { scope if !catch_var.is_unit() => rewind; let orig_scope_len = scope.len(); } + defer! { scope if !catch_var.is_unit() => rewind; let orig_scope_len = scope.len(); } if let Expr::Variable(x, ..) = catch_var { scope.push(x.3.clone(), err_value); diff --git a/src/func/call.rs b/src/func/call.rs index 61d54818..52400b3b 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -577,7 +577,7 @@ impl Engine { #[cfg(not(feature = "no_closure"))] ensure_no_data_race(fn_name, args, is_ref_mut)?; - auto_restore! { let orig_level = global.level; global.level += 1 } + defer! { let orig_level = global.level; global.level += 1 } // These may be redirected from method style calls. if hashes.is_native_only() { @@ -668,7 +668,7 @@ impl Engine { }; let orig_source = mem::replace(&mut global.source, source.clone()); - auto_restore! { global => move |g| g.source = orig_source } + defer! { global => move |g| g.source = orig_source } return if _is_method_call { // Method call of script function - map first argument to `this` @@ -696,7 +696,7 @@ impl Engine { backup.change_first_arg_to_copy(args); } - auto_restore! { args = (args) if swap => move |a| backup.restore_first_arg(a) } + defer! { args = (args) if swap => move |a| backup.restore_first_arg(a) } self.call_script_fn(global, caches, scope, None, environ, f, args, true, pos) } @@ -740,7 +740,7 @@ impl Engine { }) }); #[cfg(feature = "debugging")] - auto_restore! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } + defer! { global if Some(reset) => move |g| g.debugger_mut().reset_status(reset) } self.eval_expr(global, caches, scope, this_ptr, arg_expr) .map(|r| (r, arg_expr.start_position())) @@ -1451,7 +1451,7 @@ impl Engine { } } - auto_restore! { let orig_level = global.level; global.level += 1 } + defer! { let orig_level = global.level; global.level += 1 } match func { #[cfg(not(feature = "no_function"))] @@ -1462,7 +1462,7 @@ impl Engine { let scope = &mut Scope::new(); let orig_source = mem::replace(&mut global.source, module.id_raw().cloned()); - auto_restore! { global => move |g| g.source = orig_source } + defer! { global => move |g| g.source = orig_source } self.call_script_fn(global, caches, scope, None, environ, f, args, true, pos) } @@ -1729,7 +1729,7 @@ impl Engine { get_builtin_binary_op_fn(op_token.as_ref().unwrap(), operands[0], operands[1]) { // We may not need to bump the level because built-in's do not need it. - //auto_restore! { let orig_level = global.level; global.level += 1 } + //defer! { let orig_level = global.level; global.level += 1 } let context = need_context.then(|| (self, name.as_str(), None, &*global, pos).into()); diff --git a/src/lib.rs b/src/lib.rs index 8f4dda89..63b342ec 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -102,7 +102,7 @@ use std::prelude::v1::*; #[macro_use] mod reify; #[macro_use] -mod restore; +mod defer; mod api; mod ast; @@ -225,6 +225,7 @@ pub use api::custom_syntax::Expression; pub use api::files::{eval_file, run_file}; pub use api::{eval::eval, events::VarDefInfo, run::run}; pub use ast::{FnAccess, AST}; +use defer::Deferred; pub use engine::{Engine, OP_CONTAINS, OP_EQUALS}; pub use eval::EvalContext; #[cfg(not(feature = "no_function"))] @@ -233,7 +234,6 @@ use func::calc_typed_method_hash; use func::{calc_fn_hash, calc_fn_hash_full, calc_var_hash}; pub use func::{plugin, FuncArgs, NativeCallContext, RegisterNativeFunction}; pub use module::{FnNamespace, Module}; -use restore::RestoreOnDrop; pub use rhai_codegen::*; #[cfg(not(feature = "no_time"))] pub use types::Instant; From 2a98d38a7e0132fe2dede925d77da287a89d3fe3 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 30 Mar 2023 16:26:58 +0800 Subject: [PATCH 08/17] is_shared is made reserved. --- CHANGELOG.md | 5 ++ src/func/call.rs | 81 ++++++++--------- src/tokenizer.rs | 220 +++++++++++++++++++++++++++------------------ tools/reserved.txt | 1 + 4 files changed, 172 insertions(+), 135 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 136c1fd9..532c29e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ Version 1.14.0 The code hacks that attempt to optimize branch prediction performance are removed because benchmarks do not show any material speed improvements. +Buf fixes +---------- + +* `is_shared` is a reserved keyword and is now handled properly (e.g. it cannot be the target of a function pointer). + New features ------------ diff --git a/src/func/call.rs b/src/func/call.rs index 52400b3b..38eb7cca 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -573,58 +573,47 @@ impl Engine { _is_method_call: bool, pos: Position, ) -> RhaiResultOf<(Dynamic, bool)> { + // These may be redirected from method style calls. + if hashes.is_native_only() { + let error = match fn_name { + // Handle type_of() + KEYWORD_TYPE_OF => { + if args.len() == 1 { + let typ = self.get_interned_string(self.map_type_name(args[0].type_name())); + return Ok((typ.into(), false)); + } + true + } + + #[cfg(not(feature = "no_closure"))] + crate::engine::KEYWORD_IS_SHARED => { + if args.len() == 1 { + return Ok((args[0].is_shared().into(), false)); + } + true + } + + #[cfg(not(feature = "no_function"))] + crate::engine::KEYWORD_IS_DEF_FN => true, + + KEYWORD_FN_PTR | KEYWORD_EVAL | KEYWORD_IS_DEF_VAR | KEYWORD_FN_PTR_CALL + | KEYWORD_FN_PTR_CURRY => true, + + _ => false, + }; + + if error { + let sig = self.gen_fn_call_signature(fn_name, args); + return Err(ERR::ErrorFunctionNotFound(sig, pos).into()); + } + } + // Check for data race. #[cfg(not(feature = "no_closure"))] ensure_no_data_race(fn_name, args, is_ref_mut)?; defer! { let orig_level = global.level; global.level += 1 } - // These may be redirected from method style calls. - if hashes.is_native_only() { - match fn_name { - // Handle type_of() - KEYWORD_TYPE_OF if args.len() == 1 => { - let typ = self.get_interned_string(self.map_type_name(args[0].type_name())); - return Ok((typ.into(), false)); - } - - // Handle is_def_fn() - #[cfg(not(feature = "no_function"))] - crate::engine::KEYWORD_IS_DEF_FN - if args.len() == 2 && args[0].is_fnptr() && args[1].is_int() => - { - let fn_name = args[0].read_lock::().expect("`FnPtr`"); - let num_params = args[1].as_int().expect("`INT`"); - - return Ok(( - if (0..=crate::MAX_USIZE_INT).contains(&num_params) { - #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)] - let hash_script = - calc_fn_hash(None, fn_name.as_str(), num_params as usize); - self.has_script_fn(global, caches, hash_script) - } else { - false - } - .into(), - false, - )); - } - - // Handle is_shared() - #[cfg(not(feature = "no_closure"))] - crate::engine::KEYWORD_IS_SHARED => { - unreachable!("{} called as method", fn_name) - } - - KEYWORD_FN_PTR | KEYWORD_EVAL | KEYWORD_IS_DEF_VAR | KEYWORD_FN_PTR_CALL - | KEYWORD_FN_PTR_CURRY => { - unreachable!("{} called as method", fn_name) - } - - _ => (), - } - } - // Script-defined function call? #[cfg(not(feature = "no_function"))] if !hashes.is_native_only() { diff --git a/src/tokenizer.rs b/src/tokenizer.rs index 60d13808..da4676fb 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -514,117 +514,117 @@ static KEYWORDS_LIST: [(&str, Token); 153] = [ const MIN_RESERVED_LEN: usize = 1; const MAX_RESERVED_LEN: usize = 10; const MIN_RESERVED_HASH_VALUE: usize = 1; -const MAX_RESERVED_HASH_VALUE: usize = 112; +const MAX_RESERVED_HASH_VALUE: usize = 149; static RESERVED_ASSOC_VALUES: [u8; 256] = [ - 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, - 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 35, 113, 45, 25, 113, - 113, 113, 60, 55, 50, 50, 113, 15, 0, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, - 10, 85, 45, 5, 55, 50, 5, 113, 113, 113, 113, 113, 85, 113, 113, 113, 113, 113, 113, 113, 113, - 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 35, 113, 113, 113, 55, 113, 10, 40, - 5, 0, 5, 35, 10, 5, 0, 113, 113, 20, 25, 5, 45, 0, 113, 0, 0, 0, 15, 30, 20, 25, 20, 113, 113, - 20, 113, 0, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, - 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, - 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, - 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, - 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, - 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, - 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, 113, + 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, + 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 10, 150, 5, 35, 150, 150, + 150, 45, 35, 30, 30, 150, 20, 15, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 35, + 30, 15, 5, 25, 0, 25, 150, 150, 150, 150, 150, 65, 150, 150, 150, 150, 150, 150, 150, 150, 150, + 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 40, 150, 150, 150, 150, 150, 0, 150, 0, + 0, 0, 15, 45, 10, 15, 150, 150, 35, 25, 10, 50, 0, 150, 5, 0, 15, 0, 5, 25, 45, 15, 150, 150, + 25, 150, 20, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, + 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, + 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, + 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, + 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, + 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, + 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, 150, ]; -static RESERVED_LIST: [(&str, bool, bool, bool); 113] = [ +static RESERVED_LIST: [(&str, bool, bool, bool); 150] = [ ("", false, false, false), - ("~", true, false, false), - ("is", true, false, false), - ("...", true, false, false), - ("", false, false, false), - ("print", true, true, false), - ("@", true, false, false), - ("private", cfg!(feature = "no_function"), false, false), - ("", false, false, false), - ("this", true, false, false), - ("", false, false, false), - ("thread", true, false, false), + ("?", true, false, false), ("as", cfg!(feature = "no_module"), false, false), - ("", false, false, false), - ("", false, false, false), - ("spawn", true, false, false), - ("static", true, false, false), - (":=", true, false, false), - ("===", true, false, false), - ("case", true, false, false), - ("super", true, false, false), - ("shared", true, false, false), - ("package", true, false, false), ("use", true, false, false), - ("with", true, false, false), - ("curry", true, true, true), - ("$", true, false, false), - ("type_of", true, true, true), - ("nil", true, false, false), - ("sync", true, false, false), - ("yield", true, false, false), - ("import", cfg!(feature = "no_module"), false, false), - ("--", true, false, false), - ("new", true, false, false), - ("exit", true, false, false), + ("case", true, false, false), ("async", true, false, false), - ("export", cfg!(feature = "no_module"), false, false), - ("!.", true, false, false), + ("public", true, false, false), + ("package", true, false, false), ("", false, false, false), - ("call", true, true, true), - ("match", true, false, false), ("", false, false, false), - ("fn", cfg!(feature = "no_function"), false, false), - ("var", true, false, false), - ("null", true, false, false), - ("await", true, false, false), + ("super", true, false, false), ("#", true, false, false), + ("private", cfg!(feature = "no_function"), false, false), + ("var", true, false, false), + ("protected", true, false, false), + ("spawn", true, false, false), + ("shared", true, false, false), + ("is", true, false, false), + ("===", true, false, false), + ("sync", true, false, false), + ("curry", true, true, true), + ("static", true, false, false), ("default", true, false, false), ("!==", true, false, false), - ("eval", true, true, false), - ("debug", true, true, false), - ("?", true, false, false), + ("is_shared", cfg!(not(feature = "no_closure")), true, true), + ("print", true, true, false), + ("", false, false, false), + ("#!", true, false, false), + ("", false, false, false), + ("this", true, false, false), + ("is_def_var", true, true, false), + ("thread", true, false, false), ("?.", cfg!(feature = "no_object"), false, false), ("", false, false, false), - ("protected", true, false, false), + ("is_def_fn", cfg!(not(feature = "no_function")), true, false), + ("yield", true, false, false), + ("", false, false, false), + ("fn", cfg!(feature = "no_function"), false, false), + ("new", true, false, false), + ("call", true, true, true), + ("match", true, false, false), + ("~", true, false, false), + ("!.", true, false, false), + ("", false, false, false), + ("eval", true, true, false), + ("await", true, false, false), + ("", false, false, false), + (":=", true, false, false), + ("...", true, false, false), + ("null", true, false, false), + ("debug", true, true, false), + ("@", true, false, false), + ("type_of", true, true, true), + ("", false, false, false), + ("with", true, false, false), ("", false, false, false), ("", false, false, false), - ("go", true, false, false), - ("", false, false, false), - ("goto", true, false, false), - ("", false, false, false), - ("public", true, false, false), ("<-", true, false, false), ("", false, false, false), - ("is_def_fn", cfg!(not(feature = "no_function")), true, false), - ("is_def_var", true, true, false), + ("void", true, false, false), ("", false, false, false), + ("import", cfg!(feature = "no_module"), false, false), + ("--", true, false, false), + ("nil", true, false, false), + ("exit", true, false, false), + ("", false, false, false), + ("export", cfg!(feature = "no_module"), false, false), ("<|", true, false, false), - ("::<", true, false, false), ("", false, false, false), ("", false, false, false), ("", false, false, false), + ("$", true, false, false), ("->", true, false, false), ("", false, false, false), ("", false, false, false), ("", false, false, false), - ("module", true, false, false), + ("", false, false, false), ("|>", true, false, false), ("", false, false, false), - ("void", true, false, false), - ("", false, false, false), - ("", false, false, false), - ("#!", true, false, false), - ("", false, false, false), - ("", false, false, false), ("", false, false, false), ("", false, false, false), + ("module", true, false, false), ("?[", cfg!(feature = "no_index"), false, false), ("", false, false, false), ("", false, false, false), ("", false, false, false), ("", false, false, false), ("Fn", true, true, false), + ("::<", true, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("++", true, false, false), ("", false, false, false), ("", false, false, false), ("", false, false, false), @@ -634,17 +634,54 @@ static RESERVED_LIST: [(&str, bool, bool, bool); 113] = [ ("", false, false, false), ("", false, false, false), ("", false, false, false), - ("++", true, false, false), - ("", false, false, false), - ("", false, false, false), - ("", false, false, false), - ("", false, false, false), ("*)", true, false, false), ("", false, false, false), ("", false, false, false), ("", false, false, false), ("", false, false, false), ("(*", true, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("", false, false, false), + ("go", true, false, false), + ("", false, false, false), + ("goto", true, false, false), ]; impl Token { @@ -875,12 +912,13 @@ impl Token { // by GNU `gperf` on the list of keywords. let utf8 = syntax.as_bytes(); let len = utf8.len(); - let mut hash_val = len; if !(MIN_KEYWORD_LEN..=MAX_KEYWORD_LEN).contains(&len) { return None; } + let mut hash_val = len; + match len { 1 => (), _ => hash_val += KEYWORD_ASSOC_VALUES[(utf8[1] as usize) + 1] as usize, @@ -2306,8 +2344,10 @@ pub fn is_id_continue(x: char) -> bool { /// The first `bool` indicates whether it is a reserved keyword or symbol. /// /// The second `bool` indicates whether the keyword can be called normally as a function. +/// `false` if it is not a reserved keyword. /// /// The third `bool` indicates whether the keyword can be called in method-call style. +/// `false` if it is not a reserved keyword or it cannot be called as a function. #[inline] #[must_use] pub fn is_reserved_keyword_or_symbol(syntax: &str) -> (bool, bool, bool) { @@ -2315,16 +2355,19 @@ pub fn is_reserved_keyword_or_symbol(syntax: &str) -> (bool, bool, bool) { // by GNU `gperf` on the list of keywords. let utf8 = syntax.as_bytes(); let len = utf8.len(); - let rounds = len.min(3); - let mut hash_val = len; if !(MIN_RESERVED_LEN..=MAX_RESERVED_LEN).contains(&len) { return (false, false, false); } - for x in 0..rounds { - hash_val += RESERVED_ASSOC_VALUES[utf8[rounds - 1 - x] as usize] as usize; + let mut hash_val = len; + + match len { + 1 => (), + _ => hash_val += RESERVED_ASSOC_VALUES[(utf8[1] as usize)] as usize, } + hash_val += RESERVED_ASSOC_VALUES[utf8[0] as usize] as usize; + hash_val += RESERVED_ASSOC_VALUES[utf8[len - 1] as usize] as usize; if !(MIN_RESERVED_HASH_VALUE..=MAX_RESERVED_HASH_VALUE).contains(&hash_val) { return (false, false, false); @@ -2332,13 +2375,12 @@ pub fn is_reserved_keyword_or_symbol(syntax: &str) -> (bool, bool, bool) { match RESERVED_LIST[hash_val] { ("", ..) => (false, false, false), - (s, true, a, b) => ( + (s, true, a, b) => { // Fail early to avoid calling memcmp(). // Since we are already working with bytes, mind as well check the first one. - s.len() == len && s.as_bytes()[0] == utf8[0] && s == syntax, - a, - b, - ), + let is_reserved = s.len() == len && s.as_bytes()[0] == utf8[0] && s == syntax; + (is_reserved, is_reserved && a, is_reserved && a && b) + } _ => (false, false, false), } } diff --git a/tools/reserved.txt b/tools/reserved.txt index 2dbe79cd..4db0859a 100644 --- a/tools/reserved.txt +++ b/tools/reserved.txt @@ -91,3 +91,4 @@ struct reserved; "this", true, false, false "is_def_var", true, true, false "is_def_fn", cfg!(not(feature = "no_function")), true, false +"is_shared", cfg!(not(feature = "no_closure")), true, true From 34c7dabe4481613b2722c3b40bff43d4510c6e29 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 30 Mar 2023 16:43:15 +0800 Subject: [PATCH 09/17] Add is_def_fn with 3 parameters. --- CHANGELOG.md | 1 + src/api/definitions/builtin-functions.d.rhai | 15 +++++ src/func/call.rs | 64 ++++++++++++++++---- tests/method_call.rs | 9 +++ 4 files changed, 77 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 532c29e3..8598a79c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ New features ------------ * It is now possible to require a specific _type_ to the `this` pointer for a particular script-defined function so that it is called only when the `this` pointer contains the specified type. +* `is_def_fn` is extended to support checking for typed methods, with syntax `is_def_fn(this_type, fn_name, arity)` Version 1.13.0 diff --git a/src/api/definitions/builtin-functions.d.rhai b/src/api/definitions/builtin-functions.d.rhai index 93d07df3..0ccab0d0 100644 --- a/src/api/definitions/builtin-functions.d.rhai +++ b/src/api/definitions/builtin-functions.d.rhai @@ -112,6 +112,21 @@ fn curry(fn_ptr: FnPtr, ...args: ?) -> FnPtr; /// ``` fn is_def_fn(fn_name: String, num_params: int) -> bool; +/// Return `true` if a script-defined function exists with a specified name and +/// number of parameters, bound to a specified type for `this`. +/// +/// # Example +/// +/// ```rhai +/// // A method that requires `this` to be `MyType` +/// fn MyType.foo(x) { } +/// +/// print(is_def_fn("MyType", "foo", 1)); // prints true +/// print(is_def_fn("foo", 1)); // prints false +/// print(is_def_fn("MyType", "foo", 2)); // prints false +/// ``` +fn is_def_fn(this_type: String, fn_name: String, num_params: int) -> bool; + /// Return `true` if a variable matching a specified name is defined. /// /// # Example diff --git a/src/func/call.rs b/src/func/call.rs index 38eb7cca..5afb6e7a 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -1,6 +1,9 @@ //! Implement function-calling mechanism for [`Engine`]. -use super::{get_builtin_binary_op_fn, get_builtin_op_assignment_fn, CallableFunction}; +use super::{ + calc_typed_method_hash, get_builtin_binary_op_fn, get_builtin_op_assignment_fn, + CallableFunction, +}; use crate::api::default_limits::MAX_DYNAMIC_PARAMETERS; use crate::ast::{Expr, FnCallExpr, FnCallHashes}; use crate::engine::{ @@ -1099,7 +1102,7 @@ impl Engine { FnCallHashes::from_hash(calc_fn_hash(None, name, args_len)) }; } - // Handle Fn() + // Handle Fn(fn_name) KEYWORD_FN_PTR if total_args == 1 => { let arg = first_arg.unwrap(); let (arg_value, arg_pos) = @@ -1114,7 +1117,7 @@ impl Engine { .map_err(|err| err.fill_position(arg_pos)); } - // Handle curry() + // Handle curry(x, ...) KEYWORD_FN_PTR_CURRY if total_args > 1 => { let first = first_arg.unwrap(); let (arg_value, arg_pos) = @@ -1137,7 +1140,7 @@ impl Engine { return Ok(fn_ptr.into()); } - // Handle is_shared() + // Handle is_shared(var) #[cfg(not(feature = "no_closure"))] crate::engine::KEYWORD_IS_SHARED if total_args == 1 => { let arg = first_arg.unwrap(); @@ -1146,7 +1149,7 @@ impl Engine { return Ok(arg_value.is_shared().into()); } - // Handle is_def_fn() + // Handle is_def_fn(fn_name, arity) #[cfg(not(feature = "no_function"))] crate::engine::KEYWORD_IS_DEF_FN if total_args == 2 => { let first = first_arg.unwrap(); @@ -1164,17 +1167,54 @@ impl Engine { .as_int() .map_err(|typ| self.make_type_mismatch_err::(typ, arg_pos))?; - return Ok(if (0..=crate::MAX_USIZE_INT).contains(&num_params) { + return Ok(if num_params >= 0 { #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)] let hash_script = calc_fn_hash(None, &fn_name, num_params as usize); - self.has_script_fn(global, caches, hash_script) + self.has_script_fn(global, caches, hash_script).into() } else { - false - } - .into()); + Dynamic::FALSE + }); } - // Handle is_def_var() + // Handle is_def_fn(this_type, fn_name, arity) + #[cfg(not(feature = "no_function"))] + #[cfg(not(feature = "no_object"))] + crate::engine::KEYWORD_IS_DEF_FN if total_args == 3 => { + let first = first_arg.unwrap(); + let (arg_value, arg_pos) = + self.get_arg_value(global, caches, scope, this_ptr.as_deref_mut(), first)?; + + let this_type = arg_value + .into_immutable_string() + .map_err(|typ| self.make_type_mismatch_err::(typ, arg_pos))?; + + let (arg_value, arg_pos) = + self.get_arg_value(global, caches, scope, this_ptr.as_deref_mut(), &a_expr[0])?; + + let fn_name = arg_value + .into_immutable_string() + .map_err(|typ| self.make_type_mismatch_err::(typ, arg_pos))?; + + let (arg_value, arg_pos) = + self.get_arg_value(global, caches, scope, this_ptr, &a_expr[1])?; + + let num_params = arg_value + .as_int() + .map_err(|typ| self.make_type_mismatch_err::(typ, arg_pos))?; + + return Ok(if num_params >= 0 { + #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)] + let hash_script = calc_typed_method_hash( + calc_fn_hash(None, &fn_name, num_params as usize), + &this_type, + ); + self.has_script_fn(global, caches, hash_script).into() + } else { + Dynamic::FALSE + }); + } + + // Handle is_def_var(fn_name) KEYWORD_IS_DEF_VAR if total_args == 1 => { let arg = first_arg.unwrap(); let (arg_value, arg_pos) = @@ -1185,7 +1225,7 @@ impl Engine { return Ok(scope.contains(&var_name).into()); } - // Handle eval() + // Handle eval(script) KEYWORD_EVAL if total_args == 1 => { // eval - only in function call style let orig_scope_len = scope.len(); diff --git a/tests/method_call.rs b/tests/method_call.rs index 824f1c72..577447e7 100644 --- a/tests/method_call.rs +++ b/tests/method_call.rs @@ -108,6 +108,15 @@ fn test_method_call_typed() -> Result<(), Box> { TestStruct { x: 1002 } ); + assert!(engine.eval::( + r#" + fn "Test-Struct#ABC".foo(x) { + this.update(x); + } + is_def_fn("Test-Struct#ABC", "foo", 1) + "# + )?); + assert!(matches!( *engine .run( From 637728de6a5bb56306edc55a68d2b1f0a27fa3a8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 30 Mar 2023 17:41:21 +0800 Subject: [PATCH 10/17] Fix builds. --- src/func/call.rs | 7 ++----- src/tokenizer.rs | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/func/call.rs b/src/func/call.rs index 5afb6e7a..dac38df3 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -1,9 +1,6 @@ //! Implement function-calling mechanism for [`Engine`]. -use super::{ - calc_typed_method_hash, get_builtin_binary_op_fn, get_builtin_op_assignment_fn, - CallableFunction, -}; +use super::{get_builtin_binary_op_fn, get_builtin_op_assignment_fn, CallableFunction}; use crate::api::default_limits::MAX_DYNAMIC_PARAMETERS; use crate::ast::{Expr, FnCallExpr, FnCallHashes}; use crate::engine::{ @@ -1204,7 +1201,7 @@ impl Engine { return Ok(if num_params >= 0 { #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)] - let hash_script = calc_typed_method_hash( + let hash_script = crate::calc_typed_method_hash( calc_fn_hash(None, &fn_name, num_params as usize), &this_type, ); diff --git a/src/tokenizer.rs b/src/tokenizer.rs index da4676fb..2e0e009c 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -2364,7 +2364,7 @@ pub fn is_reserved_keyword_or_symbol(syntax: &str) -> (bool, bool, bool) { match len { 1 => (), - _ => hash_val += RESERVED_ASSOC_VALUES[(utf8[1] as usize)] as usize, + _ => hash_val += RESERVED_ASSOC_VALUES[utf8[1] as usize] as usize, } hash_val += RESERVED_ASSOC_VALUES[utf8[0] as usize] as usize; hash_val += RESERVED_ASSOC_VALUES[utf8[len - 1] as usize] as usize; From b102982d65800a7558a92eae83493eb9e2a45b03 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 31 Mar 2023 09:00:52 +0800 Subject: [PATCH 11/17] Add flags into gperf source files. --- tools/keywords.txt | 159 +++++++++++++++++++++++---------------------- tools/reserved.txt | 146 ++++++++++++++++++++++------------------- 2 files changed, 161 insertions(+), 144 deletions(-) diff --git a/tools/keywords.txt b/tools/keywords.txt index 33375c57..4d9746f3 100644 --- a/tools/keywords.txt +++ b/tools/keywords.txt @@ -3,7 +3,7 @@ // // Generate the output table via: // ```bash -// gperf -t keywords.txt +// gperf keywords.txt // ``` // // Since GNU gperf does not produce Rust output, the ANSI-C output must be hand-edited and @@ -17,86 +17,89 @@ // * Copy the `wordlist` array into `KEYWORDS_LIST` with the following modifications: // - Remove the `#line` comments // - Change the entry wrapping `{ .. }` into tuples `( .. )` -// - Replace all entries `("")` by `("", Token::EOF)` // - Put feature flags on the appropriate lines, and duplicating lines that maps to `Token::EOF` // for the opposite feature flags // +%global-table +%struct-type +%omit-struct-type +%define initializer-suffix ,Token::EOF struct keyword; %% -"{", Token::LeftBrace -"}", Token::RightBrace -"(", Token::LeftParen -")", Token::RightParen -"[", Token::LeftBracket -"]", Token::RightBracket -"()", Token::Unit -"+", Token::Plus -"-", Token::Minus -"*", Token::Multiply -"/", Token::Divide -";", Token::SemiColon -":", Token::Colon -"::", Token::DoubleColon -"=>", Token::DoubleArrow -"_", Token::Underscore -",", Token::Comma -".", Token::Period -"?.", Token::Elvis -"??", Token::DoubleQuestion -"?[", Token::QuestionBracket -"..", Token::ExclusiveRange -"..=", Token::InclusiveRange +{, Token::LeftBrace +}, Token::RightBrace +(, Token::LeftParen +), Token::RightParen +[, Token::LeftBracket +], Token::RightBracket +(), Token::Unit ++, Token::Plus +-, Token::Minus +*, Token::Multiply +/, Token::Divide +;, Token::SemiColon +:, Token::Colon +::, Token::DoubleColon +=>, Token::DoubleArrow +_, Token::Underscore +,, Token::Comma +., Token::Period +?., Token::Elvis +??, Token::DoubleQuestion +?[, Token::QuestionBracket +.., Token::ExclusiveRange +..=, Token::InclusiveRange "#{", Token::MapStart -"=", Token::Equals -"true", Token::True -"false", Token::False -"let", Token::Let -"const", Token::Const -"if", Token::If -"else", Token::Else -"switch", Token::Switch -"do", Token::Do -"while", Token::While -"until", Token::Until -"loop", Token::Loop -"for", Token::For -"in", Token::In -"!in", Token::NotIn -"<", Token::LessThan -">", Token::GreaterThan -"<=", Token::LessThanEqualsTo -">=", Token::GreaterThanEqualsTo -"==", Token::EqualsTo -"!=", Token::NotEqualsTo -"!", Token::Bang -"|", Token::Pipe -"||", Token::Or -"&", Token::Ampersand -"&&", Token::And -"continue", Token::Continue -"break", Token::Break -"return", Token::Return -"throw", Token::Throw -"try", Token::Try -"catch", Token::Catch -"+=", Token::PlusAssign -"-=", Token::MinusAssign -"*=", Token::MultiplyAssign -"/=", Token::DivideAssign -"<<=", Token::LeftShiftAssign -">>=", Token::RightShiftAssign -"&=", Token::AndAssign -"|=", Token::OrAssign -"^=", Token::XOrAssign -"<<", Token::LeftShift -">>", Token::RightShift -"^", Token::XOr -"%", Token::Modulo -"%=", Token::ModuloAssign -"**", Token::PowerOf -"**=", Token::PowerOfAssign -"fn", Token::Fn -"private", Token::Private -"import", Token::Import -"export", Token::Export -"as", Token::As +=, Token::Equals +true, Token::True +false, Token::False +let, Token::Let +const, Token::Const +if, Token::If +else, Token::Else +switch, Token::Switch +do, Token::Do +while, Token::While +until, Token::Until +loop, Token::Loop +for, Token::For +in, Token::In +!in, Token::NotIn +<, Token::LessThan +>, Token::GreaterThan +<=, Token::LessThanEqualsTo +>=, Token::GreaterThanEqualsTo +==, Token::EqualsTo +!=, Token::NotEqualsTo +!, Token::Bang +|, Token::Pipe +||, Token::Or +&, Token::Ampersand +&&, Token::And +continue, Token::Continue +break, Token::Break +return, Token::Return +throw, Token::Throw +try, Token::Try +catch, Token::Catch ++=, Token::PlusAssign +-=, Token::MinusAssign +*=, Token::MultiplyAssign +/=, Token::DivideAssign +<<=, Token::LeftShiftAssign +>>=, Token::RightShiftAssign +&=, Token::AndAssign +|=, Token::OrAssign +^=, Token::XOrAssign +<<, Token::LeftShift +>>, Token::RightShift +^, Token::XOr +%, Token::Modulo +%=, Token::ModuloAssign +**, Token::PowerOf +**=, Token::PowerOfAssign +fn, Token::Fn +private, Token::Private +import, Token::Import +export, Token::Export +as, Token::As diff --git a/tools/reserved.txt b/tools/reserved.txt index 4db0859a..4ae6d8d2 100644 --- a/tools/reserved.txt +++ b/tools/reserved.txt @@ -7,7 +7,7 @@ // // Generate the output table via: // ```bash -// gperf -t reserved.txt +// gperf reserved.txt // ``` // // Since GNU gperf does not produce Rust output, the ANSI-C output must be hand-edited and @@ -21,74 +21,88 @@ // * Copy the `wordlist` array into `RESERVED_LIST` with the following modifications: // - Remove the `#line` comments // - Change the entry wrapping `{ .. }` into tuples `( .. )` -// - Replace all entries `("")` by `("", false, false, false)` // - Feature flags can be incorporated directly into the output via the `cfg!` macro // +%global-table +%struct-type +%omit-struct-type +%define initializer-suffix ,false,false,false struct reserved; %% -"?.", cfg!(feature = "no_object"), false, false -"?[", cfg!(feature = "no_index"), false, false -"fn", cfg!(feature = "no_function"), false, false -"private", cfg!(feature = "no_function"), false, false -"import", cfg!(feature = "no_module"), false, false -"export", cfg!(feature = "no_module"), false, false -"as", cfg!(feature = "no_module"), false, false -"===", true, false, false -"!==", true, false, false -"->", true, false, false -"<-", true, false, false -"?", true, false, false -":=", true, false, false -":;", true, false, false -"~", true, false, false -"!.", true, false, false -"::<", true, false, false -"(*", true, false, false -"*)", true, false, false +# reserved under certain flags +# +?., cfg!(feature = no_object), false, false +?[, cfg!(feature = no_index), false, false +fn, cfg!(feature = no_function), false, false +private, cfg!(feature = no_function), false, false +import, cfg!(feature = no_module), false, false +export, cfg!(feature = no_module), false, false +as, cfg!(feature = no_module), false, false +# +# reserved symbols +# +===, true, false, false +!==, true, false, false +->, true, false, false +<-, true, false, false +?, true, false, false +:=, true, false, false +:;, true, false, false +~, true, false, false +!., true, false, false +::<, true, false, false +(*, true, false, false +*), true, false, false "#", true, false, false "#!", true, false, false -"@", true, false, false -"$", true, false, false -"++", true, false, false -"--", true, false, false -"...", true, false, false -"<|", true, false, false -"|>", true, false, false -"public", true, false, false -"protected", true, false, false -"super", true, false, false -"new", true, false, false -"use", true, false, false -"module", true, false, false -"package", true, false, false -"var", true, false, false -"static", true, false, false -"shared", true, false, false -"with", true, false, false -"is", true, false, false -"goto", true, false, false -"exit", true, false, false -"match", true, false, false -"case", true, false, false -"default", true, false, false -"void", true, false, false -"null", true, false, false -"nil", true, false, false -"spawn", true, false, false -"thread", true, false, false -"go", true, false, false -"sync", true, false, false -"async", true, false, false -"await", true, false, false -"yield", true, false, false -"print", true, true, false -"debug", true, true, false -"type_of", true, true, true -"eval", true, true, false -"Fn", true, true, false -"call", true, true, true -"curry", true, true, true -"this", true, false, false -"is_def_var", true, true, false -"is_def_fn", cfg!(not(feature = "no_function")), true, false -"is_shared", cfg!(not(feature = "no_closure")), true, true +@, true, false, false +$, true, false, false +++, true, false, false +--, true, false, false +..., true, false, false +<|, true, false, false +|>, true, false, false +# +# reserved keywords +# +public, true, false, false +protected, true, false, false +super, true, false, false +new, true, false, false +use, true, false, false +module, true, false, false +package, true, false, false +var, true, false, false +static, true, false, false +shared, true, false, false +with, true, false, false +is, true, false, false +goto, true, false, false +exit, true, false, false +match, true, false, false +case, true, false, false +default, true, false, false +void, true, false, false +null, true, false, false +nil, true, false, false +spawn, true, false, false +thread, true, false, false +go, true, false, false +sync, true, false, false +async, true, false, false +await, true, false, false +yield, true, false, false +# +# keyword functions +# +print, true, true, false +debug, true, true, false +type_of, true, true, true +eval, true, true, false +Fn, true, true, false +call, true, true, true +curry, true, true, true +this, true, false, false +is_def_var, true, true, false +is_def_fn, cfg!(not(feature = no_function)), true, false +is_shared, cfg!(not(feature = no_closure)), true, true From 5eaa4c52402c7f88c49f5947b896896753bd34d0 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 31 Mar 2023 10:41:55 +0800 Subject: [PATCH 12/17] Do not cache long strings and avoid caching one-hit wonders. --- src/parser.rs | 4 ++-- src/types/interner.rs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index d7518138..2300b402 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -261,7 +261,7 @@ impl<'e, 's> ParseState<'e, 's> { text: impl AsRef + Into, ) -> ImmutableString { self.interned_strings.get_with_mapper( - crate::engine::FN_GET, + b'g', |s| crate::engine::make_getter(s.as_ref()).into(), text, ) @@ -276,7 +276,7 @@ impl<'e, 's> ParseState<'e, 's> { text: impl AsRef + Into, ) -> ImmutableString { self.interned_strings.get_with_mapper( - crate::engine::FN_SET, + b's', |s| crate::engine::make_setter(s.as_ref()).into(), text, ) diff --git a/src/types/interner.rs b/src/types/interner.rs index 118950cf..22e06fef 100644 --- a/src/types/interner.rs +++ b/src/types/interner.rs @@ -62,7 +62,7 @@ impl StringsInterner { #[inline(always)] #[must_use] pub fn get + Into>(&mut self, text: S) -> ImmutableString { - self.get_with_mapper("", Into::into, text) + self.get_with_mapper(0, Into::into, text) } /// Get an identifier from a text string, adding it to the interner if necessary. @@ -70,19 +70,19 @@ impl StringsInterner { #[must_use] pub fn get_with_mapper>( &mut self, - category: &str, + category: u8, mapper: impl FnOnce(S) -> ImmutableString, text: S, ) -> ImmutableString { let key = text.as_ref(); let hasher = &mut get_hasher(); - category.hash(hasher); + hasher.write_u8(category); key.hash(hasher); let hash = hasher.finish(); - // Cache long strings only on the second try to avoid caching "one-hit wonders". - if key.len() > MAX_STRING_LEN && self.bloom_filter.is_absent_and_set(hash) { + // Do not cache long strings and avoid caching "one-hit wonders". + if key.len() > MAX_STRING_LEN || self.bloom_filter.is_absent_and_set(hash) { return mapper(text); } From df05f434607f89c64d57e8cee3a918d5c8f7f007 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 5 Apr 2023 23:15:55 +0800 Subject: [PATCH 13/17] Simplify match. --- src/ast/expr.rs | 12 ++++---- src/ast/stmt.rs | 4 +-- src/defer.rs | 14 ++++----- src/eval/chaining.rs | 71 +++++++++++++++++--------------------------- src/eval/expr.rs | 19 ++++++------ 5 files changed, 53 insertions(+), 67 deletions(-) diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 1896f0df..52d7ccd1 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -288,8 +288,8 @@ pub enum Expr { Unit(Position), /// Variable access - (optional long index, namespace, namespace hash, variable name), optional short index, position /// - /// The short index is [`u8`] which is used when the index is <= 255, which should be the vast - /// majority of cases (unless there are more than 255 variables defined!). + /// The short index is [`u8`] which is used when the index is <= 255, which should be + /// the vast majority of cases (unless there are more than 255 variables defined!). /// This is to avoid reading a pointer redirection during each variable access. Variable( Box<(Option, Namespace, u64, ImmutableString)>, @@ -315,15 +315,15 @@ pub enum Expr { /// /// ### Flags /// - /// [`NEGATED`][ASTFlags::NEGATED] = `?.` (`.` if unset) - /// [`BREAK`][ASTFlags::BREAK] = terminate the chain (recurse into the chain if unset) + /// * [`NEGATED`][ASTFlags::NEGATED] = `?.` (`.` if unset) + /// * [`BREAK`][ASTFlags::BREAK] = terminate the chain (recurse into the chain if unset) Dot(Box, ASTFlags, Position), /// lhs `[` rhs `]` /// /// ### Flags /// - /// [`NEGATED`][ASTFlags::NEGATED] = `?[` ... `]` (`[` ... `]` if unset) - /// [`BREAK`][ASTFlags::BREAK] = terminate the chain (recurse into the chain if unset) + /// * [`NEGATED`][ASTFlags::NEGATED] = `?[` ... `]` (`[` ... `]` if unset) + /// * [`BREAK`][ASTFlags::BREAK] = terminate the chain (recurse into the chain if unset) Index(Box, ASTFlags, Position), /// lhs `&&` rhs And(Box, Position), diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index 8487c4fb..65634092 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -663,7 +663,7 @@ pub enum Stmt { /// /// ### Flags /// - /// * [`NONE`][ASTFlags::NONE] = `while` + /// * [`NONE`][ASTFlags::NONE] = `while` /// * [`NEGATED`][ASTFlags::NEGATED] = `until` Do(Box, ASTFlags, Position), /// `for` `(` id `,` counter `)` `in` expr `{` stmt `}` @@ -672,7 +672,7 @@ pub enum Stmt { /// /// ### Flags /// - /// * [`EXPORTED`][ASTFlags::EXPORTED] = `export` + /// * [`EXPORTED`][ASTFlags::EXPORTED] = `export` /// * [`CONSTANT`][ASTFlags::CONSTANT] = `const` Var(Box<(Ident, Expr, Option)>, ASTFlags, Position), /// expr op`=` expr diff --git a/src/defer.rs b/src/defer.rs index f3947430..4bf0ff21 100644 --- a/src/defer.rs +++ b/src/defer.rs @@ -65,8 +65,8 @@ macro_rules! defer { /// Run custom restoration logic upon the end of scope. #[must_use] pub struct Deferred<'a, T: ?Sized, R: FnOnce(&mut T)> { - value: &'a mut T, - restore: Option, + lock: &'a mut T, + defer: Option, } impl<'a, T: ?Sized, R: FnOnce(&mut T)> Deferred<'a, T, R> { @@ -78,8 +78,8 @@ impl<'a, T: ?Sized, R: FnOnce(&mut T)> Deferred<'a, T, R> { #[inline(always)] pub fn lock(value: &'a mut T, restore: R) -> Self { Self { - value, - restore: Some(restore), + lock: value, + defer: Some(restore), } } } @@ -87,7 +87,7 @@ impl<'a, T: ?Sized, R: FnOnce(&mut T)> Deferred<'a, T, R> { impl<'a, T: ?Sized, R: FnOnce(&mut T)> Drop for Deferred<'a, T, R> { #[inline(always)] fn drop(&mut self) { - self.restore.take().unwrap()(self.value); + self.defer.take().unwrap()(self.lock); } } @@ -96,13 +96,13 @@ impl<'a, T: ?Sized, R: FnOnce(&mut T)> Deref for Deferred<'a, T, R> { #[inline(always)] fn deref(&self) -> &Self::Target { - self.value + self.lock } } impl<'a, T: ?Sized, R: FnOnce(&mut T)> DerefMut for Deferred<'a, T, R> { #[inline(always)] fn deref_mut(&mut self) -> &mut Self::Target { - self.value + self.lock } } diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 43570317..13e1433c 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -362,21 +362,14 @@ impl Engine { Expr::Property(..) if chain_type == ChainType::Dotting => (), #[cfg(not(feature = "no_object"))] Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"), - // Short-circuit for simple method call: {expr}.func() - #[cfg(not(feature = "no_object"))] - Expr::FnCall(x, ..) if chain_type == ChainType::Dotting && x.args.is_empty() => (), - // Short-circuit for method call with all literal arguments: {expr}.func(1, 2, 3) - #[cfg(not(feature = "no_object"))] - Expr::FnCall(x, ..) - if chain_type == ChainType::Dotting && x.args.iter().all(Expr::is_constant) => - { - idx_values.extend(x.args.iter().map(|expr| expr.get_literal_value().unwrap())) - } // Short-circuit for indexing with literal: {expr}[1] #[cfg(not(feature = "no_index"))] _ if chain_type == ChainType::Indexing && rhs.is_constant() => { idx_values.push(rhs.get_literal_value().unwrap()) } + // Short-circuit for simple method call: {expr}.func() + #[cfg(not(feature = "no_object"))] + Expr::MethodCall(x, ..) if chain_type == ChainType::Dotting && x.args.is_empty() => (), // All other patterns - evaluate the arguments chain _ => self.eval_dot_index_chain_arguments( global, @@ -439,27 +432,25 @@ impl Engine { ) -> RhaiResultOf<()> { self.track_operation(global, expr.position())?; - let chain_type = ChainType::from(parent); - - match expr { + match (expr, ChainType::from(parent)) { #[cfg(not(feature = "no_object"))] - Expr::MethodCall(x, ..) if chain_type == ChainType::Dotting && !x.is_qualified() => { + (Expr::MethodCall(x, ..), ChainType::Dotting) => { + debug_assert!( + !x.is_qualified(), + "function call in dot chain should not be namespace-qualified" + ); + for expr in &x.args { let arg_value = self.get_arg_value(global, caches, scope, this_ptr.as_deref_mut(), expr)?; idx_values.push(arg_value.0.flatten()); } } - #[cfg(not(feature = "no_object"))] - Expr::MethodCall(..) if chain_type == ChainType::Dotting => { - unreachable!("function call in dot chain should not be namespace-qualified") - } #[cfg(not(feature = "no_object"))] - Expr::Property(..) if chain_type == ChainType::Dotting => (), - Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"), + (Expr::Property(..), ChainType::Dotting) => (), - Expr::Index(x, ..) | Expr::Dot(x, ..) + (Expr::Index(x, ..), chain_type) | (Expr::Dot(x, ..), chain_type) if !parent.options().contains(ASTFlags::BREAK) => { let BinaryExpr { lhs, rhs, .. } = &**x; @@ -467,37 +458,34 @@ impl Engine { let mut _arg_values = FnArgsVec::new_const(); // Evaluate in left-to-right order - match lhs { + match (lhs, chain_type) { #[cfg(not(feature = "no_object"))] - Expr::Property(..) if chain_type == ChainType::Dotting => (), - Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"), + (Expr::Property(..), ChainType::Dotting) => (), #[cfg(not(feature = "no_object"))] - Expr::MethodCall(x, ..) - if chain_type == ChainType::Dotting && !x.is_qualified() => - { + (Expr::MethodCall(x, ..), ChainType::Dotting) => { + debug_assert!( + !x.is_qualified(), + "function call in dot chain should not be namespace-qualified" + ); + for expr in &x.args { let tp = this_ptr.as_deref_mut(); let arg_value = self.get_arg_value(global, caches, scope, tp, expr)?; _arg_values.push(arg_value.0.flatten()); } } - #[cfg(not(feature = "no_object"))] - Expr::MethodCall(..) if chain_type == ChainType::Dotting => { - unreachable!("function call in dot chain should not be namespace-qualified") - } - #[cfg(not(feature = "no_object"))] - expr if chain_type == ChainType::Dotting => { - unreachable!("invalid dot expression: {:?}", expr); - } #[cfg(not(feature = "no_index"))] - _ if chain_type == ChainType::Indexing => { + (_, ChainType::Indexing) => { _arg_values.push( self.eval_expr(global, caches, scope, this_ptr.as_deref_mut(), lhs)? .flatten(), ); } - expr => unreachable!("unknown chained expression: {:?}", expr), + #[allow(unreachable_patterns)] + (expr, chain_type) => { + unreachable!("unknown {:?} expression: {:?}", chain_type, expr) + } } // Push in reverse order @@ -508,16 +496,13 @@ impl Engine { idx_values.extend(_arg_values); } - #[cfg(not(feature = "no_object"))] - _ if chain_type == ChainType::Dotting => { - unreachable!("invalid dot expression: {:?}", expr); - } #[cfg(not(feature = "no_index"))] - _ if chain_type == ChainType::Indexing => idx_values.push( + (_, ChainType::Indexing) => idx_values.push( self.eval_expr(global, caches, scope, this_ptr, expr)? .flatten(), ), - _ => unreachable!("unknown chained expression: {:?}", expr), + #[allow(unreachable_patterns)] + (expr, chain_type) => unreachable!("unknown {:?} expression: {:?}", chain_type, expr), } Ok(()) diff --git a/src/eval/expr.rs b/src/eval/expr.rs index f8af1f35..f1555a3d 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -259,17 +259,18 @@ impl Engine { self.eval_fn_call_expr(global, caches, scope, this_ptr, x, *pos) } - Expr::Variable(x, index, var_pos) => { - if index.is_none() && x.0.is_none() && x.3 == KEYWORD_THIS { - this_ptr - .ok_or_else(|| ERR::ErrorUnboundThis(*var_pos).into()) - .cloned() - } else { - self.search_namespace(global, caches, scope, this_ptr, expr) - .map(Target::take_or_clone) - } + Expr::Variable(x, index, var_pos) + if index.is_none() && x.0.is_none() && x.3 == KEYWORD_THIS => + { + this_ptr + .ok_or_else(|| ERR::ErrorUnboundThis(*var_pos).into()) + .cloned() } + Expr::Variable(..) => self + .search_namespace(global, caches, scope, this_ptr, expr) + .map(Target::take_or_clone), + Expr::InterpolatedString(x, _) => { let mut concat = SmartString::new_const(); From 9de41e7559671762b4b99ff1f391025ff38cecba Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 5 Apr 2023 23:40:27 +0800 Subject: [PATCH 14/17] Further simplify matches. --- src/eval/chaining.rs | 98 ++++++++++++++++++++++---------------------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 13e1433c..28dacaa9 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -344,8 +344,6 @@ impl Engine { expr: &Expr, new_val: Option<(Dynamic, &OpAssignment)>, ) -> RhaiResult { - let chain_type = ChainType::from(expr); - let BinaryExpr { lhs, rhs } = match expr { #[cfg(not(feature = "no_index"))] Expr::Index(x, ..) => &**x, @@ -356,20 +354,22 @@ impl Engine { let idx_values = &mut FnArgsVec::new_const(); - match rhs { + match (rhs, ChainType::from(expr)) { // Short-circuit for simple property access: {expr}.prop #[cfg(not(feature = "no_object"))] - Expr::Property(..) if chain_type == ChainType::Dotting => (), + (Expr::Property(..), ChainType::Dotting) => (), #[cfg(not(feature = "no_object"))] - Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"), + (Expr::Property(..), ..) => { + unreachable!("unexpected Expr::Property for indexing") + } // Short-circuit for indexing with literal: {expr}[1] #[cfg(not(feature = "no_index"))] - _ if chain_type == ChainType::Indexing && rhs.is_constant() => { + (_, ChainType::Indexing) if rhs.is_constant() => { idx_values.push(rhs.get_literal_value().unwrap()) } // Short-circuit for simple method call: {expr}.func() #[cfg(not(feature = "no_object"))] - Expr::MethodCall(x, ..) if chain_type == ChainType::Dotting && x.args.is_empty() => (), + (Expr::MethodCall(x, ..), ChainType::Dotting) if x.args.is_empty() => (), // All other patterns - evaluate the arguments chain _ => self.eval_dot_index_chain_arguments( global, @@ -387,9 +387,9 @@ impl Engine { #[cfg(not(feature = "debugging"))] let scope2 = (); - match lhs { + match (lhs, new_val) { // id.??? or id[???] - Expr::Variable(.., var_pos) => { + (Expr::Variable(.., var_pos), new_val) => { self.track_operation(global, *var_pos)?; #[cfg(feature = "debugging")] @@ -402,9 +402,9 @@ impl Engine { ) } // {expr}.??? = ??? or {expr}[???] = ??? - _ if new_val.is_some() => unreachable!("cannot assign to an expression"), + (_, Some(..)) => unreachable!("cannot assign to an expression"), // {expr}.??? or {expr}[???] - lhs_expr => { + (lhs_expr, None) => { let value = self .eval_expr(global, caches, scope, this_ptr.as_deref_mut(), lhs_expr)? .flatten(); @@ -412,7 +412,7 @@ impl Engine { self.eval_dot_index_chain_raw( global, caches, scope2, this_ptr, lhs_expr, expr, obj_ptr, rhs, idx_values, - new_val, + None, ) } } @@ -437,7 +437,7 @@ impl Engine { (Expr::MethodCall(x, ..), ChainType::Dotting) => { debug_assert!( !x.is_qualified(), - "function call in dot chain should not be namespace-qualified" + "method call in dot chain should not be namespace-qualified" ); for expr in &x.args { @@ -450,7 +450,7 @@ impl Engine { #[cfg(not(feature = "no_object"))] (Expr::Property(..), ChainType::Dotting) => (), - (Expr::Index(x, ..), chain_type) | (Expr::Dot(x, ..), chain_type) + (Expr::Index(x, ..) | Expr::Dot(x, ..), chain_type) if !parent.options().contains(ASTFlags::BREAK) => { let BinaryExpr { lhs, rhs, .. } = &**x; @@ -466,7 +466,7 @@ impl Engine { (Expr::MethodCall(x, ..), ChainType::Dotting) => { debug_assert!( !x.is_qualified(), - "function call in dot chain should not be namespace-qualified" + "method call in dot chain should not be namespace-qualified" ); for expr in &x.args { @@ -540,9 +540,9 @@ impl Engine { let pos = rhs.start_position(); - match rhs { + match (rhs, new_val) { // xxx[idx].expr... | xxx[idx][expr]... - Expr::Dot(x, ..) | Expr::Index(x, ..) + (Expr::Dot(x, ..) | Expr::Index(x, ..), new_val) if !parent.options().contains(ASTFlags::BREAK) => { #[cfg(feature = "debugging")] @@ -589,11 +589,10 @@ impl Engine { Ok(result) } // xxx[rhs] op= new_val - _ if new_val.is_some() => { + (_, Some((new_val, op_info))) => { #[cfg(feature = "debugging")] self.run_debugger(global, caches, scope, this_ptr, parent)?; - let (new_val, op_info) = new_val.expect("`Some`"); let idx_val = &mut idx_values.pop().unwrap(); let idx = &mut idx_val.clone(); @@ -646,7 +645,7 @@ impl Engine { Ok((Dynamic::UNIT, true)) } // xxx[rhs] - _ => { + (_, None) => { #[cfg(feature = "debugging")] self.run_debugger(global, caches, scope, this_ptr, parent)?; @@ -667,9 +666,18 @@ impl Engine { return Ok((Dynamic::UNIT, false)); } - match rhs { + match (rhs, new_val, target.is_map()) { + // xxx.fn_name(...) = ??? + (Expr::MethodCall(..), Some(..), ..) => { + unreachable!("method call cannot be assigned to") + } // xxx.fn_name(arg_expr_list) - Expr::MethodCall(x, pos) if !x.is_qualified() && new_val.is_none() => { + (Expr::MethodCall(x, pos), None, ..) => { + debug_assert!( + !x.is_qualified(), + "method call in dot chain should not be namespace-qualified" + ); + #[cfg(feature = "debugging")] let reset = self.run_debugger_with_reset(global, caches, scope, this_ptr, rhs)?; @@ -690,21 +698,12 @@ impl Engine { global, caches, name, *hashes, target, call_args, arg1_pos, *pos, ) } - // xxx.fn_name(...) = ??? - Expr::MethodCall(..) if new_val.is_some() => { - unreachable!("method call cannot be assigned to") - } - // xxx.module::fn_name(...) - syntax error - Expr::MethodCall(..) => { - unreachable!("function call in dot chain should not be namespace-qualified") - } // {xxx:map}.id op= ??? - Expr::Property(x, pos) if new_val.is_some() && target.is_map() => { + (Expr::Property(x, pos), Some((new_val, op_info)), true) => { #[cfg(feature = "debugging")] self.run_debugger(global, caches, scope, this_ptr, rhs)?; let index = &mut x.2.clone().into(); - let (new_val, op_info) = new_val.expect("`Some`"); { let val_target = &mut self.get_indexed_mut( global, caches, target, index, *pos, op_pos, true, false, @@ -717,7 +716,7 @@ impl Engine { Ok((Dynamic::UNIT, true)) } // {xxx:map}.id - Expr::Property(x, pos) if target.is_map() => { + (Expr::Property(x, pos), None, true) => { #[cfg(feature = "debugging")] self.run_debugger(global, caches, scope, this_ptr, rhs)?; @@ -728,12 +727,11 @@ impl Engine { Ok((val.take_or_clone(), false)) } // xxx.id op= ??? - Expr::Property(x, pos) if new_val.is_some() => { + (Expr::Property(x, pos), Some((mut new_val, op_info)), false) => { #[cfg(feature = "debugging")] self.run_debugger(global, caches, scope, this_ptr, rhs)?; let ((getter, hash_get), (setter, hash_set), name) = &**x; - let (mut new_val, op_info) = new_val.expect("`Some`"); if op_info.is_op_assignment() { let args = &mut [target.as_mut()]; @@ -793,7 +791,7 @@ impl Engine { }) } // xxx.id - Expr::Property(x, pos) => { + (Expr::Property(x, pos), None, false) => { #[cfg(feature = "debugging")] self.run_debugger(global, caches, scope, this_ptr, rhs)?; @@ -822,7 +820,7 @@ impl Engine { ) } // {xxx:map}.sub_lhs[expr] | {xxx:map}.sub_lhs.expr - Expr::Index(x, ..) | Expr::Dot(x, ..) if target.is_map() => { + (Expr::Index(x, ..) | Expr::Dot(x, ..), new_val, true) => { let _node = &x.lhs; let mut _this_ptr = this_ptr; let _tp = _this_ptr.as_deref_mut(); @@ -838,7 +836,12 @@ impl Engine { )? } // {xxx:map}.fn_name(arg_expr_list)[expr] | {xxx:map}.fn_name(arg_expr_list).expr - Expr::MethodCall(ref x, pos) if !x.is_qualified() => { + Expr::MethodCall(ref x, pos) => { + debug_assert!( + !x.is_qualified(), + "method call in dot chain should not be namespace-qualified" + ); + #[cfg(feature = "debugging")] let reset = self .run_debugger_with_reset(global, caches, scope, _tp, _node)?; @@ -861,10 +864,6 @@ impl Engine { .0 .into() } - // {xxx:map}.module::fn_name(...) - syntax error - Expr::MethodCall(..) => unreachable!( - "function call in dot chain should not be namespace-qualified" - ), // Others - syntax error ref expr => unreachable!("invalid dot expression: {:?}", expr), }; @@ -875,7 +874,7 @@ impl Engine { ) } // xxx.sub_lhs[expr] | xxx.sub_lhs.expr - Expr::Index(x, ..) | Expr::Dot(x, ..) => { + (Expr::Index(x, ..) | Expr::Dot(x, ..), new_val, ..) => { let _node = &x.lhs; let mut _this_ptr = this_ptr; let _tp = _this_ptr.as_deref_mut(); @@ -956,7 +955,12 @@ impl Engine { Ok((result, may_be_changed)) } // xxx.fn_name(arg_expr_list)[expr] | xxx.fn_name(arg_expr_list).expr - Expr::MethodCall(ref f, pos) if !f.is_qualified() => { + Expr::MethodCall(ref f, pos) => { + debug_assert!( + !f.is_qualified(), + "method call in dot chain should not be namespace-qualified" + ); + let val = { #[cfg(feature = "debugging")] let reset = self.run_debugger_with_reset( @@ -988,16 +992,12 @@ impl Engine { idx_values, new_val, ) } - // xxx.module::fn_name(...) - syntax error - Expr::MethodCall(..) => unreachable!( - "function call in dot chain should not be namespace-qualified" - ), // Others - syntax error ref expr => unreachable!("invalid dot expression: {:?}", expr), } } // Syntax error - expr => unreachable!("invalid chaining expression: {:?}", expr), + (expr, ..) => unreachable!("invalid chaining expression: {:?}", expr), } } } From 98f227d640dbe6f038d8c02cb7f26b87b1bbff24 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 7 Apr 2023 16:14:26 +0800 Subject: [PATCH 15/17] Fix doc test. --- src/ast/ast.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ast/ast.rs b/src/ast/ast.rs index 139f5634..6734a446 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -839,6 +839,8 @@ impl AST { /// /// { // <- new block scope /// const Z = 0; // <- literal constant not at top-level + /// + /// print(Z); // make sure the block is not optimized away /// } /// ")?; /// From 8369a9bf635f56da0a9016af76754b65c8446b81 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 9 Apr 2023 16:31:06 +0800 Subject: [PATCH 16/17] Fix optimizer bug for closures. --- CHANGELOG.md | 1 + src/optimizer.rs | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8598a79c..7627368d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Buf fixes ---------- * `is_shared` is a reserved keyword and is now handled properly (e.g. it cannot be the target of a function pointer). +* Re-optimizing an AST via `optimize_ast` with constants now works correctly for closures. Previously the hidden `Share` nodes are not removed and causes variable-not-found errors during runtime if the constants are not available in the scope. New features ------------ diff --git a/src/optimizer.rs b/src/optimizer.rs index ea8f00d2..f4447556 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -840,6 +840,20 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b // return expr; Stmt::Return(Some(ref mut expr), ..) => optimize_expr(expr, state, false), + // Share nothing + Stmt::Share(x) if x.is_empty() => { + state.set_dirty(); + *stmt = Stmt::Noop(Position::NONE); + } + // Share constants + Stmt::Share(x) => { + let len = x.len(); + x.retain(|(v, _, _)| !state.find_constant(v).is_some()); + if x.len() != len { + state.set_dirty(); + } + } + // All other statements - skip _ => (), } From a82bb7b2efc6de1b938a2338fdfa3b22d5630dad Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 9 Apr 2023 16:38:19 +0800 Subject: [PATCH 17/17] Fix build --- src/optimizer.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/optimizer.rs b/src/optimizer.rs index f4447556..124e7d95 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -841,11 +841,13 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b Stmt::Return(Some(ref mut expr), ..) => optimize_expr(expr, state, false), // Share nothing + #[cfg(not(feature = "no_closure"))] Stmt::Share(x) if x.is_empty() => { state.set_dirty(); *stmt = Stmt::Noop(Position::NONE); } // Share constants + #[cfg(not(feature = "no_closure"))] Stmt::Share(x) => { let len = x.len(); x.retain(|(v, _, _)| !state.find_constant(v).is_some());