diff --git a/CHANGELOG.md b/CHANGELOG.md index 68bb17c6..8fb82683 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,29 @@ Rhai Release Notes ================== +Version 1.15.0 +============== + +Bug fixes +--------- + +* Fixes a concurrency error in static hashing keys (thanks [`garypen`](https://github.com/garypen)!). + +Enhancements +------------ + +* Expressions involving `this` should now run slightly faster due to a dedicated `AST` node `ThisPtr`. +* A `take` function is added to the standard library to take ownership of any data (replacing with `()`) in order to avoid cloning. +* `EvalAltResult::ErrorMismatchOutputType` now gives a better name for the requested generic type (e.g. `&str` is now `&str` and not `string`). + + Version 1.14.0 ============== +This new version contains a substantial number of bug fixes for edge cases. + +A new syntax is supported to facilitate writing object methods in script. + The code hacks that attempt to optimize branch prediction performance are removed because benchmarks do not show any material speed improvements. Bug fixes diff --git a/Cargo.toml b/Cargo.toml index 908b55cb..14867dad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = [".", "codegen"] [package] name = "rhai" -version = "1.14.0" +version = "1.15.0" rust-version = "1.61.0" edition = "2018" resolver = "2" @@ -25,7 +25,7 @@ bitflags = { version = "1", default-features = false } smartstring = { version = "1", default-features = false } rhai_codegen = { version = "1.5.0", path = "codegen", default-features = false } -no-std-compat = { git = "https://gitlab.com/jD91mZM2/no-std-compat", default-features = false, features = ["alloc"], optional = true } +no-std-compat = { git = "https://gitlab.com/jD91mZM2/no-std-compat", version = "0.4.1", default-features = false, features = ["alloc"], optional = true } libm = { version = "0.2", default-features = false, optional = true } hashbrown = { version = "0.13", optional = true } core-error = { version = "0.0", default-features = false, features = ["alloc"], optional = true } diff --git a/examples/event_handler_js/script.rhai b/examples/event_handler_js/script.rhai index 2df62003..08ab3960 100644 --- a/examples/event_handler_js/script.rhai +++ b/examples/event_handler_js/script.rhai @@ -22,11 +22,12 @@ fn start(data) { if this.value <= 0 { throw "Conditions not yet ready to start!"; } - this.bool_state = true; - this.value += parse_int(data); // Constant 'MY_CONSTANT' in custom scope is also visible! print(`MY_CONSTANT = ${MY_CONSTANT}`); + + this.value += parse_int(data); + this.bool_state = true; } /// 'end' event handler @@ -37,8 +38,8 @@ fn end(data) { if this.value > 0 { throw "Conditions not yet ready to end!"; } - this.bool_state = false; this.value = parse_int(data); + this.bool_state = false; } /// 'update' event handler diff --git a/examples/event_handler_main/script.rhai b/examples/event_handler_main/script.rhai index a42cadef..fdfbae52 100644 --- a/examples/event_handler_main/script.rhai +++ b/examples/event_handler_main/script.rhai @@ -23,7 +23,6 @@ fn start(data) { if value <= 0 { throw "Conditions not yet ready to start!"; } - bool_state = true; // Constants 'MY_CONSTANT' and 'EXTRA_CONSTANT' // in custom scope are also visible! @@ -31,6 +30,7 @@ fn start(data) { print(`EXTRA_CONSTANT = ${EXTRA_CONSTANT}`); value += parse_int(data); + bool_state = true; } /// 'end' event handler @@ -41,8 +41,8 @@ fn end(data) { if value > 0 { throw "Conditions not yet ready to end!"; } - bool_state = false; value = parse_int(data); + bool_state = false; } /// 'update' event handler diff --git a/examples/event_handler_map/script.rhai b/examples/event_handler_map/script.rhai index 1dd341e3..fd604f6d 100644 --- a/examples/event_handler_map/script.rhai +++ b/examples/event_handler_map/script.rhai @@ -28,11 +28,12 @@ fn start(data) { if state.value <= 0 { throw "Conditions not yet ready to start!"; } - state.bool_state = true; - state.value = parse_int(data); // Constant 'MY_CONSTANT' in custom scope is also visible! print(`MY_CONSTANT = ${MY_CONSTANT}`); + + state.value = parse_int(data); + state.bool_state = true; } /// 'end' event handler @@ -43,8 +44,8 @@ fn end(data) { if state.value > 0 { throw "Conditions not yet ready to end!"; } - state.bool_state = false; state.value = parse_int(data); + state.bool_state = false; } /// 'update' event handler diff --git a/src/api/call_fn.rs b/src/api/call_fn.rs index 120c4f4c..09e3e247 100644 --- a/src/api/call_fn.rs +++ b/src/api/call_fn.rs @@ -8,10 +8,7 @@ use crate::{ }; #[cfg(feature = "no_std")] use std::prelude::v1::*; -use std::{ - any::{type_name, TypeId}, - mem, -}; +use std::{any::type_name, mem}; /// Options for calling a script-defined function via [`Engine::call_fn_with_options`]. #[derive(Debug, Hash)] @@ -181,17 +178,14 @@ impl Engine { options, ) .and_then(|result| { - // Bail out early if the return type needs no cast - if TypeId::of::() == TypeId::of::() { - return Ok(reify! { result => T }); - } - - // Cast return type - let typ = self.map_type_name(result.type_name()); - - result.try_cast().ok_or_else(|| { - let t = self.map_type_name(type_name::()).into(); - ERR::ErrorMismatchOutputType(t, typ.into(), Position::NONE).into() + result.try_cast_raw().map_err(|r| { + let result_type = self.map_type_name(r.type_name()); + let cast_type = match type_name::() { + typ @ _ if typ.contains("::") => self.map_type_name(typ), + typ @ _ => typ, + }; + ERR::ErrorMismatchOutputType(cast_type.into(), result_type.into(), Position::NONE) + .into() }) }) } diff --git a/src/api/custom_syntax.rs b/src/api/custom_syntax.rs index 5f52d94c..4cdb3085 100644 --- a/src/api/custom_syntax.rs +++ b/src/api/custom_syntax.rs @@ -109,6 +109,8 @@ impl Expression<'_> { #[cfg(not(feature = "no_module"))] Expr::Variable(x, ..) if !x.1.is_empty() => None, Expr::Variable(x, ..) => Some(x.3.as_str()), + #[cfg(not(feature = "no_function"))] + Expr::ThisPtr(..) => Some(crate::engine::KEYWORD_THIS), Expr::StringConstant(x, ..) => Some(x.as_str()), _ => None, } diff --git a/src/api/eval.rs b/src/api/eval.rs index 60a9b816..7ac42d16 100644 --- a/src/api/eval.rs +++ b/src/api/eval.rs @@ -212,11 +212,18 @@ impl Engine { return Ok(reify! { result => T }); } - let typ = self.map_type_name(result.type_name()); + result.try_cast_raw::().map_err(|v| { + let typename = match type_name::() { + typ @ _ if typ.contains("::") => self.map_type_name(typ), + typ @ _ => typ, + }; - result.try_cast::().ok_or_else(|| { - let t = self.map_type_name(type_name::()).into(); - ERR::ErrorMismatchOutputType(t, typ.into(), Position::NONE).into() + ERR::ErrorMismatchOutputType( + typename.into(), + self.map_type_name(v.type_name()).into(), + Position::NONE, + ) + .into() }) } /// Evaluate an [`AST`] with own scope, returning the result value or an error. diff --git a/src/ast/expr.rs b/src/ast/expr.rs index e9ea368e..371ade11 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -297,6 +297,8 @@ pub enum Expr { Option, Position, ), + /// `this`. + ThisPtr(Position), /// Property access - ((getter, hash), (setter, hash), prop) Property( Box<( @@ -375,6 +377,7 @@ impl fmt::Debug for Expr { .entries(x.0.iter().map(|(k, v)| (k, v))) .finish() } + Self::ThisPtr(..) => f.debug_struct("ThisPtr").finish(), Self::Variable(x, i, ..) => { f.write_str("Variable(")?; @@ -632,6 +635,7 @@ impl Expr { | Self::Array(..) | Self::Map(..) | Self::Variable(..) + | Self::ThisPtr(..) | Self::And(..) | Self::Or(..) | Self::Coalesce(..) @@ -662,6 +666,7 @@ impl Expr { | Self::Array(.., pos) | Self::Map(.., pos) | Self::Variable(.., pos) + | Self::ThisPtr(pos) | Self::And(.., pos) | Self::Or(.., pos) | Self::Coalesce(.., pos) @@ -725,6 +730,7 @@ impl Expr { | Self::Dot(.., pos) | Self::Index(.., pos) | Self::Variable(.., pos) + | Self::ThisPtr(pos) | Self::FnCall(.., pos) | Self::MethodCall(.., pos) | Self::InterpolatedString(.., pos) @@ -816,6 +822,7 @@ impl Expr { | Self::StringConstant(..) | Self::InterpolatedString(..) | Self::FnCall(..) + | Self::ThisPtr(..) | Self::MethodCall(..) | Self::Stmt(..) | Self::Dot(..) diff --git a/src/config/hashing.rs b/src/config/hashing.rs index 95005eab..38bab240 100644 --- a/src/config/hashing.rs +++ b/src/config/hashing.rs @@ -51,9 +51,9 @@ impl HokmaLock { pub fn write(&'static self) -> WhenTheHokmaSuppression { loop { // We are only interested in error results - if let Err(previous) = self - .lock - .compare_exchange(1, 1, Ordering::SeqCst, Ordering::SeqCst) + if let Err(previous) = + self.lock + .compare_exchange(1, 1, Ordering::SeqCst, Ordering::SeqCst) { // If we failed, previous cannot be 1 return WhenTheHokmaSuppression { diff --git a/src/engine.rs b/src/engine.rs index 59516a82..eacf74ac 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -29,6 +29,7 @@ pub const KEYWORD_IS_SHARED: &str = "is_shared"; pub const KEYWORD_IS_DEF_VAR: &str = "is_def_var"; #[cfg(not(feature = "no_function"))] pub const KEYWORD_IS_DEF_FN: &str = "is_def_fn"; +#[cfg(not(feature = "no_function"))] pub const KEYWORD_THIS: &str = "this"; #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_module"))] diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 28dacaa9..103b636e 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -388,6 +388,23 @@ impl Engine { let scope2 = (); match (lhs, new_val) { + // this.??? or this[???] + (Expr::ThisPtr(var_pos), new_val) => { + self.track_operation(global, *var_pos)?; + + #[cfg(feature = "debugging")] + self.run_debugger(global, caches, scope, this_ptr.as_deref_mut(), lhs)?; + + if let Some(this_ptr) = this_ptr { + let target = &mut this_ptr.into(); + + self.eval_dot_index_chain_raw( + global, caches, scope2, None, lhs, expr, target, rhs, idx_values, new_val, + ) + } else { + Err(ERR::ErrorUnboundThis(*var_pos).into()) + } + } // id.??? or id[???] (Expr::Variable(.., var_pos), new_val) => { self.track_operation(global, *var_pos)?; diff --git a/src/eval/expr.rs b/src/eval/expr.rs index f1555a3d..066fdf0f 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -2,7 +2,6 @@ use super::{Caches, EvalContext, GlobalRuntimeState, Target}; use crate::ast::Expr; -use crate::engine::KEYWORD_THIS; use crate::packages::string_basic::{print_with_func, FUNC_TO_STRING}; use crate::types::dynamic::AccessMode; use crate::{Dynamic, Engine, RhaiResult, RhaiResultOf, Scope, SmartString, ERR}; @@ -140,15 +139,7 @@ impl Engine { let index = match expr { // Check if the variable is `this` - Expr::Variable(v, None, ..) - if v.0.is_none() && v.1.is_empty() && v.3 == KEYWORD_THIS => - { - return if let Some(this_ptr) = this_ptr { - Ok(this_ptr.into()) - } else { - Err(ERR::ErrorUnboundThis(expr.position()).into()) - }; - } + Expr::ThisPtr(..) => unreachable!("Expr::ThisPtr should have been handled outside"), _ if global.always_search_scope => 0, @@ -259,13 +250,9 @@ impl Engine { self.eval_fn_call_expr(global, caches, scope, this_ptr, x, *pos) } - Expr::Variable(x, index, var_pos) - if index.is_none() && x.0.is_none() && x.3 == KEYWORD_THIS => - { - this_ptr - .ok_or_else(|| ERR::ErrorUnboundThis(*var_pos).into()) - .cloned() - } + Expr::ThisPtr(var_pos) => this_ptr + .ok_or_else(|| ERR::ErrorUnboundThis(*var_pos).into()) + .cloned(), Expr::Variable(..) => self .search_namespace(global, caches, scope, this_ptr, expr) @@ -413,13 +400,7 @@ impl Engine { .and_then(|r| self.check_data_size(r, expr.start_position())) } - Expr::Stmt(x) => { - if x.is_empty() { - Ok(Dynamic::UNIT) - } else { - self.eval_stmt_block(global, caches, scope, this_ptr, x, true) - } - } + Expr::Stmt(x) => self.eval_stmt_block(global, caches, scope, this_ptr, x, true), #[cfg(not(feature = "no_index"))] Expr::Index(..) => { diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index a58cbbc3..e747d8fc 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -302,7 +302,26 @@ impl Engine { Stmt::Assignment(x, ..) => { let (op_info, BinaryExpr { lhs, rhs }) = &**x; - if let Expr::Variable(x, ..) = lhs { + if let Expr::ThisPtr(..) = lhs { + if this_ptr.is_none() { + return Err(ERR::ErrorUnboundThis(lhs.position()).into()); + } + + #[cfg(not(feature = "no_function"))] + { + let rhs_val = self + .eval_expr(global, caches, scope, this_ptr.as_deref_mut(), rhs)? + .flatten(); + + self.track_operation(global, lhs.position())?; + + let target = &mut this_ptr.unwrap().into(); + + self.eval_op_assignment(global, caches, op_info, lhs, target, rhs_val)?; + } + #[cfg(feature = "no_function")] + unreachable!(); + } else if let Expr::Variable(x, ..) = lhs { let rhs_val = self .eval_expr(global, caches, scope, this_ptr.as_deref_mut(), rhs)? .flatten(); @@ -342,6 +361,10 @@ impl Engine { // Must be either `var[index] op= val` or `var.prop op= val`. // The return value of any op-assignment (should be `()`) is thrown away and not used. let _ = match lhs { + // this op= rhs (handled above) + Expr::ThisPtr(..) => { + unreachable!("Expr::ThisPtr case is already handled") + } // name op= rhs (handled above) Expr::Variable(..) => { unreachable!("Expr::Variable case is already handled") @@ -861,9 +884,12 @@ impl Engine { } let v = self.eval_expr(global, caches, scope, this_ptr, expr)?; - let typ = v.type_name(); - let path = v.try_cast::().ok_or_else(|| { - self.make_type_mismatch_err::(typ, expr.position()) + + let path = v.try_cast_raw::().map_err(|v| { + self.make_type_mismatch_err::( + v.type_name(), + expr.position(), + ) })?; let path_pos = expr.start_position(); diff --git a/src/func/call.rs b/src/func/call.rs index f52fb915..9e7e153d 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -796,9 +796,11 @@ impl Engine { debug_assert!(!call_args.is_empty()); // FnPtr call on object - let typ = call_args[0].type_name(); - let fn_ptr = call_args[0].take().try_cast::().ok_or_else(|| { - self.make_type_mismatch_err::(self.map_type_name(typ), first_arg_pos) + let fn_ptr = call_args[0].take().try_cast_raw::().map_err(|v| { + self.make_type_mismatch_err::( + self.map_type_name(v.type_name()), + first_arg_pos, + ) })?; #[cfg(not(feature = "no_function"))] @@ -1025,9 +1027,11 @@ impl Engine { let (first_arg_value, first_arg_pos) = self.get_arg_value(global, caches, scope, this_ptr.as_deref_mut(), arg)?; - let typ = first_arg_value.type_name(); - let fn_ptr = first_arg_value.try_cast::().ok_or_else(|| { - self.make_type_mismatch_err::(self.map_type_name(typ), first_arg_pos) + let fn_ptr = first_arg_value.try_cast_raw::().map_err(|v| { + self.make_type_mismatch_err::( + self.map_type_name(v.type_name()), + first_arg_pos, + ) })?; #[cfg(not(feature = "no_function"))] @@ -1104,9 +1108,11 @@ impl Engine { let (first_arg_value, first_arg_pos) = self.get_arg_value(global, caches, scope, this_ptr.as_deref_mut(), first)?; - let typ = first_arg_value.type_name(); - let mut fn_ptr = first_arg_value.try_cast::().ok_or_else(|| { - self.make_type_mismatch_err::(self.map_type_name(typ), first_arg_pos) + let mut fn_ptr = first_arg_value.try_cast_raw::().map_err(|v| { + self.make_type_mismatch_err::( + self.map_type_name(v.type_name()), + first_arg_pos, + ) })?; // Append the new curried arguments to the existing list. @@ -1282,14 +1288,34 @@ impl Engine { } // Call with blank scope - if num_args > 0 || !curry.is_empty() { - // If the first argument is a variable, and there is no curried arguments, - // convert to method-call style in order to leverage potential &mut first argument and - // avoid cloning the value - if curry.is_empty() && first_arg.map_or(false, |expr| expr.is_variable_access(false)) { - let first_expr = first_arg.unwrap(); + #[cfg(not(feature = "no_closure"))] + let this_ptr_not_shared = this_ptr.as_ref().map_or(false, |v| !v.is_shared()); + #[cfg(feature = "no_closure")] + let this_ptr_not_shared = true; - self.track_operation(global, first_expr.position())?; + // If the first argument is a variable, and there are no curried arguments, + // convert to method-call style in order to leverage potential &mut first argument + // and avoid cloning the value. + match first_arg { + Some(_first_expr @ Expr::ThisPtr(pos)) if curry.is_empty() && this_ptr_not_shared => { + // Turn it into a method call only if the object is not shared + self.track_operation(global, *pos)?; + + #[cfg(feature = "debugging")] + self.run_debugger(global, caches, scope, this_ptr.as_deref_mut(), _first_expr)?; + + // func(x, ...) -> x.func(...) + for expr in args_expr { + let (value, ..) = + self.get_arg_value(global, caches, scope, this_ptr.as_deref_mut(), expr)?; + arg_values.push(value.flatten()); + } + + is_ref_mut = true; + args.push(this_ptr.unwrap()); + } + Some(first_expr @ Expr::Variable(.., pos)) if curry.is_empty() => { + self.track_operation(global, *pos)?; #[cfg(feature = "debugging")] self.run_debugger(global, caches, scope, this_ptr.as_deref_mut(), first_expr)?; @@ -1316,7 +1342,8 @@ impl Engine { let obj_ref = target.take_ref().expect("ref"); args.push(obj_ref); } - } else { + } + _ => { // func(..., ...) for expr in first_arg.into_iter().chain(args_expr.iter()) { let (value, ..) = @@ -1325,10 +1352,10 @@ impl Engine { } args.extend(curry.iter_mut()); } - - args.extend(arg_values.iter_mut()); } + args.extend(arg_values.iter_mut()); + self.exec_fn_call( global, caches, None, name, op_token, hashes, &mut args, is_ref_mut, false, pos, ) @@ -1353,18 +1380,32 @@ impl Engine { let args = &mut FnArgsVec::with_capacity(args_expr.len()); let mut first_arg_value = None; - if !args_expr.is_empty() { - // See if the first argument is a variable (not namespace-qualified). - // If so, convert to method-call style in order to leverage potential &mut first argument - // and avoid cloning the value - if !args_expr.is_empty() && args_expr[0].is_variable_access(true) { - // Get target reference to first argument - let first_arg = &args_expr[0]; + #[cfg(not(feature = "no_closure"))] + let this_ptr_not_shared = this_ptr.as_ref().map_or(false, |v| !v.is_shared()); + #[cfg(feature = "no_closure")] + let this_ptr_not_shared = true; - self.track_operation(global, first_arg.position())?; + // 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) { + Some(_first_expr @ Expr::ThisPtr(pos)) if this_ptr_not_shared => { + self.track_operation(global, *pos)?; #[cfg(feature = "debugging")] - self.run_debugger(global, caches, scope, this_ptr.as_deref_mut(), first_arg)?; + self.run_debugger(global, caches, scope, this_ptr.as_deref_mut(), _first_expr)?; + + // Turn it into a method call only if the object is not shared + let (first, rest) = arg_values.split_first_mut().unwrap(); + first_arg_value = Some(first); + args.push(this_ptr.unwrap()); + args.extend(rest.iter_mut()); + } + Some(first_expr @ Expr::Variable(.., pos)) => { + self.track_operation(global, *pos)?; + + #[cfg(feature = "debugging")] + self.run_debugger(global, caches, scope, this_ptr.as_deref_mut(), first_expr)?; // func(x, ...) -> x.func(...) arg_values.push(Dynamic::UNIT); @@ -1375,14 +1416,9 @@ impl Engine { arg_values.push(value.flatten()); } - let target = self.search_scope_only(global, caches, scope, this_ptr, first_arg)?; + let target = self.search_namespace(global, caches, scope, this_ptr, first_expr)?; - #[cfg(not(feature = "no_closure"))] - let target_is_shared = target.is_shared(); - #[cfg(feature = "no_closure")] - let target_is_shared = false; - - if target_is_shared || target.is_temp_value() { + if target.is_shared() || target.is_temp_value() { arg_values[0] = target.take_or_clone().flatten(); args.extend(arg_values.iter_mut()); } else { @@ -1393,7 +1429,8 @@ impl Engine { args.push(obj_ref); args.extend(rest.iter_mut()); } - } else { + } + Some(_) => { // func(..., ...) or func(mod::x, ...) for expr in args_expr { let (value, ..) = @@ -1402,6 +1439,7 @@ impl Engine { } args.extend(arg_values.iter_mut()); } + None => (), } // Search for the root namespace diff --git a/src/func/native.rs b/src/func/native.rs index 300daaef..28863635 100644 --- a/src/func/native.rs +++ b/src/func/native.rs @@ -10,7 +10,7 @@ use crate::{ calc_fn_hash, Dynamic, Engine, EvalContext, FnArgsVec, FuncArgs, Position, RhaiResult, RhaiResultOf, StaticVec, VarDefInfo, ERR, }; -use std::any::{type_name, TypeId}; +use std::any::type_name; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -313,16 +313,18 @@ impl<'a> NativeCallContext<'a> { self._call_fn_raw(fn_name, args, false, false, false) .and_then(|result| { - // Bail out early if the return type needs no cast - if TypeId::of::() == TypeId::of::() { - return Ok(reify! { result => T }); - } - - let typ = self.engine().map_type_name(result.type_name()); - - result.try_cast().ok_or_else(|| { - let t = self.engine().map_type_name(type_name::()).into(); - ERR::ErrorMismatchOutputType(t, typ.into(), Position::NONE).into() + result.try_cast_raw().map_err(|r| { + let result_type = self.engine().map_type_name(r.type_name()); + let cast_type = match type_name::() { + typ @ _ if typ.contains("::") => self.engine.map_type_name(typ), + typ @ _ => typ, + }; + ERR::ErrorMismatchOutputType( + cast_type.into(), + result_type.into(), + Position::NONE, + ) + .into() }) }) } @@ -344,16 +346,18 @@ impl<'a> NativeCallContext<'a> { self._call_fn_raw(fn_name, args, true, false, false) .and_then(|result| { - // Bail out early if the return type needs no cast - if TypeId::of::() == TypeId::of::() { - return Ok(reify! { result => T }); - } - - let typ = self.engine().map_type_name(result.type_name()); - - result.try_cast().ok_or_else(|| { - let t = self.engine().map_type_name(type_name::()).into(); - ERR::ErrorMismatchOutputType(t, typ.into(), Position::NONE).into() + result.try_cast_raw().map_err(|r| { + let result_type = self.engine().map_type_name(r.type_name()); + let cast_type = match type_name::() { + typ @ _ if typ.contains("::") => self.engine.map_type_name(typ), + typ @ _ => typ, + }; + ERR::ErrorMismatchOutputType( + cast_type.into(), + result_type.into(), + Position::NONE, + ) + .into() }) }) } diff --git a/src/func/script.rs b/src/func/script.rs index 4040316f..8da0e8ba 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -6,7 +6,6 @@ use crate::ast::ScriptFnDef; use crate::eval::{Caches, GlobalRuntimeState}; use crate::func::EncapsulatedEnviron; use crate::{Dynamic, Engine, Position, RhaiResult, Scope, ERR}; -use std::mem; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -100,7 +99,7 @@ impl Engine { global.lib.push(lib.clone()); - mem::replace(&mut global.constants, constants.clone()) + std::mem::replace(&mut global.constants, constants.clone()) }); #[cfg(feature = "debugging")] diff --git a/src/optimizer.rs b/src/optimizer.rs index 876702c2..9bfe5239 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -901,8 +901,8 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { *expr = mem::take(&mut m.0).into_iter().find(|(x, ..)| x.as_str() == prop) .map_or_else(|| Expr::Unit(*pos), |(.., mut expr)| { expr.set_position(*pos); expr }); } - // var.rhs - (Expr::Variable(..), rhs) => optimize_expr(rhs, state, true), + // var.rhs or this.rhs + (Expr::Variable(..) | Expr::ThisPtr(..), rhs) => optimize_expr(rhs, state, true), // const.type_of() (lhs, Expr::MethodCall(x, pos)) if lhs.is_constant() && x.name == KEYWORD_TYPE_OF && x.args.is_empty() => { if let Some(value) = lhs.get_literal_value() { @@ -987,8 +987,8 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { state.set_dirty(); *expr = Expr::CharConstant(s.chars().rev().nth(i.unsigned_abs() as usize - 1).unwrap(), *pos); } - // var[rhs] - (Expr::Variable(..), rhs) => optimize_expr(rhs, state, true), + // var[rhs] or this[rhs] + (Expr::Variable(..) | Expr::ThisPtr(..), rhs) => optimize_expr(rhs, state, true), // lhs[rhs] (lhs, rhs) => { optimize_expr(lhs, state, false); optimize_expr(rhs, state, true); } }, diff --git a/src/packages/lang_core.rs b/src/packages/lang_core.rs index 26815f43..6020c7ee 100644 --- a/src/packages/lang_core.rs +++ b/src/packages/lang_core.rs @@ -26,6 +26,30 @@ def_package! { #[export_module] mod core_functions { + /// Take ownership of the data in a `Dynamic` value and return it. + /// The data is _NOT_ cloned. + /// + /// The original value is replaced with `()`. + /// + /// # Example + /// + /// ```rhai + /// let x = 42; + /// + /// print(take(x)); // prints 42 + /// + /// print(x); // prints () + /// ``` + #[rhai_fn(return_raw)] + pub fn take(value: &mut Dynamic) -> RhaiResultOf { + if value.is_read_only() { + return Err( + ERR::ErrorNonPureMethodCallOnConstant("take".to_string(), Position::NONE).into(), + ); + } + + Ok(std::mem::take(value)) + } /// Return the _tag_ of a `Dynamic` value. /// /// # Example diff --git a/src/parser.rs b/src/parser.rs index 27ae8301..4009ad92 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -7,7 +7,7 @@ use crate::ast::{ FnCallHashes, Ident, Namespace, OpAssignment, RangeCase, ScriptFnDef, Stmt, StmtBlock, StmtBlockContainer, SwitchCasesCollection, }; -use crate::engine::{Precedence, KEYWORD_THIS, OP_CONTAINS, OP_NOT}; +use crate::engine::{Precedence, OP_CONTAINS, OP_NOT}; use crate::eval::{Caches, GlobalRuntimeState}; use crate::func::{hashing::get_hasher, StraightHashMap}; use crate::tokenizer::{ @@ -1750,21 +1750,19 @@ impl Engine { settings.pos, ) } - // Access to `this` as a variable is OK within a function scope + // Access to `this` as a variable #[cfg(not(feature = "no_function"))] - _ if *s == KEYWORD_THIS && settings.has_flag(ParseSettingFlags::FN_SCOPE) => { - Expr::Variable( - (None, ns, 0, state.get_interned_string(*s)).into(), - None, - settings.pos, - ) - } - // Cannot access to `this` as a variable not in a function scope - _ if *s == KEYWORD_THIS => { - let msg = format!("'{s}' can only be used in functions"); - return Err( - LexError::ImproperSymbol(s.to_string(), msg).into_err(settings.pos) - ); + _ if *s == crate::engine::KEYWORD_THIS => { + // OK within a function scope + if settings.has_flag(ParseSettingFlags::FN_SCOPE) { + Expr::ThisPtr(settings.pos) + } else { + // Cannot access to `this` as a variable not in a function scope + let msg = format!("'{s}' can only be used in functions"); + return Err( + LexError::ImproperSymbol(s.to_string(), msg).into_err(settings.pos) + ); + } } _ => return Err(PERR::Reserved(s.to_string()).into_err(settings.pos)), } @@ -2148,10 +2146,8 @@ impl Engine { }; match lhs { - // const_expr = rhs - ref expr if expr.is_constant() => { - Err(PERR::AssignmentToConstant(String::new()).into_err(lhs.start_position())) - } + // this = rhs + Expr::ThisPtr(_) => Ok(Stmt::Assignment((op_info, (lhs, rhs).into()).into())), // var (non-indexed) = rhs Expr::Variable(ref x, None, _) if x.0.is_none() => { Ok(Stmt::Assignment((op_info, (lhs, rhs).into()).into())) @@ -2186,8 +2182,8 @@ impl Engine { match valid_lvalue { None => { match x.lhs { - // var[???] = rhs, var.??? = rhs - Expr::Variable(..) => { + // var[???] = rhs, this[???] = rhs, var.??? = rhs, this.??? = rhs + Expr::Variable(..) | Expr::ThisPtr(..) => { Ok(Stmt::Assignment((op_info, (lhs, rhs).into()).into())) } // expr[???] = rhs, expr.??? = rhs @@ -2200,6 +2196,10 @@ impl Engine { } } } + // const_expr = rhs + ref expr if expr.is_constant() => { + Err(PERR::AssignmentToConstant(String::new()).into_err(lhs.start_position())) + } // ??? && ??? = rhs, ??? || ??? = rhs, xxx ?? xxx = rhs Expr::And(..) | Expr::Or(..) | Expr::Coalesce(..) if !op_info.is_op_assignment() => { Err(LexError::ImproperSymbol( diff --git a/src/types/dynamic.rs b/src/types/dynamic.rs index c3887d86..7650ad9c 100644 --- a/src/types/dynamic.rs +++ b/src/types/dynamic.rs @@ -1174,108 +1174,120 @@ impl Dynamic { /// /// assert_eq!(x.try_cast::().expect("x should be u32"), 42); /// ``` - #[inline] + #[inline(always)] #[must_use] #[allow(unused_mut)] pub fn try_cast(mut self) -> Option { + self.try_cast_raw().ok() + } + /// Convert the [`Dynamic`] value into specific type. + /// + /// Casting to a [`Dynamic`] just returns as is, but if it contains a shared value, + /// it is cloned into a [`Dynamic`] with a normal value. + /// + /// Returns itself if types mismatched. + #[allow(unused_mut)] + pub(crate) fn try_cast_raw(mut self) -> Result { // Coded this way in order to maximally leverage potentials for dead-code removal. #[cfg(not(feature = "no_closure"))] self.flatten_in_place(); if TypeId::of::() == TypeId::of::() { - return Some(reify! { self => !!! T }); + return Ok(reify! { self => !!! T }); } if TypeId::of::() == TypeId::of::<()>() { return match self.0 { - Union::Unit(..) => Some(reify! { () => !!! T }), - _ => None, + Union::Unit(..) => Ok(reify! { () => !!! T }), + _ => Err(self), }; } if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Int(n, ..) => Some(reify! { n => !!! T }), - _ => None, + Union::Int(n, ..) => Ok(reify! { n => !!! T }), + _ => Err(self), }; } #[cfg(not(feature = "no_float"))] if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Float(v, ..) => Some(reify! { *v => !!! T }), - _ => None, + Union::Float(v, ..) => Ok(reify! { *v => !!! T }), + _ => Err(self), }; } #[cfg(feature = "decimal")] if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Decimal(v, ..) => Some(reify! { *v => !!! T }), - _ => None, + Union::Decimal(v, ..) => Ok(reify! { *v => !!! T }), + _ => Err(self), }; } if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Bool(b, ..) => Some(reify! { b => !!! T }), - _ => None, + Union::Bool(b, ..) => Ok(reify! { b => !!! T }), + _ => Err(self), }; } if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Str(s, ..) => Some(reify! { s => !!! T }), - _ => None, + Union::Str(s, ..) => Ok(reify! { s => !!! T }), + _ => Err(self), }; } if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Str(s, ..) => Some(reify! { s.to_string() => !!! T }), - _ => None, + Union::Str(s, ..) => Ok(reify! { s.to_string() => !!! T }), + _ => Err(self), }; } if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Char(c, ..) => Some(reify! { c => !!! T }), - _ => None, + Union::Char(c, ..) => Ok(reify! { c => !!! T }), + _ => Err(self), }; } #[cfg(not(feature = "no_index"))] if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Array(a, ..) => Some(reify! { *a => !!! T }), - _ => None, + Union::Array(a, ..) => Ok(reify! { *a => !!! T }), + _ => Err(self), }; } #[cfg(not(feature = "no_index"))] if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Blob(b, ..) => Some(reify! { *b => !!! T }), - _ => None, + Union::Blob(b, ..) => Ok(reify! { *b => !!! T }), + _ => Err(self), }; } #[cfg(not(feature = "no_object"))] if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::Map(m, ..) => Some(reify! { *m => !!! T }), - _ => None, + Union::Map(m, ..) => Ok(reify! { *m => !!! T }), + _ => Err(self), }; } if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::FnPtr(f, ..) => Some(reify! { *f => !!! T }), - _ => None, + Union::FnPtr(f, ..) => Ok(reify! { *f => !!! T }), + _ => Err(self), }; } #[cfg(not(feature = "no_time"))] if TypeId::of::() == TypeId::of::() { return match self.0 { - Union::TimeStamp(t, ..) => Some(reify! { *t => !!! T }), - _ => None, + Union::TimeStamp(t, ..) => Ok(reify! { *t => !!! T }), + _ => Err(self), }; } match self.0 { - Union::Variant(v, ..) => (*v).as_boxed_any().downcast().ok().map(|x| *x), + Union::Variant(v, ..) if TypeId::of::() == (**v).type_id() => { + Ok((*v).as_boxed_any().downcast().map(|x| *x).unwrap()) + } #[cfg(not(feature = "no_closure"))] Union::Shared(..) => unreachable!("Union::Shared case should be already handled"), - _ => None, + _ => Err(self), } } /// Convert the [`Dynamic`] value into a specific type. diff --git a/src/types/fn_ptr.rs b/src/types/fn_ptr.rs index abe6e713..ba13ab9d 100644 --- a/src/types/fn_ptr.rs +++ b/src/types/fn_ptr.rs @@ -11,7 +11,7 @@ use crate::{ #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{ - any::{type_name, TypeId}, + any::type_name, convert::{TryFrom, TryInto}, fmt, hash::{Hash, Hasher}, @@ -218,16 +218,14 @@ impl FnPtr { let ctx = (engine, self.fn_name(), None, &*global, Position::NONE).into(); self.call_raw(&ctx, None, arg_values).and_then(|result| { - // Bail out early if the return type needs no cast - if TypeId::of::() == TypeId::of::() { - return Ok(reify! { result => T }); - } - - let typ = engine.map_type_name(result.type_name()); - - result.try_cast().ok_or_else(|| { - let t = engine.map_type_name(type_name::()).into(); - ERR::ErrorMismatchOutputType(t, typ.into(), Position::NONE).into() + result.try_cast_raw().map_err(|r| { + let result_type = engine.map_type_name(r.type_name()); + let cast_type = match type_name::() { + typ @ _ if typ.contains("::") => engine.map_type_name(typ), + typ @ _ => typ, + }; + ERR::ErrorMismatchOutputType(cast_type.into(), result_type.into(), Position::NONE) + .into() }) }) } @@ -247,16 +245,14 @@ impl FnPtr { args.parse(&mut arg_values); self.call_raw(context, None, arg_values).and_then(|result| { - // Bail out early if the return type needs no cast - if TypeId::of::() == TypeId::of::() { - return Ok(reify! { result => T }); - } - - let typ = context.engine().map_type_name(result.type_name()); - - result.try_cast().ok_or_else(|| { - let t = context.engine().map_type_name(type_name::()).into(); - ERR::ErrorMismatchOutputType(t, typ.into(), Position::NONE).into() + result.try_cast_raw().map_err(|r| { + let result_type = context.engine().map_type_name(r.type_name()); + let cast_type = match type_name::() { + typ @ _ if typ.contains("::") => context.engine().map_type_name(typ), + typ @ _ => typ, + }; + ERR::ErrorMismatchOutputType(cast_type.into(), result_type.into(), Position::NONE) + .into() }) }) } diff --git a/tests/internal_fn.rs b/tests/internal_fn.rs index f440e479..38c06741 100644 --- a/tests/internal_fn.rs +++ b/tests/internal_fn.rs @@ -64,6 +64,48 @@ fn test_internal_fn() -> Result<(), Box> { Ok(()) } +#[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Default)] +struct TestStruct(INT); + +impl Clone for TestStruct { + fn clone(&self) -> Self { + Self(self.0 + 1) + } +} + +#[test] +fn test_internal_fn_take() -> Result<(), Box> { + let mut engine = Engine::new(); + + engine + .register_type_with_name::("TestStruct") + .register_fn("new_ts", |x: INT| TestStruct(x)); + + assert_eq!( + engine.eval::( + " + let x = new_ts(0); + for n in 0..41 { x = x } + x + ", + )?, + TestStruct(42) + ); + + assert_eq!( + engine.eval::( + " + let x = new_ts(0); + for n in 0..41 { x = take(x) } + take(x) + ", + )?, + TestStruct(0) + ); + + Ok(()) +} + #[test] fn test_internal_fn_big() -> Result<(), Box> { let engine = Engine::new(); diff --git a/tests/mismatched_op.rs b/tests/mismatched_op.rs index dd094a45..ca7b5d93 100644 --- a/tests/mismatched_op.rs +++ b/tests/mismatched_op.rs @@ -10,6 +10,21 @@ fn test_mismatched_op() { )); } +#[test] +fn test_mismatched_op_name() { + let engine = Engine::new(); + + assert!(matches!( + *engine.eval::("true").expect_err("expects error"), + EvalAltResult::ErrorMismatchOutputType(need, actual, ..) if need == "string" && actual == "bool" + )); + + assert!(matches!( + *engine.eval::<&str>("true").expect_err("expects error"), + EvalAltResult::ErrorMismatchOutputType(need, actual, ..) if need == "&str" && actual == "bool" + )); +} + #[test] #[cfg(not(feature = "no_object"))] fn test_mismatched_op_custom_type() -> Result<(), Box> {