From b88089315453c451776bee324575c77f2b0373bf Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 18 Dec 2022 14:51:38 +0800 Subject: [PATCH] Allow negative shift bits. --- CHANGELOG.md | 1 + src/func/builtin.rs | 75 ++++++++++++++++++++++++++++++++++---- src/packages/arithmetic.rs | 32 +++++++++------- tests/binary_ops.rs | 2 + 4 files changed, 88 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db5d1fec..ac07a872 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ Enhancements * The traits `Index` and `IndexMut` are added to `FnPtr`. * `FnPtr::iter_curry` and `FnPtr::iter_curry_mut` are added. * `Dynamic::deep_scan` is added to recursively scan for `Dynamic` values. +* `>>` and `<<` operators on integers no longer throw errors when the number of bits to shift is out of bounds. Shifting by a negative number of bits simply reverses the shift direction. Version 1.11.0 diff --git a/src/func/builtin.rs b/src/func/builtin.rs index 10c8e537..ad15da11 100644 --- a/src/func/builtin.rs +++ b/src/func/builtin.rs @@ -118,6 +118,11 @@ pub fn get_builtin_binary_op_fn(op: Token, x: &Dynamic, y: &Dynamic) -> Option Ok($func:ident ( $xx:ident, $yy:ident ))) => { (|_, args| { + let x = args[0].$xx().unwrap() as $base; + let y = args[1].$yy().unwrap() as $base; + Ok($func(x, y).into()) + }, false) }; ($base:ty => $func:ident ( $xx:ident, $yy:ident )) => { (|_, args| { let x = args[0].$xx().unwrap() as $base; let y = args[1].$yy().unwrap() as $base; @@ -133,6 +138,11 @@ pub fn get_builtin_binary_op_fn(op: Token, x: &Dynamic, y: &Dynamic) -> Option::from(args[1].$yy().unwrap()); Ok(x.$func(y).into()) }, false) }; + (from $base:ty => Ok($func:ident ( $xx:ident, $yy:ident ))) => { (|_, args| { + let x = <$base>::from(args[0].$xx().unwrap()); + let y = <$base>::from(args[1].$yy().unwrap()); + Ok($func(x, y).into()) + }, false) }; (from $base:ty => $func:ident ( $xx:ident, $yy:ident )) => { (|_, args| { let x = <$base>::from(args[0].$xx().unwrap()); let y = <$base>::from(args[1].$yy().unwrap()); @@ -155,8 +165,8 @@ pub fn get_builtin_binary_op_fn(op: Token, x: &Dynamic, y: &Dynamic) -> Option return Some(impl_op!(INT => divide(as_int, as_int))), Modulo => return Some(impl_op!(INT => modulo(as_int, as_int))), PowerOf => return Some(impl_op!(INT => power(as_int, as_int))), - RightShift => return Some(impl_op!(INT => shift_right(as_int, as_int))), - LeftShift => return Some(impl_op!(INT => shift_left(as_int, as_int))), + RightShift => return Some(impl_op!(INT => Ok(shift_right(as_int, as_int)))), + LeftShift => return Some(impl_op!(INT => Ok(shift_left(as_int, as_int)))), _ => (), } @@ -168,8 +178,26 @@ pub fn get_builtin_binary_op_fn(op: Token, x: &Dynamic, y: &Dynamic) -> Option return Some(impl_op!(INT => as_int / as_int)), Modulo => return Some(impl_op!(INT => as_int % as_int)), PowerOf => return Some(impl_op!(INT => as_int.pow(as_int as u32))), - RightShift => return Some(impl_op!(INT => as_int >> as_int)), - LeftShift => return Some(impl_op!(INT => as_int << as_int)), + RightShift => { + return Some(( + |_, args| { + let x = args[0].as_int().unwrap(); + let y = args[1].as_int().unwrap(); + Ok((if y < 0 { x << -y } else { x >> y }).into()) + }, + false, + )) + } + LeftShift => { + return Some(( + |_, args| { + let x = args[0].as_int().unwrap(); + let y = args[1].as_int().unwrap(); + Ok((if y < 0 { x >> -y } else { x << y }).into()) + }, + false, + )) + } _ => (), } @@ -614,6 +642,12 @@ pub fn get_builtin_op_assignment_fn(op: Token, x: &Dynamic, y: &Dynamic) -> Opti let y = args[1].$yy().unwrap() as $x; Ok((*args[0].write_lock::<$x>().unwrap() = x.$func(y as $yyy)).into()) }, false) }; + ($x:ty => Ok($func:ident ( $xx:ident, $yy:ident ))) => { (|_, args| { + let x = args[0].$xx().unwrap(); + let y = args[1].$yy().unwrap() as $x; + let v: Dynamic = $func(x, y).into(); + Ok((*args[0].write_lock().unwrap() = v).into()) + }, false) }; ($x:ty => $func:ident ( $xx:ident, $yy:ident )) => { (|_, args| { let x = args[0].$xx().unwrap(); let y = args[1].$yy().unwrap() as $x; @@ -628,6 +662,11 @@ pub fn get_builtin_op_assignment_fn(op: Token, x: &Dynamic, y: &Dynamic) -> Opti let y = <$x>::from(args[1].$yy().unwrap()); Ok((*args[0].write_lock::<$x>().unwrap() = x.$func(y)).into()) }, false) }; + (from $x:ty => Ok($func:ident ( $xx:ident, $yy:ident ))) => { (|_, args| { + let x = args[0].$xx().unwrap(); + let y = <$x>::from(args[1].$yy().unwrap()); + Ok((*args[0].write_lock().unwrap() = $func(x, y).into()).into()) + }, false) }; (from $x:ty => $func:ident ( $xx:ident, $yy:ident )) => { (|_, args| { let x = args[0].$xx().unwrap(); let y = <$x>::from(args[1].$yy().unwrap()); @@ -650,8 +689,8 @@ pub fn get_builtin_op_assignment_fn(op: Token, x: &Dynamic, y: &Dynamic) -> Opti DivideAssign => return Some(impl_op!(INT => divide(as_int, as_int))), ModuloAssign => return Some(impl_op!(INT => modulo(as_int, as_int))), PowerOfAssign => return Some(impl_op!(INT => power(as_int, as_int))), - RightShiftAssign => return Some(impl_op!(INT => shift_right(as_int, as_int))), - LeftShiftAssign => return Some(impl_op!(INT => shift_left(as_int, as_int))), + RightShiftAssign => return Some(impl_op!(INT => Ok(shift_right(as_int, as_int)))), + LeftShiftAssign => return Some(impl_op!(INT => Ok(shift_left(as_int, as_int)))), _ => (), } @@ -663,8 +702,28 @@ pub fn get_builtin_op_assignment_fn(op: Token, x: &Dynamic, y: &Dynamic) -> Opti DivideAssign => return Some(impl_op!(INT /= as_int)), ModuloAssign => return Some(impl_op!(INT %= as_int)), PowerOfAssign => return Some(impl_op!(INT => as_int.pow(as_int as u32))), - RightShiftAssign => return Some(impl_op!(INT >>= as_int)), - LeftShiftAssign => return Some(impl_op!(INT <<= as_int)), + RightShiftAssign => { + return Some(( + |_, args| { + let x = args[0].as_int().unwrap(); + let y = args[1].as_int().unwrap(); + let v = if y < 0 { x << -y } else { x >> y }; + Ok((*args[0].write_lock::().unwrap() = v.into()).into()) + }, + false, + )) + } + LeftShiftAssign => { + return Some(( + |_, args| { + let x = args[0].as_int().unwrap(); + let y = args[1].as_int().unwrap(); + let v = if y < 0 { x >> -y } else { x << y }; + Ok((*args[0].write_lock::().unwrap() = v.into()).into()) + }, + false, + )) + } _ => (), } diff --git a/src/packages/arithmetic.rs b/src/packages/arithmetic.rs index 49fd6c1b..771982b7 100644 --- a/src/packages/arithmetic.rs +++ b/src/packages/arithmetic.rs @@ -72,9 +72,9 @@ macro_rules! gen_arithmetic_functions { pub fn power(x: $arg_type, y: INT) -> RhaiResultOf<$arg_type> { if cfg!(not(feature = "unchecked")) { if cfg!(not(feature = "only_i32")) && y > (u32::MAX as INT) { - Err(make_err(format!("Integer raised to too large an index: {x} ** {y}"))) + Err(make_err(format!("Exponential overflow: {x} ** {y}"))) } else if y < 0 { - Err(make_err(format!("Integer raised to a negative index: {x} ** {y}"))) + Err(make_err(format!("Integer raised to a negative power: {x} ** {y}"))) } else { x.checked_pow(y as u32).ok_or_else(|| make_err(format!("Exponential overflow: {x} ** {y}"))) } @@ -83,32 +83,36 @@ macro_rules! gen_arithmetic_functions { } } - #[rhai_fn(name = "<<", return_raw)] - pub fn shift_left(x: $arg_type, y: INT) -> RhaiResultOf<$arg_type> { + #[rhai_fn(name = "<<")] + pub fn shift_left(x: $arg_type, y: INT) -> $arg_type { if cfg!(not(feature = "unchecked")) { if cfg!(not(feature = "only_i32")) && y > (u32::MAX as INT) { - Err(make_err(format!("Left-shift by too many bits: {x} << {y}"))) + 0 } else if y < 0 { - Err(make_err(format!("Left-shift by a negative number: {x} << {y}"))) + shift_right(x, y.checked_abs().unwrap_or(INT::MAX)) } else { - x.checked_shl(y as u32).ok_or_else(|| make_err(format!("Left-shift by too many bits: {x} << {y}"))) + x.checked_shl(y as u32).unwrap_or_else(|| 0) } + } else if y < 0 { + x >> -y } else { - Ok(x << y) + x << y } } - #[rhai_fn(name = ">>", return_raw)] - pub fn shift_right(x: $arg_type, y: INT) -> RhaiResultOf<$arg_type> { + #[rhai_fn(name = ">>")] + pub fn shift_right(x: $arg_type, y: INT) -> $arg_type { if cfg!(not(feature = "unchecked")) { if cfg!(not(feature = "only_i32")) && y > (u32::MAX as INT) { - Err(make_err(format!("Right-shift by too many bits: {x} >> {y}"))) + x.wrapping_shr(u32::MAX) } else if y < 0 { - Err(make_err(format!("Right-shift by a negative number: {x} >> {y}"))) + shift_left(x, y.checked_abs().unwrap_or(INT::MAX)) } else { - x.checked_shr(y as u32).ok_or_else(|| make_err(format!("Right-shift by too many bits: {x} >> {y}"))) + x.checked_shr(y as u32).unwrap_or_else(|| x.wrapping_shr(u32::MAX)) } + } else if y < 0 { + x << -y } else { - Ok(x >> y) + x >> y } } #[rhai_fn(name = "&")] diff --git a/tests/binary_ops.rs b/tests/binary_ops.rs index a470a155..6d0a9d70 100644 --- a/tests/binary_ops.rs +++ b/tests/binary_ops.rs @@ -30,7 +30,9 @@ fn test_binary_ops() -> Result<(), Box> { assert_eq!(engine.eval::("let x = 10; x %= 4; x")?, 2); assert_eq!(engine.eval::("let x = 10; x **= 4; x")?, 10000); assert_eq!(engine.eval::("let x = 10; x <<= 4; x")?, 160); + assert_eq!(engine.eval::("let x = 10; x <<= -1; x")?, 5); assert_eq!(engine.eval::("let x = 10; x >>= 4; x")?, 0); + assert_eq!(engine.eval::("let x = 10; x >>= -2; x")?, 40); assert_eq!(engine.eval::("let x = 10; x &= 4; x")?, 0); assert_eq!(engine.eval::("let x = 10; x |= 4; x")?, 14); assert_eq!(engine.eval::("let x = 10; x ^= 4; x")?, 14);