From 4c5ea8decc37e7bf4316523926e048300c5c3c39 Mon Sep 17 00:00:00 2001
From: Stephen Chung <schungx@live.com>
Date: Fri, 5 Mar 2021 10:33:48 +0800
Subject: [PATCH] Fix switch of non-hashable value.

---
 CHANGELOG.md    |  1 +
 src/dynamic.rs  | 28 +++++++++++++++++++++++++++-
 src/engine.rs   | 29 ++++++++++++++++++++---------
 tests/switch.rs |  4 ++++
 4 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index dcd5265f..97aea371 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ Bug fixes
 * Fixed error types in `EvalAltResult::ErrorMismatchDataType` which were swapped.
 * Some expressions involving shared variables now work properly, for example `x in shared_value`, `return shared_value`, `obj.field = shared_value` etc. Previously, the resultant value is still shared which is counter-intuitive.
 * Potential overflow panics in `range(from, to, step)` is fixed.
+* `switch` statements no longer panic on custom types.
 
 Breaking changes
 ----------------
diff --git a/src/dynamic.rs b/src/dynamic.rs
index 3354e2bf..b0168ccb 100644
--- a/src/dynamic.rs
+++ b/src/dynamic.rs
@@ -414,7 +414,7 @@ impl Hash for Dynamic {
             #[cfg(feature = "sync")]
             Union::Shared(cell, _) => (*cell.read().unwrap()).hash(state),
 
-            _ => unimplemented!(),
+            _ => unimplemented!("{} cannot be hashed", self.type_name()),
         }
     }
 }
@@ -761,6 +761,32 @@ impl Dynamic {
             _ => self.access_mode().is_read_only(),
         }
     }
+    /// Can this [`Dynamic`] be hashed?
+    pub(crate) fn is_hashable(&self) -> bool {
+        match &self.0 {
+            Union::Unit(_, _)
+            | Union::Bool(_, _)
+            | Union::Str(_, _)
+            | Union::Char(_, _)
+            | Union::Int(_, _) => true,
+
+            #[cfg(not(feature = "no_float"))]
+            Union::Float(_, _) => true,
+            #[cfg(not(feature = "no_index"))]
+            Union::Array(_, _) => true,
+            #[cfg(not(feature = "no_object"))]
+            Union::Map(_, _) => true,
+
+            #[cfg(not(feature = "no_closure"))]
+            #[cfg(not(feature = "sync"))]
+            Union::Shared(cell, _) => cell.borrow().is_hashable(),
+            #[cfg(not(feature = "no_closure"))]
+            #[cfg(feature = "sync")]
+            Union::Shared(cell, _) => cell.read().unwrap().is_hashable(),
+
+            _ => false,
+        }
+    }
     /// Create a [`Dynamic`] from any type.  A [`Dynamic`] value is simply returned as is.
     ///
     /// # Safety
diff --git a/src/engine.rs b/src/engine.rs
index e8ac7b90..7a57d972 100644
--- a/src/engine.rs
+++ b/src/engine.rs
@@ -2111,18 +2111,29 @@ impl Engine {
             Stmt::Switch(match_expr, x, _) => {
                 let (table, def_stmt) = x.as_ref();
 
-                let hasher = &mut get_hasher();
-                self.eval_expr(scope, mods, state, lib, this_ptr, match_expr, level)?
-                    .hash(hasher);
-                let hash = hasher.finish();
+                let value = self.eval_expr(scope, mods, state, lib, this_ptr, match_expr, level)?;
 
-                if let Some(stmt) = table.get(&hash) {
-                    self.eval_stmt(scope, mods, state, lib, this_ptr, stmt, level)
-                } else if let Some(def_stmt) = def_stmt {
-                    self.eval_stmt(scope, mods, state, lib, this_ptr, def_stmt, level)
+                if value.is_hashable() {
+                    let hasher = &mut get_hasher();
+                    value.hash(hasher);
+                    let hash = hasher.finish();
+
+                    table
+                        .get(&hash)
+                        .map(|stmt| self.eval_stmt(scope, mods, state, lib, this_ptr, stmt, level))
                 } else {
-                    Ok(Dynamic::UNIT)
+                    // Non-hashable values never match any specific clause
+                    None
                 }
+                .unwrap_or_else(|| {
+                    // Default match clause
+                    def_stmt.as_ref().map_or_else(
+                        || Ok(Dynamic::UNIT),
+                        |def_stmt| {
+                            self.eval_stmt(scope, mods, state, lib, this_ptr, def_stmt, level)
+                        },
+                    )
+                })
             }
 
             // While loop
diff --git a/tests/switch.rs b/tests/switch.rs
index 9f2cca35..537dfd04 100644
--- a/tests/switch.rs
+++ b/tests/switch.rs
@@ -18,6 +18,10 @@ fn test_switch() -> Result<(), Box<EvalAltResult>> {
         engine.eval_with_scope::<()>(&mut scope, "switch x { 1 => 123, 2 => 'a' }")?,
         ()
     );
+    assert_eq!(
+        engine.eval::<INT>("let x = timestamp(); switch x { 1 => 123, _ => 42 }")?,
+        42
+    );
     assert_eq!(
         engine.eval_with_scope::<INT>(
             &mut scope,