From 0b04d05afe1be17aa09b165a41b1936f72517704 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 22 Aug 2020 11:08:27 +0800 Subject: [PATCH 1/3] Do not parse closures when allow_anonymous_fn is false. --- RELEASES.md | 9 +++++ doc/src/engine/expressions.md | 4 +- src/parser.rs | 4 +- tests/closures.rs | 70 ++++++++++++++++++++++++++++++++++- 4 files changed, 84 insertions(+), 3 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index d42f41e2..6ed51c4a 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -7,6 +7,15 @@ Version 0.19.0 Bug fixes --------- +* `Engine::compile_expression`, `Engine::eval_expression` etc. no longer parse anonymous functions and closures. + + +Version 0.18.2 +============== + +Bug fixes +--------- + * Fixes bug that prevents calling functions in closures. * Fixes bug that erroneously consumes the first argument to a module-qualified function call. diff --git a/doc/src/engine/expressions.md b/doc/src/engine/expressions.md index e9e890bf..a87a901c 100644 --- a/doc/src/engine/expressions.md +++ b/doc/src/engine/expressions.md @@ -11,9 +11,11 @@ In these cases, use the `Engine::compile_expression` and `Engine::eval_expressio let result = engine.eval_expression::("2 + (10 + 10) * 2")?; ``` -When evaluating _expressions_, no full-blown statement (e.g. `if`, `while`, `for`) - not even variable assignment - +When evaluating _expressions_, no full-blown statement (e.g. `if`, `while`, `for`, `fn`) - not even variable assignment - is supported and will be considered parse errors when encountered. +[Closures] and [anonymous functions] are also not supported because in the background they compile to functions. + ```rust // The following are all syntax errors because the script is not an expression. diff --git a/src/parser.rs b/src/parser.rs index b8bb9cd8..39ffaada 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1862,7 +1862,7 @@ fn parse_unary( } // | ... #[cfg(not(feature = "no_function"))] - Token::Pipe | Token::Or => { + Token::Pipe | Token::Or if settings.allow_anonymous_fn => { let mut new_state = ParseState::new( state.engine, #[cfg(not(feature = "unchecked"))] @@ -3353,6 +3353,8 @@ impl Engine { }; let expr = parse_expr(input, &mut state, &mut functions, settings)?; + assert!(functions.is_empty()); + match input.peek().unwrap() { (Token::EOF, _) => (), // Return error if the expression doesn't end diff --git a/tests/closures.rs b/tests/closures.rs index 94b8a9ca..4eb8d81b 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -1,7 +1,11 @@ #![cfg(not(feature = "no_function"))] -use rhai::{Dynamic, Engine, EvalAltResult, FnPtr, Module, RegisterFn, INT}; +use rhai::{ + Dynamic, Engine, EvalAltResult, FnPtr, Map, Module, ParseErrorType, RegisterFn, Scope, INT, +}; use std::any::TypeId; +use std::cell::RefCell; use std::mem::take; +use std::rc::Rc; #[test] fn test_fn_ptr_curry_call() -> Result<(), Box> { @@ -39,6 +43,14 @@ fn test_fn_ptr_curry_call() -> Result<(), Box> { fn test_closures() -> Result<(), Box> { let mut engine = Engine::new(); + assert!(matches!( + *engine + .compile_expression("let f = |x| {};") + .expect_err("should error") + .0, + ParseErrorType::BadInput(_) + )); + assert_eq!( engine.eval::( r#" @@ -181,3 +193,59 @@ fn test_closures_data_race() -> Result<(), Box> { Ok(()) } + +type MyType = Rc>; + +#[test] +#[cfg(not(feature = "no_object"))] +#[cfg(not(feature = "sync"))] +fn test_closure_shared_obj() -> Result<(), Box> { + let mut engine = Engine::new(); + + // Register API on MyType + engine + .register_type_with_name::("MyType") + .register_get_set( + "data", + |p: &mut MyType| *p.borrow(), + |p: &mut MyType, value: INT| *p.borrow_mut() = value, + ) + .register_fn("+=", |p1: &mut MyType, p2: MyType| { + *p1.borrow_mut() += *p2.borrow() + }) + .register_fn("-=", |p1: &mut MyType, p2: MyType| { + *p1.borrow_mut() -= *p2.borrow() + }); + + let engine = engine; // Make engine immutable + + let code = r#" + #{ + name: "A", + description: "B", + cost: 1, + health_added: 0, + action: |p1, p2| { p1 += p2 } + } + "#; + + let ast = engine.compile(code)?; + let res = engine.eval_ast::(&ast)?; + + // Make closure + let f = move |p1: MyType, p2: MyType| -> Result<(), Box> { + let action_ptr = res["action"].clone().cast::(); + let name = action_ptr.fn_name(); + engine.call_fn::<_, ()>(&mut Scope::new(), &ast, name, (p1, p2)) + }; + + // Test closure + let p1 = Rc::new(RefCell::new(41)); + let p2 = Rc::new(RefCell::new(1)); + + f(p1.clone(), p2.clone())?; + + assert_eq!(*p1.borrow(), 42); + + Ok(()) +} From 0ece75aba364226d531c2f4f6409c598f01b2f30 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 22 Aug 2020 22:44:24 +0800 Subject: [PATCH 2/3] Allow module access in closures. --- Cargo.toml | 2 +- RELEASES.md | 3 ++- src/parser.rs | 15 +++++++++++++++ tests/call_fn.rs | 14 ++++++++++++++ 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index df785e3e..25305837 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rhai" -version = "0.19.0" +version = "0.18.3" edition = "2018" authors = ["Jonathan Turner", "Lukáš Hozda", "Stephen Chung"] description = "Embedded scripting for Rust" diff --git a/RELEASES.md b/RELEASES.md index 6ed51c4a..db26c661 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,13 +1,14 @@ Rhai Release Notes ================== -Version 0.19.0 +Version 0.18.3 ============== Bug fixes --------- * `Engine::compile_expression`, `Engine::eval_expression` etc. no longer parse anonymous functions and closures. +* Imported modules now work inside closures. Version 0.18.2 diff --git a/src/parser.rs b/src/parser.rs index 39ffaada..01be7715 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1645,11 +1645,22 @@ fn parse_primary( } Expr::Variable(Box::new(((s, settings.pos), None, 0, None))) } + // Module qualification + #[cfg(not(feature = "no_module"))] + Token::Identifier(s) if *next_token == Token::DoubleColon => { + // Once the identifier consumed we must enable next variables capturing + #[cfg(not(feature = "no_closure"))] + { + state.allow_capture = true; + } + Expr::Variable(Box::new(((s, settings.pos), None, 0, None))) + } // Normal variable access Token::Identifier(s) => { let index = state.access_var(&s, settings.pos); Expr::Variable(Box::new(((s, settings.pos), None, 0, index))) } + // Function call is allowed to have reserved keyword Token::Reserved(s) if *next_token == Token::LeftParen || *next_token == Token::Bang => { if is_keyword_function(&s) { @@ -1658,6 +1669,7 @@ fn parse_primary( return Err(PERR::Reserved(s).into_err(settings.pos)); } } + // Access to `this` as a variable is OK Token::Reserved(s) if s == KEYWORD_THIS && *next_token != Token::LeftParen => { if !settings.is_function_scope { @@ -1669,9 +1681,11 @@ fn parse_primary( Expr::Variable(Box::new(((s, settings.pos), None, 0, None))) } } + Token::Reserved(s) if is_valid_identifier(s.chars()) => { return Err(PERR::Reserved(s).into_err(settings.pos)); } + Token::LeftParen => parse_paren_expr(input, state, lib, settings.level_up())?, #[cfg(not(feature = "no_index"))] Token::LeftBracket => parse_array_literal(input, state, lib, settings.level_up())?, @@ -1680,6 +1694,7 @@ fn parse_primary( Token::True => Expr::True(settings.pos), Token::False => Expr::False(settings.pos), Token::LexError(err) => return Err(err.into_err(settings.pos)), + _ => { return Err( PERR::BadInput(format!("Unexpected '{}'", token.syntax())).into_err(settings.pos) diff --git a/tests/call_fn.rs b/tests/call_fn.rs index a7a5d514..5d22abd1 100644 --- a/tests/call_fn.rs +++ b/tests/call_fn.rs @@ -120,6 +120,20 @@ fn test_fn_ptr_raw() -> Result<(), Box> { 42 ); + assert_eq!( + engine.eval::( + r#" + fn foo(x, y) { this += x + y; } + + let x = 40; + let v = 1; + x.bar(Fn("foo").curry(v), 1); + x + "# + )?, + 42 + ); + assert!(matches!( *engine.eval::( r#" From 177a0de23cd44f0145af68f7ffe6b8b1cb26521b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 22 Aug 2020 23:01:25 +0800 Subject: [PATCH 3/3] Fix closure test. --- src/api.rs | 8 ++++---- tests/closures.rs | 7 ++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/api.rs b/src/api.rs index 852fd2a7..899b1ab6 100644 --- a/src/api.rs +++ b/src/api.rs @@ -2,7 +2,7 @@ use crate::any::{Dynamic, Variant}; use crate::engine::{Engine, Imports, State}; -use crate::error::{ParseError, ParseErrorType}; +use crate::error::ParseError; use crate::fn_native::{IteratorFn, SendSync}; use crate::module::{FuncReturn, Module}; use crate::optimize::OptimizationLevel; @@ -18,6 +18,7 @@ use crate::engine::{FN_IDX_GET, FN_IDX_SET}; #[cfg(not(feature = "no_object"))] use crate::{ engine::{make_getter, make_setter, Map}, + error::ParseErrorType, fn_register::{RegisterFn, RegisterResultFn}, token::Token, }; @@ -34,7 +35,6 @@ use crate::optimize::optimize_into_ast; use crate::stdlib::{ any::{type_name, TypeId}, boxed::Box, - string::ToString, }; #[cfg(not(feature = "no_optimize"))] @@ -944,8 +944,8 @@ impl Engine { ["#", json_text] } else { return Err(ParseErrorType::MissingToken( - Token::LeftBrace.syntax().to_string(), - "to start a JSON object hash".to_string(), + Token::LeftBrace.syntax().into(), + "to start a JSON object hash".into(), ) .into_err(Position::new(1, (json.len() - json_text.len() + 1) as u16)) .into()); diff --git a/tests/closures.rs b/tests/closures.rs index 4eb8d81b..348aa162 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -1,12 +1,13 @@ #![cfg(not(feature = "no_function"))] -use rhai::{ - Dynamic, Engine, EvalAltResult, FnPtr, Map, Module, ParseErrorType, RegisterFn, Scope, INT, -}; +use rhai::{Dynamic, Engine, EvalAltResult, FnPtr, Module, ParseErrorType, RegisterFn, Scope, INT}; use std::any::TypeId; use std::cell::RefCell; use std::mem::take; use std::rc::Rc; +#[cfg(not(feature = "no_object"))] +use rhai::Map; + #[test] fn test_fn_ptr_curry_call() -> Result<(), Box> { let mut engine = Engine::new();