From 7262d63909d5a0500ee32047421bda0d855758b6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 2 Mar 2021 23:08:54 +0800 Subject: [PATCH] Move default comparisons into builtin. --- src/engine_api.rs | 17 ++++++++- src/{builtin.rs => fn_builtin.rs} | 46 +++++++++++++++++++--- src/fn_call.rs | 5 +-- src/fn_register.rs | 2 +- src/lib.rs | 2 +- src/optimize.rs | 2 +- src/packages/logic.rs | 63 ------------------------------- 7 files changed, 60 insertions(+), 77 deletions(-) rename src/{builtin.rs => fn_builtin.rs} (96%) diff --git a/src/engine_api.rs b/src/engine_api.rs index 2e8e31f6..45edace6 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -71,6 +71,7 @@ impl Engine { /// /// impl TestStruct { /// fn new() -> Self { Self { field: 1 } } + /// /// fn update(&mut self, offset: i64) { self.field += offset; } /// } /// @@ -170,6 +171,7 @@ impl Engine { /// /// impl TestStruct { /// fn new() -> Self { Self { field: 1 } } + /// /// // Even a getter must start with `&mut self` and not `&self`. /// fn get_field(&mut self) -> i64 { self.field } /// } @@ -216,8 +218,9 @@ impl Engine { /// /// impl TestStruct { /// fn new() -> Self { Self { field: 1 } } + /// /// // Even a getter must start with `&mut self` and not `&self`. - /// fn get_field(&mut self) -> RhaiResult { + /// fn get_field(&mut self) -> Result> { /// Ok(self.field.into()) /// } /// } @@ -258,6 +261,7 @@ impl Engine { /// /// impl TestStruct { /// fn new() -> Self { Self { field: 1 } } + /// /// fn set_field(&mut self, new_val: i64) { self.field = new_val; } /// } /// @@ -305,6 +309,7 @@ impl Engine { /// /// impl TestStruct { /// fn new() -> Self { Self { field: 1 } } + /// /// fn set_field(&mut self, new_val: i64) -> Result<(), Box> { /// self.field = new_val; /// Ok(()) @@ -356,8 +361,10 @@ impl Engine { /// /// impl TestStruct { /// fn new() -> Self { Self { field: 1 } } + /// /// // Even a getter must start with `&mut self` and not `&self`. /// fn get_field(&mut self) -> i64 { self.field } + /// /// fn set_field(&mut self, new_val: i64) { self.field = new_val; } /// } /// @@ -407,6 +414,7 @@ impl Engine { /// /// impl TestStruct { /// fn new() -> Self { Self { fields: vec![1, 2, 3, 4, 5] } } + /// /// // Even a getter must start with `&mut self` and not `&self`. /// fn get_field(&mut self, index: i64) -> i64 { self.fields[index as usize] } /// } @@ -473,8 +481,9 @@ impl Engine { /// /// impl TestStruct { /// fn new() -> Self { Self { fields: vec![1, 2, 3, 4, 5] } } + /// /// // Even a getter must start with `&mut self` and not `&self`. - /// fn get_field(&mut self, index: i64) -> RhaiResult { + /// fn get_field(&mut self, index: i64) -> Result> { /// Ok(self.fields[index as usize].into()) /// } /// } @@ -535,6 +544,7 @@ impl Engine { /// /// impl TestStruct { /// fn new() -> Self { Self { fields: vec![1, 2, 3, 4, 5] } } + /// /// fn set_field(&mut self, index: i64, value: i64) { self.fields[index as usize] = value; } /// } /// @@ -601,6 +611,7 @@ impl Engine { /// /// impl TestStruct { /// fn new() -> Self { Self { fields: vec![1, 2, 3, 4, 5] } } + /// /// fn set_field(&mut self, index: i64, value: i64) -> Result<(), Box> { /// self.fields[index as usize] = value; /// Ok(()) @@ -672,8 +683,10 @@ impl Engine { /// /// impl TestStruct { /// fn new() -> Self { Self { fields: vec![1, 2, 3, 4, 5] } } + /// /// // Even a getter must start with `&mut self` and not `&self`. /// fn get_field(&mut self, index: i64) -> i64 { self.fields[index as usize] } + /// /// fn set_field(&mut self, index: i64, value: i64) { self.fields[index as usize] = value; } /// } /// diff --git a/src/builtin.rs b/src/fn_builtin.rs similarity index 96% rename from src/builtin.rs rename to src/fn_builtin.rs index 6090ec07..6a48f263 100644 --- a/src/builtin.rs +++ b/src/fn_builtin.rs @@ -14,6 +14,29 @@ use rust_decimal::Decimal; #[cfg(not(feature = "no_float"))] use num_traits::float::Float; +/// Is the type a numeric type? +fn is_numeric(type_id: TypeId) -> bool { + let result = type_id == TypeId::of::() + || type_id == TypeId::of::() + || type_id == TypeId::of::() + || type_id == TypeId::of::() + || type_id == TypeId::of::() + || type_id == TypeId::of::() + || type_id == TypeId::of::() + || type_id == TypeId::of::(); + + #[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))] + let result = result || type_id == TypeId::of::() || type_id == TypeId::of::(); + + #[cfg(not(feature = "no_float"))] + let result = result || type_id == TypeId::of::() || type_id == TypeId::of::(); + + #[cfg(feature = "decimal")] + let result = result || type_id == TypeId::of::(); + + result +} + /// Build in common binary operator implementations to avoid the cost of calling a registered function. pub fn get_builtin_binary_op_fn( op: &str, @@ -23,13 +46,24 @@ pub fn get_builtin_binary_op_fn( let type1 = x.type_id(); let type2 = y.type_id(); + // One of the operands is a custom type, so it is never built-in if x.is_variant() || y.is_variant() { - // One of the operands is a custom type, so it is never built-in - return match op { - "!=" if type1 != type2 => Some(|_, _| Ok(Dynamic::TRUE)), - "==" | ">" | ">=" | "<" | "<=" if type1 != type2 => Some(|_, _| Ok(Dynamic::FALSE)), - _ => None, - }; + if is_numeric(type1) && is_numeric(type2) { + // Disallow comparisons between different numeric types + return None; + } + + // If the types are not the same, default to not compare + if type1 != type2 { + return match op { + "!=" => Some(|_, _| Ok(Dynamic::TRUE)), + "==" | ">" | ">=" | "<" | "<=" => Some(|_, _| Ok(Dynamic::FALSE)), + _ => None, + }; + } + + // Disallow comparisons between the same type + return None; } let types_pair = (type1, type2); diff --git a/src/fn_call.rs b/src/fn_call.rs index ef11cd4c..20b09bf4 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -1,11 +1,11 @@ //! Implement function-calling mechanism for [`Engine`]. -use crate::builtin::{get_builtin_binary_op_fn, get_builtin_op_assignment_fn}; use crate::engine::{ Imports, State, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_FN_PTR_CALL, KEYWORD_FN_PTR_CURRY, KEYWORD_IS_DEF_VAR, KEYWORD_PRINT, KEYWORD_TYPE_OF, MAX_DYNAMIC_PARAMETERS, }; +use crate::fn_builtin::{get_builtin_binary_op_fn, get_builtin_op_assignment_fn}; use crate::fn_native::{FnAny, FnCallArgs}; use crate::module::NamespaceRef; use crate::optimize::OptimizationLevel; @@ -243,8 +243,7 @@ impl Engine { // Stop when all permutations are exhausted None if bitmask >= max_bitmask => { - return if num_args != 2 || args[0].is_variant() || args[1].is_variant() - { + return if num_args != 2 { None } else if !is_op_assignment { if let Some(f) = diff --git a/src/fn_register.rs b/src/fn_register.rs index b63572b1..77329412 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -52,7 +52,7 @@ pub trait RegisterResultFn { /// use rhai::{Engine, Dynamic, RegisterResultFn, EvalAltResult}; /// /// // Normal function - /// fn div(x: i64, y: i64) -> RhaiResult { + /// fn div(x: i64, y: i64) -> Result> { /// if y == 0 { /// // '.into()' automatically converts to 'Box' /// Err("division by zero!".into()) diff --git a/src/lib.rs b/src/lib.rs index aee23e1d..166b0c07 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -64,12 +64,12 @@ extern crate alloc; // Internal modules mod ast; -mod builtin; mod dynamic; mod engine; mod engine_api; mod engine_settings; mod fn_args; +mod fn_builtin; mod fn_call; mod fn_func; mod fn_native; diff --git a/src/optimize.rs b/src/optimize.rs index 967a657d..b484e643 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -1,9 +1,9 @@ //! Module implementing the [`AST`] optimizer. use crate::ast::{Expr, ScriptFnDef, Stmt}; -use crate::builtin::get_builtin_binary_op_fn; use crate::dynamic::AccessMode; use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_PRINT, KEYWORD_TYPE_OF}; +use crate::fn_builtin::get_builtin_binary_op_fn; use crate::parser::map_dynamic_to_expr; use crate::stdlib::{ boxed::Box, diff --git a/src/packages/logic.rs b/src/packages/logic.rs index 5466b0a2..e6b6af54 100644 --- a/src/packages/logic.rs +++ b/src/packages/logic.rs @@ -1,6 +1,5 @@ #![allow(non_snake_case)] -use crate::builtin::get_builtin_binary_op_fn; use crate::def_package; use crate::plugin::*; @@ -40,8 +39,6 @@ macro_rules! reg_functions { } def_package!(crate:LogicPackage:"Logical operators.", lib, { - combine_with_exported_module!(lib, "logic", logic_functions); - #[cfg(not(feature = "only_i32"))] #[cfg(not(feature = "only_i64"))] { @@ -97,66 +94,6 @@ gen_cmp_functions!(float => f64); #[cfg(feature = "decimal")] gen_cmp_functions!(decimal => Decimal); -#[export_module] -mod logic_functions { - fn is_numeric(type_id: TypeId) -> bool { - let result = type_id == TypeId::of::() - || type_id == TypeId::of::() - || type_id == TypeId::of::() - || type_id == TypeId::of::() - || type_id == TypeId::of::() - || type_id == TypeId::of::() - || type_id == TypeId::of::() - || type_id == TypeId::of::(); - - #[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))] - let result = result || type_id == TypeId::of::() || type_id == TypeId::of::(); - - #[cfg(not(feature = "no_float"))] - let result = result || type_id == TypeId::of::() || type_id == TypeId::of::(); - - #[cfg(feature = "decimal")] - let result = result || type_id == TypeId::of::(); - - result - } - - #[rhai_fn( - name = "==", - name = "!=", - name = ">", - name = ">=", - name = "<", - name = "<=", - return_raw, - pure - )] - pub fn cmp( - ctx: NativeCallContext, - x: &mut Dynamic, - mut y: Dynamic, - ) -> Result> { - let type_x = x.type_id(); - let type_y = y.type_id(); - - if type_x != type_y && is_numeric(type_x) && is_numeric(type_y) { - // Disallow comparisons between different number types - } else if let Some(f) = get_builtin_binary_op_fn(ctx.fn_name(), x, &y) { - return f(ctx, &mut [x, &mut y]); - } - - Err(Box::new(EvalAltResult::ErrorFunctionNotFound( - format!( - "{} ({}, {})", - ctx.fn_name(), - ctx.engine().map_type_name(x.type_name()), - ctx.engine().map_type_name(y.type_name()) - ), - Position::NONE, - ))) - } -} - #[cfg(not(feature = "no_float"))] #[export_module] mod f32_functions {