From d119e13b7936d88349a08805e481a0908796255f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 17 Jul 2020 10:18:07 +0800 Subject: [PATCH] Allow binding of this pointer in FnPtr calls. --- RELEASES.md | 1 + doc/src/language/fn-ptr.md | 46 +++++++++-- src/engine.rs | 26 ++++++- src/error.rs | 4 + src/parser.rs | 153 ++++++++++++++++++++++++------------- src/token.rs | 19 ++++- src/utils.rs | 6 ++ tests/call_fn.rs | 2 +- tests/fn_ptr.rs | 80 +++++++++++++++++++ 9 files changed, 267 insertions(+), 70 deletions(-) create mode 100644 tests/fn_ptr.rs diff --git a/RELEASES.md b/RELEASES.md index ed848b2e..9a48b709 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -9,6 +9,7 @@ New features * `call` can now be called function-call style for function pointers - this is to handle builds with `no_object`. * Disallow many keywords as variables, such as `print`, `eval`, `call`, `this` etc. +* `x.call(f, ...)` allows binding `x` to `this` for the function referenced by the function pointer `f`. Version 0.17.0 diff --git a/doc/src/language/fn-ptr.md b/doc/src/language/fn-ptr.md index 51c7d39c..57f2f24f 100644 --- a/doc/src/language/fn-ptr.md +++ b/doc/src/language/fn-ptr.md @@ -66,20 +66,18 @@ Because of their dynamic nature, function pointers cannot refer to functions in See [function namespaces] for more details. ```rust -import "foo" as f; // assume there is 'f::do_something()' +import "foo" as f; // assume there is 'f::do_work()' -f::do_something(); // works! +f::do_work(); // works! -let p = Fn("f::do_something"); +let p = Fn("f::do_work"); // error: invalid function name -p.call(); // error: function not found - 'f::do_something' - -fn do_something_now() { // call it from a local function +fn do_work_now() { // call it from a local function import "foo" as f; - f::do_something(); + f::do_work(); } -let p = Fn("do_something_now"); +let p = Fn("do_work_now"); p.call(); // works! ``` @@ -134,3 +132,35 @@ let func = sign(x) + 1; // Dynamic dispatch map[func].call(42); ``` + + +Binding the `this` Pointer +------------------------- + +When `call` is called as a _method_ but not on a `FnPtr` value, it is possible to dynamically dispatch +to a function call while binding the object in the method call to the `this` pointer of the function. + +To achieve this, pass the `FnPtr` value as the _first_ argument to `call`: + +```rust +fn add(x) { this += x; } // define function which uses 'this' + +let func = Fn("add"); // function pointer to 'add' + +func.call(1); // error: 'this' pointer is not bound + +let x = 41; + +func.call(x, 1); // error: function 'add (i64, i64)' not found + +call(func, x, 1); // error: function 'add (i64, i64)' not found + +x.call(func, 1); // 'this' is bound to 'x', dispatched to 'func' + +x == 42; +``` + +Beware that this only works for _method-call_ style. Normal function-call style cannot bind +the `this` pointer (for syntactic reasons). + +Therefore, obviously, binding the `this` pointer is unsupported under [`no_function`]. diff --git a/src/engine.rs b/src/engine.rs index f0bdfc48..dd1879cc 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -96,7 +96,7 @@ pub const MARKER_BLOCK: &str = "$block$"; pub const MARKER_IDENT: &str = "$ident$"; #[cfg(feature = "internals")] -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Hash)] pub struct Expression<'a>(&'a Expr); #[cfg(feature = "internals")] @@ -1088,10 +1088,10 @@ impl Engine { let idx = idx_val.downcast_mut::>().unwrap(); let mut fn_name = name.as_ref(); - // Check if it is a FnPtr call let (result, updated) = if fn_name == KEYWORD_FN_PTR_CALL && obj.is::() { + // FnPtr call // Redirect function name - fn_name = obj.as_str().unwrap(); + let fn_name = obj.as_str().unwrap(); // Recalculate hash let hash = calc_fn_hash(empty(), fn_name, idx.len(), empty()); // Arguments are passed as-is @@ -1102,6 +1102,26 @@ impl Engine { self.exec_fn_call( state, lib, fn_name, *native, hash, args, false, false, def_val, level, ) + } else if fn_name == KEYWORD_FN_PTR_CALL && idx.len() > 0 && idx[0].is::() { + // FnPtr call on object + // Redirect function name + let fn_name = idx[0] + .downcast_ref::() + .unwrap() + .get_fn_name() + .clone(); + // Recalculate hash + let hash = calc_fn_hash(empty(), &fn_name, idx.len() - 1, empty()); + // Replace the first argument with the object pointer + let mut arg_values = once(obj) + .chain(idx.iter_mut().skip(1)) + .collect::>(); + let args = arg_values.as_mut(); + + // Map it to name(args) in function-call style + self.exec_fn_call( + state, lib, &fn_name, *native, hash, args, is_ref, true, def_val, level, + ) } else { let redirected: Option; let mut hash = *hash; diff --git a/src/error.rs b/src/error.rs index 6b3308ba..1f4b08bf 100644 --- a/src/error.rs +++ b/src/error.rs @@ -98,6 +98,8 @@ pub enum ParseErrorType { PropertyExpected, /// Missing a variable name after the `let`, `const` or `for` keywords. VariableExpected, + /// An identifier is a reserved keyword. + Reserved(String), /// Missing an expression. Wrapped value is the expression type. ExprExpected(String), /// Defining a function `fn` in an appropriate place (e.g. inside another function). @@ -163,6 +165,7 @@ impl ParseErrorType { Self::ForbiddenConstantExpr(_) => "Expecting a constant", Self::PropertyExpected => "Expecting name of a property", Self::VariableExpected => "Expecting name of a variable", + Self::Reserved(_) => "Invalid use of reserved keyword", Self::ExprExpected(_) => "Expecting an expression", Self::FnMissingName => "Expecting name in function declaration", Self::FnMissingParams(_) => "Expecting parameters in function declaration", @@ -224,6 +227,7 @@ impl fmt::Display for ParseErrorType { Self::LiteralTooLarge(typ, max) => { write!(f, "{} exceeds the maximum limit ({})", typ, max) } + Self::Reserved(s) => write!(f, "'{}' is a reserved keyword", s), _ => f.write_str(self.desc()), } } diff --git a/src/parser.rs b/src/parser.rs index eca7ef65..e2f0c3f2 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -25,6 +25,7 @@ use crate::stdlib::{ char, collections::HashMap, fmt, format, + hash::Hash, iter::empty, mem, num::NonZeroUsize, @@ -340,7 +341,7 @@ impl fmt::Display for FnAccess { } /// A scripted function definition. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Hash)] pub struct ScriptFnDef { /// Function name. pub name: String, @@ -374,7 +375,7 @@ impl fmt::Display for ScriptFnDef { } /// `return`/`throw` statement. -#[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)] +#[derive(Debug, Eq, PartialEq, Clone, Copy, Hash)] pub enum ReturnType { /// `return` statement. Return, @@ -478,7 +479,7 @@ impl ParseSettings { /// /// Each variant is at most one pointer in size (for speed), /// with everything being allocated together in one single tuple. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Hash)] pub enum Stmt { /// No-op. Noop(Position), @@ -590,6 +591,13 @@ impl fmt::Debug for CustomExpr { } } +#[cfg(feature = "internals")] +impl Hash for CustomExpr { + fn hash(&self, state: &mut H) { + self.0.hash(state); + } +} + /// An expression. /// /// Each variant is at most one pointer in size (for speed), @@ -665,6 +673,18 @@ impl Default for Expr { } } +impl Hash for Expr { + fn hash(&self, state: &mut H) { + match self { + Self::FloatConstant(x) => { + state.write(&x.0.to_le_bytes()); + x.1.hash(state); + } + _ => self.hash(state), + } + } +} + impl Expr { /// Get the `Dynamic` value of a constant expression. /// @@ -1345,56 +1365,57 @@ fn parse_map_literal( eat_token(input, Token::RightBrace); break; } - _ => { - let (name, pos) = match input.next().unwrap() { - (Token::Identifier(s), pos) => (s, pos), - (Token::StringConstant(s), pos) => (s, pos), - (Token::LexError(err), pos) => return Err(err.into_err(pos)), - (_, pos) if map.is_empty() => { - return Err(PERR::MissingToken( - Token::RightBrace.into(), - MISSING_RBRACE.into(), - ) - .into_err(pos)) - } - (Token::EOF, pos) => { - return Err(PERR::MissingToken( - Token::RightBrace.into(), - MISSING_RBRACE.into(), - ) - .into_err(pos)) - } - (_, pos) => return Err(PERR::PropertyExpected.into_err(pos)), - }; - - match input.next().unwrap() { - (Token::Colon, _) => (), - (Token::LexError(err), pos) => return Err(err.into_err(pos)), - (_, pos) => { - return Err(PERR::MissingToken( - Token::Colon.into(), - format!( - "to follow the property '{}' in this object map literal", - name - ), - ) - .into_err(pos)) - } - }; - - if state.engine.max_map_size > 0 && map.len() >= state.engine.max_map_size { - return Err(PERR::LiteralTooLarge( - "Number of properties in object map literal".to_string(), - state.engine.max_map_size, - ) - .into_err(input.peek().unwrap().1)); - } - - let expr = parse_expr(input, state, lib, settings.level_up())?; - map.push(((Into::::into(name), pos), expr)); - } + _ => (), } + let (name, pos) = match input.next().unwrap() { + (Token::Identifier(s), pos) => (s, pos), + (Token::StringConstant(s), pos) => (s, pos), + (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { + return Err(PERR::Reserved(s).into_err(pos)); + } + (Token::LexError(err), pos) => return Err(err.into_err(pos)), + (_, pos) if map.is_empty() => { + return Err( + PERR::MissingToken(Token::RightBrace.into(), MISSING_RBRACE.into()) + .into_err(pos), + ); + } + (Token::EOF, pos) => { + return Err( + PERR::MissingToken(Token::RightBrace.into(), MISSING_RBRACE.into()) + .into_err(pos), + ); + } + (_, pos) => return Err(PERR::PropertyExpected.into_err(pos)), + }; + + match input.next().unwrap() { + (Token::Colon, _) => (), + (Token::LexError(err), pos) => return Err(err.into_err(pos)), + (_, pos) => { + return Err(PERR::MissingToken( + Token::Colon.into(), + format!( + "to follow the property '{}' in this object map literal", + name + ), + ) + .into_err(pos)) + } + }; + + if state.engine.max_map_size > 0 && map.len() >= state.engine.max_map_size { + return Err(PERR::LiteralTooLarge( + "Number of properties in object map literal".to_string(), + state.engine.max_map_size, + ) + .into_err(input.peek().unwrap().1)); + } + + let expr = parse_expr(input, state, lib, settings.level_up())?; + map.push(((Into::::into(name), pos), expr)); + match input.peek().unwrap() { (Token::Comma, _) => { eat_token(input, Token::Comma); @@ -1477,6 +1498,9 @@ 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())?, @@ -1488,7 +1512,7 @@ fn parse_primary( _ => { return Err( PERR::BadInput(format!("Unexpected '{}'", token.syntax())).into_err(settings.pos) - ) + ); } }; @@ -1526,6 +1550,9 @@ fn parse_primary( Expr::Variable(Box::new(((id2, pos2), modules, 0, index))) } + (Token::Reserved(id2), pos2) if is_valid_identifier(id2.chars()) => { + return Err(PERR::Reserved(id2).into_err(pos2)); + } (_, pos2) => return Err(PERR::VariableExpected.into_err(pos2)), }, // Indexing @@ -2112,6 +2139,9 @@ fn parse_expr( (Token::Identifier(s), pos) => { exprs.push(Expr::Variable(Box::new(((s, pos), None, 0, None)))); } + (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { + return Err(PERR::Reserved(s).into_err(pos)); + } (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), }, MARKER_EXPR => exprs.push(parse_expr(input, state, lib, settings)?), @@ -2279,10 +2309,12 @@ fn parse_for( let name = match input.next().unwrap() { // Variable name (Token::Identifier(s), _) => s, + // Reserved keyword + (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { + return Err(PERR::Reserved(s).into_err(pos)); + } // Bad identifier (Token::LexError(err), pos) => return Err(err.into_err(pos)), - // EOF - (Token::EOF, pos) => return Err(PERR::VariableExpected.into_err(pos)), // Not a variable name (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), }; @@ -2329,6 +2361,9 @@ fn parse_let( // let name ... let (name, pos) = match input.next().unwrap() { (Token::Identifier(s), pos) => (s, pos), + (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { + return Err(PERR::Reserved(s).into_err(pos)); + } (Token::LexError(err), pos) => return Err(err.into_err(pos)), (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), }; @@ -2398,6 +2433,9 @@ fn parse_import( // import expr as name ... let (name, _) = match input.next().unwrap() { (Token::Identifier(s), pos) => (s, pos), + (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { + return Err(PERR::Reserved(s).into_err(pos)); + } (Token::LexError(err), pos) => return Err(err.into_err(pos)), (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), }; @@ -2422,6 +2460,9 @@ fn parse_export( loop { let (id, id_pos) = match input.next().unwrap() { (Token::Identifier(s), pos) => (s.clone(), pos), + (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { + return Err(PERR::Reserved(s).into_err(pos)); + } (Token::LexError(err), pos) => return Err(err.into_err(pos)), (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), }; @@ -2429,6 +2470,10 @@ fn parse_export( let rename = if match_token(input, Token::As)? { match input.next().unwrap() { (Token::Identifier(s), pos) => Some((s.clone(), pos)), + (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { + return Err(PERR::Reserved(s).into_err(pos)); + } + (Token::LexError(err), pos) => return Err(err.into_err(pos)), (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), } } else { diff --git a/src/token.rs b/src/token.rs index 25e39fc8..f3579317 100644 --- a/src/token.rs +++ b/src/token.rs @@ -279,8 +279,6 @@ impl Token { Or => "||", Ampersand => "&", And => "&&", - #[cfg(not(feature = "no_function"))] - Fn => "fn", Continue => "continue", Break => "break", Return => "return", @@ -301,8 +299,12 @@ impl Token { ModuloAssign => "%=", PowerOf => "~", PowerOfAssign => "~=", + + #[cfg(not(feature = "no_function"))] + Fn => "fn", #[cfg(not(feature = "no_function"))] Private => "private", + #[cfg(not(feature = "no_module"))] Import => "import", #[cfg(not(feature = "no_module"))] @@ -359,8 +361,6 @@ impl Token { "||" => Or, "&" => Ampersand, "&&" => And, - #[cfg(not(feature = "no_function"))] - "fn" => Fn, "continue" => Continue, "break" => Break, "return" => Return, @@ -381,14 +381,25 @@ impl Token { "%=" => ModuloAssign, "~" => PowerOf, "~=" => PowerOfAssign, + + #[cfg(not(feature = "no_function"))] + "fn" => Fn, #[cfg(not(feature = "no_function"))] "private" => Private, + #[cfg(not(feature = "no_module"))] "import" => Import, #[cfg(not(feature = "no_module"))] "export" => Export, #[cfg(not(feature = "no_module"))] "as" => As, + + #[cfg(feature = "no_function")] + "fn" | "private" => Reserved(syntax.into()), + + #[cfg(feature = "no_module")] + "import" | "export" | "as" => Reserved(syntax.into()), + "===" | "!==" | "->" | "<-" | "=>" | ":=" | "::<" | "(*" | "*)" | "#" => { Reserved(syntax.into()) } diff --git a/src/utils.rs b/src/utils.rs index 5dd619ba..01fdd26b 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -156,6 +156,12 @@ impl Drop for StaticVec { } } +impl Hash for StaticVec { + fn hash(&self, state: &mut H) { + self.iter().for_each(|x| x.hash(state)); + } +} + impl Default for StaticVec { fn default() -> Self { Self { diff --git a/tests/call_fn.rs b/tests/call_fn.rs index 2eecab90..a04bd614 100644 --- a/tests/call_fn.rs +++ b/tests/call_fn.rs @@ -116,7 +116,7 @@ fn test_anonymous_fn() -> Result<(), Box> { #[test] #[cfg(not(feature = "no_object"))] -fn test_fn_ptr() -> Result<(), Box> { +fn test_fn_ptr_raw() -> Result<(), Box> { let mut engine = Engine::new(); engine.register_raw_fn( diff --git a/tests/fn_ptr.rs b/tests/fn_ptr.rs new file mode 100644 index 00000000..e9121475 --- /dev/null +++ b/tests/fn_ptr.rs @@ -0,0 +1,80 @@ +use rhai::{Engine, EvalAltResult, RegisterFn, INT}; + +#[test] +fn test_fn_ptr() -> Result<(), Box> { + let mut engine = Engine::new(); + + engine.register_fn("bar", |x: &mut INT, y: INT| *x += y); + + #[cfg(not(feature = "no_object"))] + assert_eq!( + engine.eval::( + r#" + let f = Fn("bar"); + let x = 40; + f.call(x, 2); + x + "# + )?, + 40 + ); + + #[cfg(not(feature = "no_object"))] + assert_eq!( + engine.eval::( + r#" + let f = Fn("bar"); + let x = 40; + x.call(f, 2); + x + "# + )?, + 42 + ); + + assert_eq!( + engine.eval::( + r#" + let f = Fn("bar"); + let x = 40; + call(f, x, 2); + x + "# + )?, + 42 + ); + + #[cfg(not(feature = "no_function"))] + #[cfg(not(feature = "no_object"))] + assert_eq!( + engine.eval::( + r#" + fn foo(x) { this += x; } + + let f = Fn("foo"); + let x = 40; + x.call(f, 2); + x + "# + )?, + 42 + ); + + #[cfg(not(feature = "no_function"))] + assert!(matches!( + *engine + .eval::( + r#" + fn foo(x) { this += x; } + + let f = Fn("foo"); + call(f, 2); + x + "# + ) + .expect_err("should error"), + EvalAltResult::ErrorInFunctionCall(fn_name, err, _) if fn_name == "foo" && matches!(*err, EvalAltResult::ErrorUnboundedThis(_)) + )); + + Ok(()) +}