Do not default compares of different types.

CHANGELOG.md

@@ -13,6 +13,7 @@ Bug fixes
 * `x += y` where `x` and `y` are `char` now works correctly.
 * Expressions such as `!inside` now parses correctly instead of as `!in` followed by `side`.
 * Custom syntax starting with symbols now works correctly and no longer raises a parse error.
+* Comparing different custom types now works correctly when the appropriate comparison operators are registered.
 
 Potentially breaking changes
 ----------------------------

src/eval/data_check.rs

@@ -8,17 +8,16 @@ use std::borrow::Borrow;
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
 
-impl Dynamic {
+/// Recursively calculate the sizes of an array.
-    /// Recursively calculate the sizes of an array.
+///
-    ///
+/// Sizes returned are `(` [`Array`][crate::Array], [`Map`][crate::Map] and [`String`] `)`.
-    /// Sizes returned are `(` [`Array`][crate::Array], [`Map`][crate::Map] and [`String`] `)`.
+///
-    ///
+/// # Panics
-    /// # Panics
+///
-    ///
+/// Panics if any interior data is shared (should never happen).
-    /// Panics if any interior data is shared (should never happen).
+#[cfg(not(feature = "no_index"))]
-    #[cfg(not(feature = "no_index"))]
+#[inline]
-    #[inline]
+pub fn calc_array_sizes(array: &crate::Array) -> (usize, usize, usize) {
-    pub(crate) fn calc_array_sizes(array: &crate::Array) -> (usize, usize, usize) {
     let (mut ax, mut mx, mut sx) = (0, 0, 0);
 
     for value in array {
@@ -26,7 +25,7 @@ impl Dynamic {
 
         match value.0 {
             Union::Array(ref a, ..) => {
-                    let (a, m, s) = Self::calc_array_sizes(a);
+                let (a, m, s) = calc_array_sizes(a);
                 ax += a;
                 mx += m;
                 sx += s;
@@ -34,7 +33,7 @@ impl Dynamic {
             Union::Blob(ref a, ..) => ax += 1 + a.len(),
             #[cfg(not(feature = "no_object"))]
             Union::Map(ref m, ..) => {
-                    let (a, m, s) = Self::calc_map_sizes(m);
+                let (a, m, s) = calc_map_sizes(m);
                 ax += a;
                 mx += m;
                 sx += s;
@@ -49,17 +48,17 @@ impl Dynamic {
     }
 
     (ax, mx, sx)
-    }
+}
-    /// Recursively calculate the sizes of a map.
+/// Recursively calculate the sizes of a map.
-    ///
+///
-    /// Sizes returned are `(` [`Array`][crate::Array], [`Map`][crate::Map] and [`String`] `)`.
+/// Sizes returned are `(` [`Array`][crate::Array], [`Map`][crate::Map] and [`String`] `)`.
-    ///
+///
-    /// # Panics
+/// # Panics
-    ///
+///
-    /// Panics if any interior data is shared (should never happen).
+/// Panics if any interior data is shared (should never happen).
-    #[cfg(not(feature = "no_object"))]
+#[cfg(not(feature = "no_object"))]
-    #[inline]
+#[inline]
-    pub(crate) fn calc_map_sizes(map: &crate::Map) -> (usize, usize, usize) {
+pub fn calc_map_sizes(map: &crate::Map) -> (usize, usize, usize) {
     let (mut ax, mut mx, mut sx) = (0, 0, 0);
 
     for value in map.values() {
@@ -68,7 +67,7 @@ impl Dynamic {
         match value.0 {
             #[cfg(not(feature = "no_index"))]
             Union::Array(ref a, ..) => {
-                    let (a, m, s) = Self::calc_array_sizes(a);
+                let (a, m, s) = calc_array_sizes(a);
                 ax += a;
                 mx += m;
                 sx += s;
@@ -76,7 +75,7 @@ impl Dynamic {
             #[cfg(not(feature = "no_index"))]
             Union::Blob(ref a, ..) => ax += 1 + a.len(),
             Union::Map(ref m, ..) => {
-                    let (a, m, s) = Self::calc_map_sizes(m);
+                let (a, m, s) = calc_map_sizes(m);
                 ax += a;
                 mx += m;
                 sx += s;
@@ -91,7 +90,9 @@ impl Dynamic {
     }
 
     (ax, mx, sx)
-    }
+}
+
+impl Dynamic {
     /// Recursively calculate the sizes of a value.
     ///
     /// Sizes returned are `(` [`Array`][crate::Array], [`Map`][crate::Map] and [`String`] `)`.
@@ -103,11 +104,11 @@ impl Dynamic {
     pub(crate) fn calc_data_sizes(&self, _top: bool) -> (usize, usize, usize) {
         match self.0 {
             #[cfg(not(feature = "no_index"))]
-            Union::Array(ref arr, ..) => Self::calc_array_sizes(arr),
+            Union::Array(ref arr, ..) => calc_array_sizes(arr),
             #[cfg(not(feature = "no_index"))]
             Union::Blob(ref blob, ..) => (blob.len(), 0, 0),
             #[cfg(not(feature = "no_object"))]
-            Union::Map(ref map, ..) => Self::calc_map_sizes(map),
+            Union::Map(ref map, ..) => calc_map_sizes(map),
             Union::Str(ref s, ..) => (0, 0, s.len()),
             #[cfg(not(feature = "no_closure"))]
             Union::Shared(..) if _top => self.read_lock::<Self>().unwrap().calc_data_sizes(true),

src/eval/global_state.rs

@@ -127,20 +127,6 @@ impl GlobalRuntimeState {
     pub fn get_shared_import(&self, index: usize) -> Option<crate::SharedModule> {
         self.modules.as_ref().and_then(|m| m.get(index).cloned())
     }
-    /// Get a mutable reference to the globally-imported [module][crate::Module] at a
-    /// particular index.
-    ///
-    /// Not available under `no_module`.
-    #[cfg(not(feature = "no_module"))]
-    #[allow(dead_code)]
-    #[inline]
-    #[must_use]
-    pub(crate) fn get_shared_import_mut(
-        &mut self,
-        index: usize,
-    ) -> Option<&mut crate::SharedModule> {
-        self.modules.as_deref_mut().and_then(|m| m.get_mut(index))
-    }
     /// Get the index of a globally-imported [module][crate::Module] by name.
     ///
     /// Not available under `no_module`.

src/eval/mod.rs

@@ -11,6 +11,10 @@ mod target;
 pub use cache::{Caches, FnResolutionCache, FnResolutionCacheEntry};
 #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))]
 pub use chaining::ChainType;
+#[cfg(not(feature = "no_index"))]
+pub use data_check::calc_array_sizes;
+#[cfg(not(feature = "no_object"))]
+pub use data_check::calc_map_sizes;
 #[cfg(feature = "debugging")]
 pub use debugger::{
     BreakPoint, CallStackFrame, Debugger, DebuggerCommand, DebuggerEvent, DebuggerStatus,

src/func/builtin.rs

@@ -27,48 +27,6 @@ use rust_decimal::Decimal;
 /// The `unchecked` feature is not active.
 const CHECKED_BUILD: bool = cfg!(not(feature = "unchecked"));
 
-/// Is the type a numeric type?
-#[inline]
-#[must_use]
-fn is_numeric(type_id: TypeId) -> bool {
-    if type_id == TypeId::of::<INT>() {
-        return true;
-    }
-
-    #[cfg(not(feature = "only_i64"))]
-    #[cfg(not(feature = "only_i32"))]
-    if type_id == TypeId::of::<u8>()
-        || type_id == TypeId::of::<u16>()
-        || type_id == TypeId::of::<u32>()
-        || type_id == TypeId::of::<u64>()
-        || type_id == TypeId::of::<i8>()
-        || type_id == TypeId::of::<i16>()
-        || type_id == TypeId::of::<i32>()
-        || type_id == TypeId::of::<i64>()
-    {
-        return true;
-    }
-
-    #[cfg(not(feature = "only_i64"))]
-    #[cfg(not(feature = "only_i32"))]
-    #[cfg(not(target_family = "wasm"))]
-    if type_id == TypeId::of::<u128>() || type_id == TypeId::of::<i128>() {
-        return true;
-    }
-
-    #[cfg(not(feature = "no_float"))]
-    if type_id == TypeId::of::<f32>() || type_id == TypeId::of::<f64>() {
-        return true;
-    }
-
-    #[cfg(feature = "decimal")]
-    if type_id == TypeId::of::<rust_decimal::Decimal>() {
-        return true;
-    }
-
-    false
-}
-
 /// A function that returns `true`.
 #[inline(always)]
 #[allow(clippy::unnecessary_wraps)]
@@ -584,22 +542,7 @@ pub fn get_builtin_binary_op_fn(op: &Token, x: &Dynamic, y: &Dynamic) -> Option<
 
     // One of the operands is a custom type, so it is never built-in
     if x.is_variant() || y.is_variant() {
-        return if is_numeric(type1) && is_numeric(type2) {
+        return None;
-            // Disallow comparisons between different numeric types
-            None
-        } else if type1 != type2 {
-            // If the types are not the same, default to not compare
-            match op {
-                NotEqualsTo => Some((const_true_fn, false)),
-                EqualsTo | GreaterThan | GreaterThanEqualsTo | LessThan | LessThanEqualsTo => {
-                    Some((const_false_fn, false))
-                }
-                _ => None,
-            }
-        } else {
-            // Disallow comparisons between the same type
-            None
-        };
     }
 
     // Default comparison operators for different types

src/packages/array_basic.rs

@@ -2,9 +2,10 @@
 
 use crate::api::deprecated::deprecated_array_functions;
 use crate::engine::OP_EQUALS;
-use crate::eval::{calc_index, calc_offset_len};
+use crate::eval::{calc_index, calc_offset_len, calc_array_sizes};
 use crate::module::ModuleFlags;
 use crate::plugin::*;
+
 use crate::{
     def_package, Array, Dynamic, ExclusiveRange, FnPtr, InclusiveRange, NativeCallContext,
     Position, RhaiResultOf, StaticVec, ERR, INT, MAX_USIZE_INT,
@@ -237,7 +238,7 @@ pub mod array_functions {
         #[cfg(not(feature = "unchecked"))]
         if _ctx.engine().max_array_size() > 0 {
             let pad = len - array.len();
-            let (a, m, s) = Dynamic::calc_array_sizes(array);
+            let (a, m, s) = calc_array_sizes(array);
             let (ax, mx, sx) = item.calc_data_sizes(true);
 
             _ctx.engine()

tests/mismatched_op.rs

@@ -39,7 +39,10 @@ fn test_mismatched_op_custom_type() -> Result<(), Box<EvalAltResult>> {
         ").expect_err("should error"),
         EvalAltResult::ErrorFunctionNotFound(f, ..) if f == "== (TestStruct, TestStruct)"));
 
-    assert!(!engine.eval::<bool>("new_ts() == 42")?);
+    assert!(
+        matches!(*engine.eval::<bool>("new_ts() == 42").expect_err("should error"),
+        EvalAltResult::ErrorFunctionNotFound(f, ..) if f.starts_with("== (TestStruct, "))
+    );
 
     assert!(matches!(
         *engine.eval::<INT>("60 + new_ts()").expect_err("should error"),

tests/ops.rs

@@ -35,7 +35,11 @@ fn test_ops_other_number_types() -> Result<(), Box<EvalAltResult>> {
         EvalAltResult::ErrorFunctionNotFound(f, ..) if f.starts_with("== (u16,")
     ));
 
-    assert!(!engine.eval_with_scope::<bool>(&mut scope, r#"x == "hello""#)?);
+    assert!(
+        matches!(*engine.eval_with_scope::<bool>(&mut scope, r#"x == "hello""#).expect_err("should error"),
+            EvalAltResult::ErrorFunctionNotFound(f, ..) if f.starts_with("== (u16,")
+        )
+    );
 
     Ok(())
 }
@@ -63,3 +67,24 @@ fn test_ops_precedence() -> Result<(), Box<EvalAltResult>> {
 
     Ok(())
 }
+
+#[test]
+fn test_ops_custom_types() -> Result<(), Box<EvalAltResult>> {
+    #[derive(Debug, Clone, PartialEq, Eq)]
+    struct Test1;
+    #[derive(Debug, Clone, PartialEq, Eq)]
+    struct Test2;
+
+    let mut engine = Engine::new();
+
+    engine
+        .register_type_with_name::<Test1>("Test1")
+        .register_type_with_name::<Test2>("Test2")
+        .register_fn("new_ts1", || Test1)
+        .register_fn("new_ts2", || Test2)
+        .register_fn("==", |x: Test1, y: Test2| true);
+
+    assert!(engine.eval::<bool>("let x = new_ts1(); let y = new_ts2(); x == y")?);
+
+    Ok(())
+}