From e50fcc385f3271456729d5db518173730e434be0 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 8 May 2020 22:38:56 +0800 Subject: [PATCH] Pre-calculate index for module-qualified calls. --- README.md | 2 +- src/engine.rs | 45 ++++++++++++++++++++++++++------------------- src/module.rs | 40 +++++++++++++++++++++++++--------------- src/parser.rs | 10 ++++++---- tests/modules.rs | 8 +++++--- 5 files changed, 63 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index bb5f3597..5a4ebd16 100644 --- a/README.md +++ b/README.md @@ -2158,7 +2158,7 @@ let ast = engine.compile(r#" "#)?; // Convert the 'AST' into a module, using the 'Engine' to evaluate it first -let module = Module::eval_ast_as_new(&ast, &engine)?; +let module = Module::eval_ast_as_new(Scope::new(), &ast, &engine)?; // 'module' now can be loaded into a custom 'Scope' for future use. It contains: // - sub-module: 'extra' diff --git a/src/engine.rs b/src/engine.rs index d026ecb0..0832152b 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -434,7 +434,7 @@ fn default_print(s: &str) { fn search_scope<'a>( scope: &'a mut Scope, name: &str, - #[cfg(not(feature = "no_module"))] mut modules: &Option>, + #[cfg(not(feature = "no_module"))] modules: &Option>, #[cfg(feature = "no_module")] _: &Option, index: Option, pos: Position, @@ -444,7 +444,7 @@ fn search_scope<'a>( if let Some(modules) = modules { let (id, root_pos) = modules.get(0); - let module = if let Some(index) = index { + let module = if let Some(index) = modules.index() { scope .get_mut(scope.len() - index.get()) .0 @@ -1369,9 +1369,17 @@ impl Engine { let (id, root_pos) = modules.get(0); // First module - let module = scope.find_module(id).ok_or_else(|| { - Box::new(EvalAltResult::ErrorModuleNotFound(id.into(), *root_pos)) - })?; + let module = if let Some(index) = modules.index() { + scope + .get_mut(scope.len() - index.get()) + .0 + .downcast_mut::() + .unwrap() + } else { + scope.find_module(id).ok_or_else(|| { + Box::new(EvalAltResult::ErrorModuleNotFound(id.into(), *root_pos)) + })? + }; // First search in script-defined functions (can override built-in) if let Some(fn_def) = module.get_qualified_scripted_fn(modules.key()) { @@ -1623,9 +1631,8 @@ impl Engine { { if let Some(resolver) = self.module_resolver.as_ref() { // Use an empty scope to create a module - let mut mod_scope = Scope::new(); let module = - resolver.resolve(self, mod_scope, &path, expr.position())?; + resolver.resolve(self, Scope::new(), &path, expr.position())?; // TODO - avoid copying module name in inner block? let mod_name = name.as_ref().clone(); @@ -1649,18 +1656,18 @@ impl Engine { let mut found = false; // Mark scope variables as public - match scope.get_index(id) { - Some((index, ScopeEntryType::Normal)) - | Some((index, ScopeEntryType::Constant)) => { - let alias = rename - .as_ref() - .map(|(n, _)| n.clone()) - .unwrap_or_else(|| id.clone()); - scope.set_entry_alias(index, alias); - found = true; - } - Some((_, ScopeEntryType::Module)) => unreachable!(), - _ => (), + if let Some(index) = scope + .get_index(id) + .map(|(i, _)| i) + .or_else(|| scope.get_module_index(id)) + { + let alias = rename + .as_ref() + .map(|(n, _)| n.clone()) + .unwrap_or_else(|| id.clone()); + + scope.set_entry_alias(index, alias); + found = true; } if !found { diff --git a/src/module.rs b/src/module.rs index 4eac6dfa..10636ee8 100644 --- a/src/module.rs +++ b/src/module.rs @@ -17,6 +17,7 @@ use crate::stdlib::{ fmt, iter::{empty, repeat}, mem, + num::NonZeroUsize, ops::{Deref, DerefMut}, rc::Rc, string::{String, ToString}, @@ -568,12 +569,11 @@ impl Module { /// /// ``` /// # fn main() -> Result<(), Box> { - /// use rhai::{Engine, Module}; + /// use rhai::{Engine, Module, Scope}; /// /// let engine = Engine::new(); - /// let mut scope = Scope::new(); - /// let ast = engine.compile("let answer = 42;")?; - /// let module = Module::eval_ast_as_new(scope, &ast, &engine)?; + /// let ast = engine.compile("let answer = 42; export answer;")?; + /// let module = Module::eval_ast_as_new(Scope::new(), &ast, &engine)?; /// assert!(module.contains_var("answer")); /// assert_eq!(module.get_var_value::("answer").unwrap(), 42); /// # Ok(()) @@ -599,14 +599,14 @@ impl Module { ScopeEntryType::Normal | ScopeEntryType::Constant if alias.is_some() => { module.variables.insert(*alias.unwrap(), value); } - // Variables with no alias are private and not exported - ScopeEntryType::Normal | ScopeEntryType::Constant => (), // Modules left in the scope become sub-modules - ScopeEntryType::Module => { + ScopeEntryType::Module if alias.is_some() => { module .modules - .insert(name.into_owned(), value.cast::()); + .insert(*alias.unwrap(), value.cast::()); } + // Variables and modules with no alias are private and not exported + _ => (), } }, ); @@ -830,14 +830,18 @@ mod file { /// A `StaticVec` is used because most module-level access contains only one level, /// and it is wasteful to always allocate a `Vec` with one element. #[derive(Clone, Hash, Default)] -pub struct ModuleRef(StaticVec<(String, Position)>, u64); +pub struct ModuleRef(StaticVec<(String, Position)>, Option, u64); impl fmt::Debug for ModuleRef { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Debug::fmt(&self.0, f)?; - if self.1 > 0 { - write!(f, " -> {:0>16x}", self.1) + if self.2 > 0 { + if let Some(index) = self.1 { + write!(f, " -> {},{:0>16x}", index, self.2) + } else { + write!(f, " -> {:0>16x}", self.2) + } } else { Ok(()) } @@ -869,16 +873,22 @@ impl fmt::Display for ModuleRef { impl From> for ModuleRef { fn from(modules: StaticVec<(String, Position)>) -> Self { - Self(modules, 0) + Self(modules, None, 0) } } impl ModuleRef { - pub fn key(&self) -> u64 { + pub(crate) fn key(&self) -> u64 { + self.2 + } + pub(crate) fn set_key(&mut self, key: u64) { + self.2 = key + } + pub(crate) fn index(&self) -> Option { self.1 } - pub fn set_key(&mut self, key: u64) { - self.1 = key + pub(crate) fn set_index(&mut self, index: Option) { + self.1 = index } } diff --git a/src/parser.rs b/src/parser.rs index ad93943c..274854b4 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -719,6 +719,7 @@ fn parse_call_expr<'a>( // Calculate hash let hash = calc_fn_hash(modules.iter().map(|(m, _)| m.as_str()), &id, empty()); modules.set_key(hash); + modules.set_index(stack.find_module(&modules.get(0).0)); } } return Ok(Expr::FnCall( @@ -751,6 +752,7 @@ fn parse_call_expr<'a>( repeat(EMPTY_TYPE_ID()).take(args.len()), ); modules.set_key(hash); + modules.set_index(stack.find_module(&modules.get(0).0)); } } return Ok(Expr::FnCall( @@ -1147,14 +1149,12 @@ fn parse_primary<'a>( } // module access #[cfg(not(feature = "no_module"))] - (Expr::Variable(id, mut modules, mut index, pos), Token::DoubleColon) => { + (Expr::Variable(id, mut modules, index, pos), Token::DoubleColon) => { match input.next().unwrap() { (Token::Identifier(id2), pos2) => { if let Some(ref mut modules) = modules { modules.push((*id, pos)); } else { - index = stack.find_module(id.as_ref()); - let mut m: ModuleRef = Default::default(); m.push((*id, pos)); modules = Some(Box::new(m)); @@ -1181,6 +1181,7 @@ fn parse_primary<'a>( Expr::Variable(id, Some(modules), _, _) => { let hash = calc_fn_hash(modules.iter().map(|(v, _)| v.as_str()), id, empty()); modules.set_key(hash); + modules.set_index(stack.find_module(&modules.get(0).0)); } _ => (), } @@ -2131,7 +2132,8 @@ fn parse_stmt<'a>( Token::LeftBrace => parse_block(input, stack, breakable, allow_stmt_expr), // fn ... - Token::Fn => Err(PERR::WrongFnDefinition.into_err(*pos)), + Token::Fn if !is_global => Err(PERR::WrongFnDefinition.into_err(*pos)), + Token::Fn => unreachable!(), Token::If => parse_if(input, stack, breakable, allow_stmt_expr), Token::While => parse_while(input, stack, allow_stmt_expr), diff --git a/tests/modules.rs b/tests/modules.rs index e7475c5c..4876e9fb 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -114,23 +114,25 @@ fn test_module_from_ast() -> Result<(), Box> { // Final variable values become constant module variable values foo = calc(foo); hello = "hello, " + foo + " worlds!"; + + export x as abc, foo, hello, extra as foobar; "#, )?; - let module = Module::eval_ast_as_new(&ast, &engine)?; + let module = Module::eval_ast_as_new(Scope::new(), &ast, &engine)?; let mut scope = Scope::new(); scope.push_module("testing", module); assert_eq!( - engine.eval_expression_with_scope::(&mut scope, "testing::x")?, + engine.eval_expression_with_scope::(&mut scope, "testing::abc")?, 123 ); assert_eq!( engine.eval_expression_with_scope::(&mut scope, "testing::foo")?, 42 ); - assert!(engine.eval_expression_with_scope::(&mut scope, "testing::extra::foo")?); + assert!(engine.eval_expression_with_scope::(&mut scope, "testing::foobar::foo")?); assert_eq!( engine.eval_expression_with_scope::(&mut scope, "testing::hello")?, "hello, 42 worlds!"