Skip to content

Commit

Permalink
Fix Dynamic hashing.
Browse files Browse the repository at this point in the history
  • Loading branch information
schungx committed Jan 25, 2024
1 parent 6824d94 commit f475167
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Fixes to bugs found via fuzzing
* Fixed exponential running time when raising a decimal number to a very large power (> 1 million) -- it now returns an overflow error.
* Shared values that contain reference loops no longer cause a stack overflow when printing.
* `sleep` no longer panics on `NaN`.
* `switch` on ranges now work properly.

Other bug fixes
---------------
Expand Down
12 changes: 11 additions & 1 deletion src/optimizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,16 @@ impl StmtBlock {
}
}

/// Is this [`Expr`] a constant that is hashable?
#[inline(always)]
fn is_hashable_constant(expr: &Expr) -> bool {
match expr {
_ if !expr.is_constant() => false,
Expr::DynamicConstant(v, ..) => v.is_hashable(),
_ => false,
}
}

/// Optimize a [statement][Stmt].
fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: bool) {
#[inline(always)]
Expand Down Expand Up @@ -505,7 +515,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
}

// switch const { ... }
Stmt::Switch(x, pos) if x.0.is_constant() => {
Stmt::Switch(x, pos) if is_hashable_constant(&x.0) => {
let (
match_expr,
SwitchCasesCollection {
Expand Down
7 changes: 2 additions & 5 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1286,14 +1286,11 @@ impl Engine {

if let Some(mut r) = range_value {
if !r.is_empty() {
// Other range
r.set_index(index);
ranges.push(r);
}
continue;
}

if !ranges.is_empty() {
} else if !ranges.is_empty() {
// Check for numeric values after ranges
let forbidden = match value {
Dynamic(Union::Int(..)) => true,
#[cfg(not(feature = "no_float"))]
Expand Down
114 changes: 107 additions & 7 deletions src/types/dynamic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,15 +400,65 @@ impl Hash for Dynamic {
Union::Blob(ref a, ..) => a.hash(state),
#[cfg(not(feature = "no_object"))]
Union::Map(ref m, ..) => m.hash(state),
Union::FnPtr(ref f, ..) => f.hash(state),
Union::FnPtr(..) => unimplemented!("FnPtr cannot be hashed"),

#[cfg(not(feature = "no_closure"))]
Union::Shared(ref cell, ..) => (*crate::func::locked_read(cell)).hash(state),

Union::Variant(..) => unimplemented!("{} cannot be hashed", self.type_name()),
Union::Variant(ref v, ..) => {
let _value_any = (***v).as_any();

#[cfg(not(feature = "only_i32"))]
#[cfg(not(feature = "only_i64"))]
if let Some(value) = _value_any.downcast_ref::<u8>() {
return value.hash(state);
} else if let Some(value) = _value_any.downcast_ref::<u16>() {
return value.hash(state);
} else if let Some(value) = _value_any.downcast_ref::<u32>() {
return value.hash(state);
} else if let Some(value) = _value_any.downcast_ref::<u64>() {
return value.hash(state);
} else if let Some(value) = _value_any.downcast_ref::<i8>() {
return value.hash(state);
} else if let Some(value) = _value_any.downcast_ref::<i16>() {
return value.hash(state);
} else if let Some(value) = _value_any.downcast_ref::<i32>() {
return value.hash(state);
} else if let Some(value) = _value_any.downcast_ref::<i64>() {
return value.hash(state);
}

#[cfg(not(feature = "no_float"))]
#[cfg(not(feature = "f32_float"))]
if let Some(value) = _value_any.downcast_ref::<f32>() {
return value.to_ne_bytes().hash(state);
}
#[cfg(not(feature = "no_float"))]
#[cfg(feature = "f32_float")]
if let Some(value) = _value_any.downcast_ref::<f64>() {
return value.to_ne_bytes().hash(state);
}

#[cfg(not(feature = "only_i32"))]
#[cfg(not(feature = "only_i64"))]
#[cfg(not(target_family = "wasm"))]
if let Some(value) = _value_any.downcast_ref::<u128>() {
return value.hash(state);
} else if let Some(value) = _value_any.downcast_ref::<i128>() {
return value.hash(state);
}

if let Some(range) = _value_any.downcast_ref::<ExclusiveRange>() {
return range.hash(state);
} else if let Some(range) = _value_any.downcast_ref::<InclusiveRange>() {
return range.hash(state);
}

unimplemented!("Custom type {} cannot be hashed", self.type_name())
}

#[cfg(not(feature = "no_time"))]
Union::TimeStamp(..) => unimplemented!("{} cannot be hashed", self.type_name()),
Union::TimeStamp(..) => unimplemented!("Timestamp cannot be hashed"),
}
}
}
Expand Down Expand Up @@ -1153,15 +1203,65 @@ impl Dynamic {

#[cfg(not(feature = "no_float"))]
Union::Float(..) => true,
#[cfg(feature = "decimal")]
Union::Decimal(..) => true,
#[cfg(not(feature = "no_index"))]
Union::Array(..) => true,
Union::Array(ref a, ..) => a.iter().all(Self::is_hashable),
#[cfg(not(feature = "no_index"))]
Union::Blob(..) => true,
#[cfg(not(feature = "no_object"))]
Union::Map(..) => true,
Union::Map(ref m, ..) => m.values().all(Self::is_hashable),
Union::FnPtr(..) => false,
#[cfg(not(feature = "no_time"))]
Union::TimeStamp(..) => false,

Union::Variant(ref v, ..) => {
let _value_any = (***v).as_any();
let _type_id = _value_any.type_id();

#[cfg(not(feature = "only_i32"))]
#[cfg(not(feature = "only_i64"))]
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 = "no_float"))]
#[cfg(not(feature = "f32_float"))]
if _type_id == TypeId::of::<f32>() {
return true;
}
#[cfg(not(feature = "no_float"))]
#[cfg(feature = "f32_float")]
if _type_id == TypeId::of::<f64>() {
return true;
}

#[cfg(not(feature = "only_i32"))]
#[cfg(not(feature = "only_i64"))]
#[cfg(not(target_family = "wasm"))]
if _type_id == TypeId::of::<u128>() || _type_id == TypeId::of::<i128>() {
return true;
}

if _type_id == TypeId::of::<ExclusiveRange>()
|| _type_id == TypeId::of::<InclusiveRange>()
{
return true;
}

false
}

#[cfg(not(feature = "no_closure"))]
Union::Shared(ref cell, ..) => crate::func::locked_read(cell).is_hashable(),

_ => false,
}
}
/// Create a [`Dynamic`] from any type. A [`Dynamic`] value is simply returned as is.
Expand Down
19 changes: 1 addition & 18 deletions src/types/fn_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ use std::prelude::v1::*;
use std::{
any::type_name,
convert::{TryFrom, TryInto},
fmt,
hash::{Hash, Hasher},
mem,
fmt, mem,
ops::{Index, IndexMut},
};
use thin_vec::ThinVec;
Expand All @@ -31,21 +29,6 @@ pub struct FnPtr {
pub(crate) fn_def: Option<Shared<crate::ast::ScriptFuncDef>>,
}

impl Hash for FnPtr {
#[inline(always)]
fn hash<H: Hasher>(&self, state: &mut H) {
self.name.hash(state);
self.curry.hash(state);

// Hash the shared [`EncapsulatedEnviron`] by hashing its shared pointer.
self.environ.as_ref().map(Shared::as_ptr).hash(state);

// Hash the linked [`ScriptFuncDef`][crate::ast::ScriptFuncDef] by hashing its shared pointer.
#[cfg(not(feature = "no_function"))]
self.fn_def.as_ref().map(Shared::as_ptr).hash(state);
}
}

impl fmt::Display for FnPtr {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "Fn({})", self.fn_name())
Expand Down
1 change: 1 addition & 0 deletions tests/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ fn test_switch() {
.unwrap(),
3
);
assert_eq!(engine.eval::<bool>("let x = 1..42; switch x { 1 => (), 2 => 'a', 1..42 => true }").unwrap(), true);

assert_eq!(engine.eval_with_scope::<INT>(&mut scope, "switch 42 { 42 => 123, 42 => 999 }").unwrap(), 123);

Expand Down

0 comments on commit f475167

Please sign in to comment.