From 402041d43251017ab772fe6f9920e41f496a359d Mon Sep 17 00:00:00 2001 From: hello2dj Date: Thu, 14 May 2020 03:01:42 +0800 Subject: [PATCH] impl abstract-equality-comparison (#395) --- boa/src/builtins/array/mod.rs | 12 ++-- boa/src/builtins/number/mod.rs | 44 +++++++++++++ boa/src/builtins/number/tests.rs | 31 +++++++++ boa/src/builtins/object/mod.rs | 2 +- boa/src/builtins/string/mod.rs | 4 +- boa/src/builtins/value/mod.rs | 21 ++++-- boa/src/builtins/value/operations.rs | 97 ++++++++++++++++++++++------ boa/src/builtins/value/tests.rs | 48 ++++++++++++++ boa/src/exec/mod.rs | 14 ++-- 9 files changed, 230 insertions(+), 43 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 110e7547d26..eb578a49811 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -16,7 +16,7 @@ use crate::{ builtins::{ object::{Object, ObjectInternalMethods, ObjectKind, INSTANCE_PROTOTYPE, PROTOTYPE}, property::Property, - value::{ResultValue, Value, ValueData}, + value::{same_value_zero, ResultValue, Value, ValueData}, }, exec::Interpreter, }; @@ -418,7 +418,7 @@ pub fn shift(this: &mut Value, _: &[Value], _: &mut Interpreter) -> ResultValue let to = (k.wrapping_sub(1)).to_string(); let from_value = this.get_field_slice(&from); - if from_value == Value::undefined() { + if from_value.is_undefined() { this.remove_property(&to); } else { this.set_field_slice(&to, from_value); @@ -454,7 +454,7 @@ pub fn unshift(this: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultV let to = (k.wrapping_add(arg_c).wrapping_sub(1)).to_string(); let from_value = this.get_field_slice(&from); - if from_value == Value::undefined() { + if from_value.is_undefined() { this.remove_property(&to); } else { this.set_field_slice(&to, from_value); @@ -601,7 +601,7 @@ pub fn index_of(this: &mut Value, args: &[Value], _: &mut Interpreter) -> Result while idx < len { let check_element = this.get_field_slice(&idx.to_string()).clone(); - if check_element == search_element { + if check_element.strict_equals(&search_element) { return Ok(Value::from(idx)); } @@ -654,7 +654,7 @@ pub fn last_index_of(this: &mut Value, args: &[Value], _: &mut Interpreter) -> R while idx >= 0 { let check_element = this.get_field_slice(&idx.to_string()).clone(); - if check_element == search_element { + if check_element.strict_equals(&search_element) { return Ok(Value::from(idx)); } @@ -797,7 +797,7 @@ pub fn includes_value(this: &mut Value, args: &[Value], _: &mut Interpreter) -> for idx in 0..length { let check_element = this.get_field_slice(&idx.to_string()).clone(); - if check_element == search_element { + if same_value_zero(&check_element, &search_element) { return Ok(Value::from(true)); } } diff --git a/boa/src/builtins/number/mod.rs b/boa/src/builtins/number/mod.rs index d9e864ece54..45558004378 100644 --- a/boa/src/builtins/number/mod.rs +++ b/boa/src/builtins/number/mod.rs @@ -368,3 +368,47 @@ pub fn create(global: &Value) -> Value { pub fn init(global: &Value) { global.set_field_slice("Number", create(global)); } + +/// The abstract operation Number::equal takes arguments +/// x (a Number) and y (a Number). It performs the following steps when called: +/// +/// https://tc39.es/ecma262/#sec-numeric-types-number-equal +#[allow(clippy::float_cmp)] +pub fn equals(a: f64, b: f64) -> bool { + a == b +} + +/// The abstract operation Number::sameValue takes arguments +/// x (a Number) and y (a Number). It performs the following steps when called: +/// +/// https://tc39.es/ecma262/#sec-numeric-types-number-sameValue +#[allow(clippy::float_cmp)] +pub fn same_value(a: f64, b: f64) -> bool { + if a.is_nan() && b.is_nan() { + return true; + } + + if a == 0.0 && b == 0.0 { + if (a.is_sign_negative() && b.is_sign_positive()) + || (a.is_sign_positive() && b.is_sign_negative()) + { + return false; + }; + true + } else { + a == b + } +} + +/// The abstract operation Number::sameValueZero takes arguments +/// x (a Number) and y (a Number). It performs the following steps when called: +/// +/// https://tc39.es/ecma262/#sec-numeric-types-number-sameValueZero +#[allow(clippy::float_cmp)] +pub fn same_value_zero(a: f64, b: f64) -> bool { + if a.is_nan() && b.is_nan() { + return true; + } + + a == b +} diff --git a/boa/src/builtins/number/tests.rs b/boa/src/builtins/number/tests.rs index e4cb7a4d2ad..1b5a1b24b85 100644 --- a/boa/src/builtins/number/tests.rs +++ b/boa/src/builtins/number/tests.rs @@ -395,3 +395,34 @@ fn value_of() { assert_eq!(exp_val.to_number(), 12_000_f64); assert_eq!(neg_val.to_number(), -12_000_f64); } + +#[test] +fn equal() { + assert_eq!(super::equals(0.0, 0.0), true); + assert_eq!(super::equals(-0.0, 0.0), true); + assert_eq!(super::equals(0.0, -0.0), true); + assert_eq!(super::equals(f64::NAN, -0.0), false); + assert_eq!(super::equals(0.0, f64::NAN), false); + + assert_eq!(super::equals(1.0, 1.0), true); +} + +#[test] +fn same_value() { + assert_eq!(super::same_value(0.0, 0.0), true); + assert_eq!(super::same_value(-0.0, 0.0), false); + assert_eq!(super::same_value(0.0, -0.0), false); + assert_eq!(super::same_value(f64::NAN, -0.0), false); + assert_eq!(super::same_value(0.0, f64::NAN), false); + assert_eq!(super::equals(1.0, 1.0), true); +} + +#[test] +fn same_value_zero() { + assert_eq!(super::same_value_zero(0.0, 0.0), true); + assert_eq!(super::same_value_zero(-0.0, 0.0), true); + assert_eq!(super::same_value_zero(0.0, -0.0), true); + assert_eq!(super::same_value_zero(f64::NAN, -0.0), false); + assert_eq!(super::same_value_zero(0.0, f64::NAN), false); + assert_eq!(super::equals(1.0, 1.0), true); +} diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index d7b6ed6f087..d950235c320 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -91,7 +91,7 @@ impl ObjectInternalMethods for Object { fn set_prototype_of(&mut self, val: Value) -> bool { debug_assert!(val.is_object() || val.is_null()); let current = self.get_internal_slot(PROTOTYPE); - if current == val { + if same_value(¤t, &val, false) { return true; } let extensible = self.get_internal_slot("extensible"); diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index 7ece2e74c30..f67d037b18d 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -969,7 +969,7 @@ pub fn value_of(this: &mut Value, args: &[Value], ctx: &mut Interpreter) -> Resu pub fn match_all(this: &mut Value, args: &[Value], ctx: &mut Interpreter) -> ResultValue { let mut re: Value = match args.get(0) { Some(arg) => { - if arg == &Value::null() { + if arg.is_null() { make_regexp( &mut Value::from(Object::default()), &[ @@ -978,7 +978,7 @@ pub fn match_all(this: &mut Value, args: &[Value], ctx: &mut Interpreter) -> Res ], ctx, ) - } else if arg == &Value::undefined() { + } else if arg.is_undefined() { make_regexp( &mut Value::from(Object::default()), &[Value::undefined(), Value::from(String::from("g"))], diff --git a/boa/src/builtins/value/mod.rs b/boa/src/builtins/value/mod.rs index 87249d29d08..8998f6937d1 100644 --- a/boa/src/builtins/value/mod.rs +++ b/boa/src/builtins/value/mod.rs @@ -258,7 +258,10 @@ impl ValueData { /// Returns true if the value is a number pub fn is_number(&self) -> bool { - self.is_double() + match self { + Self::Rational(_) | Self::Integer(_) => true, + _ => false, + } } /// Returns true if the value is a string @@ -295,13 +298,19 @@ impl ValueData { pub fn to_number(&self) -> f64 { match *self { Self::Object(_) | Self::Symbol(_) | Self::Undefined => NAN, - Self::String(ref str) => match FromStr::from_str(str) { - Ok(num) => num, - Err(_) => NAN, - }, - Self::Rational(num) => num, + Self::String(ref str) => { + if str.is_empty() { + return 0.0; + } + + match FromStr::from_str(str) { + Ok(num) => num, + Err(_) => NAN, + } + } Self::Boolean(true) => 1.0, Self::Boolean(false) | Self::Null => 0.0, + Self::Rational(num) => num, Self::Integer(num) => f64::from(num), } } diff --git a/boa/src/builtins/value/operations.rs b/boa/src/builtins/value/operations.rs index 66c550639a8..c42e3831bc4 100644 --- a/boa/src/builtins/value/operations.rs +++ b/boa/src/builtins/value/operations.rs @@ -1,23 +1,63 @@ use super::*; +use crate::builtins::number; +use crate::Interpreter; + +use std::borrow::Borrow; + +impl Value { + /// Strict equality comparison. + /// + /// This method is executed when doing strict equality comparisons with the `===` operator. + /// For more information, check . + pub fn strict_equals(&self, other: &Self) -> bool { + if self.get_type() != other.get_type() { + return false; + } + + if self.is_number() { + return number::equals(f64::from(self), f64::from(other)); + } + + same_value_non_number(self, other) + } + + /// Abstract equality comparison. + /// + /// This method is executed when doing abstract equality comparisons with the `==` operator. + /// For more information, check + pub fn equals(&mut self, other: &mut Self, interpreter: &mut Interpreter) -> bool { + if self.get_type() == other.get_type() { + return self.strict_equals(other); + } -impl PartialEq for Value { - fn eq(&self, other: &Self) -> bool { match (self.data(), other.data()) { - // TODO: fix this - // _ if self.ptr.to_inner() == &other.ptr.to_inner() => true, _ if self.is_null_or_undefined() && other.is_null_or_undefined() => true, - (ValueData::String(_), _) | (_, ValueData::String(_)) => { - self.to_string() == other.to_string() + + // https://github.com/rust-lang/rust/issues/54883 + (ValueData::Integer(_), ValueData::String(_)) + | (ValueData::Rational(_), ValueData::String(_)) + | (ValueData::String(_), ValueData::Integer(_)) + | (ValueData::String(_), ValueData::Rational(_)) + | (ValueData::Rational(_), ValueData::Boolean(_)) + | (ValueData::Integer(_), ValueData::Boolean(_)) => { + let a: &Value = self.borrow(); + let b: &Value = other.borrow(); + number::equals(f64::from(a), f64::from(b)) + } + (ValueData::Boolean(_), _) => { + other.equals(&mut Value::from(self.to_integer()), interpreter) + } + (_, ValueData::Boolean(_)) => { + self.equals(&mut Value::from(other.to_integer()), interpreter) + } + (ValueData::Object(_), _) => { + let mut primitive = interpreter.to_primitive(self, None); + primitive.equals(other, interpreter) } - (ValueData::Boolean(a), ValueData::Boolean(b)) if a == b => true, - (ValueData::Rational(a), ValueData::Rational(b)) - if a == b && !a.is_nan() && !b.is_nan() => - { - true + (_, ValueData::Object(_)) => { + let mut primitive = interpreter.to_primitive(other, None); + primitive.equals(self, interpreter) } - (ValueData::Rational(a), _) if *a == other.to_number() => true, - (_, ValueData::Rational(a)) if *a == self.to_number() => true, - (ValueData::Integer(a), ValueData::Integer(b)) if a == b => true, _ => false, } } @@ -96,6 +136,25 @@ impl Not for Value { } } +/// The internal comparison abstract operation SameValueZero(x, y), +/// where x and y are ECMAScript language values, produces true or false. +/// SameValueZero differs from SameValue only in its treatment of +0 and -0. +/// +/// Such a comparison is performed as follows: +/// +/// +pub fn same_value_zero(x: &Value, y: &Value) -> bool { + if x.get_type() != y.get_type() { + return false; + } + + if x.is_number() { + return number::same_value_zero(f64::from(x), f64::from(y)); + } + + same_value_non_number(x, y) +} + /// The internal comparison abstract operation SameValue(x, y), /// where x and y are ECMAScript language values, produces true or false. /// Such a comparison is performed as follows: @@ -114,10 +173,10 @@ pub fn same_value(x: &Value, y: &Value, strict: bool) -> bool { return false; } - if x.get_type() == "number" { - let native_x = f64::from(x); - let native_y = f64::from(y); - return native_x.abs() - native_y.abs() == 0.0; + // TODO: check BigInt + // https://github.com/jasonwilliams/boa/pull/358 + if x.is_number() { + return number::same_value(f64::from(x), f64::from(y)); } same_value_non_number(x, y) @@ -135,7 +194,7 @@ pub fn same_value_non_number(x: &Value, y: &Value) -> bool { false } "boolean" => bool::from(x) == bool::from(y), - "object" => *x == *y, + "object" => std::ptr::eq(x, y), _ => false, } } diff --git a/boa/src/builtins/value/tests.rs b/boa/src/builtins/value/tests.rs index af94f1b57b6..bfccf0fe438 100644 --- a/boa/src/builtins/value/tests.rs +++ b/boa/src/builtins/value/tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::{forward, Executor, Realm}; #[test] fn check_is_object() { @@ -46,3 +47,50 @@ fn check_number_is_true() { assert_eq!(Value::from(-1.0).is_true(), true); assert_eq!(Value::from(NAN).is_true(), false); } + +// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness +#[test] +fn abstract_equality_comparison() { + let realm = Realm::create(); + let mut engine = Executor::new(realm); + + assert_eq!(forward(&mut engine, "undefined == undefined"), "true"); + assert_eq!(forward(&mut engine, "null == null"), "true"); + assert_eq!(forward(&mut engine, "true == true"), "true"); + assert_eq!(forward(&mut engine, "false == false"), "true"); + assert_eq!(forward(&mut engine, "'foo' == 'foo'"), "true"); + assert_eq!(forward(&mut engine, "0 == 0"), "true"); + assert_eq!(forward(&mut engine, "+0 == -0"), "true"); + assert_eq!(forward(&mut engine, "+0 == 0"), "true"); + assert_eq!(forward(&mut engine, "-0 == 0"), "true"); + assert_eq!(forward(&mut engine, "0 == false"), "true"); + assert_eq!(forward(&mut engine, "'' == false"), "true"); + assert_eq!(forward(&mut engine, "'' == 0"), "true"); + assert_eq!(forward(&mut engine, "'17' == 17"), "true"); + assert_eq!(forward(&mut engine, "[1,2] == '1,2'"), "true"); + assert_eq!(forward(&mut engine, "new String('foo') == 'foo'"), "true"); + assert_eq!(forward(&mut engine, "null == undefined"), "true"); + assert_eq!(forward(&mut engine, "undefined == null"), "true"); + assert_eq!(forward(&mut engine, "null == false"), "false"); + assert_eq!(forward(&mut engine, "[] == ![]"), "true"); + assert_eq!( + forward(&mut engine, "a = { foo: 'bar' }; b = { foo: 'bar'}; a == b"), + "false" + ); + assert_eq!( + forward(&mut engine, "new String('foo') == new String('foo')"), + "false" + ); + assert_eq!(forward(&mut engine, "0 == null"), "false"); + + // https://github.com/jasonwilliams/boa/issues/357 + assert_eq!(forward(&mut engine, "0 == '-0'"), "true"); + assert_eq!(forward(&mut engine, "0 == '+0'"), "true"); + assert_eq!(forward(&mut engine, "'+0' == 0"), "true"); + assert_eq!(forward(&mut engine, "'-0' == 0"), "true"); + + // https://github.com/jasonwilliams/boa/issues/393 + // assert_eq!(forward(&mut engine, "0 == NaN"), "false"); + // assert_eq!(forward(&mut engine, "'foo' == NaN"), "false"); + // assert_eq!(forward(&mut engine, "NaN == NaN"), "false"); +} diff --git a/boa/src/exec/mod.rs b/boa/src/exec/mod.rs index f5e4deb83aa..524b693771a 100644 --- a/boa/src/exec/mod.rs +++ b/boa/src/exec/mod.rs @@ -209,7 +209,7 @@ impl Executor for Interpreter { for tup in vals.iter() { let cond = &tup.0; let block = &tup.1; - if val == self.run(cond)? { + if val.strict_equals(&self.run(cond)?) { matched = true; let last_expr = block.last().expect("Block has no expressions"); for expr in block.iter() { @@ -446,14 +446,10 @@ impl Executor for Interpreter { let mut v_a = v_r_a.borrow_mut(); let mut v_b = v_r_b.borrow_mut(); Ok(Value::from(match *op { - CompOp::Equal if v_a.is_object() => v_r_a == v_r_b, - CompOp::Equal => v_a == v_b, - CompOp::NotEqual if v_a.is_object() => v_r_a != v_r_b, - CompOp::NotEqual => v_a != v_b, - CompOp::StrictEqual if v_a.is_object() => v_r_a == v_r_b, - CompOp::StrictEqual => v_a == v_b, - CompOp::StrictNotEqual if v_a.is_object() => v_r_a != v_r_b, - CompOp::StrictNotEqual => v_a != v_b, + CompOp::Equal => v_r_a.equals(v_b, self), + CompOp::NotEqual => !v_r_a.equals(v_b, self), + CompOp::StrictEqual => v_r_a.strict_equals(v_b), + CompOp::StrictNotEqual => !v_r_a.strict_equals(v_b), CompOp::GreaterThan => v_a.to_number() > v_b.to_number(), CompOp::GreaterThanOrEqual => v_a.to_number() >= v_b.to_number(), CompOp::LessThan => v_a.to_number() < v_b.to_number(),