kjuulh/rhai
1
0
Fork 0
Code Issues 1 Pull Requests 1 Packages Projects Releases Wiki Activity

Do not return default for comparisons between same types.

Browse Source
This commit is contained in:
Stephen Chung 2021-02-25 10:59:11 +08:00
parent 02057ef1d2
commit 49e5382ab0
3 changed files with 80 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
RELEASES.md
View File

@ -14,6 +14,7 @@ Breaking changes
* For plugin functions, constants passed to methods (i.e. `&mut` parameter) now raise an error unless the functions are marked with `#[rhai_fn(pure)]`. * For plugin functions, constants passed to methods (i.e. `&mut` parameter) now raise an error unless the functions are marked with `#[rhai_fn(pure)]`.
* Visibility (i.e. `pub` or not) for generated _plugin_ modules now follow the visibility of the underlying module. * Visibility (i.e. `pub` or not) for generated _plugin_ modules now follow the visibility of the underlying module.
* Comparison operators between the sames types now throw errors when they're not defined instead of returning the default. Only comparing between _different_ types will return the default.
* Default stack-overflow and top-level expression nesting limits for release builds are lowered to 64 from 128. * Default stack-overflow and top-level expression nesting limits for release builds are lowered to 64 from 128.
New features New features

109
src/fn_call.rs
View File

@ -21,7 +21,6 @@ use crate::stdlib::{
string::ToString, string::ToString,
vec::Vec, vec::Vec,
}; };
use crate::token::is_assignment_operator;
use crate::utils::combine_hashes; use crate::utils::combine_hashes;
use crate::{ use crate::{
calc_native_fn_hash, calc_script_fn_hash, Dynamic, Engine, EvalAltResult, FnPtr, calc_native_fn_hash, calc_script_fn_hash, Dynamic, Engine, EvalAltResult, FnPtr,
@ -174,6 +173,7 @@ impl Engine {
hash_fn: NonZeroU64, hash_fn: NonZeroU64,
args: &mut FnCallArgs, args: &mut FnCallArgs,
is_ref: bool, is_ref: bool,
is_op_assignment: bool,
pos: Position, pos: Position,
) -> Result<(Dynamic, bool), Box<EvalAltResult>> { ) -> Result<(Dynamic, bool), Box<EvalAltResult>> {
self.inc_operations(state, pos)?; self.inc_operations(state, pos)?;
@ -286,15 +286,16 @@ impl Engine {
// See if it is built in. // See if it is built in.
if args.len() == 2 && !args[0].is_variant() && !args[1].is_variant() { if args.len() == 2 && !args[0].is_variant() && !args[1].is_variant() {
if is_assignment_operator(fn_name) { // Op-assignment?
if is_ref { if is_op_assignment {
// Op-assignment if !is_ref {
let (first, second) = args.split_first_mut().unwrap(); unreachable!("op-assignments must have ref argument");
}
let (first, second) = args.split_first_mut().unwrap();
match run_builtin_op_assignment(fn_name, first, second[0])? { match run_builtin_op_assignment(fn_name, first, second[0])? {
Some(_) => return Ok((Dynamic::UNIT, false)), Some(_) => return Ok((Dynamic::UNIT, false)),
None => (), None => (),
}
} }
} else { } else {
match run_builtin_binary_op(fn_name, args[0], args[1])? { match run_builtin_binary_op(fn_name, args[0], args[1])? {
@ -756,12 +757,14 @@ impl Engine {
Ok((result, false)) Ok((result, false))
} else { } else {
// Native function call // Native function call
self.call_native_fn(mods, state, lib, fn_name, hash_fn, args, is_ref, pos) self.call_native_fn(
mods, state, lib, fn_name, hash_fn, args, is_ref, false, pos,
)
} }
} }
// Native function call // Native function call
_ => self.call_native_fn(mods, state, lib, fn_name, hash_fn, args, is_ref, pos), _ => self.call_native_fn(mods, state, lib, fn_name, hash_fn, args, is_ref, false, pos),
} }
} }
@ -1368,17 +1371,29 @@ pub fn run_builtin_binary_op(
x: &Dynamic, x: &Dynamic,
y: &Dynamic, y: &Dynamic,
) -> Result<Option<Dynamic>, Box<EvalAltResult>> { ) -> Result<Option<Dynamic>, Box<EvalAltResult>> {
let first_type = x.type_id(); let type1 = x.type_id();
let second_type = y.type_id(); let type2 = y.type_id();
let type_id = (first_type, second_type); if x.is_variant() || y.is_variant() {
// One of the operands is a custom type, so it is never built-in
return Ok(match op {
"!=" if type1 != type2 => Some(Dynamic::TRUE),
"==" | ">" | ">=" | "<" | "<=" if type1 != type2 => Some(Dynamic::FALSE),
_ => None,
});
}
let types_pair = (type1, type2);
#[cfg(not(feature = "no_float"))] #[cfg(not(feature = "no_float"))]
if let Some((x, y)) = if type_id == (TypeId::of::<FLOAT>(), TypeId::of::<FLOAT>()) { if let Some((x, y)) = if types_pair == (TypeId::of::<FLOAT>(), TypeId::of::<FLOAT>()) {
// FLOAT op FLOAT
Some((x.clone().cast::<FLOAT>(), y.clone().cast::<FLOAT>())) Some((x.clone().cast::<FLOAT>(), y.clone().cast::<FLOAT>()))
} else if type_id == (TypeId::of::<FLOAT>(), TypeId::of::<INT>()) { } else if types_pair == (TypeId::of::<FLOAT>(), TypeId::of::<INT>()) {
// FLOAT op INT
Some((x.clone().cast::<FLOAT>(), y.clone().cast::<INT>() as FLOAT)) Some((x.clone().cast::<FLOAT>(), y.clone().cast::<INT>() as FLOAT))
} else if type_id == (TypeId::of::<INT>(), TypeId::of::<FLOAT>()) { } else if types_pair == (TypeId::of::<INT>(), TypeId::of::<FLOAT>()) {
// INT op FLOAT
Some((x.clone().cast::<INT>() as FLOAT, y.clone().cast::<FLOAT>())) Some((x.clone().cast::<INT>() as FLOAT, y.clone().cast::<FLOAT>()))
} else { } else {
None None
@ -1402,16 +1417,19 @@ pub fn run_builtin_binary_op(
#[cfg(feature = "decimal")] #[cfg(feature = "decimal")]
if let Some((x, y)) = if type_id == (TypeId::of::<Decimal>(), TypeId::of::<Decimal>()) { if let Some((x, y)) = if type_id == (TypeId::of::<Decimal>(), TypeId::of::<Decimal>()) {
// Decimal op Decimal
Some(( Some((
*x.read_lock::<Decimal>().unwrap(), *x.read_lock::<Decimal>().unwrap(),
*y.read_lock::<Decimal>().unwrap(), *y.read_lock::<Decimal>().unwrap(),
)) ))
} else if type_id == (TypeId::of::<Decimal>(), TypeId::of::<INT>()) { } else if type_id == (TypeId::of::<Decimal>(), TypeId::of::<INT>()) {
// Decimal op INT
Some(( Some((
*x.read_lock::<Decimal>().unwrap(), *x.read_lock::<Decimal>().unwrap(),
y.clone().cast::<INT>().into(), y.clone().cast::<INT>().into(),
)) ))
} else if type_id == (TypeId::of::<INT>(), TypeId::of::<Decimal>()) { } else if type_id == (TypeId::of::<INT>(), TypeId::of::<Decimal>()) {
// INT op Decimal
Some(( Some((
x.clone().cast::<INT>().into(), x.clone().cast::<INT>().into(),
*y.read_lock::<Decimal>().unwrap(), *y.read_lock::<Decimal>().unwrap(),
@ -1452,8 +1470,9 @@ pub fn run_builtin_binary_op(
} }
} }
if second_type != first_type { if type2 != type1 {
if type_id == (TypeId::of::<char>(), TypeId::of::<ImmutableString>()) { // char op string
if types_pair == (TypeId::of::<char>(), TypeId::of::<ImmutableString>()) {
let x = x.clone().cast::<char>(); let x = x.clone().cast::<char>();
let y = &*y.read_lock::<ImmutableString>().unwrap(); let y = &*y.read_lock::<ImmutableString>().unwrap();
@ -1462,8 +1481,8 @@ pub fn run_builtin_binary_op(
_ => return Ok(None), _ => return Ok(None),
} }
} }
// string op char
if type_id == (TypeId::of::<ImmutableString>(), TypeId::of::<char>()) { if types_pair == (TypeId::of::<ImmutableString>(), TypeId::of::<char>()) {
let x = &*x.read_lock::<ImmutableString>().unwrap(); let x = &*x.read_lock::<ImmutableString>().unwrap();
let y = y.clone().cast::<char>(); let y = y.clone().cast::<char>();
@ -1472,7 +1491,7 @@ pub fn run_builtin_binary_op(
_ => return Ok(None), _ => return Ok(None),
} }
} }
// Default comparison operators for different types
return Ok(match op { return Ok(match op {
"!=" => Some(Dynamic::TRUE), "!=" => Some(Dynamic::TRUE),
"==" | ">" | ">=" | "<" | "<=" => Some(Dynamic::FALSE), "==" | ">" | ">=" | "<" | "<=" => Some(Dynamic::FALSE),
@ -1480,7 +1499,9 @@ pub fn run_builtin_binary_op(
}); });
} }
if first_type == TypeId::of::<INT>() { // Beyond here, type1 == type2
if type1 == TypeId::of::<INT>() {
let x = x.clone().cast::<INT>(); let x = x.clone().cast::<INT>();
let y = y.clone().cast::<INT>(); let y = y.clone().cast::<INT>();
@ -1526,7 +1547,7 @@ pub fn run_builtin_binary_op(
} }
} }
if first_type == TypeId::of::<bool>() { if type1 == TypeId::of::<bool>() {
let x = x.clone().cast::<bool>(); let x = x.clone().cast::<bool>();
let y = y.clone().cast::<bool>(); let y = y.clone().cast::<bool>();
@ -1540,7 +1561,7 @@ pub fn run_builtin_binary_op(
} }
} }
if first_type == TypeId::of::<ImmutableString>() { if type1 == TypeId::of::<ImmutableString>() {
let x = &*x.read_lock::<ImmutableString>().unwrap(); let x = &*x.read_lock::<ImmutableString>().unwrap();
let y = &*y.read_lock::<ImmutableString>().unwrap(); let y = &*y.read_lock::<ImmutableString>().unwrap();
@ -1556,7 +1577,7 @@ pub fn run_builtin_binary_op(
} }
} }
if first_type == TypeId::of::<char>() { if type1 == TypeId::of::<char>() {
let x = x.clone().cast::<char>(); let x = x.clone().cast::<char>();
let y = y.clone().cast::<char>(); let y = y.clone().cast::<char>();
@ -1572,7 +1593,7 @@ pub fn run_builtin_binary_op(
} }
} }
if first_type == TypeId::of::<()>() { if type1 == TypeId::of::<()>() {
match op { match op {
"==" => return Ok(Some(true.into())), "==" => return Ok(Some(true.into())),
"!=" | ">" | ">=" | "<" | "<=" => return Ok(Some(false.into())), "!=" | ">" | ">=" | "<" | "<=" => return Ok(Some(false.into())),
@ -1580,11 +1601,7 @@ pub fn run_builtin_binary_op(
} }
} }
Ok(match op { Ok(None)
"!=" => Some(Dynamic::TRUE),
"==" | ">" | ">=" | "<" | "<=" => Some(Dynamic::FALSE),
_ => None,
})
} }
/// Build in common operator assignment implementations to avoid the cost of calling a registered function. /// Build in common operator assignment implementations to avoid the cost of calling a registered function.
@ -1593,16 +1610,18 @@ pub fn run_builtin_op_assignment(
x: &mut Dynamic, x: &mut Dynamic,
y: &Dynamic, y: &Dynamic,
) -> Result<Option<()>, Box<EvalAltResult>> { ) -> Result<Option<()>, Box<EvalAltResult>> {
let first_type = x.type_id(); let type1 = x.type_id();
let second_type = y.type_id(); let type2 = y.type_id();
let type_id = (first_type, second_type); let types_pair = (type1, type2);
#[cfg(not(feature = "no_float"))] #[cfg(not(feature = "no_float"))]
if let Some((mut x, y)) = if type_id == (TypeId::of::<FLOAT>(), TypeId::of::<FLOAT>()) { if let Some((mut x, y)) = if types_pair == (TypeId::of::<FLOAT>(), TypeId::of::<FLOAT>()) {
// FLOAT op= FLOAT
let y = y.clone().cast::<FLOAT>(); let y = y.clone().cast::<FLOAT>();
Some((x.write_lock::<FLOAT>().unwrap(), y)) Some((x.write_lock::<FLOAT>().unwrap(), y))
} else if type_id == (TypeId::of::<FLOAT>(), TypeId::of::<INT>()) { } else if types_pair == (TypeId::of::<FLOAT>(), TypeId::of::<INT>()) {
// FLOAT op= INT
let y = y.clone().cast::<INT>() as FLOAT; let y = y.clone().cast::<INT>() as FLOAT;
Some((x.write_lock::<FLOAT>().unwrap(), y)) Some((x.write_lock::<FLOAT>().unwrap(), y))
} else { } else {
@ -1621,9 +1640,11 @@ pub fn run_builtin_op_assignment(
#[cfg(feature = "decimal")] #[cfg(feature = "decimal")]
if let Some((mut x, y)) = if type_id == (TypeId::of::<Decimal>(), TypeId::of::<Decimal>()) { if let Some((mut x, y)) = if type_id == (TypeId::of::<Decimal>(), TypeId::of::<Decimal>()) {
// Decimal op= Decimal
let y = *y.read_lock::<Decimal>().unwrap(); let y = *y.read_lock::<Decimal>().unwrap();
Some((x.write_lock::<Decimal>().unwrap(), y)) Some((x.write_lock::<Decimal>().unwrap(), y))
} else if type_id == (TypeId::of::<Decimal>(), TypeId::of::<INT>()) { } else if type_id == (TypeId::of::<Decimal>(), TypeId::of::<INT>()) {
// Decimal op= INT
let y = y.clone().cast::<INT>().into(); let y = y.clone().cast::<INT>().into();
Some((x.write_lock::<Decimal>().unwrap(), y)) Some((x.write_lock::<Decimal>().unwrap(), y))
} else { } else {
@ -1652,8 +1673,8 @@ pub fn run_builtin_op_assignment(
} }
} }
if second_type != first_type { if type2 != type1 {
if type_id == (TypeId::of::<ImmutableString>(), TypeId::of::<char>()) { if types_pair == (TypeId::of::<ImmutableString>(), TypeId::of::<char>()) {
let y = y.read_lock::<char>().unwrap().deref().clone(); let y = y.read_lock::<char>().unwrap().deref().clone();
let mut x = x.write_lock::<ImmutableString>().unwrap(); let mut x = x.write_lock::<ImmutableString>().unwrap();
@ -1666,7 +1687,9 @@ pub fn run_builtin_op_assignment(
return Ok(None); return Ok(None);
} }
if first_type == TypeId::of::<INT>() { // Beyond here, type1 == type2
if type1 == TypeId::of::<INT>() {
let y = y.clone().cast::<INT>(); let y = y.clone().cast::<INT>();
let mut x = x.write_lock::<INT>().unwrap(); let mut x = x.write_lock::<INT>().unwrap();
@ -1706,7 +1729,7 @@ pub fn run_builtin_op_assignment(
} }
} }
if first_type == TypeId::of::<bool>() { if type1 == TypeId::of::<bool>() {
let y = y.clone().cast::<bool>(); let y = y.clone().cast::<bool>();
let mut x = x.write_lock::<bool>().unwrap(); let mut x = x.write_lock::<bool>().unwrap();
@ -1717,7 +1740,7 @@ pub fn run_builtin_op_assignment(
} }
} }
if first_type == TypeId::of::<char>() { if type1 == TypeId::of::<char>() {
let y = y.read_lock::<char>().unwrap().deref().clone(); let y = y.read_lock::<char>().unwrap().deref().clone();
let mut x = x.write_lock::<Dynamic>().unwrap(); let mut x = x.write_lock::<Dynamic>().unwrap();
@ -1727,7 +1750,7 @@ pub fn run_builtin_op_assignment(
} }
} }
if first_type == TypeId::of::<ImmutableString>() { if type1 == TypeId::of::<ImmutableString>() {
let y = y.read_lock::<ImmutableString>().unwrap().deref().clone(); let y = y.read_lock::<ImmutableString>().unwrap().deref().clone();
let mut x = x.write_lock::<ImmutableString>().unwrap(); let mut x = x.write_lock::<ImmutableString>().unwrap();

15
tests/mismatched_op.rs
View File

@ -12,7 +12,7 @@ fn test_mismatched_op() {
#[test] #[test]
#[cfg(not(feature = "no_object"))] #[cfg(not(feature = "no_object"))]
fn test_mismatched_op_custom_type() { fn test_mismatched_op_custom_type() -> Result<(), Box<EvalAltResult>> {
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
struct TestStruct { struct TestStruct {
x: INT, x: INT,
@ -30,9 +30,18 @@ fn test_mismatched_op_custom_type() {
.register_type_with_name::<TestStruct>("TestStruct") .register_type_with_name::<TestStruct>("TestStruct")
.register_fn("new_ts", TestStruct::new); .register_fn("new_ts", TestStruct::new);
assert!(matches!(*engine.eval::<bool>(r"
let x = new_ts();
let y = new_ts();
x == y
").expect_err("should error"),
EvalAltResult::ErrorFunctionNotFound(f, _) if f == "== (TestStruct, TestStruct)"));
assert!(!engine.eval::<bool>("new_ts() == 42")?);
assert!(matches!( assert!(matches!(
*engine.eval::<INT>("60 + new_ts()").expect_err("should error"), *engine.eval::<INT>("60 + new_ts()").expect_err("should error"),
EvalAltResult::ErrorFunctionNotFound(err, _) if err == format!("+ ({}, TestStruct)", std::any::type_name::<INT>()) EvalAltResult::ErrorFunctionNotFound(f, _) if f == format!("+ ({}, TestStruct)", std::any::type_name::<INT>())
)); ));
assert!(matches!( assert!(matches!(
@ -40,4 +49,6 @@ fn test_mismatched_op_custom_type() {
EvalAltResult::ErrorMismatchOutputType(need, actual, _) EvalAltResult::ErrorMismatchOutputType(need, actual, _)
if need == "TestStruct" && actual == std::any::type_name::<INT>() if need == "TestStruct" && actual == std::any::type_name::<INT>()
)); ));
Ok(())
} }