From 634dfb7e80c767946cbafd4a73450ee41ed61a8a Mon Sep 17 00:00:00 2001 From: Paul Lancaster Date: Wed, 10 Jun 2020 11:25:40 +0100 Subject: [PATCH] Optimise type comparisons (#441) Co-authored-by: Iban Eguia --- boa/src/builtins/value/equality.rs | 15 ++--- boa/src/builtins/value/mod.rs | 27 ++------ boa/src/builtins/value/tests.rs | 61 ++++++++++++++++++- boa/src/builtins/value/val_type.rs | 59 ++++++++++++++++++ boa/src/exec/conditional/mod.rs | 7 +-- boa/src/exec/expression/mod.rs | 4 +- boa/src/exec/field/mod.rs | 6 +- boa/src/exec/mod.rs | 4 +- boa/src/exec/operator/mod.rs | 4 +- .../syntax/parser/statement/break_stm/mod.rs | 1 + .../parser/statement/continue_stm/mod.rs | 5 +- .../parser/statement/declaration/mod.rs | 1 + boa/src/syntax/parser/statement/if_stm/mod.rs | 1 + 13 files changed, 145 insertions(+), 50 deletions(-) create mode 100644 boa/src/builtins/value/val_type.rs diff --git a/boa/src/builtins/value/equality.rs b/boa/src/builtins/value/equality.rs index f76fd357693..caa6573bcde 100644 --- a/boa/src/builtins/value/equality.rs +++ b/boa/src/builtins/value/equality.rs @@ -200,16 +200,11 @@ pub fn same_value_zero(x: &Value, y: &Value) -> bool { pub fn same_value_non_numeric(x: &Value, y: &Value) -> bool { debug_assert!(x.get_type() == y.get_type()); match x.get_type() { - "undefined" => true, - "null" => true, - "string" => { - if x.to_string() == y.to_string() { - return true; - } - false - } - "boolean" => bool::from(x) == bool::from(y), - "object" => std::ptr::eq(x, y), + Type::Undefined => true, + Type::Null => true, + Type::String => x.to_string() == y.to_string(), + Type::Boolean => bool::from(x) == bool::from(y), + Type::Object => std::ptr::eq(x, y), _ => false, } } diff --git a/boa/src/builtins/value/mod.rs b/boa/src/builtins/value/mod.rs index ad8cac3e3d6..56c7564dc38 100644 --- a/boa/src/builtins/value/mod.rs +++ b/boa/src/builtins/value/mod.rs @@ -5,6 +5,10 @@ #[cfg(test)] mod tests; +pub mod val_type; + +pub use crate::builtins::value::val_type::Type; + use crate::builtins::{ object::{ internal_methods_trait::ObjectInternalMethods, InternalState, InternalStateCell, Object, @@ -765,29 +769,6 @@ impl ValueData { new_func_val.set_field("length", Value::from(length)); new_func_val } - - /// Get the type of the value - /// - /// https://tc39.es/ecma262/#sec-typeof-operator - pub fn get_type(&self) -> &'static str { - let _timer = BoaProfiler::global().start_event("Value::get_type", "value"); - match *self { - Self::Rational(_) | Self::Integer(_) => "number", - Self::String(_) => "string", - Self::Boolean(_) => "boolean", - Self::Symbol(_) => "symbol", - Self::Null => "object", - Self::Undefined => "undefined", - Self::Object(ref o) => { - if o.deref().borrow().is_callable() { - "function" - } else { - "object" - } - } - Self::BigInt(_) => "bigint", - } - } } impl Default for ValueData { diff --git a/boa/src/builtins/value/tests.rs b/boa/src/builtins/value/tests.rs index 193efee084c..a13883f19de 100644 --- a/boa/src/builtins/value/tests.rs +++ b/boa/src/builtins/value/tests.rs @@ -1,5 +1,5 @@ use super::*; -use crate::{forward, Interpreter, Realm}; +use crate::{forward, forward_val, Interpreter, Realm}; #[test] fn check_is_object() { @@ -18,7 +18,7 @@ fn check_string_to_value() { #[test] fn check_undefined() { let u = ValueData::Undefined; - assert_eq!(u.get_type(), "undefined"); + assert_eq!(u.get_type(), Type::Undefined); assert_eq!(u.to_string(), "undefined"); } @@ -92,3 +92,60 @@ fn abstract_equality_comparison() { assert_eq!(forward(&mut engine, "'foo' == NaN"), "false"); assert_eq!(forward(&mut engine, "NaN == NaN"), "false"); } + +#[test] +fn get_types() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + + assert_eq!( + forward_val(&mut engine, "undefined").unwrap().get_type(), + Type::Undefined + ); + assert_eq!( + forward_val(&mut engine, "1").unwrap().get_type(), + Type::Number + ); + assert_eq!( + forward_val(&mut engine, "1.5").unwrap().get_type(), + Type::Number + ); + assert_eq!( + forward_val(&mut engine, "BigInt(\"123442424242424424242424242\")") + .unwrap() + .get_type(), + Type::BigInt + ); + assert_eq!( + forward_val(&mut engine, "true").unwrap().get_type(), + Type::Boolean + ); + assert_eq!( + forward_val(&mut engine, "false").unwrap().get_type(), + Type::Boolean + ); + assert_eq!( + forward_val(&mut engine, "function foo() {console.log(\"foo\");}") + .unwrap() + .get_type(), + Type::Function + ); + assert_eq!( + forward_val(&mut engine, "null").unwrap().get_type(), + Type::Null + ); + assert_eq!( + forward_val(&mut engine, "var x = {arg: \"hi\", foo: \"hello\"}; x") + .unwrap() + .get_type(), + Type::Object + ); + assert_eq!( + forward_val(&mut engine, "\"Hi\"").unwrap().get_type(), + Type::String + ); + assert_eq!( + forward_val(&mut engine, "Symbol()").unwrap().get_type(), + Type::Symbol + ); +} diff --git a/boa/src/builtins/value/val_type.rs b/boa/src/builtins/value/val_type.rs new file mode 100644 index 00000000000..9f49fdea9fe --- /dev/null +++ b/boa/src/builtins/value/val_type.rs @@ -0,0 +1,59 @@ +use crate::builtins::value::ValueData; +use std::ops::Deref; + +/// Possible types of val as defined at https://tc39.es/ecma262/#sec-typeof-operator. +/// Note that an object which implements call is referred to here as 'Function'. +#[derive(Eq, PartialEq, Debug, Clone, Copy)] +pub enum Type { + Undefined, + Null, + Boolean, + Number, + String, + Symbol, + BigInt, + Object, + Function, +} + +impl Type { + pub fn as_str(&self) -> &str { + match self { + Self::Number => "number", + Self::String => "string", + Self::Boolean => "boolean", + Self::Symbol => "symbol", + Self::Null => "object", + Self::Undefined => "undefined", + Self::Function => "function", + Self::Object => "object", + Self::BigInt => "bigint", + } + } +} + +impl ValueData { + /// Get the type of the value. + /// + /// This is similar to typeof as described at https://tc39.es/ecma262/#sec-typeof-operator but instead of + /// returning a string it returns a Type enum which implements fmt::Display to allow getting the string if + /// required using to_string(). + pub fn get_type(&self) -> Type { + match *self { + Self::Rational(_) | Self::Integer(_) => Type::Number, + Self::String(_) => Type::String, + Self::Boolean(_) => Type::Boolean, + Self::Symbol(_) => Type::Symbol, + Self::Null => Type::Null, + Self::Undefined => Type::Undefined, + Self::Object(ref o) => { + if o.deref().borrow().is_callable() { + Type::Function + } else { + Type::Object + } + } + Self::BigInt(_) => Type::BigInt, + } + } +} diff --git a/boa/src/exec/conditional/mod.rs b/boa/src/exec/conditional/mod.rs index 582f9bdc212..1649ef77686 100644 --- a/boa/src/exec/conditional/mod.rs +++ b/boa/src/exec/conditional/mod.rs @@ -9,11 +9,10 @@ impl Executable for If { fn run(&self, interpreter: &mut Interpreter) -> ResultValue { Ok(if self.cond().run(interpreter)?.borrow().is_true() { self.body().run(interpreter)? + } else if let Some(ref else_e) = self.else_node() { + else_e.run(interpreter)? } else { - match self.else_node() { - Some(ref else_e) => else_e.run(interpreter)?, - None => Value::undefined(), - } + Value::undefined() }) } } diff --git a/boa/src/exec/expression/mod.rs b/boa/src/exec/expression/mod.rs index 838719b8ecc..4062bfc71f5 100644 --- a/boa/src/exec/expression/mod.rs +++ b/boa/src/exec/expression/mod.rs @@ -4,7 +4,7 @@ use super::{Executable, Interpreter}; use crate::{ builtins::{ object::{INSTANCE_PROTOTYPE, PROTOTYPE}, - value::{ResultValue, Value, ValueData}, + value::{ResultValue, Type, Value, ValueData}, }, syntax::ast::node::{Call, New, Node}, BoaProfiler, @@ -16,7 +16,7 @@ impl Executable for Call { let (mut this, func) = match self.expr() { Node::GetConstField(ref get_const_field) => { let mut obj = get_const_field.obj().run(interpreter)?; - if obj.get_type() != "object" || obj.get_type() != "symbol" { + if obj.get_type() != Type::Object || obj.get_type() != Type::Symbol { obj = interpreter .to_object(&obj) .expect("failed to convert to object"); diff --git a/boa/src/exec/field/mod.rs b/boa/src/exec/field/mod.rs index 872d0c25303..ee5b5489262 100644 --- a/boa/src/exec/field/mod.rs +++ b/boa/src/exec/field/mod.rs @@ -1,13 +1,13 @@ use super::{Executable, Interpreter}; use crate::{ - builtins::value::ResultValue, + builtins::value::{ResultValue, Type}, syntax::ast::node::{GetConstField, GetField}, }; impl Executable for GetConstField { fn run(&self, interpreter: &mut Interpreter) -> ResultValue { let mut obj = self.obj().run(interpreter)?; - if obj.get_type() != "object" || obj.get_type() != "symbol" { + if obj.get_type() != Type::Object || obj.get_type() != Type::Symbol { obj = interpreter .to_object(&obj) .expect("failed to convert to object"); @@ -20,7 +20,7 @@ impl Executable for GetConstField { impl Executable for GetField { fn run(&self, interpreter: &mut Interpreter) -> ResultValue { let mut obj = self.obj().run(interpreter)?; - if obj.get_type() != "object" || obj.get_type() != "symbol" { + if obj.get_type() != Type::Object || obj.get_type() != Type::Symbol { obj = interpreter .to_object(&obj) .expect("failed to convert to object"); diff --git a/boa/src/exec/mod.rs b/boa/src/exec/mod.rs index a0a4b05889d..4aec876a762 100644 --- a/boa/src/exec/mod.rs +++ b/boa/src/exec/mod.rs @@ -27,7 +27,7 @@ use crate::{ PROTOTYPE, }, property::Property, - value::{ResultValue, Value, ValueData}, + value::{ResultValue, Type, Value, ValueData}, BigInt, Number, }, realm::Realm, @@ -222,7 +222,7 @@ impl Interpreter { /// pub(crate) fn ordinary_to_primitive(&mut self, o: &mut Value, hint: &str) -> Value { - debug_assert!(o.get_type() == "object"); + debug_assert!(o.get_type() == Type::Object); debug_assert!(hint == "string" || hint == "number"); let method_names: Vec<&str> = if hint == "string" { vec!["toString", "valueOf"] diff --git a/boa/src/exec/operator/mod.rs b/boa/src/exec/operator/mod.rs index bc8d2d63286..35a099dc51b 100644 --- a/boa/src/exec/operator/mod.rs +++ b/boa/src/exec/operator/mod.rs @@ -91,7 +91,7 @@ impl Executable for BinOp { if !v_b.is_object() { return interpreter.throw_type_error(format!( "right-hand side of 'in' should be an object, got {}", - v_b.get_type() + v_b.get_type().as_str() )); } let key = interpreter.to_property_key(&mut v_a)?; @@ -219,7 +219,7 @@ impl Executable for UnaryOp { | Node::UnaryOp(_) => Value::boolean(true), _ => panic!("SyntaxError: wrong delete argument {}", self), }, - op::UnaryOp::TypeOf => Value::from(v_a.get_type()), + op::UnaryOp::TypeOf => Value::from(v_a.get_type().as_str()), }) } } diff --git a/boa/src/syntax/parser/statement/break_stm/mod.rs b/boa/src/syntax/parser/statement/break_stm/mod.rs index 526cd547a0f..e0a070de1f0 100644 --- a/boa/src/syntax/parser/statement/break_stm/mod.rs +++ b/boa/src/syntax/parser/statement/break_stm/mod.rs @@ -11,6 +11,7 @@ mod tests; use super::LabelIdentifier; + use crate::{ syntax::{ ast::{node::Break, Keyword, Punctuator, TokenKind}, diff --git a/boa/src/syntax/parser/statement/continue_stm/mod.rs b/boa/src/syntax/parser/statement/continue_stm/mod.rs index 8925f655fc5..433f41dd0fc 100644 --- a/boa/src/syntax/parser/statement/continue_stm/mod.rs +++ b/boa/src/syntax/parser/statement/continue_stm/mod.rs @@ -10,11 +10,12 @@ #[cfg(test)] mod tests; -use super::LabelIdentifier; use crate::{ syntax::{ ast::{node::Continue, Keyword, Punctuator, TokenKind}, - parser::{AllowAwait, AllowYield, Cursor, ParseError, TokenParser}, + parser::{ + statement::LabelIdentifier, AllowAwait, AllowYield, Cursor, ParseError, TokenParser, + }, }, BoaProfiler, }; diff --git a/boa/src/syntax/parser/statement/declaration/mod.rs b/boa/src/syntax/parser/statement/declaration/mod.rs index 0e0bc52e928..c22aa676569 100644 --- a/boa/src/syntax/parser/statement/declaration/mod.rs +++ b/boa/src/syntax/parser/statement/declaration/mod.rs @@ -13,6 +13,7 @@ mod lexical; mod tests; use self::{hoistable::HoistableDeclaration, lexical::LexicalDeclaration}; + use crate::{ syntax::{ ast::{Keyword, Node, TokenKind}, diff --git a/boa/src/syntax/parser/statement/if_stm/mod.rs b/boa/src/syntax/parser/statement/if_stm/mod.rs index ff1e47de71d..d8e6b1c7294 100644 --- a/boa/src/syntax/parser/statement/if_stm/mod.rs +++ b/boa/src/syntax/parser/statement/if_stm/mod.rs @@ -2,6 +2,7 @@ mod tests; use super::Statement; + use crate::{ syntax::{ ast::{node::If, Keyword, Node, Punctuator, TokenKind},