Disallow implicit comparisons between different numeric types.

This commit is contained in:
Stephen Chung 2021-02-24 15:45:29 +08:00
parent 0d933d865a
commit 4ac05aee8b
11 changed files with 125 additions and 114 deletions

View File

@ -1112,8 +1112,6 @@ pub struct FnCallExpr {
pub hash_script: Option<NonZeroU64>,
/// Does this function call capture the parent scope?
pub capture: bool,
/// Default value when the function is not found, mostly used to provide a default for comparison functions.
pub def_value: Option<Dynamic>,
/// Namespace of the function, if any. Boxed because it occurs rarely.
pub namespace: Option<NamespaceRef>,
/// Function name.

View File

@ -1163,7 +1163,7 @@ impl Engine {
self.exec_fn_call(
mods, state, lib, FN_IDX_SET, None, args, is_ref, true, false,
val_pos, None, None, level,
val_pos, None, level,
)
.map_err(|err| match *err {
EvalAltResult::ErrorFunctionNotFound(fn_sig, _)
@ -1199,14 +1199,11 @@ impl Engine {
let FnCallExpr {
name,
hash_script: hash,
def_value,
..
} = x.as_ref();
let def_value = def_value.as_ref();
let args = idx_val.as_fn_call_args();
self.make_method_call(
mods, state, lib, name, *hash, target, args, def_value, false, *pos,
level,
mods, state, lib, name, *hash, target, args, false, *pos, level,
)
}
// xxx.fn_name(...) = ???
@ -1247,7 +1244,7 @@ impl Engine {
let mut args = [target_val, &mut (new_val.as_mut().unwrap().0).0];
self.exec_fn_call(
mods, state, lib, setter, None, &mut args, is_ref, true, false, *pos,
None, None, level,
None, level,
)
.map(|(v, _)| (v, true))
}
@ -1257,7 +1254,7 @@ impl Engine {
let mut args = [target_val];
self.exec_fn_call(
mods, state, lib, getter, None, &mut args, is_ref, true, false, *pos,
None, None, level,
None, level,
)
.map(|(v, _)| (v, false))
}
@ -1277,14 +1274,11 @@ impl Engine {
let FnCallExpr {
name,
hash_script: hash,
def_value,
..
} = x.as_ref();
let def_value = def_value.as_ref();
let args = idx_val.as_fn_call_args();
let (val, _) = self.make_method_call(
mods, state, lib, name, *hash, target, args, def_value, false,
*pos, level,
mods, state, lib, name, *hash, target, args, false, *pos, level,
)?;
val.into()
}
@ -1312,7 +1306,7 @@ impl Engine {
let args = &mut arg_values[..1];
let (mut val, updated) = self.exec_fn_call(
mods, state, lib, getter, None, args, is_ref, true, false,
*pos, None, None, level,
*pos, None, level,
)?;
let val = &mut val;
@ -1338,7 +1332,7 @@ impl Engine {
arg_values[1] = val;
self.exec_fn_call(
mods, state, lib, setter, None, arg_values, is_ref, true,
false, *pos, None, None, level,
false, *pos, None, level,
)
.or_else(
|err| match *err {
@ -1359,14 +1353,11 @@ impl Engine {
let FnCallExpr {
name,
hash_script: hash,
def_value,
..
} = f.as_ref();
let def_value = def_value.as_ref();
let args = idx_val.as_fn_call_args();
let (mut val, _) = self.make_method_call(
mods, state, lib, name, *hash, target, args, def_value, false,
*pos, level,
mods, state, lib, name, *hash, target, args, false, *pos, level,
)?;
let val = &mut val;
let target = &mut val.into();
@ -1637,7 +1628,7 @@ impl Engine {
let args = &mut [target, &mut idx];
self.exec_fn_call(
_mods, state, _lib, FN_IDX_GET, None, args, _is_ref, true, false, idx_pos,
None, None, _level,
None, _level,
)
.map(|(v, _)| v.into())
.map_err(|err| match *err {
@ -1680,9 +1671,6 @@ impl Engine {
#[cfg(not(feature = "no_index"))]
Dynamic(Union::Array(mut rhs_value, _)) => {
// Call the `==` operator to compare each value
let def_value = Some(false.into());
let def_value = def_value.as_ref();
for value in rhs_value.iter_mut() {
let args = &mut [&mut lhs_value.clone(), value];
let hash_fn =
@ -1693,7 +1681,6 @@ impl Engine {
if self
.call_native_fn(
mods, state, lib, OP_EQUALS, hash_fn, args, false, false, pos,
def_value,
)?
.0
.as_bool()
@ -1799,13 +1786,11 @@ impl Engine {
capture: cap_scope,
hash_script: hash,
args,
def_value,
..
} = x.as_ref();
let def_value = def_value.as_ref();
self.make_function_call(
scope, mods, state, lib, this_ptr, name, args, def_value, *hash, false, *pos,
*cap_scope, level,
scope, mods, state, lib, this_ptr, name, args, *hash, false, *pos, *cap_scope,
level,
)
}
@ -1816,15 +1801,12 @@ impl Engine {
namespace,
hash_script,
args,
def_value,
..
} = x.as_ref();
let namespace = namespace.as_ref();
let hash = hash_script.unwrap();
let def_value = def_value.as_ref();
self.make_qualified_function_call(
scope, mods, state, lib, this_ptr, namespace, name, args, def_value, hash,
*pos, level,
scope, mods, state, lib, this_ptr, namespace, name, args, hash, *pos, level,
)
}
@ -1979,9 +1961,7 @@ impl Engine {
let hash_fn =
calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())).unwrap();
match self.call_native_fn(
mods, state, lib, op, hash_fn, args, true, false, op_pos, None,
) {
match self.call_native_fn(mods, state, lib, op, hash_fn, args, true, false, op_pos) {
Ok(_) => (),
Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) =>
{
@ -1990,7 +1970,7 @@ impl Engine {
// Run function
let (value, _) = self.call_native_fn(
mods, state, lib, op, hash_fn, args, false, false, op_pos, None,
mods, state, lib, op, hash_fn, args, false, false, op_pos,
)?;
*args[0] = value.flatten();

View File

@ -174,7 +174,6 @@ impl Engine {
is_ref: bool,
pub_only: bool,
pos: Position,
def_val: Option<&Dynamic>,
) -> Result<(Dynamic, bool), Box<EvalAltResult>> {
self.inc_operations(state, pos)?;
@ -313,11 +312,6 @@ impl Engine {
}
}
// Return default value (if any)
if let Some(val) = def_val {
return Ok((val.clone(), false));
}
// Getter function not found?
#[cfg(not(feature = "no_object"))]
if let Some(prop) = extract_prop_from_getter(fn_name) {
@ -616,7 +610,6 @@ impl Engine {
pub_only: bool,
pos: Position,
_capture_scope: Option<Scope>,
def_val: Option<&Dynamic>,
_level: usize,
) -> Result<(Dynamic, bool), Box<EvalAltResult>> {
// Check for data race.
@ -772,14 +765,14 @@ impl Engine {
} else {
// Native function call
self.call_native_fn(
mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, pos, def_val,
mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, pos,
)
}
}
// Native function call
_ => self.call_native_fn(
mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, pos, def_val,
mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, pos,
),
}
}
@ -861,7 +854,6 @@ impl Engine {
hash_script: Option<NonZeroU64>,
target: &mut crate::engine::Target,
mut call_args: StaticVec<Dynamic>,
def_val: Option<&Dynamic>,
pub_only: bool,
pos: Position,
level: usize,
@ -890,8 +882,7 @@ impl Engine {
// Map it to name(args) in function-call style
self.exec_fn_call(
mods, state, lib, fn_name, hash, args, false, false, pub_only, pos, None, def_val,
level,
mods, state, lib, fn_name, hash, args, false, false, pub_only, pos, None, level,
)
} else if fn_name == KEYWORD_FN_PTR_CALL
&& call_args.len() > 0
@ -914,8 +905,7 @@ impl Engine {
// Map it to name(args) in function-call style
self.exec_fn_call(
mods, state, lib, fn_name, hash, args, is_ref, true, pub_only, pos, None, def_val,
level,
mods, state, lib, fn_name, hash, args, is_ref, true, pub_only, pos, None, level,
)
} else if fn_name == KEYWORD_FN_PTR_CURRY && obj.is::<FnPtr>() {
// Curry call
@ -980,8 +970,7 @@ impl Engine {
let args = arg_values.as_mut();
self.exec_fn_call(
mods, state, lib, fn_name, hash, args, is_ref, true, pub_only, pos, None, def_val,
level,
mods, state, lib, fn_name, hash, args, is_ref, true, pub_only, pos, None, level,
)
}?;
@ -1003,7 +992,6 @@ impl Engine {
this_ptr: &mut Option<&mut Dynamic>,
fn_name: &str,
args_expr: impl AsRef<[Expr]>,
def_val: Option<&Dynamic>,
mut hash_script: Option<NonZeroU64>,
pub_only: bool,
pos: Position,
@ -1228,7 +1216,6 @@ impl Engine {
pub_only,
pos,
capture,
def_val,
level,
)
.map(|(v, _)| v)
@ -1245,7 +1232,6 @@ impl Engine {
namespace: Option<&NamespaceRef>,
fn_name: &str,
args_expr: impl AsRef<[Expr]>,
def_val: Option<&Dynamic>,
hash_script: NonZeroU64,
pos: Position,
level: usize,
@ -1367,7 +1353,6 @@ impl Engine {
args.as_mut(),
),
Some(f) => unreachable!("unknown function type: {:?}", f),
None if def_val.is_some() => Ok(def_val.unwrap().clone()),
None => EvalAltResult::ErrorFunctionNotFound(
format!(
"{}{} ({})",
@ -1500,7 +1485,11 @@ pub fn run_builtin_binary_op(
}
}
return Ok(None);
return Ok(match op {
"!=" => Some(Dynamic::TRUE),
"==" | ">" | ">=" | "<" | "<=" => Some(Dynamic::FALSE),
_ => None,
});
}
if first_type == TypeId::of::<INT>() {
@ -1603,7 +1592,11 @@ pub fn run_builtin_binary_op(
}
}
Ok(None)
Ok(match op {
"!=" => Some(Dynamic::TRUE),
"==" | ">" | ">=" | "<" | "<=" => Some(Dynamic::FALSE),
_ => None,
})
}
/// Build in common operator assignment implementations to avoid the cost of calling a registered function.

View File

@ -186,7 +186,6 @@ impl<'e, 'n, 's, 'a, 'm> NativeCallContext<'e, 'n, 's, 'a, 'm> {
is_method: bool,
public_only: bool,
args: &mut [&mut Dynamic],
def_value: Option<&Dynamic>,
) -> Result<Dynamic, Box<EvalAltResult>> {
self.engine()
.exec_fn_call(
@ -201,7 +200,6 @@ impl<'e, 'n, 's, 'a, 'm> NativeCallContext<'e, 'n, 's, 'a, 'm> {
public_only,
Position::NONE,
None,
def_value,
0,
)
.map(|(r, _)| r)
@ -339,7 +337,7 @@ impl FnPtr {
args.insert(0, obj);
}
ctx.call_fn_dynamic_raw(self.fn_name(), is_method, true, args.as_mut(), None)
ctx.call_fn_dynamic_raw(self.fn_name(), is_method, true, args.as_mut())
}
}

View File

@ -151,7 +151,6 @@ fn call_fn_with_constant_arguments(
false,
true,
Position::NONE,
None,
)
.ok()
.map(|(v, _)| v)
@ -717,8 +716,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) {
// Handle `type_of()`
Some(arg_for_type_of.to_string().into())
} else {
// Otherwise use the default value, if any
x.def_value.clone()
None
}
})
.and_then(|result| map_dynamic_to_expr(result, *pos))

View File

@ -647,11 +647,9 @@ mod array_functions {
return Ok(true.into());
}
let def_value = Some(false.into());
for (a1, a2) in array.iter_mut().zip(array2.iter_mut()) {
let equals = ctx
.call_fn_dynamic_raw(OP_EQUALS, true, false, &mut [a1, a2], def_value.as_ref())
.call_fn_dynamic_raw(OP_EQUALS, true, false, &mut [a1, a2])
.map(|v| v.as_bool().unwrap_or(false))?;
if !equals {

View File

@ -1,6 +1,7 @@
#![allow(non_snake_case)]
use crate::def_package;
use crate::fn_call::run_builtin_binary_op;
use crate::plugin::*;
#[cfg(feature = "decimal")]
@ -17,30 +18,12 @@ macro_rules! gen_cmp_functions {
#[export_module]
pub mod functions {
#[rhai_fn(name = "<")]
pub fn lt(x: $arg_type, y: $arg_type) -> bool {
x < y
}
#[rhai_fn(name = "<=")]
pub fn lte(x: $arg_type, y: $arg_type) -> bool {
x <= y
}
#[rhai_fn(name = ">")]
pub fn gt(x: $arg_type, y: $arg_type) -> bool {
x > y
}
#[rhai_fn(name = ">=")]
pub fn gte(x: $arg_type, y: $arg_type) -> bool {
x >= y
}
#[rhai_fn(name = "==")]
pub fn eq(x: $arg_type, y: $arg_type) -> bool {
x == y
}
#[rhai_fn(name = "!=")]
pub fn ne(x: $arg_type, y: $arg_type) -> bool {
x != y
}
#[rhai_fn(name = "<")] pub fn lt(x: $arg_type, y: $arg_type) -> bool { x < y }
#[rhai_fn(name = "<=")] pub fn lte(x: $arg_type, y: $arg_type) -> bool { x <= y }
#[rhai_fn(name = ">")] pub fn gt(x: $arg_type, y: $arg_type) -> bool { x > y }
#[rhai_fn(name = ">=")] pub fn gte(x: $arg_type, y: $arg_type) -> bool { x >= y }
#[rhai_fn(name = "==")] pub fn eq(x: $arg_type, y: $arg_type) -> bool { x == y }
#[rhai_fn(name = "!=")] pub fn ne(x: $arg_type, y: $arg_type) -> bool { x != y }
}
})* }
};
@ -57,6 +40,8 @@ macro_rules! reg_functions {
}
def_package!(crate:LogicPackage:"Logical operators.", lib, {
combine_with_exported_module!(lib, "logic", logic_functions);
#[cfg(not(feature = "only_i32"))]
#[cfg(not(feature = "only_i64"))]
{
@ -112,6 +97,60 @@ gen_cmp_functions!(float => f64);
#[cfg(feature = "decimal")]
gen_cmp_functions!(decimal => Decimal);
#[export_module]
mod logic_functions {
fn is_numeric(type_id: TypeId) -> bool {
let result = 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>();
#[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))]
let result = result || type_id == TypeId::of::<u128>() || type_id == TypeId::of::<i128>();
#[cfg(not(feature = "no_float"))]
let result = result || type_id == TypeId::of::<f32>() || type_id == TypeId::of::<f64>();
#[cfg(feature = "decimal")]
let result = result || type_id_x == TypeId::of::<rust_decimal::Decimal>();
result
}
#[rhai_fn(
name = "==",
name = "!=",
name = ">",
name = ">=",
name = "<",
name = "<=",
return_raw
)]
pub fn cmp(
ctx: NativeCallContext,
x: Dynamic,
y: Dynamic,
) -> Result<Dynamic, Box<EvalAltResult>> {
let type_x = x.type_id();
let type_y = y.type_id();
if type_x != type_y && is_numeric(type_x) && is_numeric(type_y) {
// Disallow comparisons between different number types
} else if let Some(x) = run_builtin_binary_op(ctx.fn_name(), &x, &y)? {
return Ok(x);
}
Err(Box::new(EvalAltResult::ErrorFunctionNotFound(
format!("{} ({}, {})", ctx.fn_name(), x.type_name(), y.type_name()),
Position::NONE,
)))
}
}
#[cfg(not(feature = "no_float"))]
#[export_module]
mod f32_functions {

View File

@ -58,12 +58,10 @@ mod map_functions {
return Ok(true.into());
}
let def_value = Some(false.into());
for (m1, v1) in map.iter_mut() {
if let Some(v2) = map2.get_mut(m1) {
let equals = ctx
.call_fn_dynamic_raw(OP_EQUALS, true, false, &mut [v1, v2], def_value.as_ref())
.call_fn_dynamic_raw(OP_EQUALS, true, false, &mut [v1, v2])
.map(|v| v.as_bool().unwrap_or(false))?;
if !equals {

View File

@ -21,7 +21,7 @@ def_package!(crate:BasicStringPackage:"Basic string utilities, including printin
#[cfg(any(not(feature = "no_index"), not(feature = "no_object")))]
#[inline(always)]
fn print_with_func(fn_name: &str, ctx: &NativeCallContext, value: &mut Dynamic) -> ImmutableString {
match ctx.call_fn_dynamic_raw(fn_name, true, false, &mut [value], None) {
match ctx.call_fn_dynamic_raw(fn_name, true, false, &mut [value]) {
Ok(result) if result.is::<ImmutableString>() => result.take_immutable_string().unwrap(),
Ok(result) => ctx.engine().map_type_name(result.type_name()).into(),
Err(_) => ctx.engine().map_type_name(value.type_name()).into(),

View File

@ -1347,7 +1347,6 @@ fn parse_unary(
Box::new(FnCallExpr {
name: op.into(),
args,
def_value: Some(false.into()), // NOT operator, when operating on invalid operand, defaults to false
..Default::default()
}),
pos,
@ -1792,7 +1791,6 @@ fn parse_binary_op(
#[cfg(not(feature = "unchecked"))]
settings.ensure_level_within_max_limit(state.max_expr_depth)?;
let cmp_def = Some(false.into());
let op = op_token.syntax();
let op_base = FnCallExpr {
@ -1819,28 +1817,16 @@ fn parse_binary_op(
| Token::XOr => Expr::FnCall(Box::new(FnCallExpr { args, ..op_base }), pos),
// '!=' defaults to true when passed invalid operands
Token::NotEqualsTo => Expr::FnCall(
Box::new(FnCallExpr {
args,
def_value: Some(true.into()),
..op_base
}),
pos,
),
Token::NotEqualsTo => Expr::FnCall(Box::new(FnCallExpr { args, ..op_base }), pos),
// Comparison operators default to false when passed invalid operands
Token::EqualsTo
| Token::LessThan
| Token::LessThanEqualsTo
| Token::GreaterThan
| Token::GreaterThanEqualsTo => Expr::FnCall(
Box::new(FnCallExpr {
args,
def_value: cmp_def,
..op_base
}),
pos,
),
| Token::GreaterThanEqualsTo => {
Expr::FnCall(Box::new(FnCallExpr { args, ..op_base }), pos)
}
Token::Or => {
let rhs = args.pop().unwrap();

View File

@ -1,4 +1,4 @@
use rhai::{Engine, EvalAltResult, INT};
use rhai::{Engine, EvalAltResult, Scope, INT};
#[test]
fn test_ops() -> Result<(), Box<EvalAltResult>> {
@ -10,6 +10,29 @@ fn test_ops() -> Result<(), Box<EvalAltResult>> {
Ok(())
}
#[test]
fn test_ops_numbers() -> Result<(), Box<EvalAltResult>> {
let engine = Engine::new();
let mut scope = Scope::new();
scope.push("x", 42_u16);
assert!(matches!(
*engine.eval_with_scope::<bool>(&mut scope, "x == 42").expect_err("should error"),
EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with("== (u16,")
));
#[cfg(not(feature = "no_float"))]
assert!(matches!(
*engine.eval_with_scope::<bool>(&mut scope, "x == 42.0").expect_err("should error"),
EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with("== (u16,")
));
assert!(!engine.eval_with_scope::<bool>(&mut scope, r#"x == "hello""#)?);
Ok(())
}
#[test]
fn test_op_precedence() -> Result<(), Box<EvalAltResult>> {
let engine = Engine::new();