Merge pull request #630 from schungx/master

Allow if and switch expressions.

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
 --------------
@@ -22,10 +23,17 @@ 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
 ------------
 
 * `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

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();

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`.

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) => {

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<Item = (&str, &Module)> {
-        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<Item = (&crate::ImmutableString, &Shared<Module>)> {
-        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<Item = &Module> {
-        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)]

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.

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<Identifier>, 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)]

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 (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 has_condition = !matches!(condition, Expr::BoolConstant(true, ..));
 
                 let stmt_block: StmtBlock = stmt.into();
-            expressions.push((condition, Expr::Stmt(stmt_block.into())).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, ..));
+
+            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,

tests/expressions.rs

@@ -12,8 +12,47 @@ fn test_expressions() -> Result<(), Box<EvalAltResult>> {
         engine.eval_expression_with_scope::<INT>(&mut scope, "2 + (x + 10) * 2")?,
         42
     );
+    assert_eq!(
+        engine.eval_expression_with_scope::<INT>(&mut scope, "if x > 0 { 42 } else { 123 }")?,
+        42
+    );
     assert!(engine
-        .eval_expression_with_scope::<INT>(&mut scope, "if x > 0 { 42 } else { 123 }")
+        .eval_expression_with_scope::<INT>(&mut scope, "if x > 0 { let y = 42; y } else { 123 }")
+        .is_err());
+    assert!(engine
+        .eval_expression_with_scope::<INT>(&mut scope, "if x > 0 { 42 } else { let y = 123; y }")
+        .is_err());
+    assert!(engine
+        .eval_expression_with_scope::<INT>(&mut scope, "if x > 0 { 42 } else {}")
+        .is_err());
+
+    assert_eq!(
+        engine.eval_expression_with_scope::<INT>(
+            &mut scope,
+            "
+                switch x {
+                    0 => 1,
+                    1..10 => 123,
+                    10 => 42,
+                }
+            "
+        )?,
+        42
+    );
+    assert!(engine
+        .eval_expression_with_scope::<INT>(
+            &mut scope,
+            "
+                switch x {
+                    0 => 1,
+                    1..10 => {
+                        let y = 123;
+                        y
+                    }
+                    10 => 42,
+                }
+            "
+        )
         .is_err());
 
     assert!(engine.eval_expression::<()>("40 + 2;").is_err());

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,
+    Dynamic, Engine, EvalAltResult, FnNamespace, FnPtr, ImmutableString, Module, NativeCallContext,
-    ParseErrorType, Scope, INT,
+    ParseError, ParseErrorType, Scope, Shared, INT,
 };
 
 #[test]
@@ -459,10 +459,10 @@ fn test_module_str() -> Result<(), Box<EvalAltResult>> {
 #[cfg(not(feature = "no_function"))]
 #[test]
 fn test_module_ast_namespace() -> Result<(), Box<EvalAltResult>> {
-    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<EvalAltResult>> {
 fn test_module_ast_namespace2() -> Result<(), Box<EvalAltResult>> {
     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<EvalAltResult>> {
     Ok(())
 }
 
+#[cfg(not(feature = "no_function"))]
+#[cfg(feature = "internals")]
+#[test]
+fn test_module_context() -> Result<(), Box<EvalAltResult>> {
+    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<INT, Box<EvalAltResult>> {
+            // 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<String> = 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::<Shared<Module>>::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::<INT>(r#"import "testing" as t; t::bar()"#)?,
+        42
+    );
+
+    Ok(())
+}
+
 #[test]
 fn test_module_file() -> Result<(), Box<EvalAltResult>> {
     let engine = Engine::new();