From 5c80157e7a346786a635a16b9c50905375b4751f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 25 Aug 2022 22:17:01 +0800 Subject: [PATCH 1/5] Store path in module id. --- CHANGELOG.md | 1 + src/api/compile.rs | 2 +- src/module/resolvers/stat.rs | 8 +++++++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1e53315..421f305a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Enhancements ------------ * `is_empty` method is added to arrays, BLOB's, object maps, strings and ranges. +* `StaticModuleResolver` now stores the path in the module's `id` field. Version 1.9.0 diff --git a/src/api/compile.rs b/src/api/compile.rs index 73a3bf21..5322dbeb 100644 --- a/src/api/compile.rs +++ b/src/api/compile.rs @@ -113,7 +113,7 @@ impl Engine { }); } - let mut ast = self.compile_scripts_with_scope(scope, &[script])?; + let mut ast = self.compile_with_scope(scope, script)?; let mut resolver = StaticModuleResolver::new(); let mut imports = BTreeSet::new(); diff --git a/src/module/resolvers/stat.rs b/src/module/resolvers/stat.rs index b2d418c7..4019fc6d 100644 --- a/src/module/resolvers/stat.rs +++ b/src/module/resolvers/stat.rs @@ -54,8 +54,14 @@ impl StaticModuleResolver { /// Add a [module][Module] keyed by its path. #[inline] pub fn insert(&mut self, path: impl Into, mut module: Module) { + let path = path.into(); + + if module.id().is_none() { + module.set_id(path.clone()); + } + module.build_index(); - self.0.insert(path.into(), module.into()); + self.0.insert(path, module.into()); } /// Remove a [module][Module] given its path. #[inline(always)] From b36f746dbe1fe9c9e68d417327e86963b9794c76 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 25 Aug 2022 22:25:41 +0800 Subject: [PATCH 2/5] Add Engine::module_resolver. --- CHANGELOG.md | 1 + src/api/mod.rs | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 421f305a..7f17154f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Enhancements * `is_empty` method is added to arrays, BLOB's, object maps, strings and ranges. * `StaticModuleResolver` now stores the path in the module's `id` field. +* `Engine::module_resolver` is added to grant access to the `Engine`'s module resolver. Version 1.9.0 diff --git a/src/api/mod.rs b/src/api/mod.rs index b5120787..7f128243 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -70,6 +70,15 @@ pub mod default_limits { } impl Engine { + /// The module resolution service used by the [`Engine`]. + /// + /// Not available under `no_module`. + #[cfg(not(feature = "no_module"))] + #[inline(always)] + pub fn module_resolver(&self) -> &dyn crate::ModuleResolver { + &*self.module_resolver + } + /// Set the module resolution service used by the [`Engine`]. /// /// Not available under `no_module`. From 296d5c054c9b6e2e262a0b2971e4a1c556d50389 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 26 Aug 2022 11:23:16 +0800 Subject: [PATCH 3/5] Use simple optimization for rhai-run. --- CHANGELOG.md | 1 + src/bin/rhai-run.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f17154f..d936b051 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Bug fixes * API for registering property getters/setters and indexers to an `Engine` now works with functions that take a first parameter of `NativeCallContext`. * Missing API function `Module::set_getter_setter_fn` is added. +* To avoid subtle errors, simple optimization is used for `rhai-run`; previous it was full optimization. Deprecated API -------------- diff --git a/src/bin/rhai-run.rs b/src/bin/rhai-run.rs index ec4816d6..e87e99b4 100644 --- a/src/bin/rhai-run.rs +++ b/src/bin/rhai-run.rs @@ -51,7 +51,7 @@ fn main() { let mut engine = Engine::new(); #[cfg(not(feature = "no_optimize"))] - engine.set_optimization_level(rhai::OptimizationLevel::Full); + engine.set_optimization_level(rhai::OptimizationLevel::Simple); let mut f = match File::open(&filename) { Err(err) => { From 204284f4f716114c27280316474c4f88876cd661 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 26 Aug 2022 16:20:23 +0800 Subject: [PATCH 4/5] Add test to recreate NativeCallContext. --- src/func/native.rs | 10 +++--- src/module/mod.rs | 6 ---- tests/modules.rs | 82 ++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 81 insertions(+), 17 deletions(-) diff --git a/src/func/native.rs b/src/func/native.rs index 26801793..851f52e7 100644 --- a/src/func/native.rs +++ b/src/func/native.rs @@ -227,7 +227,7 @@ impl<'a> NativeCallContext<'a> { #[cfg(not(feature = "no_module"))] #[inline] pub fn iter_imports(&self) -> impl Iterator { - self.global.iter().flat_map(|&m| m.iter_imports()) + self.global.iter().flat_map(|&g| g.iter_imports()) } /// Get an iterator over the current set of modules imported via `import` statements in reverse order. #[cfg(not(feature = "no_module"))] @@ -236,7 +236,7 @@ impl<'a> NativeCallContext<'a> { pub(crate) fn iter_imports_raw( &self, ) -> impl Iterator)> { - self.global.iter().flat_map(|&m| m.iter_imports_raw()) + self.global.iter().flat_map(|&g| g.iter_imports_raw()) } /// _(internals)_ The current [`GlobalRuntimeState`], if any. /// Exported under the `internals` feature only. @@ -249,12 +249,12 @@ impl<'a> NativeCallContext<'a> { self.global } /// Get an iterator over the namespaces containing definitions of all script-defined functions - /// in reverse order. + /// in reverse order (i.e. parent namespaces are iterated after child namespaces). #[inline] pub fn iter_namespaces(&self) -> impl Iterator { - self.lib.iter().rev().copied() + self.lib.iter().copied() } - /// _(internals)_ The current set of namespaces containing definitions of all script-defined functions. + /// _(internals)_ The current stack of namespaces containing definitions of all script-defined functions. /// Exported under the `internals` feature only. #[cfg(feature = "internals")] #[inline(always)] diff --git a/src/module/mod.rs b/src/module/mod.rs index 5a11eae1..805c821a 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1960,9 +1960,6 @@ impl Module { /// The entire [`AST`][crate::AST] is encapsulated into each function, allowing functions to /// cross-call each other. /// - /// Functions in the global namespace, plus all functions defined in the [`Module`], are - /// _merged_ into a _unified_ namespace. Therefore, all functions will be found. - /// /// # Example /// /// ``` @@ -1993,9 +1990,6 @@ impl Module { /// The entire [`AST`][crate::AST] is encapsulated into each function, allowing functions to /// cross-call each other. /// - /// Functions in the global namespace, plus all functions defined in the [`Module`], are - /// _merged_ into a _unified_ namespace. Therefore, all functions will be found. - /// /// # WARNING - Low Level API /// /// This function is very low level. diff --git a/tests/modules.rs b/tests/modules.rs index 87bc2b76..77e60b2b 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -1,8 +1,8 @@ #![cfg(not(feature = "no_module"))] use rhai::{ module_resolvers::{DummyModuleResolver, StaticModuleResolver}, - Dynamic, Engine, EvalAltResult, FnNamespace, ImmutableString, Module, ParseError, - ParseErrorType, Scope, INT, + Dynamic, Engine, EvalAltResult, FnNamespace, FnPtr, ImmutableString, Module, NativeCallContext, + ParseError, ParseErrorType, Scope, Shared, INT, }; #[test] @@ -459,10 +459,10 @@ fn test_module_str() -> Result<(), Box> { #[cfg(not(feature = "no_function"))] #[test] fn test_module_ast_namespace() -> Result<(), Box> { - let script = r#" + let script = " fn foo(x) { x + 1 } fn bar(x) { foo(x) } - "#; + "; let mut engine = Engine::new(); @@ -499,11 +499,11 @@ fn test_module_ast_namespace() -> Result<(), Box> { fn test_module_ast_namespace2() -> Result<(), Box> { use rhai::{Engine, Module, Scope}; - const MODULE_TEXT: &str = r#" + const MODULE_TEXT: &str = " fn run_function(function) { call(function) } - "#; + "; const SCRIPT: &str = r#" import "test_module" as test; @@ -527,6 +527,76 @@ fn test_module_ast_namespace2() -> Result<(), Box> { Ok(()) } +#[cfg(not(feature = "no_function"))] +#[cfg(feature = "internals")] +#[test] +fn test_module_context() -> Result<(), Box> { + let script = "fn bar() { calc(|x| x + 1) }"; + + let mut engine = Engine::new(); + + let ast = engine.compile(script)?; + + let module = Module::eval_ast_as_new(Scope::new(), &ast, &engine)?; + + let mut resolver = StaticModuleResolver::new(); + resolver.insert("testing", module); + engine.set_module_resolver(resolver); + + engine.register_fn( + "calc", + |context: NativeCallContext, fp: FnPtr| -> Result> { + // Store fields for later use + let engine = context.engine(); + let fn_name = context.fn_name().to_string(); + let source = context.source().map(|s| s.to_string()); + let global = context.global_runtime_state().unwrap().clone(); + let pos = context.position(); + let call_level = context.call_level(); + + // Store the paths of the stack of call modules up to this point + let modules_list: Vec = context + .iter_namespaces() + .map(|m| m.id().unwrap_or("testing")) + .filter(|id| !id.is_empty()) + .map(|id| id.to_string()) + .collect(); + + // Recreate the 'NativeCallContext' - requires the 'internals' feature + let mut libraries = Vec::>::new(); + + for path in modules_list { + // Recreate the stack of call modules by resolving each path with + // the module resolver. + let module = engine.module_resolver().resolve(engine, None, &path, pos)?; + + libraries.push(module); + } + + let lib: Vec<&Module> = libraries.iter().map(|m| m.as_ref()).collect(); + + let new_context = NativeCallContext::new_with_all_fields( + engine, + &fn_name, + source.as_ref().map(|s| s.as_str()), + &global, + &lib, + pos, + call_level, + ); + + fp.call_within_context(&new_context, (41 as INT,)) + }, + ); + + assert_eq!( + engine.eval::(r#"import "testing" as t; t::bar()"#)?, + 42 + ); + + Ok(()) +} + #[test] fn test_module_file() -> Result<(), Box> { let engine = Engine::new(); From d80184ba14f9a4b1c000d5989a5d4dfa7449d806 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 26 Aug 2022 23:10:58 +0800 Subject: [PATCH 5/5] Allow if-expressions and switch-expressions in Engine::eval_expression. --- CHANGELOG.md | 5 +++++ src/parser.rs | 40 +++++++++++++++++++++++++++++++++++----- tests/expressions.rs | 41 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 80 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d936b051..115f189d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,11 @@ New features * For very special needs, the ability to register fallible type iterators is added. +### Expressions + +* `if`-expressions are allowed in `Engine::eval_expression` and `Engine::compile_expression` provided that both statement blocks each contain at most a single expression. +* `switch`-expressions are allowed in `Engine::eval_expression` and `Engine::compile_expression` provided that match actions are expressions only. + Enhancements ------------ diff --git a/src/parser.rs b/src/parser.rs index 48a8a97b..8498b90d 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -291,6 +291,8 @@ struct ParseSettings { in_closure: bool, /// Is the construct being parsed located inside a breakable loop? is_breakable: bool, + /// Allow statements in blocks? + allow_statements: bool, /// Language options in effect (overrides Engine options). options: LangOptions, /// Current expression nesting level. @@ -1193,12 +1195,21 @@ impl Engine { } }; - let stmt = self.parse_stmt(input, state, lib, settings.level_up())?; - let need_comma = !stmt.is_self_terminated(); + let (action_expr, need_comma) = if settings.allow_statements { + let stmt = self.parse_stmt(input, state, lib, settings.level_up())?; + let need_comma = !stmt.is_self_terminated(); + + let stmt_block: StmtBlock = stmt.into(); + (Expr::Stmt(stmt_block.into()), need_comma) + } else { + ( + self.parse_expr(input, state, lib, settings.level_up())?, + true, + ) + }; let has_condition = !matches!(condition, Expr::BoolConstant(true, ..)); - let stmt_block: StmtBlock = stmt.into(); - expressions.push((condition, Expr::Stmt(stmt_block.into())).into()); + expressions.push((condition, action_expr).into()); let index = expressions.len() - 1; if case_expr_list.is_empty() { @@ -3076,6 +3087,22 @@ impl Engine { let mut statements = StaticVec::new_const(); + if !settings.allow_statements { + let stmt = self.parse_expr_stmt(input, state, lib, settings.level_up())?; + statements.push(stmt); + + // Must end with } + return match input.next().expect(NEVER_ENDS) { + (Token::RightBrace, pos) => Ok((statements, settings.pos, pos).into()), + (Token::LexError(err), pos) => Err(err.into_err(pos)), + (.., pos) => Err(PERR::MissingToken( + Token::LeftBrace.into(), + "to start a statement block".into(), + ) + .into_err(pos)), + }; + } + let prev_entry_stack_len = state.block_stack_len; state.block_stack_len = state.stack.len(); @@ -3290,6 +3317,7 @@ impl Engine { #[cfg(not(feature = "no_closure"))] in_closure: false, is_breakable: false, + allow_statements: true, level: 0, options, pos, @@ -3761,7 +3789,7 @@ impl Engine { let mut functions = BTreeMap::new(); let mut options = self.options; - options.remove(LangOptions::IF_EXPR | LangOptions::SWITCH_EXPR | LangOptions::STMT_EXPR); + options.remove(LangOptions::STMT_EXPR); #[cfg(not(feature = "no_function"))] options.remove(LangOptions::ANON_FN); @@ -3773,6 +3801,7 @@ impl Engine { #[cfg(not(feature = "no_closure"))] in_closure: false, is_breakable: false, + allow_statements: false, level: 0, options, pos: Position::NONE, @@ -3828,6 +3857,7 @@ impl Engine { #[cfg(not(feature = "no_closure"))] in_closure: false, is_breakable: false, + allow_statements: true, options: self.options, level: 0, pos: Position::NONE, diff --git a/tests/expressions.rs b/tests/expressions.rs index 6c0f16b1..4d0b4888 100644 --- a/tests/expressions.rs +++ b/tests/expressions.rs @@ -12,8 +12,47 @@ fn test_expressions() -> Result<(), Box> { engine.eval_expression_with_scope::(&mut scope, "2 + (x + 10) * 2")?, 42 ); + assert_eq!( + engine.eval_expression_with_scope::(&mut scope, "if x > 0 { 42 } else { 123 }")?, + 42 + ); assert!(engine - .eval_expression_with_scope::(&mut scope, "if x > 0 { 42 } else { 123 }") + .eval_expression_with_scope::(&mut scope, "if x > 0 { let y = 42; y } else { 123 }") + .is_err()); + assert!(engine + .eval_expression_with_scope::(&mut scope, "if x > 0 { 42 } else { let y = 123; y }") + .is_err()); + assert!(engine + .eval_expression_with_scope::(&mut scope, "if x > 0 { 42 } else {}") + .is_err()); + + assert_eq!( + engine.eval_expression_with_scope::( + &mut scope, + " + switch x { + 0 => 1, + 1..10 => 123, + 10 => 42, + } + " + )?, + 42 + ); + assert!(engine + .eval_expression_with_scope::( + &mut scope, + " + switch x { + 0 => 1, + 1..10 => { + let y = 123; + y + } + 10 => 42, + } + " + ) .is_err()); assert!(engine.eval_expression::<()>("40 + 2;").is_err());