From 16fbfbb606347c24777718a9e29e6c68fd31d5b4 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 16 Jul 2020 12:09:31 +0800 Subject: [PATCH] Enable call in function-call style. --- RELEASES.md | 5 ++++ doc/src/language/fn-ptr.md | 4 +-- src/engine.rs | 54 ++++++++++++++++++++++++++++++-------- src/parser.rs | 39 ++++++++++++++++++--------- src/token.rs | 13 ++++++--- tests/functions.rs | 16 ++++++++++- 6 files changed, 102 insertions(+), 29 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 938b4d2c..ed848b2e 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -4,6 +4,11 @@ Rhai Release Notes Version 0.18.0 ============== +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. Version 0.17.0 diff --git a/doc/src/language/fn-ptr.md b/doc/src/language/fn-ptr.md index e4d9bc5d..51c7d39c 100644 --- a/doc/src/language/fn-ptr.md +++ b/doc/src/language/fn-ptr.md @@ -6,7 +6,7 @@ Function Pointers It is possible to store a _function pointer_ in a variable just like a normal value. In fact, internally a function pointer simply stores the _name_ of the function as a string. -Call a function pointer using the `call` method, which needs to be called in method-call style. +Call a function pointer using the `call` method. Built-in methods @@ -40,7 +40,7 @@ func.call(1) == 42; // call a function pointer with the 'call' method foo(1) == 42; // <- the above de-sugars to this -call(func, 1); //<- error: 'call (Fn, i64)' is not a registered function +call(func, 1); // normal function call style also works for 'call' let len = Fn("len"); // 'Fn' also works with registered native Rust functions diff --git a/src/engine.rs b/src/engine.rs index 69eaf371..f0bdfc48 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -944,7 +944,7 @@ impl Engine { } // Has a system function an override? - fn has_override(&self, lib: &Module, (hash_fn, hash_script): (u64, u64)) -> bool { + fn has_override(&self, lib: &Module, hash_fn: u64, hash_script: u64) -> bool { // NOTE: We skip script functions for global_module and packages, and native functions for lib // First check script-defined functions @@ -986,13 +986,15 @@ impl Engine { match fn_name { // type_of - KEYWORD_TYPE_OF if args.len() == 1 && !self.has_override(lib, hashes) => Ok(( - self.map_type_name(args[0].type_name()).to_string().into(), - false, - )), + KEYWORD_TYPE_OF if args.len() == 1 && !self.has_override(lib, hashes.0, hashes.1) => { + Ok(( + self.map_type_name(args[0].type_name()).to_string().into(), + false, + )) + } // Fn - KEYWORD_FN_PTR if args.len() == 1 && !self.has_override(lib, hashes) => { + KEYWORD_FN_PTR if args.len() == 1 && !self.has_override(lib, hashes.0, hashes.1) => { Err(Box::new(EvalAltResult::ErrorRuntime( "'Fn' should not be called in method style. Try Fn(...);".into(), Position::none(), @@ -1000,7 +1002,7 @@ impl Engine { } // eval - reaching this point it must be a method-style call - KEYWORD_EVAL if args.len() == 1 && !self.has_override(lib, hashes) => { + KEYWORD_EVAL if args.len() == 1 && !self.has_override(lib, hashes.0, hashes.1) => { Err(Box::new(EvalAltResult::ErrorRuntime( "'eval' should not be called in method style. Try eval(...);".into(), Position::none(), @@ -1926,7 +1928,7 @@ impl Engine { let hash_fn = calc_fn_hash(empty(), name, 1, once(TypeId::of::())); - if !self.has_override(lib, (hash_fn, *hash)) { + if !self.has_override(lib, hash_fn, *hash) { // Fn - only in function call style let expr = args_expr.get(0); let arg_value = @@ -1952,7 +1954,7 @@ impl Engine { let hash_fn = calc_fn_hash(empty(), name, 1, once(TypeId::of::())); - if !self.has_override(lib, (hash_fn, *hash)) { + if !self.has_override(lib, hash_fn, *hash) { // eval - only in function call style let prev_len = scope.len(); let expr = args_expr.get(0); @@ -1972,6 +1974,36 @@ impl Engine { } } + // Handle call() - Redirect function call + let redirected; + let mut name = name.as_ref(); + let mut args_expr = args_expr.as_ref(); + let mut hash = *hash; + + if name == KEYWORD_FN_PTR_CALL + && args_expr.len() >= 1 + && !self.has_override(lib, 0, hash) + { + let expr = args_expr.get(0).unwrap(); + let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + + if fn_ptr.is::() { + // Redirect function name + redirected = Some(fn_ptr.cast::().take_fn_name()); + name = redirected.as_ref().unwrap(); + // Skip the first argument + args_expr = &args_expr.as_ref()[1..]; + // Recalculate hash + hash = calc_fn_hash(empty(), name, args_expr.len(), empty()); + } else { + return Err(Box::new(EvalAltResult::ErrorMismatchOutputType( + self.map_type_name(type_name::()).into(), + fn_ptr.type_name().into(), + expr.position(), + ))); + } + } + // Normal function call - except for Fn and eval (handled above) let mut arg_values: StaticVec; let mut args: StaticVec<_>; @@ -1983,7 +2015,7 @@ impl Engine { } else { // See if the first argument is a variable, if so, convert to method-call style // in order to leverage potential &mut first argument and avoid cloning the value - match args_expr.get(0) { + match args_expr.get(0).unwrap() { // func(x, ...) -> x.func(...) lhs @ Expr::Variable(_) => { arg_values = args_expr @@ -2020,7 +2052,7 @@ impl Engine { let args = args.as_mut(); self.exec_fn_call( - state, lib, name, *native, *hash, args, is_ref, false, def_val, level, + state, lib, name, *native, hash, args, is_ref, false, def_val, level, ) .map(|(v, _)| v) .map_err(|err| err.new_position(*pos)) diff --git a/src/parser.rs b/src/parser.rs index 55fbdabf..eca7ef65 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -7,7 +7,7 @@ use crate::error::{LexError, ParseError, ParseErrorType}; use crate::module::{Module, ModuleRef}; use crate::optimize::{optimize_into_ast, OptimizationLevel}; use crate::scope::{EntryType as ScopeEntryType, Scope}; -use crate::token::{Position, Token, TokenStream}; +use crate::token::{is_valid_identifier, Position, Token, TokenStream}; use crate::utils::{StaticVec, StraightHasherBuilder}; #[cfg(feature = "internals")] @@ -440,6 +440,8 @@ struct ParseSettings { pos: Position, /// Is the construct being parsed located at global level? is_global: bool, + /// Is the construct being parsed located at function definition level? + is_function_scope: bool, /// Is the current position inside a loop? is_breakable: bool, /// Is anonymous function allowed? @@ -1460,6 +1462,21 @@ fn parse_primary( let index = state.find_var(&s); Expr::Variable(Box::new(((s, settings.pos), None, 0, index))) } + // Function call is allowed to have reserved keyword + Token::Reserved(s) if s != KEYWORD_THIS && input.peek().unwrap().0 == Token::LeftParen => { + Expr::Variable(Box::new(((s, settings.pos), None, 0, None))) + } + // Access to `this` as a variable is OK + Token::Reserved(s) if s == KEYWORD_THIS && input.peek().unwrap().0 != Token::LeftParen => { + if !settings.is_function_scope { + return Err( + PERR::BadInput(format!("'{}' can only be used in functions", s)) + .into_err(settings.pos), + ); + } else { + Expr::Variable(Box::new(((s, settings.pos), None, 0, None))) + } + } 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())?, @@ -1538,6 +1555,7 @@ fn parse_primary( _ => (), } + // Make sure identifiers are valid Ok(root_expr) } @@ -2315,16 +2333,6 @@ fn parse_let( (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), }; - // Check if the name is allowed - match name.as_str() { - KEYWORD_THIS => { - return Err( - PERR::BadInput(LexError::MalformedIdentifier(name).to_string()).into_err(pos), - ) - } - _ => (), - } - // let name = ... if match_token(input, Token::Equals)? { // let name = expr @@ -2598,6 +2606,7 @@ fn parse_stmt( allow_stmt_expr: true, allow_anonymous_fn: true, is_global: false, + is_function_scope: true, is_breakable: false, level: 0, pos: pos, @@ -2695,7 +2704,11 @@ fn parse_fn( settings.ensure_level_within_max_limit(state.max_expr_depth)?; let name = match input.next().unwrap() { - (Token::Identifier(s), _) | (Token::Custom(s), _) => s, + (Token::Identifier(s), _) | (Token::Custom(s), _) | (Token::Reserved(s), _) + if s != KEYWORD_THIS && is_valid_identifier(s.chars()) => + { + s + } (_, pos) => return Err(PERR::FnMissingName.into_err(pos)), }; @@ -2790,6 +2803,7 @@ impl Engine { allow_stmt_expr: false, allow_anonymous_fn: false, is_global: true, + is_function_scope: false, is_breakable: false, level: 0, pos: Position::none(), @@ -2829,6 +2843,7 @@ impl Engine { allow_stmt_expr: true, allow_anonymous_fn: true, is_global: true, + is_function_scope: false, is_breakable: false, level: 0, pos: Position::none(), diff --git a/src/token.rs b/src/token.rs index 88196856..25e39fc8 100644 --- a/src/token.rs +++ b/src/token.rs @@ -1,6 +1,10 @@ //! Main module defining the lexer and parser. -use crate::engine::Engine; +use crate::engine::{ + Engine, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_FN_PTR_CALL, KEYWORD_PRINT, + KEYWORD_THIS, KEYWORD_TYPE_OF, +}; + use crate::error::LexError; use crate::parser::INT; use crate::utils::StaticVec; @@ -388,6 +392,8 @@ impl Token { "===" | "!==" | "->" | "<-" | "=>" | ":=" | "::<" | "(*" | "*)" | "#" => { Reserved(syntax.into()) } + KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_TYPE_OF | KEYWORD_EVAL | KEYWORD_FN_PTR + | KEYWORD_FN_PTR_CALL | KEYWORD_THIS => Reserved(syntax.into()), _ => return None, }) @@ -1353,9 +1359,10 @@ impl<'a> Iterator for TokenIterator<'a, '_> { "'#' is not a valid symbol. Should it be '#{'?" .to_string(), ))), - token => Token::LexError(Box::new(LERR::ImproperSymbol( - format!("'{}' is not a valid symbol.", token) + token if !is_valid_identifier(token.chars()) => Token::LexError(Box::new(LERR::ImproperSymbol( + format!("'{}' is a reserved symbol.", token) ))), + _ => Token::Reserved(s) }, pos)), (r @ Some(_), None, None) => r, (Some((Token::Identifier(s), pos)), _, Some(custom)) if custom.contains_key(&s) => { diff --git a/tests/functions.rs b/tests/functions.rs index ed3eeb73..3c0a080b 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -52,12 +52,24 @@ fn test_functions() -> Result<(), Box> { } #[test] -#[cfg(not(feature = "no_object"))] fn test_function_pointers() -> Result<(), Box> { let engine = Engine::new(); assert_eq!(engine.eval::(r#"type_of(Fn("abc"))"#)?, "Fn"); + assert_eq!( + engine.eval::( + r#" + fn foo(x) { 40 + x } + + let f = Fn("foo"); + call(f, 2) + "# + )?, + 42 + ); + + #[cfg(not(feature = "no_object"))] assert_eq!( engine.eval::( r#" @@ -73,11 +85,13 @@ fn test_function_pointers() -> Result<(), Box> { 42 ); + #[cfg(not(feature = "no_object"))] assert!(matches!( *engine.eval::(r#"let f = Fn("abc"); f.call(0)"#).expect_err("should error"), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with("abc (") )); + #[cfg(not(feature = "no_object"))] assert_eq!( engine.eval::( r#"