From d79b1c72594e1779086fe7b01897d5b9b94eb702 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 18:45:54 +0100 Subject: [PATCH 01/31] Initial implementation of Map() --- boa/src/builtins/map/mod.rs | 312 ++++++++++++++++++++++++++++++ boa/src/builtins/map/tests.rs | 168 ++++++++++++++++ boa/src/builtins/mod.rs | 3 + boa/src/builtins/object/mod.rs | 29 ++- boa/src/builtins/value/display.rs | 25 +++ boa/src/exec/mod.rs | 32 +++ 6 files changed, 568 insertions(+), 1 deletion(-) create mode 100644 boa/src/builtins/map/mod.rs create mode 100644 boa/src/builtins/map/tests.rs diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs new file mode 100644 index 00000000000..afbcc190ad5 --- /dev/null +++ b/boa/src/builtins/map/mod.rs @@ -0,0 +1,312 @@ +use super::function::{make_builtin_fn, make_constructor_fn}; +use crate::{ + builtins::{ + object::{InternalState, ObjectData, INSTANCE_PROTOTYPE, PROTOTYPE}, + property::Property, + value::{ResultValue, Value}, + }, + exec::Interpreter, + BoaProfiler, +}; +use std::collections::HashMap; + +#[cfg(test)] +mod tests; + +#[derive(Debug, Clone)] +pub(crate) struct Map(HashMap); + +impl Map { + pub(crate) const NAME: &'static str = "Map"; + + pub(crate) const LENGTH: usize = 1; + + /// Helper function to get the map object. + fn get_map(this: &Value, ctx: &mut Interpreter) -> Result, Value> { + if let Value::Object(ref object) = this { + let object = object.borrow(); + if let Some(map) = object.as_map() { + return Ok(map); + } + } + Err(ctx.construct_type_error("'this' is not a Map")) + } + + /// Helper function to set the size property. + fn set_size(this: &Value, size: usize) { + let size = Property::new() + .value(Value::from(size)) + .writable(false) + .configurable(false) + .enumerable(false); + + this.set_property("size".to_string(), size); + } + + /// `Map.prototype.set( key, value )` + /// + /// This method associates the value with the key. Returns the map object. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.set + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/set + pub(crate) fn set(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultValue { + let (key, value) = match args.len() { + 0 => (Value::Undefined, Value::Undefined), + 1 => (args[0].clone(), Value::Undefined), + _ => (args[0].clone(), args[1].clone()), + }; + + let mut map = Self::get_map(this, ctx)?; + + map.insert(key, value); + Self::set_size(this, map.len()); + + this.set_data(ObjectData::Map(map)); + Ok(this.clone()) + } + + /// `Map.prototype.delete( key )` + /// + /// This method removes the element associated with the key, if it exists. Returns true if there was an element, false otherwise. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.delete + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/delete + pub(crate) fn delete(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultValue { + let undefined = Value::Undefined; + let key = match args.len() { + 0 => &undefined, + _ => &args[0], + }; + + let mut map = Self::get_map(this, ctx)?; + + let deleted = map.remove(key).is_some(); + Self::set_size(this, map.len()); + + this.set_data(ObjectData::Map(map)); + Ok(Value::boolean(deleted)) + } + + /// `Map.prototype.get( key )` + /// + /// This method returns the value associated with the key, or undefined if there is none. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.get + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/get + pub(crate) fn get(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultValue { + let undefined = Value::Undefined; + let key = match args.len() { + 0 => &undefined, + _ => &args[0], + }; + + if let Value::Object(ref object) = this { + let object = object.borrow(); + if let Some(map) = object.as_map_ref() { + return Ok(if let Some(result) = map.get(key) { + result.clone() + } else { + Value::Undefined + }); + } + } + + Err(ctx.construct_type_error("'this' is not a Map")) + } + + /// `Map.prototype.clear( )` + /// + /// This method removes all entries from the map. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.clear + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/clear + pub(crate) fn clear(this: &Value, _: &[Value], _: &mut Interpreter) -> ResultValue { + this.set_data(ObjectData::Map(HashMap::new())); + + Self::set_size(this, 0); + + Ok(Value::Undefined) + } + + /// `Map.prototype.has( key )` + /// + /// This method checks if the map contains an entry with the given key. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.has + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/has + pub(crate) fn has(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultValue { + let undefined = Value::Undefined; + let key = match args.len() { + 0 => &undefined, + _ => &args[0], + }; + + if let Value::Object(ref object) = this { + let object = object.borrow(); + if let Some(map) = object.as_map_ref() { + return Ok(Value::Boolean(map.contains_key(key))); + } + } + + Err(ctx.construct_type_error("'this' is not a Map")) + } + + /// `Map.prototype.forEach( callbackFn [ , thisArg ] )` + /// + /// This method executes the provided callback function for each key-value pair in the map. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.foreach + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/forEach + pub(crate) fn for_each( + this: &Value, + args: &[Value], + interpreter: &mut Interpreter, + ) -> ResultValue { + if args.is_empty() { + return Err(Value::from("Missing argument for Map.prototype.forEach")); + } + + let callback_arg = &args[0]; + let this_arg = args.get(1).cloned().unwrap_or_else(Value::undefined); + + if let Value::Object(ref object) = this { + let object = object.borrow(); + if let Some(map) = object.as_map() { + for (key, value) in map { + let arguments = [value, key, this.clone()]; + + interpreter.call(callback_arg, &this_arg, &arguments)?; + } + } + } + + Ok(Value::Undefined) + } + + /// Helper function to get a key-value pair from an array. + fn get_key_value(value: &Value) -> Option<(Value, Value)> { + if let Value::Object(object) = value { + if object.borrow().is_array() { + let (key, value) = match i32::from(&value.get_field("length")) { + 0 => (Value::Undefined, Value::Undefined), + 1 => (value.get_field("0"), Value::Undefined), + _ => (value.get_field("0"), value.get_field("1")), + }; + return Some((key, value)); + } + } + None + } + + /// Create a new map + pub(crate) fn make_map(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultValue { + // Make a new Object which will internally represent the Array (mapping + // between indices and values): this creates an Object with no prototype + + // Set Prototype + let prototype = ctx.realm.global_obj.get_field("Map").get_field(PROTOTYPE); + + this.set_internal_slot(INSTANCE_PROTOTYPE, prototype); + // This value is used by console.log and other routines to match Object type + // to its Javascript Identifier (global constructor method name) + + // add our arguments in + let data = match args.len() { + 0 => HashMap::new(), + _ => match &args[0] { + Value::Object(object) => { + let object = object.borrow(); + if let Some(map) = object.as_map() { + map.clone() + } else if object.is_array() { + let mut map = HashMap::new(); + let len = i32::from(&args[0].get_field("length")); + for i in 0..len { + let val = &args[0].get_field(i.to_string()); + let (key, value) = + Self::get_key_value(val).ok_or(ctx.construct_type_error( + "iterable for Map should have array-like objects", + ))?; + map.insert(key, value); + } + map + } else { + return Err(ctx.construct_type_error( + "iterable for Map should have array-like objects", + )); + } + } + _ => { + return Err( + ctx.construct_type_error("iterable for Map should have array-like objects") + ) + } + }, + }; + + // finally create length property + Self::set_size(this, data.len()); + + this.set_data(ObjectData::Map(data)); + + Ok(this.clone()) + } + + /// Create a new `Map` object. + pub(crate) fn create(global: &Value) -> Value { + // Create prototype + let prototype = Value::new_object(Some(global)); + + make_builtin_fn(Self::set, "set", &prototype, 2); + make_builtin_fn(Self::delete, "delete", &prototype, 1); + make_builtin_fn(Self::get, "get", &prototype, 1); + make_builtin_fn(Self::clear, "clear", &prototype, 0); + make_builtin_fn(Self::has, "has", &prototype, 1); + make_builtin_fn(Self::for_each, "forEach", &prototype, 1); + + let map = make_constructor_fn( + Self::NAME, + Self::LENGTH, + Self::make_map, + global, + prototype, + true, + ); + + map + } + + /// Initialise the `Map` object on the global object. + #[inline] + pub(crate) fn init(global: &Value) -> (&str, Value) { + let _timer = BoaProfiler::global().start_event(Self::NAME, "init"); + + (Self::NAME, Self::create(global)) + } +} + +impl InternalState for HashMap {} diff --git a/boa/src/builtins/map/tests.rs b/boa/src/builtins/map/tests.rs new file mode 100644 index 00000000000..fb416cc509b --- /dev/null +++ b/boa/src/builtins/map/tests.rs @@ -0,0 +1,168 @@ +use crate::{exec::Interpreter, forward, realm::Realm}; + +#[test] +fn construct_empty() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + var empty = new Map(); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "empty.size"); + assert_eq!(result, "0"); +} + +#[test] +fn construct_from_array() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([["1", "one"], ["2", "two"]]); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.size"); + assert_eq!(result, "2"); +} + +#[test] +fn clone() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let original = new Map([["1", "one"], ["2", "two"]]); + let clone = new Map(original); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "clone.size"); + assert_eq!(result, "2"); + let result = forward( + &mut engine, + r#" + original.set("3", "three"); + original.size"#, + ); + assert_eq!(result, "3"); + let result = forward(&mut engine, "clone.size"); + assert_eq!(result, "2"); +} + +#[test] +fn merge() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let first = new Map([["1", "one"], ["2", "two"]]); + let second = new Map([["2", "second two"], ["3", "three"]]); + let third = new Map([["4", "four"], ["5", "five"]]); + let merged1 = new Map([...first, ...second]); + let merged2 = new Map([...second, ...third]); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "merged1.size"); + assert_eq!(result, "3"); + let result = forward(&mut engine, "merged1.get('2')"); + assert_eq!(result, "second two"); + let result = forward(&mut engine, "merged2.size"); + assert_eq!(result, "4"); +} + +#[test] +fn get() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([["1", "one"], ["2", "two"]]); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.get('1')"); + assert_eq!(result, "one"); + let result = forward(&mut engine, "map.get('2')"); + assert_eq!(result, "two"); + let result = forward(&mut engine, "map.get('3')"); + assert_eq!(result, "undefined"); + let result = forward(&mut engine, "map.get()"); + assert_eq!(result, "undefined"); +} + +#[test] +fn set() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map(); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.set();map.size"); + assert_eq!(result, "1"); + let result = forward(&mut engine, "map.set('1', 'one');map.size"); + assert_eq!(result, "2"); + let result = forward(&mut engine, "map.set('2');map.size"); + assert_eq!(result, "3"); +} + +#[test] +fn clear() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([["1", "one"], ["2", "two"]]); + map.clear(); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.size"); + assert_eq!(result, "0"); +} + +#[test] +fn delete() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([["1", "one"], ["2", "two"]]); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.delete('1')"); + assert_eq!(result, "true"); + let result = forward(&mut engine, "map.size"); + assert_eq!(result, "1"); + let result = forward(&mut engine, "map.delete('1')"); + assert_eq!(result, "false"); +} + +#[test] +fn has() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([["1", "one"]]); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.has()"); + assert_eq!(result, "false"); + let result = forward(&mut engine, "map.has('1')"); + assert_eq!(result, "true"); + let result = forward(&mut engine, "map.has('2')"); + assert_eq!(result, "false"); +} + +#[test] +fn for_each() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([[1, 5], [2, 10], [3, 15]]); + let valueSum = 0; + let keySum = 0; + let sizeSum = 0; + function callingCallback(value, key, map) { + valueSum += value; + keySum += key; + sizeSum += map.size; + } + map.forEach(callingCallback); + "#; + forward(&mut engine, init); + assert_eq!(forward(&mut engine, "valueSum"), "30"); + assert_eq!(forward(&mut engine, "keySum"), "6"); + assert_eq!(forward(&mut engine, "sizeSum"), "9"); +} diff --git a/boa/src/builtins/mod.rs b/boa/src/builtins/mod.rs index 46ba43b777d..61ee7779bd1 100644 --- a/boa/src/builtins/mod.rs +++ b/boa/src/builtins/mod.rs @@ -9,6 +9,7 @@ pub mod function; pub mod global_this; pub mod infinity; pub mod json; +pub mod map; pub mod math; pub mod nan; pub mod number; @@ -28,6 +29,7 @@ pub(crate) use self::{ global_this::GlobalThis, infinity::Infinity, json::Json, + map::Map, math::Math, nan::NaN, number::Number, @@ -49,6 +51,7 @@ pub fn init(global: &Value) { BigInt::init, Boolean::init, Json::init, + Map::init, Math::init, Number::init, RegExp::init, diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index 58c2aeedaa1..c78b37c05a1 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -25,7 +25,10 @@ use crate::{ }; use gc::{Finalize, Trace}; use rustc_hash::FxHashMap; -use std::fmt::{Debug, Display, Error, Formatter}; +use std::{ + collections::HashMap, + fmt::{Debug, Display, Error, Formatter}, +}; use super::function::{make_builtin_fn, make_constructor_fn}; use crate::builtins::value::same_value; @@ -67,6 +70,7 @@ pub struct Object { #[derive(Debug, Trace, Finalize, Clone)] pub enum ObjectData { Array, + Map(HashMap), BigInt(RcBigInt), Boolean(bool), Function(Function), @@ -85,6 +89,7 @@ impl Display for ObjectData { match self { Self::Function(_) => "Function", Self::Array => "Array", + Self::Map(_) => "Map", Self::String(_) => "String", Self::Symbol(_) => "Symbol", Self::Error => "Error", @@ -251,6 +256,28 @@ impl Object { } } + /// Checks if it is a `Map` object.pub + #[inline] + pub fn is_map(&self) -> bool { + matches!(self.data, ObjectData::Map(_)) + } + + #[inline] + pub fn as_map(&self) -> Option> { + match self.data { + ObjectData::Map(ref map) => Some(map.clone()), + _ => None, + } + } + + #[inline] + pub fn as_map_ref(&self) -> Option<&HashMap> { + match self.data { + ObjectData::Map(ref map) => Some(map), + _ => None, + } + } + /// Checks if it a `String` object. #[inline] pub fn is_string(&self) -> bool { diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index 5637c04abb6..7b352940c72 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -102,6 +102,31 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool) -> String { format!("[ {} ]", arr) } + ObjectData::Map(ref map) => { + let size = i32::from( + &v.borrow() + .properties() + .get("size") + .unwrap() + .value + .clone() + .expect("Could not borrow value"), + ); + if size == 0 { + return String::from("Map(0)"); + } + + let mappings = map + .iter() + .map(|(key, value)| { + let key = log_string_from(key, print_internals); + let value = log_string_from(value, print_internals); + format!("{} → {}", key, value) + }) + .collect::>() + .join(", "); + format!("Map {{ {} }}", mappings) + } _ => display_obj(&x, print_internals), } } diff --git a/boa/src/exec/mod.rs b/boa/src/exec/mod.rs index 479ebf775a9..da926109d7c 100644 --- a/boa/src/exec/mod.rs +++ b/boa/src/exec/mod.rs @@ -38,6 +38,7 @@ use crate::{ }, BoaProfiler, }; +use std::borrow::Borrow; use std::convert::TryFrom; use std::ops::Deref; @@ -373,6 +374,37 @@ impl Interpreter { .collect(); return Ok(values); } + // Check if object is a Map + else if let ObjectData::Map(ref map) = x.deref().borrow().data { + let values = map + .iter() + .map(|(key, value)| { + // Construct a new array containing the key-value pair + let array = Value::new_object(Some( + &self + .realm() + .environment + .get_global_object() + .expect("Could not get global object"), + )); + array.set_data(ObjectData::Array); + array.borrow().set_internal_slot( + INSTANCE_PROTOTYPE, + self.realm() + .environment + .get_binding_value("Array") + .expect("Array was not initialized") + .borrow() + .get_field(PROTOTYPE), + ); + array.borrow().set_field("0", key); + array.borrow().set_field("1", value); + array.borrow().set_field("length", Value::from(2)); + array + }) + .collect(); + return Ok(values); + } return Err(()); } From 352a78d71ab4d820b6f82946f7306e48929a919f Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 19:00:54 +0100 Subject: [PATCH 02/31] Remove impl InternalState for HashMap --- boa/src/builtins/map/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index afbcc190ad5..9dd1d9781fb 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -1,7 +1,7 @@ use super::function::{make_builtin_fn, make_constructor_fn}; use crate::{ builtins::{ - object::{InternalState, ObjectData, INSTANCE_PROTOTYPE, PROTOTYPE}, + object::{ObjectData, INSTANCE_PROTOTYPE, PROTOTYPE}, property::Property, value::{ResultValue, Value}, }, @@ -308,5 +308,3 @@ impl Map { (Self::NAME, Self::create(global)) } } - -impl InternalState for HashMap {} From aadf7717a0983e28fd1319fe59adeaf8313e07f4 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 19:04:27 +0100 Subject: [PATCH 03/31] Fix clippy lint --- boa/src/builtins/map/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 9dd1d9781fb..7768482ec4b 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -288,7 +288,7 @@ impl Map { make_builtin_fn(Self::has, "has", &prototype, 1); make_builtin_fn(Self::for_each, "forEach", &prototype, 1); - let map = make_constructor_fn( + make_constructor_fn( Self::NAME, Self::LENGTH, Self::make_map, @@ -296,8 +296,6 @@ impl Map { prototype, true, ); - - map } /// Initialise the `Map` object on the global object. From cf925ce1ac37eedbce82bb453de7e74a92127504 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 19:06:57 +0100 Subject: [PATCH 04/31] Actually fix clippy lint --- boa/src/builtins/map/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 7768482ec4b..98ac13d6b3e 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -295,7 +295,7 @@ impl Map { global, prototype, true, - ); + ) } /// Initialise the `Map` object on the global object. From 029863fd7c3a99474966852eae337a3b4cc353d5 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 19:17:08 +0100 Subject: [PATCH 05/31] Fix ok_or lint --- boa/src/builtins/map/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 98ac13d6b3e..05f68c7d6e6 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -241,14 +241,14 @@ impl Map { Value::Object(object) => { let object = object.borrow(); if let Some(map) = object.as_map() { - map.clone() + map } else if object.is_array() { let mut map = HashMap::new(); let len = i32::from(&args[0].get_field("length")); for i in 0..len { let val = &args[0].get_field(i.to_string()); let (key, value) = - Self::get_key_value(val).ok_or(ctx.construct_type_error( + Self::get_key_value(val).ok_or_else(|| ctx.construct_type_error( "iterable for Map should have array-like objects", ))?; map.insert(key, value); From 85a3702a080e8099d9e48237294ca383460cd2bd Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 19:18:10 +0100 Subject: [PATCH 06/31] format code --- boa/src/builtins/map/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 05f68c7d6e6..315505d1839 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -247,10 +247,11 @@ impl Map { let len = i32::from(&args[0].get_field("length")); for i in 0..len { let val = &args[0].get_field(i.to_string()); - let (key, value) = - Self::get_key_value(val).ok_or_else(|| ctx.construct_type_error( + let (key, value) = Self::get_key_value(val).ok_or_else(|| { + ctx.construct_type_error( "iterable for Map should have array-like objects", - ))?; + ) + })?; map.insert(key, value); } map From 11c4004db0d19963d7143b63c80fa44fd3b39692 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Mon, 6 Jul 2020 01:50:25 +0100 Subject: [PATCH 07/31] Use IndexMap instead of HashMap --- Cargo.lock | 18 +++- boa/Cargo.toml | 1 + boa/src/builtins/map/mod.rs | 65 ++++++++----- boa/src/builtins/map/ordered_map.rs | 144 ++++++++++++++++++++++++++++ boa/src/builtins/map/tests.rs | 50 ++++++++-- boa/src/builtins/object/mod.rs | 16 ++-- boa/src/builtins/value/display.rs | 1 + boa/src/exec/mod.rs | 1 + 8 files changed, 254 insertions(+), 42 deletions(-) create mode 100644 boa/src/builtins/map/ordered_map.rs diff --git a/Cargo.lock b/Cargo.lock index 36214179c49..def4d178b45 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7,6 +7,7 @@ dependencies = [ "bitflags", "criterion", "gc", + "indexmap", "jemallocator", "measureme", "num-bigint", @@ -114,9 +115,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.0.56" +version = "1.0.57" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77c1f1d60091c1b73e2b1f4560ab419204b178e625fa945ded7b660becd2bd46" +checksum = "0fde55d2a2bfaa4c9668bbc63f531fbdeee3ffe188f4662511ce2c22b3eedebe" [[package]] name = "cfg-if" @@ -315,6 +316,15 @@ dependencies = [ "libc", ] +[[package]] +name = "indexmap" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c398b2b113b55809ceb9ee3e753fcbac793f1956663f3c36549c1346015c2afe" +dependencies = [ + "autocfg", +] + [[package]] name = "itertools" version = "0.9.0" @@ -426,9 +436,9 @@ dependencies = [ [[package]] name = "memoffset" -version = "0.5.4" +version = "0.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b4fc2c02a7e374099d4ee95a193111f72d2110197fe200272371758f6c3643d8" +checksum = "c198b026e1bbf08a937e94c6c60f9ec4a2267f5b0d2eec9c1b21b061ce2be55f" dependencies = [ "autocfg", ] diff --git a/boa/Cargo.toml b/boa/Cargo.toml index ecf5b94c038..82ea99314a8 100644 --- a/boa/Cargo.toml +++ b/boa/Cargo.toml @@ -23,6 +23,7 @@ rustc-hash = "1.1.0" num-bigint = { version = "0.3.0", features = ["serde"] } num-integer = "0.1.43" bitflags = "1.2.1" +indexmap = "1.4.0" # Optional Dependencies serde = { version = "1.0.114", features = ["derive"], optional = true } diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 315505d1839..a8613e52bbe 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -1,3 +1,5 @@ +#![allow(clippy::mutable_key_type)] + use super::function::{make_builtin_fn, make_constructor_fn}; use crate::{ builtins::{ @@ -8,13 +10,15 @@ use crate::{ exec::Interpreter, BoaProfiler, }; -use std::collections::HashMap; +use gc::GcCell; +use ordered_map::OrderedMap; +pub mod ordered_map; #[cfg(test)] mod tests; #[derive(Debug, Clone)] -pub(crate) struct Map(HashMap); +pub(crate) struct Map(OrderedMap); impl Map { pub(crate) const NAME: &'static str = "Map"; @@ -22,7 +26,7 @@ impl Map { pub(crate) const LENGTH: usize = 1; /// Helper function to get the map object. - fn get_map(this: &Value, ctx: &mut Interpreter) -> Result, Value> { + /*fn get_map(this: &Value, ctx: &mut Interpreter) -> Result, Value> { if let Value::Object(ref object) = this { let object = object.borrow(); if let Some(map) = object.as_map() { @@ -30,7 +34,7 @@ impl Map { } } Err(ctx.construct_type_error("'this' is not a Map")) - } + }*/ /// Helper function to set the size property. fn set_size(this: &Value, size: usize) { @@ -60,12 +64,20 @@ impl Map { _ => (args[0].clone(), args[1].clone()), }; - let mut map = Self::get_map(this, ctx)?; - - map.insert(key, value); - Self::set_size(this, map.len()); + let size = if let Value::Object(ref object) = this { + let object = object.borrow(); + if let Some(map) = object.as_map_ref() { + let mut map = map.borrow_mut(); + map.insert(key, value); + map.len() + } else { + return Err(ctx.construct_type_error("'this' is not a Map")); + } + } else { + return Err(ctx.construct_type_error("'this' is not a Map")); + }; - this.set_data(ObjectData::Map(map)); + Self::set_size(this, size); Ok(this.clone()) } @@ -86,12 +98,19 @@ impl Map { _ => &args[0], }; - let mut map = Self::get_map(this, ctx)?; - - let deleted = map.remove(key).is_some(); - Self::set_size(this, map.len()); - - this.set_data(ObjectData::Map(map)); + let (deleted, size) = if let Value::Object(ref object) = this { + let object = object.borrow(); + if let Some(map) = object.as_map_ref() { + let mut map = map.borrow_mut(); + let deleted = map.remove(key).is_some(); + (deleted, map.len()) + } else { + return Err(ctx.construct_type_error("'this' is not a Map")); + } + } else { + return Err(ctx.construct_type_error("'this' is not a Map")); + }; + Self::set_size(this, size); Ok(Value::boolean(deleted)) } @@ -115,7 +134,7 @@ impl Map { if let Value::Object(ref object) = this { let object = object.borrow(); if let Some(map) = object.as_map_ref() { - return Ok(if let Some(result) = map.get(key) { + return Ok(if let Some(result) = map.borrow().get(key) { result.clone() } else { Value::Undefined @@ -137,7 +156,7 @@ impl Map { /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.clear /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/clear pub(crate) fn clear(this: &Value, _: &[Value], _: &mut Interpreter) -> ResultValue { - this.set_data(ObjectData::Map(HashMap::new())); + this.set_data(ObjectData::Map(GcCell::new(OrderedMap::new()))); Self::set_size(this, 0); @@ -164,7 +183,7 @@ impl Map { if let Value::Object(ref object) = this { let object = object.borrow(); if let Some(map) = object.as_map_ref() { - return Ok(Value::Boolean(map.contains_key(key))); + return Ok(Value::Boolean(map.borrow().contains_key(key))); } } @@ -195,7 +214,7 @@ impl Map { if let Value::Object(ref object) = this { let object = object.borrow(); - if let Some(map) = object.as_map() { + if let Some(map) = object.as_map_clone() { for (key, value) in map { let arguments = [value, key, this.clone()]; @@ -236,14 +255,14 @@ impl Map { // add our arguments in let data = match args.len() { - 0 => HashMap::new(), + 0 => OrderedMap::new(), _ => match &args[0] { Value::Object(object) => { let object = object.borrow(); - if let Some(map) = object.as_map() { + if let Some(map) = object.as_map_clone() { map } else if object.is_array() { - let mut map = HashMap::new(); + let mut map = OrderedMap::new(); let len = i32::from(&args[0].get_field("length")); for i in 0..len { let val = &args[0].get_field(i.to_string()); @@ -272,7 +291,7 @@ impl Map { // finally create length property Self::set_size(this, data.len()); - this.set_data(ObjectData::Map(data)); + this.set_data(ObjectData::Map(GcCell::new(data))); Ok(this.clone()) } diff --git a/boa/src/builtins/map/ordered_map.rs b/boa/src/builtins/map/ordered_map.rs new file mode 100644 index 00000000000..d29f9b556b2 --- /dev/null +++ b/boa/src/builtins/map/ordered_map.rs @@ -0,0 +1,144 @@ +use gc::{custom_trace, Finalize, Trace}; +use indexmap::{map::IntoIter, map::Iter, map::IterMut, IndexMap}; +use std::collections::hash_map::RandomState; +use std::fmt::Debug; +use std::hash::{BuildHasher, Hash}; + +/// A newtype wrapping indexmap::IndexMap +#[derive(Clone)] +pub struct OrderedMap(IndexMap) +where + K: Hash + Eq; + +impl Finalize for OrderedMap {} +unsafe impl Trace for OrderedMap { + custom_trace!(this, { + for (k, v) in this.0.iter() { + mark(k); + mark(v); + } + }); +} + +impl Debug for OrderedMap { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { + self.0.fmt(formatter) + } +} + +impl Default for OrderedMap { + fn default() -> Self { + Self::new() + } +} + +impl OrderedMap +where + K: Hash + Eq, +{ + pub fn new() -> Self { + OrderedMap(IndexMap::new()) + } + + pub fn with_capacity(capacity: usize) -> Self { + OrderedMap(IndexMap::with_capacity(capacity)) + } + + /// Return the number of key-value pairs in the map. + /// + /// Computes in **O(1)** time. + pub fn len(&self) -> usize { + self.0.len() + } + + /// Returns true if the map contains no elements. + /// + /// Computes in **O(1)** time. + pub fn is_empty(&self) -> bool { + self.0.len() == 0 + } + + /// Insert a key-value pair in the map. + /// + /// If an equivalent key already exists in the map: the key remains and + /// retains in its place in the order, its corresponding value is updated + /// with `value` and the older value is returned inside `Some(_)`. + /// + /// If no equivalent key existed in the map: the new key-value pair is + /// inserted, last in order, and `None` is returned. + /// + /// Computes in **O(1)** time (amortized average). + pub fn insert(&mut self, key: K, value: V) -> Option { + self.0.insert(key, value) + } + + /// Remove the key-value pair equivalent to `key` and return + /// its value. + /// + /// Like `Vec::remove`, the pair is removed by shifting all of the + /// elements that follow it, preserving their relative order. + /// **This perturbs the index of all of those elements!** + /// + /// Return `None` if `key` is not in map. + /// + /// Computes in **O(n)** time (average). + pub fn remove(&mut self, key: &K) -> Option { + self.0.shift_remove(key) + } + + /// Return a reference to the value stored for `key`, if it is present, + /// else `None`. + /// + /// Computes in **O(1)** time (average). + pub fn get(&self, key: &K) -> Option<&V> { + self.0.get(key) + } + + /// Return an iterator over the key-value pairs of the map, in their order + pub fn iter(&self) -> Iter<'_, K, V> { + self.0.iter() + } + + /// Return `true` if an equivalent to `key` exists in the map. + /// + /// Computes in **O(1)** time (average). + pub fn contains_key(&self, key: &K) -> bool { + self.0.contains_key(key) + } +} + +impl<'a, K, V, S> IntoIterator for &'a OrderedMap +where + K: Hash + Eq, + S: BuildHasher, +{ + type Item = (&'a K, &'a V); + type IntoIter = Iter<'a, K, V>; + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} + +impl<'a, K, V, S> IntoIterator for &'a mut OrderedMap +where + K: Hash + Eq, + S: BuildHasher, +{ + type Item = (&'a K, &'a mut V); + type IntoIter = IterMut<'a, K, V>; + fn into_iter(self) -> Self::IntoIter { + self.0.iter_mut() + } +} + +impl IntoIterator for OrderedMap +where + K: Hash + Eq, + S: BuildHasher, +{ + type Item = (K, V); + type IntoIter = IntoIter; + fn into_iter(self) -> IntoIter { + self.0.into_iter() + } +} diff --git a/boa/src/builtins/map/tests.rs b/boa/src/builtins/map/tests.rs index fb416cc509b..3f0b2fe12d4 100644 --- a/boa/src/builtins/map/tests.rs +++ b/boa/src/builtins/map/tests.rs @@ -92,12 +92,15 @@ fn set() { let map = new Map(); "#; forward(&mut engine, init); - let result = forward(&mut engine, "map.set();map.size"); - assert_eq!(result, "1"); - let result = forward(&mut engine, "map.set('1', 'one');map.size"); - assert_eq!(result, "2"); - let result = forward(&mut engine, "map.set('2');map.size"); - assert_eq!(result, "3"); + let result = forward(&mut engine, "map.set()"); + assert_eq!(result, "Map { undefined → undefined }"); + let result = forward(&mut engine, "map.set('1', 'one')"); + assert_eq!(result, "Map { undefined → undefined, 1 → one }"); + let result = forward(&mut engine, "map.set('2')"); + assert_eq!( + result, + "Map { undefined → undefined, 1 → one, 2 → undefined }" + ); } #[test] @@ -166,3 +169,38 @@ fn for_each() { assert_eq!(forward(&mut engine, "keySum"), "6"); assert_eq!(forward(&mut engine, "sizeSum"), "9"); } + +#[test] +fn modify_key() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let obj = new Object(); + let map = new Map([[obj, "one"]]); + obj.field = "Value"; + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.get(obj)"); + assert_eq!(result, "one"); +} + +#[test] +fn order() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([[1, "one"]]); + map.set(2, "two"); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map"); + assert_eq!(result, "Map { 1 → one, 2 → two }"); + let result = forward(&mut engine, "map.set(1, \"five\");map"); + assert_eq!(result, "Map { 1 → five, 2 → two }"); + let result = forward(&mut engine, "map.set();map"); + assert_eq!(result, "Map { 1 → five, 2 → two, undefined → undefined }"); + let result = forward(&mut engine, "map.delete(2);map"); + assert_eq!(result, "Map { 1 → five, undefined → undefined }"); + let result = forward(&mut engine, "map.set(2, \"two\");map"); + assert_eq!(result, "Map { 1 → five, undefined → undefined, 2 → two }"); +} diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index c78b37c05a1..e9a9ab40456 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -16,6 +16,7 @@ use crate::{ builtins::{ function::Function, + map::ordered_map::OrderedMap, property::Property, value::{RcBigInt, RcString, RcSymbol, ResultValue, Value}, BigInt, @@ -23,12 +24,9 @@ use crate::{ exec::Interpreter, BoaProfiler, }; -use gc::{Finalize, Trace}; +use gc::{Finalize, GcCell, Trace}; use rustc_hash::FxHashMap; -use std::{ - collections::HashMap, - fmt::{Debug, Display, Error, Formatter}, -}; +use std::fmt::{Debug, Display, Error, Formatter}; use super::function::{make_builtin_fn, make_constructor_fn}; use crate::builtins::value::same_value; @@ -70,7 +68,7 @@ pub struct Object { #[derive(Debug, Trace, Finalize, Clone)] pub enum ObjectData { Array, - Map(HashMap), + Map(GcCell>), BigInt(RcBigInt), Boolean(bool), Function(Function), @@ -263,15 +261,15 @@ impl Object { } #[inline] - pub fn as_map(&self) -> Option> { + pub fn as_map_clone(&self) -> Option> { match self.data { - ObjectData::Map(ref map) => Some(map.clone()), + ObjectData::Map(ref map) => Some(map.borrow().clone()), _ => None, } } #[inline] - pub fn as_map_ref(&self) -> Option<&HashMap> { + pub fn as_map_ref(&self) -> Option<&GcCell>> { match self.data { ObjectData::Map(ref map) => Some(map), _ => None, diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index 7b352940c72..81a67057de1 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -117,6 +117,7 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool) -> String { } let mappings = map + .borrow() .iter() .map(|(key, value)| { let key = log_string_from(key, print_internals); diff --git a/boa/src/exec/mod.rs b/boa/src/exec/mod.rs index da926109d7c..91c73f33c4b 100644 --- a/boa/src/exec/mod.rs +++ b/boa/src/exec/mod.rs @@ -377,6 +377,7 @@ impl Interpreter { // Check if object is a Map else if let ObjectData::Map(ref map) = x.deref().borrow().data { let values = map + .borrow() .iter() .map(|(key, value)| { // Construct a new array containing the key-value pair From 72d87860017f192f8858137f7836e2d8bb4ee144 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 18:45:54 +0100 Subject: [PATCH 08/31] Initial implementation of Map() --- boa/src/builtins/map/mod.rs | 312 ++++++++++++++++++++++++++++++ boa/src/builtins/map/tests.rs | 168 ++++++++++++++++ boa/src/builtins/mod.rs | 3 + boa/src/builtins/object/mod.rs | 29 ++- boa/src/builtins/value/display.rs | 25 +++ boa/src/exec/mod.rs | 32 +++ 6 files changed, 568 insertions(+), 1 deletion(-) create mode 100644 boa/src/builtins/map/mod.rs create mode 100644 boa/src/builtins/map/tests.rs diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs new file mode 100644 index 00000000000..afbcc190ad5 --- /dev/null +++ b/boa/src/builtins/map/mod.rs @@ -0,0 +1,312 @@ +use super::function::{make_builtin_fn, make_constructor_fn}; +use crate::{ + builtins::{ + object::{InternalState, ObjectData, INSTANCE_PROTOTYPE, PROTOTYPE}, + property::Property, + value::{ResultValue, Value}, + }, + exec::Interpreter, + BoaProfiler, +}; +use std::collections::HashMap; + +#[cfg(test)] +mod tests; + +#[derive(Debug, Clone)] +pub(crate) struct Map(HashMap); + +impl Map { + pub(crate) const NAME: &'static str = "Map"; + + pub(crate) const LENGTH: usize = 1; + + /// Helper function to get the map object. + fn get_map(this: &Value, ctx: &mut Interpreter) -> Result, Value> { + if let Value::Object(ref object) = this { + let object = object.borrow(); + if let Some(map) = object.as_map() { + return Ok(map); + } + } + Err(ctx.construct_type_error("'this' is not a Map")) + } + + /// Helper function to set the size property. + fn set_size(this: &Value, size: usize) { + let size = Property::new() + .value(Value::from(size)) + .writable(false) + .configurable(false) + .enumerable(false); + + this.set_property("size".to_string(), size); + } + + /// `Map.prototype.set( key, value )` + /// + /// This method associates the value with the key. Returns the map object. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.set + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/set + pub(crate) fn set(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultValue { + let (key, value) = match args.len() { + 0 => (Value::Undefined, Value::Undefined), + 1 => (args[0].clone(), Value::Undefined), + _ => (args[0].clone(), args[1].clone()), + }; + + let mut map = Self::get_map(this, ctx)?; + + map.insert(key, value); + Self::set_size(this, map.len()); + + this.set_data(ObjectData::Map(map)); + Ok(this.clone()) + } + + /// `Map.prototype.delete( key )` + /// + /// This method removes the element associated with the key, if it exists. Returns true if there was an element, false otherwise. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.delete + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/delete + pub(crate) fn delete(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultValue { + let undefined = Value::Undefined; + let key = match args.len() { + 0 => &undefined, + _ => &args[0], + }; + + let mut map = Self::get_map(this, ctx)?; + + let deleted = map.remove(key).is_some(); + Self::set_size(this, map.len()); + + this.set_data(ObjectData::Map(map)); + Ok(Value::boolean(deleted)) + } + + /// `Map.prototype.get( key )` + /// + /// This method returns the value associated with the key, or undefined if there is none. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.get + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/get + pub(crate) fn get(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultValue { + let undefined = Value::Undefined; + let key = match args.len() { + 0 => &undefined, + _ => &args[0], + }; + + if let Value::Object(ref object) = this { + let object = object.borrow(); + if let Some(map) = object.as_map_ref() { + return Ok(if let Some(result) = map.get(key) { + result.clone() + } else { + Value::Undefined + }); + } + } + + Err(ctx.construct_type_error("'this' is not a Map")) + } + + /// `Map.prototype.clear( )` + /// + /// This method removes all entries from the map. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.clear + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/clear + pub(crate) fn clear(this: &Value, _: &[Value], _: &mut Interpreter) -> ResultValue { + this.set_data(ObjectData::Map(HashMap::new())); + + Self::set_size(this, 0); + + Ok(Value::Undefined) + } + + /// `Map.prototype.has( key )` + /// + /// This method checks if the map contains an entry with the given key. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.has + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/has + pub(crate) fn has(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultValue { + let undefined = Value::Undefined; + let key = match args.len() { + 0 => &undefined, + _ => &args[0], + }; + + if let Value::Object(ref object) = this { + let object = object.borrow(); + if let Some(map) = object.as_map_ref() { + return Ok(Value::Boolean(map.contains_key(key))); + } + } + + Err(ctx.construct_type_error("'this' is not a Map")) + } + + /// `Map.prototype.forEach( callbackFn [ , thisArg ] )` + /// + /// This method executes the provided callback function for each key-value pair in the map. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.foreach + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/forEach + pub(crate) fn for_each( + this: &Value, + args: &[Value], + interpreter: &mut Interpreter, + ) -> ResultValue { + if args.is_empty() { + return Err(Value::from("Missing argument for Map.prototype.forEach")); + } + + let callback_arg = &args[0]; + let this_arg = args.get(1).cloned().unwrap_or_else(Value::undefined); + + if let Value::Object(ref object) = this { + let object = object.borrow(); + if let Some(map) = object.as_map() { + for (key, value) in map { + let arguments = [value, key, this.clone()]; + + interpreter.call(callback_arg, &this_arg, &arguments)?; + } + } + } + + Ok(Value::Undefined) + } + + /// Helper function to get a key-value pair from an array. + fn get_key_value(value: &Value) -> Option<(Value, Value)> { + if let Value::Object(object) = value { + if object.borrow().is_array() { + let (key, value) = match i32::from(&value.get_field("length")) { + 0 => (Value::Undefined, Value::Undefined), + 1 => (value.get_field("0"), Value::Undefined), + _ => (value.get_field("0"), value.get_field("1")), + }; + return Some((key, value)); + } + } + None + } + + /// Create a new map + pub(crate) fn make_map(this: &Value, args: &[Value], ctx: &mut Interpreter) -> ResultValue { + // Make a new Object which will internally represent the Array (mapping + // between indices and values): this creates an Object with no prototype + + // Set Prototype + let prototype = ctx.realm.global_obj.get_field("Map").get_field(PROTOTYPE); + + this.set_internal_slot(INSTANCE_PROTOTYPE, prototype); + // This value is used by console.log and other routines to match Object type + // to its Javascript Identifier (global constructor method name) + + // add our arguments in + let data = match args.len() { + 0 => HashMap::new(), + _ => match &args[0] { + Value::Object(object) => { + let object = object.borrow(); + if let Some(map) = object.as_map() { + map.clone() + } else if object.is_array() { + let mut map = HashMap::new(); + let len = i32::from(&args[0].get_field("length")); + for i in 0..len { + let val = &args[0].get_field(i.to_string()); + let (key, value) = + Self::get_key_value(val).ok_or(ctx.construct_type_error( + "iterable for Map should have array-like objects", + ))?; + map.insert(key, value); + } + map + } else { + return Err(ctx.construct_type_error( + "iterable for Map should have array-like objects", + )); + } + } + _ => { + return Err( + ctx.construct_type_error("iterable for Map should have array-like objects") + ) + } + }, + }; + + // finally create length property + Self::set_size(this, data.len()); + + this.set_data(ObjectData::Map(data)); + + Ok(this.clone()) + } + + /// Create a new `Map` object. + pub(crate) fn create(global: &Value) -> Value { + // Create prototype + let prototype = Value::new_object(Some(global)); + + make_builtin_fn(Self::set, "set", &prototype, 2); + make_builtin_fn(Self::delete, "delete", &prototype, 1); + make_builtin_fn(Self::get, "get", &prototype, 1); + make_builtin_fn(Self::clear, "clear", &prototype, 0); + make_builtin_fn(Self::has, "has", &prototype, 1); + make_builtin_fn(Self::for_each, "forEach", &prototype, 1); + + let map = make_constructor_fn( + Self::NAME, + Self::LENGTH, + Self::make_map, + global, + prototype, + true, + ); + + map + } + + /// Initialise the `Map` object on the global object. + #[inline] + pub(crate) fn init(global: &Value) -> (&str, Value) { + let _timer = BoaProfiler::global().start_event(Self::NAME, "init"); + + (Self::NAME, Self::create(global)) + } +} + +impl InternalState for HashMap {} diff --git a/boa/src/builtins/map/tests.rs b/boa/src/builtins/map/tests.rs new file mode 100644 index 00000000000..fb416cc509b --- /dev/null +++ b/boa/src/builtins/map/tests.rs @@ -0,0 +1,168 @@ +use crate::{exec::Interpreter, forward, realm::Realm}; + +#[test] +fn construct_empty() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + var empty = new Map(); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "empty.size"); + assert_eq!(result, "0"); +} + +#[test] +fn construct_from_array() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([["1", "one"], ["2", "two"]]); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.size"); + assert_eq!(result, "2"); +} + +#[test] +fn clone() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let original = new Map([["1", "one"], ["2", "two"]]); + let clone = new Map(original); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "clone.size"); + assert_eq!(result, "2"); + let result = forward( + &mut engine, + r#" + original.set("3", "three"); + original.size"#, + ); + assert_eq!(result, "3"); + let result = forward(&mut engine, "clone.size"); + assert_eq!(result, "2"); +} + +#[test] +fn merge() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let first = new Map([["1", "one"], ["2", "two"]]); + let second = new Map([["2", "second two"], ["3", "three"]]); + let third = new Map([["4", "four"], ["5", "five"]]); + let merged1 = new Map([...first, ...second]); + let merged2 = new Map([...second, ...third]); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "merged1.size"); + assert_eq!(result, "3"); + let result = forward(&mut engine, "merged1.get('2')"); + assert_eq!(result, "second two"); + let result = forward(&mut engine, "merged2.size"); + assert_eq!(result, "4"); +} + +#[test] +fn get() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([["1", "one"], ["2", "two"]]); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.get('1')"); + assert_eq!(result, "one"); + let result = forward(&mut engine, "map.get('2')"); + assert_eq!(result, "two"); + let result = forward(&mut engine, "map.get('3')"); + assert_eq!(result, "undefined"); + let result = forward(&mut engine, "map.get()"); + assert_eq!(result, "undefined"); +} + +#[test] +fn set() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map(); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.set();map.size"); + assert_eq!(result, "1"); + let result = forward(&mut engine, "map.set('1', 'one');map.size"); + assert_eq!(result, "2"); + let result = forward(&mut engine, "map.set('2');map.size"); + assert_eq!(result, "3"); +} + +#[test] +fn clear() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([["1", "one"], ["2", "two"]]); + map.clear(); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.size"); + assert_eq!(result, "0"); +} + +#[test] +fn delete() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([["1", "one"], ["2", "two"]]); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.delete('1')"); + assert_eq!(result, "true"); + let result = forward(&mut engine, "map.size"); + assert_eq!(result, "1"); + let result = forward(&mut engine, "map.delete('1')"); + assert_eq!(result, "false"); +} + +#[test] +fn has() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([["1", "one"]]); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.has()"); + assert_eq!(result, "false"); + let result = forward(&mut engine, "map.has('1')"); + assert_eq!(result, "true"); + let result = forward(&mut engine, "map.has('2')"); + assert_eq!(result, "false"); +} + +#[test] +fn for_each() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([[1, 5], [2, 10], [3, 15]]); + let valueSum = 0; + let keySum = 0; + let sizeSum = 0; + function callingCallback(value, key, map) { + valueSum += value; + keySum += key; + sizeSum += map.size; + } + map.forEach(callingCallback); + "#; + forward(&mut engine, init); + assert_eq!(forward(&mut engine, "valueSum"), "30"); + assert_eq!(forward(&mut engine, "keySum"), "6"); + assert_eq!(forward(&mut engine, "sizeSum"), "9"); +} diff --git a/boa/src/builtins/mod.rs b/boa/src/builtins/mod.rs index d5a4f9cf41c..127426153aa 100644 --- a/boa/src/builtins/mod.rs +++ b/boa/src/builtins/mod.rs @@ -9,6 +9,7 @@ pub mod function; pub mod global_this; pub mod infinity; pub mod json; +pub mod map; pub mod math; pub mod nan; pub mod number; @@ -29,6 +30,7 @@ pub(crate) use self::{ global_this::GlobalThis, infinity::Infinity, json::Json, + map::Map, math::Math, nan::NaN, number::Number, @@ -50,6 +52,7 @@ pub fn init(global: &Value) { BigInt::init, Boolean::init, Json::init, + Map::init, Math::init, Number::init, RegExp::init, diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index 45cfbaa65c5..e00e4c74c9e 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -25,7 +25,10 @@ use crate::{ }; use gc::{Finalize, Trace}; use rustc_hash::FxHashMap; -use std::fmt::{Debug, Display, Error, Formatter}; +use std::{ + collections::HashMap, + fmt::{Debug, Display, Error, Formatter}, +}; use super::function::{make_builtin_fn, make_constructor_fn}; use crate::builtins::value::same_value; @@ -67,6 +70,7 @@ pub struct Object { #[derive(Debug, Trace, Finalize, Clone)] pub enum ObjectData { Array, + Map(HashMap), BigInt(RcBigInt), Boolean(bool), Function(Function), @@ -85,6 +89,7 @@ impl Display for ObjectData { match self { Self::Function(_) => "Function", Self::Array => "Array", + Self::Map(_) => "Map", Self::String(_) => "String", Self::Symbol(_) => "Symbol", Self::Error => "Error", @@ -251,6 +256,28 @@ impl Object { } } + /// Checks if it is a `Map` object.pub + #[inline] + pub fn is_map(&self) -> bool { + matches!(self.data, ObjectData::Map(_)) + } + + #[inline] + pub fn as_map(&self) -> Option> { + match self.data { + ObjectData::Map(ref map) => Some(map.clone()), + _ => None, + } + } + + #[inline] + pub fn as_map_ref(&self) -> Option<&HashMap> { + match self.data { + ObjectData::Map(ref map) => Some(map), + _ => None, + } + } + /// Checks if it a `String` object. #[inline] pub fn is_string(&self) -> bool { diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index 5637c04abb6..7b352940c72 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -102,6 +102,31 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool) -> String { format!("[ {} ]", arr) } + ObjectData::Map(ref map) => { + let size = i32::from( + &v.borrow() + .properties() + .get("size") + .unwrap() + .value + .clone() + .expect("Could not borrow value"), + ); + if size == 0 { + return String::from("Map(0)"); + } + + let mappings = map + .iter() + .map(|(key, value)| { + let key = log_string_from(key, print_internals); + let value = log_string_from(value, print_internals); + format!("{} → {}", key, value) + }) + .collect::>() + .join(", "); + format!("Map {{ {} }}", mappings) + } _ => display_obj(&x, print_internals), } } diff --git a/boa/src/exec/mod.rs b/boa/src/exec/mod.rs index fc244830e76..b9dcfe6ef30 100644 --- a/boa/src/exec/mod.rs +++ b/boa/src/exec/mod.rs @@ -38,6 +38,7 @@ use crate::{ }, BoaProfiler, }; +use std::borrow::Borrow; use std::convert::TryFrom; use std::ops::Deref; @@ -377,6 +378,37 @@ impl Interpreter { .collect(); return Ok(values); } + // Check if object is a Map + else if let ObjectData::Map(ref map) = x.deref().borrow().data { + let values = map + .iter() + .map(|(key, value)| { + // Construct a new array containing the key-value pair + let array = Value::new_object(Some( + &self + .realm() + .environment + .get_global_object() + .expect("Could not get global object"), + )); + array.set_data(ObjectData::Array); + array.borrow().set_internal_slot( + INSTANCE_PROTOTYPE, + self.realm() + .environment + .get_binding_value("Array") + .expect("Array was not initialized") + .borrow() + .get_field(PROTOTYPE), + ); + array.borrow().set_field("0", key); + array.borrow().set_field("1", value); + array.borrow().set_field("length", Value::from(2)); + array + }) + .collect(); + return Ok(values); + } return Err(()); } From f11be4d40226e65b27f67c398f5f49e95676fa71 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 19:00:54 +0100 Subject: [PATCH 09/31] Remove impl InternalState for HashMap --- boa/src/builtins/map/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index afbcc190ad5..9dd1d9781fb 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -1,7 +1,7 @@ use super::function::{make_builtin_fn, make_constructor_fn}; use crate::{ builtins::{ - object::{InternalState, ObjectData, INSTANCE_PROTOTYPE, PROTOTYPE}, + object::{ObjectData, INSTANCE_PROTOTYPE, PROTOTYPE}, property::Property, value::{ResultValue, Value}, }, @@ -308,5 +308,3 @@ impl Map { (Self::NAME, Self::create(global)) } } - -impl InternalState for HashMap {} From 57c87d9c980b7aff6a7097572f51b62fc5b6d113 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 19:04:27 +0100 Subject: [PATCH 10/31] Fix clippy lint --- boa/src/builtins/map/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 9dd1d9781fb..7768482ec4b 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -288,7 +288,7 @@ impl Map { make_builtin_fn(Self::has, "has", &prototype, 1); make_builtin_fn(Self::for_each, "forEach", &prototype, 1); - let map = make_constructor_fn( + make_constructor_fn( Self::NAME, Self::LENGTH, Self::make_map, @@ -296,8 +296,6 @@ impl Map { prototype, true, ); - - map } /// Initialise the `Map` object on the global object. From 329e5809434d204bee20f13d1e49fbeeed546976 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 19:06:57 +0100 Subject: [PATCH 11/31] Actually fix clippy lint --- boa/src/builtins/map/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 7768482ec4b..98ac13d6b3e 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -295,7 +295,7 @@ impl Map { global, prototype, true, - ); + ) } /// Initialise the `Map` object on the global object. From 3bc79d9c71a110c3171dbd7bc9cbe45775dffe70 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 19:17:08 +0100 Subject: [PATCH 12/31] Fix ok_or lint --- boa/src/builtins/map/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 98ac13d6b3e..05f68c7d6e6 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -241,14 +241,14 @@ impl Map { Value::Object(object) => { let object = object.borrow(); if let Some(map) = object.as_map() { - map.clone() + map } else if object.is_array() { let mut map = HashMap::new(); let len = i32::from(&args[0].get_field("length")); for i in 0..len { let val = &args[0].get_field(i.to_string()); let (key, value) = - Self::get_key_value(val).ok_or(ctx.construct_type_error( + Self::get_key_value(val).ok_or_else(|| ctx.construct_type_error( "iterable for Map should have array-like objects", ))?; map.insert(key, value); From 07f64af822cefd3ff5a1181a3d25c16b9a880363 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 19:18:10 +0100 Subject: [PATCH 13/31] format code --- boa/src/builtins/map/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 05f68c7d6e6..315505d1839 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -247,10 +247,11 @@ impl Map { let len = i32::from(&args[0].get_field("length")); for i in 0..len { let val = &args[0].get_field(i.to_string()); - let (key, value) = - Self::get_key_value(val).ok_or_else(|| ctx.construct_type_error( + let (key, value) = Self::get_key_value(val).ok_or_else(|| { + ctx.construct_type_error( "iterable for Map should have array-like objects", - ))?; + ) + })?; map.insert(key, value); } map From 5e9358b4293539958cf5e2c099600955bbb469de Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sat, 11 Jul 2020 14:33:16 +0100 Subject: [PATCH 14/31] Rebase upstream --- Cargo.lock | 14 ++- boa/Cargo.toml | 1 + boa/src/builtins/map/mod.rs | 65 ++++++++----- boa/src/builtins/map/ordered_map.rs | 144 ++++++++++++++++++++++++++++ boa/src/builtins/map/tests.rs | 50 ++++++++-- boa/src/builtins/object/mod.rs | 16 ++-- boa/src/builtins/value/display.rs | 1 + boa/src/exec/mod.rs | 1 + 8 files changed, 252 insertions(+), 40 deletions(-) create mode 100644 boa/src/builtins/map/ordered_map.rs diff --git a/Cargo.lock b/Cargo.lock index 47f366ee8f4..56a2b429104 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7,6 +7,7 @@ dependencies = [ "bitflags", "criterion", "gc", + "indexmap", "jemallocator", "measureme", "num-bigint", @@ -144,9 +145,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.0.58" +version = "1.0.57" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f9a06fb2e53271d7c279ec1efea6ab691c35a2ae67ec0d91d7acec0caf13b518" +checksum = "0fde55d2a2bfaa4c9668bbc63f531fbdeee3ffe188f4662511ce2c22b3eedebe" [[package]] name = "cfg-if" @@ -372,6 +373,15 @@ dependencies = [ "libc", ] +[[package]] +name = "indexmap" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c398b2b113b55809ceb9ee3e753fcbac793f1956663f3c36549c1346015c2afe" +dependencies = [ + "autocfg", +] + [[package]] name = "itertools" version = "0.9.0" diff --git a/boa/Cargo.toml b/boa/Cargo.toml index 1b7d6c0ee7e..1da7616c6b5 100644 --- a/boa/Cargo.toml +++ b/boa/Cargo.toml @@ -23,6 +23,7 @@ rustc-hash = "1.1.0" num-bigint = { version = "0.3.0", features = ["serde"] } num-integer = "0.1.43" bitflags = "1.2.1" +indexmap = "1.4.0" # Optional Dependencies serde = { version = "1.0.114", features = ["derive"], optional = true } diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 315505d1839..a8613e52bbe 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -1,3 +1,5 @@ +#![allow(clippy::mutable_key_type)] + use super::function::{make_builtin_fn, make_constructor_fn}; use crate::{ builtins::{ @@ -8,13 +10,15 @@ use crate::{ exec::Interpreter, BoaProfiler, }; -use std::collections::HashMap; +use gc::GcCell; +use ordered_map::OrderedMap; +pub mod ordered_map; #[cfg(test)] mod tests; #[derive(Debug, Clone)] -pub(crate) struct Map(HashMap); +pub(crate) struct Map(OrderedMap); impl Map { pub(crate) const NAME: &'static str = "Map"; @@ -22,7 +26,7 @@ impl Map { pub(crate) const LENGTH: usize = 1; /// Helper function to get the map object. - fn get_map(this: &Value, ctx: &mut Interpreter) -> Result, Value> { + /*fn get_map(this: &Value, ctx: &mut Interpreter) -> Result, Value> { if let Value::Object(ref object) = this { let object = object.borrow(); if let Some(map) = object.as_map() { @@ -30,7 +34,7 @@ impl Map { } } Err(ctx.construct_type_error("'this' is not a Map")) - } + }*/ /// Helper function to set the size property. fn set_size(this: &Value, size: usize) { @@ -60,12 +64,20 @@ impl Map { _ => (args[0].clone(), args[1].clone()), }; - let mut map = Self::get_map(this, ctx)?; - - map.insert(key, value); - Self::set_size(this, map.len()); + let size = if let Value::Object(ref object) = this { + let object = object.borrow(); + if let Some(map) = object.as_map_ref() { + let mut map = map.borrow_mut(); + map.insert(key, value); + map.len() + } else { + return Err(ctx.construct_type_error("'this' is not a Map")); + } + } else { + return Err(ctx.construct_type_error("'this' is not a Map")); + }; - this.set_data(ObjectData::Map(map)); + Self::set_size(this, size); Ok(this.clone()) } @@ -86,12 +98,19 @@ impl Map { _ => &args[0], }; - let mut map = Self::get_map(this, ctx)?; - - let deleted = map.remove(key).is_some(); - Self::set_size(this, map.len()); - - this.set_data(ObjectData::Map(map)); + let (deleted, size) = if let Value::Object(ref object) = this { + let object = object.borrow(); + if let Some(map) = object.as_map_ref() { + let mut map = map.borrow_mut(); + let deleted = map.remove(key).is_some(); + (deleted, map.len()) + } else { + return Err(ctx.construct_type_error("'this' is not a Map")); + } + } else { + return Err(ctx.construct_type_error("'this' is not a Map")); + }; + Self::set_size(this, size); Ok(Value::boolean(deleted)) } @@ -115,7 +134,7 @@ impl Map { if let Value::Object(ref object) = this { let object = object.borrow(); if let Some(map) = object.as_map_ref() { - return Ok(if let Some(result) = map.get(key) { + return Ok(if let Some(result) = map.borrow().get(key) { result.clone() } else { Value::Undefined @@ -137,7 +156,7 @@ impl Map { /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.clear /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/clear pub(crate) fn clear(this: &Value, _: &[Value], _: &mut Interpreter) -> ResultValue { - this.set_data(ObjectData::Map(HashMap::new())); + this.set_data(ObjectData::Map(GcCell::new(OrderedMap::new()))); Self::set_size(this, 0); @@ -164,7 +183,7 @@ impl Map { if let Value::Object(ref object) = this { let object = object.borrow(); if let Some(map) = object.as_map_ref() { - return Ok(Value::Boolean(map.contains_key(key))); + return Ok(Value::Boolean(map.borrow().contains_key(key))); } } @@ -195,7 +214,7 @@ impl Map { if let Value::Object(ref object) = this { let object = object.borrow(); - if let Some(map) = object.as_map() { + if let Some(map) = object.as_map_clone() { for (key, value) in map { let arguments = [value, key, this.clone()]; @@ -236,14 +255,14 @@ impl Map { // add our arguments in let data = match args.len() { - 0 => HashMap::new(), + 0 => OrderedMap::new(), _ => match &args[0] { Value::Object(object) => { let object = object.borrow(); - if let Some(map) = object.as_map() { + if let Some(map) = object.as_map_clone() { map } else if object.is_array() { - let mut map = HashMap::new(); + let mut map = OrderedMap::new(); let len = i32::from(&args[0].get_field("length")); for i in 0..len { let val = &args[0].get_field(i.to_string()); @@ -272,7 +291,7 @@ impl Map { // finally create length property Self::set_size(this, data.len()); - this.set_data(ObjectData::Map(data)); + this.set_data(ObjectData::Map(GcCell::new(data))); Ok(this.clone()) } diff --git a/boa/src/builtins/map/ordered_map.rs b/boa/src/builtins/map/ordered_map.rs new file mode 100644 index 00000000000..d29f9b556b2 --- /dev/null +++ b/boa/src/builtins/map/ordered_map.rs @@ -0,0 +1,144 @@ +use gc::{custom_trace, Finalize, Trace}; +use indexmap::{map::IntoIter, map::Iter, map::IterMut, IndexMap}; +use std::collections::hash_map::RandomState; +use std::fmt::Debug; +use std::hash::{BuildHasher, Hash}; + +/// A newtype wrapping indexmap::IndexMap +#[derive(Clone)] +pub struct OrderedMap(IndexMap) +where + K: Hash + Eq; + +impl Finalize for OrderedMap {} +unsafe impl Trace for OrderedMap { + custom_trace!(this, { + for (k, v) in this.0.iter() { + mark(k); + mark(v); + } + }); +} + +impl Debug for OrderedMap { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { + self.0.fmt(formatter) + } +} + +impl Default for OrderedMap { + fn default() -> Self { + Self::new() + } +} + +impl OrderedMap +where + K: Hash + Eq, +{ + pub fn new() -> Self { + OrderedMap(IndexMap::new()) + } + + pub fn with_capacity(capacity: usize) -> Self { + OrderedMap(IndexMap::with_capacity(capacity)) + } + + /// Return the number of key-value pairs in the map. + /// + /// Computes in **O(1)** time. + pub fn len(&self) -> usize { + self.0.len() + } + + /// Returns true if the map contains no elements. + /// + /// Computes in **O(1)** time. + pub fn is_empty(&self) -> bool { + self.0.len() == 0 + } + + /// Insert a key-value pair in the map. + /// + /// If an equivalent key already exists in the map: the key remains and + /// retains in its place in the order, its corresponding value is updated + /// with `value` and the older value is returned inside `Some(_)`. + /// + /// If no equivalent key existed in the map: the new key-value pair is + /// inserted, last in order, and `None` is returned. + /// + /// Computes in **O(1)** time (amortized average). + pub fn insert(&mut self, key: K, value: V) -> Option { + self.0.insert(key, value) + } + + /// Remove the key-value pair equivalent to `key` and return + /// its value. + /// + /// Like `Vec::remove`, the pair is removed by shifting all of the + /// elements that follow it, preserving their relative order. + /// **This perturbs the index of all of those elements!** + /// + /// Return `None` if `key` is not in map. + /// + /// Computes in **O(n)** time (average). + pub fn remove(&mut self, key: &K) -> Option { + self.0.shift_remove(key) + } + + /// Return a reference to the value stored for `key`, if it is present, + /// else `None`. + /// + /// Computes in **O(1)** time (average). + pub fn get(&self, key: &K) -> Option<&V> { + self.0.get(key) + } + + /// Return an iterator over the key-value pairs of the map, in their order + pub fn iter(&self) -> Iter<'_, K, V> { + self.0.iter() + } + + /// Return `true` if an equivalent to `key` exists in the map. + /// + /// Computes in **O(1)** time (average). + pub fn contains_key(&self, key: &K) -> bool { + self.0.contains_key(key) + } +} + +impl<'a, K, V, S> IntoIterator for &'a OrderedMap +where + K: Hash + Eq, + S: BuildHasher, +{ + type Item = (&'a K, &'a V); + type IntoIter = Iter<'a, K, V>; + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} + +impl<'a, K, V, S> IntoIterator for &'a mut OrderedMap +where + K: Hash + Eq, + S: BuildHasher, +{ + type Item = (&'a K, &'a mut V); + type IntoIter = IterMut<'a, K, V>; + fn into_iter(self) -> Self::IntoIter { + self.0.iter_mut() + } +} + +impl IntoIterator for OrderedMap +where + K: Hash + Eq, + S: BuildHasher, +{ + type Item = (K, V); + type IntoIter = IntoIter; + fn into_iter(self) -> IntoIter { + self.0.into_iter() + } +} diff --git a/boa/src/builtins/map/tests.rs b/boa/src/builtins/map/tests.rs index fb416cc509b..3f0b2fe12d4 100644 --- a/boa/src/builtins/map/tests.rs +++ b/boa/src/builtins/map/tests.rs @@ -92,12 +92,15 @@ fn set() { let map = new Map(); "#; forward(&mut engine, init); - let result = forward(&mut engine, "map.set();map.size"); - assert_eq!(result, "1"); - let result = forward(&mut engine, "map.set('1', 'one');map.size"); - assert_eq!(result, "2"); - let result = forward(&mut engine, "map.set('2');map.size"); - assert_eq!(result, "3"); + let result = forward(&mut engine, "map.set()"); + assert_eq!(result, "Map { undefined → undefined }"); + let result = forward(&mut engine, "map.set('1', 'one')"); + assert_eq!(result, "Map { undefined → undefined, 1 → one }"); + let result = forward(&mut engine, "map.set('2')"); + assert_eq!( + result, + "Map { undefined → undefined, 1 → one, 2 → undefined }" + ); } #[test] @@ -166,3 +169,38 @@ fn for_each() { assert_eq!(forward(&mut engine, "keySum"), "6"); assert_eq!(forward(&mut engine, "sizeSum"), "9"); } + +#[test] +fn modify_key() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let obj = new Object(); + let map = new Map([[obj, "one"]]); + obj.field = "Value"; + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map.get(obj)"); + assert_eq!(result, "one"); +} + +#[test] +fn order() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map([[1, "one"]]); + map.set(2, "two"); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map"); + assert_eq!(result, "Map { 1 → one, 2 → two }"); + let result = forward(&mut engine, "map.set(1, \"five\");map"); + assert_eq!(result, "Map { 1 → five, 2 → two }"); + let result = forward(&mut engine, "map.set();map"); + assert_eq!(result, "Map { 1 → five, 2 → two, undefined → undefined }"); + let result = forward(&mut engine, "map.delete(2);map"); + assert_eq!(result, "Map { 1 → five, undefined → undefined }"); + let result = forward(&mut engine, "map.set(2, \"two\");map"); + assert_eq!(result, "Map { 1 → five, undefined → undefined, 2 → two }"); +} diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index e00e4c74c9e..ab3e949e431 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -16,6 +16,7 @@ use crate::{ builtins::{ function::Function, + map::ordered_map::OrderedMap, property::Property, value::{RcBigInt, RcString, RcSymbol, ResultValue, Value}, BigInt, @@ -23,12 +24,9 @@ use crate::{ exec::Interpreter, BoaProfiler, }; -use gc::{Finalize, Trace}; +use gc::{Finalize, GcCell, Trace}; use rustc_hash::FxHashMap; -use std::{ - collections::HashMap, - fmt::{Debug, Display, Error, Formatter}, -}; +use std::fmt::{Debug, Display, Error, Formatter}; use super::function::{make_builtin_fn, make_constructor_fn}; use crate::builtins::value::same_value; @@ -70,7 +68,7 @@ pub struct Object { #[derive(Debug, Trace, Finalize, Clone)] pub enum ObjectData { Array, - Map(HashMap), + Map(GcCell>), BigInt(RcBigInt), Boolean(bool), Function(Function), @@ -263,15 +261,15 @@ impl Object { } #[inline] - pub fn as_map(&self) -> Option> { + pub fn as_map_clone(&self) -> Option> { match self.data { - ObjectData::Map(ref map) => Some(map.clone()), + ObjectData::Map(ref map) => Some(map.borrow().clone()), _ => None, } } #[inline] - pub fn as_map_ref(&self) -> Option<&HashMap> { + pub fn as_map_ref(&self) -> Option<&GcCell>> { match self.data { ObjectData::Map(ref map) => Some(map), _ => None, diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index 7b352940c72..81a67057de1 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -117,6 +117,7 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool) -> String { } let mappings = map + .borrow() .iter() .map(|(key, value)| { let key = log_string_from(key, print_internals); diff --git a/boa/src/exec/mod.rs b/boa/src/exec/mod.rs index b9dcfe6ef30..7eb4664a20e 100644 --- a/boa/src/exec/mod.rs +++ b/boa/src/exec/mod.rs @@ -381,6 +381,7 @@ impl Interpreter { // Check if object is a Map else if let ObjectData::Map(ref map) = x.deref().borrow().data { let values = map + .borrow() .iter() .map(|(key, value)| { // Construct a new array containing the key-value pair From 84293d9156ac702b5555132dceb3c225d8b8d077 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sat, 11 Jul 2020 15:14:42 +0100 Subject: [PATCH 15/31] Prevent recursive display for map and array --- boa/src/builtins/map/tests.rs | 16 ++++++ boa/src/builtins/value/display.rs | 81 ++++++++++++++++++------------- 2 files changed, 62 insertions(+), 35 deletions(-) diff --git a/boa/src/builtins/map/tests.rs b/boa/src/builtins/map/tests.rs index 3f0b2fe12d4..68a9b72ec1e 100644 --- a/boa/src/builtins/map/tests.rs +++ b/boa/src/builtins/map/tests.rs @@ -204,3 +204,19 @@ fn order() { let result = forward(&mut engine, "map.set(2, \"two\");map"); assert_eq!(result, "Map { 1 → five, undefined → undefined, 2 → two }"); } + +#[test] +fn recursive_display() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = r#" + let map = new Map(); + let array = new Array([map]); + map.set("y", map); + "#; + forward(&mut engine, init); + let result = forward(&mut engine, "map"); + assert_eq!(result, "Map { y → Map(1) }"); + let result = forward(&mut engine, "map.set(\"z\", array)"); + assert_eq!(result, "Map { y → Map(2), z → Array(1) }"); +} \ No newline at end of file diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index 81a67057de1..5b6e0d14251 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -58,7 +58,7 @@ macro_rules! print_obj_value { }; } -pub(crate) fn log_string_from(x: &Value, print_internals: bool) -> String { +pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: bool) -> String { match x { // We don't want to print private (compiler) or prototype properties Value::Object(ref v) => { @@ -78,29 +78,35 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool) -> String { .expect("Could not borrow value"), ); - if len == 0 { - return String::from("[]"); + if print_children { + if len == 0 { + return String::from("[]"); + } + + let arr = (0..len) + .map(|i| { + // Introduce recursive call to stringify any objects + // which are part of the Array + log_string_from( + &v.borrow() + .properties() + .get(i.to_string().as_str()) + .unwrap() + .value + .clone() + .expect("Could not borrow value"), + print_internals, + false + ) + }) + .collect::>() + .join(", "); + + format!("[ {} ]", arr) + } + else { + format!("Array({})", len) } - - let arr = (0..len) - .map(|i| { - // Introduce recursive call to stringify any objects - // which are part of the Array - log_string_from( - &v.borrow() - .properties() - .get(i.to_string().as_str()) - .unwrap() - .value - .clone() - .expect("Could not borrow value"), - print_internals, - ) - }) - .collect::>() - .join(", "); - - format!("[ {} ]", arr) } ObjectData::Map(ref map) => { let size = i32::from( @@ -116,17 +122,22 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool) -> String { return String::from("Map(0)"); } - let mappings = map - .borrow() - .iter() - .map(|(key, value)| { - let key = log_string_from(key, print_internals); - let value = log_string_from(value, print_internals); - format!("{} → {}", key, value) - }) - .collect::>() - .join(", "); - format!("Map {{ {} }}", mappings) + if print_children { + let mappings = map + .borrow() + .iter() + .map(|(key, value)| { + let key = log_string_from(key, print_internals, false); + let value = log_string_from(value, print_internals, false); + format!("{} → {}", key, value) + }) + .collect::>() + .join(", "); + format!("Map {{ {} }}", mappings) + } + else { + format!("Map({})", size) + } } _ => display_obj(&x, print_internals), } @@ -213,7 +224,7 @@ impl Display for Value { _ => v.to_string(), } ), - Self::Object(_) => write!(f, "{}", log_string_from(self, true)), + Self::Object(_) => write!(f, "{}", log_string_from(self, true, true)), Self::Integer(v) => write!(f, "{}", v), Self::BigInt(ref num) => write!(f, "{}n", num), } From e6636663c52816bdff3e6c59d5e7a607f14afe72 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sat, 11 Jul 2020 15:20:29 +0100 Subject: [PATCH 16/31] Run rustfmt --- boa/src/builtins/map/tests.rs | 2 +- boa/src/builtins/value/display.rs | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/boa/src/builtins/map/tests.rs b/boa/src/builtins/map/tests.rs index 68a9b72ec1e..04e9e95445b 100644 --- a/boa/src/builtins/map/tests.rs +++ b/boa/src/builtins/map/tests.rs @@ -219,4 +219,4 @@ fn recursive_display() { assert_eq!(result, "Map { y → Map(1) }"); let result = forward(&mut engine, "map.set(\"z\", array)"); assert_eq!(result, "Map { y → Map(2), z → Array(1) }"); -} \ No newline at end of file +} diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index 5b6e0d14251..d0ec4d3fbf0 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -96,15 +96,14 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: .clone() .expect("Could not borrow value"), print_internals, - false + false, ) }) .collect::>() .join(", "); format!("[ {} ]", arr) - } - else { + } else { format!("Array({})", len) } } @@ -134,8 +133,7 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: .collect::>() .join(", "); format!("Map {{ {} }}", mappings) - } - else { + } else { format!("Map({})", size) } } From 96e82d210724c22cf123c4c3af17948d0b5f0401 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sat, 11 Jul 2020 16:09:50 +0100 Subject: [PATCH 17/31] Merge create into init Co-authored-by: HalidOdat --- boa/src/builtins/map/mod.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index a8613e52bbe..1c31197eaa0 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -296,8 +296,10 @@ impl Map { Ok(this.clone()) } - /// Create a new `Map` object. - pub(crate) fn create(global: &Value) -> Value { + /// Initialise the `Map` object on the global object. + pub(crate) fn init(global: &Value) -> Value { + let _timer = BoaProfiler::global().start_event(Self::NAME, "init"); + // Create prototype let prototype = Value::new_object(Some(global)); @@ -308,21 +310,15 @@ impl Map { make_builtin_fn(Self::has, "has", &prototype, 1); make_builtin_fn(Self::for_each, "forEach", &prototype, 1); - make_constructor_fn( + let map_object = make_constructor_fn( Self::NAME, Self::LENGTH, Self::make_map, global, prototype, true, - ) - } - - /// Initialise the `Map` object on the global object. - #[inline] - pub(crate) fn init(global: &Value) -> (&str, Value) { - let _timer = BoaProfiler::global().start_event(Self::NAME, "init"); - - (Self::NAME, Self::create(global)) + ); + + (Self::NAME, map_object) } } From cce249a993cc76e421f6fd076cc5fd1acf39e139 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sat, 11 Jul 2020 16:12:40 +0100 Subject: [PATCH 18/31] Fix return type on init --- boa/src/builtins/map/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 1c31197eaa0..05ba74bcbc4 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -297,8 +297,8 @@ impl Map { } /// Initialise the `Map` object on the global object. - pub(crate) fn init(global: &Value) -> Value { - let _timer = BoaProfiler::global().start_event(Self::NAME, "init"); + pub(crate) fn init(global: &Value) -> (&str, Value) { + let _timer = BoaProfiler::global().start_event(Self::NAME, "init"); // Create prototype let prototype = Value::new_object(Some(global)); @@ -318,7 +318,7 @@ impl Map { prototype, true, ); - + (Self::NAME, map_object) } } From a6e093a033a030de39b7a016ae1ad01115150bf9 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sat, 11 Jul 2020 16:32:36 +0100 Subject: [PATCH 19/31] Initial implementation of preventing Map() being called as a function --- boa/src/builtins/array/mod.rs | 1 + boa/src/builtins/bigint/mod.rs | 1 + boa/src/builtins/boolean/mod.rs | 1 + boa/src/builtins/error/mod.rs | 1 + boa/src/builtins/error/range.rs | 1 + boa/src/builtins/error/reference.rs | 1 + boa/src/builtins/error/syntax.rs | 1 + boa/src/builtins/error/type.rs | 1 + boa/src/builtins/function/mod.rs | 4 +++- boa/src/builtins/map/mod.rs | 1 + boa/src/builtins/map/tests.rs | 9 +++++++++ boa/src/builtins/number/mod.rs | 1 + boa/src/builtins/object/mod.rs | 2 +- boa/src/builtins/regexp/mod.rs | 1 + boa/src/builtins/string/mod.rs | 1 + boa/src/builtins/symbol/mod.rs | 1 + 16 files changed, 26 insertions(+), 2 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index c6650a7c875..9da719751fb 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -996,6 +996,7 @@ impl Array { global, prototype, true, + true, ); // Static Methods diff --git a/boa/src/builtins/bigint/mod.rs b/boa/src/builtins/bigint/mod.rs index d6352364c18..01f0ade220f 100644 --- a/boa/src/builtins/bigint/mod.rs +++ b/boa/src/builtins/bigint/mod.rs @@ -215,6 +215,7 @@ impl BigInt { global, prototype, false, + true, ); make_builtin_fn(Self::as_int_n, "asIntN", &bigint_object, 2); diff --git a/boa/src/builtins/boolean/mod.rs b/boa/src/builtins/boolean/mod.rs index 81826821eff..6edad1e2810 100644 --- a/boa/src/builtins/boolean/mod.rs +++ b/boa/src/builtins/boolean/mod.rs @@ -115,6 +115,7 @@ impl Boolean { global, prototype, true, + true, ); (Self::NAME, boolean_object) diff --git a/boa/src/builtins/error/mod.rs b/boa/src/builtins/error/mod.rs index 24943f00d01..828ab554cee 100644 --- a/boa/src/builtins/error/mod.rs +++ b/boa/src/builtins/error/mod.rs @@ -92,6 +92,7 @@ impl Error { global, prototype, true, + true, ); (Self::NAME, error_object) diff --git a/boa/src/builtins/error/range.rs b/boa/src/builtins/error/range.rs index 400122dd956..ba8e76574cb 100644 --- a/boa/src/builtins/error/range.rs +++ b/boa/src/builtins/error/range.rs @@ -78,6 +78,7 @@ impl RangeError { global, prototype, true, + true, ); (Self::NAME, range_error_object) diff --git a/boa/src/builtins/error/reference.rs b/boa/src/builtins/error/reference.rs index 93d66488fce..842bebf1a89 100644 --- a/boa/src/builtins/error/reference.rs +++ b/boa/src/builtins/error/reference.rs @@ -76,6 +76,7 @@ impl ReferenceError { global, prototype, true, + true, ); (Self::NAME, reference_error_object) diff --git a/boa/src/builtins/error/syntax.rs b/boa/src/builtins/error/syntax.rs index 22a3da1f140..783551651a5 100644 --- a/boa/src/builtins/error/syntax.rs +++ b/boa/src/builtins/error/syntax.rs @@ -80,6 +80,7 @@ impl SyntaxError { global, prototype, true, + true, ); (Self::NAME, syntax_error_object) diff --git a/boa/src/builtins/error/type.rs b/boa/src/builtins/error/type.rs index 727d836c509..27d3dbbf46f 100644 --- a/boa/src/builtins/error/type.rs +++ b/boa/src/builtins/error/type.rs @@ -84,6 +84,7 @@ impl TypeError { global, prototype, true, + true, ); (Self::NAME, type_error_object) diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index 50f955f928b..67afd2a1c8e 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -420,6 +420,7 @@ pub fn make_constructor_fn( global: &Value, prototype: Value, constructable: bool, + callable: bool, ) -> Value { let _timer = BoaProfiler::global().start_event(&format!("make_constructor_fn: {}", name), "init"); @@ -427,6 +428,7 @@ pub fn make_constructor_fn( // Create the native function let mut function = Function::builtin(Vec::new(), body); function.constructable = constructable; + function.callable = callable; let mut constructor = Object::function(function); @@ -507,7 +509,7 @@ pub fn init(global: &Value) -> (&str, Value) { let prototype = Value::new_object(Some(global)); let function_object = - make_constructor_fn("Function", 1, make_function, global, prototype, true); + make_constructor_fn("Function", 1, make_function, global, prototype, true, true); ("Function", function_object) } diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 05ba74bcbc4..e8f066b92ad 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -317,6 +317,7 @@ impl Map { global, prototype, true, + false, ); (Self::NAME, map_object) diff --git a/boa/src/builtins/map/tests.rs b/boa/src/builtins/map/tests.rs index 04e9e95445b..2627737b893 100644 --- a/boa/src/builtins/map/tests.rs +++ b/boa/src/builtins/map/tests.rs @@ -220,3 +220,12 @@ fn recursive_display() { let result = forward(&mut engine, "map.set(\"z\", array)"); assert_eq!(result, "Map { y → Map(2), z → Array(1) }"); } + +#[test] +#[should_panic] +fn not_a_function() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let init = "let map = Map()"; + forward(&mut engine, init); +} diff --git a/boa/src/builtins/number/mod.rs b/boa/src/builtins/number/mod.rs index a69e94c5199..99a303ebd93 100644 --- a/boa/src/builtins/number/mod.rs +++ b/boa/src/builtins/number/mod.rs @@ -584,6 +584,7 @@ impl Number { global, prototype, true, + true, ); // Constants from: diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index ab3e949e431..ec0a670a278 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -573,7 +573,7 @@ pub fn init(global: &Value) -> (&str, Value) { ); make_builtin_fn(to_string, "toString", &prototype, 0); - let object = make_constructor_fn("Object", 1, make_object, global, prototype, true); + let object = make_constructor_fn("Object", 1, make_object, global, prototype, true, true); // static methods of the builtin Object make_builtin_fn(create, "create", &object, 2); diff --git a/boa/src/builtins/regexp/mod.rs b/boa/src/builtins/regexp/mod.rs index fb6eca2e7b7..cf96bfb96e8 100644 --- a/boa/src/builtins/regexp/mod.rs +++ b/boa/src/builtins/regexp/mod.rs @@ -486,6 +486,7 @@ impl RegExp { global, prototype, true, + true, ) } diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index fa0e00eaf92..3819fcb6346 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -1042,6 +1042,7 @@ impl String { global, prototype, true, + true, ); (Self::NAME, string_object) diff --git a/boa/src/builtins/symbol/mod.rs b/boa/src/builtins/symbol/mod.rs index 0a615378d4c..6548963db8b 100644 --- a/boa/src/builtins/symbol/mod.rs +++ b/boa/src/builtins/symbol/mod.rs @@ -115,6 +115,7 @@ impl Symbol { global, prototype, false, + true, ); (Self::NAME, symbol_object) From 77a08331ebb39f9680ecff589450432b25ad6aaa Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sat, 11 Jul 2020 17:00:54 +0100 Subject: [PATCH 20/31] Use bitflags for callable and constructable --- boa/src/builtins/function/mod.rs | 58 +++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 12 deletions(-) diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index 67afd2a1c8e..8d5b17b6ab5 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -24,6 +24,7 @@ use crate::{ syntax::ast::node::{FormalParameter, StatementList}, BoaProfiler, }; +use bitflags::bitflags; use gc::{unsafe_empty_trace, Finalize, Trace}; use std::fmt::{self, Debug}; @@ -90,6 +91,43 @@ unsafe impl Trace for FunctionBody { unsafe_empty_trace!(); } +bitflags! { + #[derive(Finalize, Default)] + pub struct FunctionFlags: u8 { + const CALLABLE = 0b0000_0001; + const CONSTRUCTABLE = 0b0000_0010; + } +} + +impl FunctionFlags { + pub fn from_parameters(callable: bool, constructable: bool) -> Self { + let mut flags = Self::default(); + + if callable { + flags |= Self::CALLABLE; + } + if constructable { + flags |= Self::CONSTRUCTABLE; + } + + flags + } + + #[inline] + pub fn is_callable(&self) -> bool { + self.contains(Self::CALLABLE) + } + + #[inline] + pub fn is_constructable(&self) -> bool { + self.contains(Self::CONSTRUCTABLE) + } +} + +unsafe impl Trace for FunctionFlags { + unsafe_empty_trace!(); +} + /// Boa representation of a Function Object. /// /// @@ -103,10 +141,8 @@ pub struct Function { pub this_mode: ThisMode, // Environment, built-in functions don't need Environments pub environment: Option, - /// Is it constructable - constructable: bool, - /// Is it callable. - callable: bool, + /// Is it constructable or + flags: FunctionFlags, } impl Function { @@ -126,8 +162,7 @@ impl Function { environment: scope, params: parameter_list.into(), this_mode, - constructable, - callable, + flags: FunctionFlags::from_parameters(callable, constructable), } } @@ -183,7 +218,7 @@ impl Function { interpreter: &mut Interpreter, ) -> ResultValue { let _timer = BoaProfiler::global().start_event("function::call", "function"); - if self.callable { + if self.flags.is_callable() { match self.body { FunctionBody::BuiltIn(func) => func(this, args_list, interpreter), FunctionBody::Ordinary(ref body) => { @@ -249,7 +284,7 @@ impl Function { args_list: &[Value], interpreter: &mut Interpreter, ) -> ResultValue { - if self.constructable { + if self.flags.is_constructable() { match self.body { FunctionBody::BuiltIn(func) => { func(this, args_list, interpreter)?; @@ -351,12 +386,12 @@ impl Function { /// Returns true if the function object is callable. pub fn is_callable(&self) -> bool { - self.callable + self.flags.is_callable() } /// Returns true if the function object is constructable. pub fn is_constructable(&self) -> bool { - self.constructable + self.flags.is_constructable() } } @@ -427,8 +462,7 @@ pub fn make_constructor_fn( // Create the native function let mut function = Function::builtin(Vec::new(), body); - function.constructable = constructable; - function.callable = callable; + function.flags = FunctionFlags::from_parameters(callable, constructable); let mut constructor = Object::function(function); From 374f8544f769794661b9a53a927293757e6b1847 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sat, 11 Jul 2020 17:06:28 +0100 Subject: [PATCH 21/31] FunctionFlags doesn't have to be public --- boa/src/builtins/function/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index 8d5b17b6ab5..83e5de29653 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -93,14 +93,14 @@ unsafe impl Trace for FunctionBody { bitflags! { #[derive(Finalize, Default)] - pub struct FunctionFlags: u8 { + struct FunctionFlags: u8 { const CALLABLE = 0b0000_0001; const CONSTRUCTABLE = 0b0000_0010; } } impl FunctionFlags { - pub fn from_parameters(callable: bool, constructable: bool) -> Self { + fn from_parameters(callable: bool, constructable: bool) -> Self { let mut flags = Self::default(); if callable { @@ -114,12 +114,12 @@ impl FunctionFlags { } #[inline] - pub fn is_callable(&self) -> bool { + fn is_callable(&self) -> bool { self.contains(Self::CALLABLE) } #[inline] - pub fn is_constructable(&self) -> bool { + fn is_constructable(&self) -> bool { self.contains(Self::CONSTRUCTABLE) } } From f21e21784f66f3ee2cc76af74f7aa1b7e54f7187 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 18:45:54 +0100 Subject: [PATCH 22/31] Initial implementation of Map() --- boa/src/builtins/value/display.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index d0ec4d3fbf0..05ef76c6789 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -137,6 +137,31 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: format!("Map({})", size) } } + ObjectData::Map(ref map) => { + let size = i32::from( + &v.borrow() + .properties() + .get("size") + .unwrap() + .value + .clone() + .expect("Could not borrow value"), + ); + if size == 0 { + return String::from("Map(0)"); + } + + let mappings = map + .iter() + .map(|(key, value)| { + let key = log_string_from(key, print_internals); + let value = log_string_from(value, print_internals); + format!("{} → {}", key, value) + }) + .collect::>() + .join(", "); + format!("Map {{ {} }}", mappings) + } _ => display_obj(&x, print_internals), } } From 3c3729495d7cc5854015a5540f04e153ba0f86e7 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sat, 11 Jul 2020 14:33:16 +0100 Subject: [PATCH 23/31] Rebase upstream --- boa/src/builtins/value/display.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index 05ef76c6789..3df232f3ff6 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -152,6 +152,7 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: } let mappings = map + .borrow() .iter() .map(|(key, value)| { let key = log_string_from(key, print_internals); From a9853b8b0bc6bb104baacbeb36421f72b27ba847 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sat, 11 Jul 2020 15:14:42 +0100 Subject: [PATCH 24/31] Prevent recursive display for map and array --- boa/src/builtins/value/display.rs | 29 ++--------------------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index 3df232f3ff6..b38d67fcf47 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -133,35 +133,10 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: .collect::>() .join(", "); format!("Map {{ {} }}", mappings) - } else { - format!("Map({})", size) } - } - ObjectData::Map(ref map) => { - let size = i32::from( - &v.borrow() - .properties() - .get("size") - .unwrap() - .value - .clone() - .expect("Could not borrow value"), - ); - if size == 0 { - return String::from("Map(0)"); + else { + format!("Map({})", size) } - - let mappings = map - .borrow() - .iter() - .map(|(key, value)| { - let key = log_string_from(key, print_internals); - let value = log_string_from(value, print_internals); - format!("{} → {}", key, value) - }) - .collect::>() - .join(", "); - format!("Map {{ {} }}", mappings) } _ => display_obj(&x, print_internals), } From 2a6d23ec31eaf85775fe53b3552a94084514bef9 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sun, 5 Jul 2020 18:45:54 +0100 Subject: [PATCH 25/31] Initial implementation of Map() --- boa/src/builtins/value/display.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index b38d67fcf47..96325d1de66 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -138,6 +138,31 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: format!("Map({})", size) } } + ObjectData::Map(ref map) => { + let size = i32::from( + &v.borrow() + .properties() + .get("size") + .unwrap() + .value + .clone() + .expect("Could not borrow value"), + ); + if size == 0 { + return String::from("Map(0)"); + } + + let mappings = map + .iter() + .map(|(key, value)| { + let key = log_string_from(key, print_internals); + let value = log_string_from(value, print_internals); + format!("{} → {}", key, value) + }) + .collect::>() + .join(", "); + format!("Map {{ {} }}", mappings) + } _ => display_obj(&x, print_internals), } } From 0af0d8e07ff882e3f1478edd4c6872b45d43a5ba Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Mon, 6 Jul 2020 01:50:25 +0100 Subject: [PATCH 26/31] Use IndexMap instead of HashMap --- boa/src/builtins/value/display.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index 96325d1de66..2cecb7d3c88 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -153,6 +153,7 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: } let mappings = map + .borrow() .iter() .map(|(key, value)| { let key = log_string_from(key, print_internals); From c05028e8940b95cd5dfdc40e0186da9b78c231c6 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Sat, 11 Jul 2020 15:20:29 +0100 Subject: [PATCH 27/31] Run rustfmt --- boa/src/builtins/value/display.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index 2cecb7d3c88..3df232f3ff6 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -133,8 +133,7 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: .collect::>() .join(", "); format!("Map {{ {} }}", mappings) - } - else { + } else { format!("Map({})", size) } } From 1db2ee42b62fb0b0fdbe435487264a2f987c2fb3 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Tue, 14 Jul 2020 02:25:54 +0100 Subject: [PATCH 28/31] Remove artifact from rebase --- boa/src/builtins/value/display.rs | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index 3df232f3ff6..d0ec4d3fbf0 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -137,32 +137,6 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: format!("Map({})", size) } } - ObjectData::Map(ref map) => { - let size = i32::from( - &v.borrow() - .properties() - .get("size") - .unwrap() - .value - .clone() - .expect("Could not borrow value"), - ); - if size == 0 { - return String::from("Map(0)"); - } - - let mappings = map - .borrow() - .iter() - .map(|(key, value)| { - let key = log_string_from(key, print_internals); - let value = log_string_from(value, print_internals); - format!("{} → {}", key, value) - }) - .collect::>() - .join(", "); - format!("Map {{ {} }}", mappings) - } _ => display_obj(&x, print_internals), } } From 1730256a8a1a09e33b1ffd1cd35e1dbcf6585b92 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Tue, 14 Jul 2020 02:37:05 +0100 Subject: [PATCH 29/31] Use into() for value creation --- boa/src/builtins/map/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index e8f066b92ad..4fae899a07d 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -111,7 +111,7 @@ impl Map { return Err(ctx.construct_type_error("'this' is not a Map")); }; Self::set_size(this, size); - Ok(Value::boolean(deleted)) + Ok(deleted.into()) } /// `Map.prototype.get( key )` @@ -183,7 +183,7 @@ impl Map { if let Value::Object(ref object) = this { let object = object.borrow(); if let Some(map) = object.as_map_ref() { - return Ok(Value::Boolean(map.borrow().contains_key(key))); + return Ok(map.borrow().contains_key(key).into()); } } From 7716cbda9c30f77da403372c52725bd1ba637e19 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Tue, 14 Jul 2020 20:00:12 +0100 Subject: [PATCH 30/31] Remove GcCell around OrderedMap --- boa/src/builtins/map/mod.rs | 23 ++++++++++------------- boa/src/builtins/object/mod.rs | 14 +++++++------- boa/src/builtins/value/display.rs | 1 - 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 4fae899a07d..4d0b3c3aae0 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -10,7 +10,6 @@ use crate::{ exec::Interpreter, BoaProfiler, }; -use gc::GcCell; use ordered_map::OrderedMap; pub mod ordered_map; @@ -65,9 +64,8 @@ impl Map { }; let size = if let Value::Object(ref object) = this { - let object = object.borrow(); - if let Some(map) = object.as_map_ref() { - let mut map = map.borrow_mut(); + let mut object = object.borrow_mut(); + if let Some(map) = object.as_map_mut() { map.insert(key, value); map.len() } else { @@ -99,9 +97,8 @@ impl Map { }; let (deleted, size) = if let Value::Object(ref object) = this { - let object = object.borrow(); - if let Some(map) = object.as_map_ref() { - let mut map = map.borrow_mut(); + let mut object = object.borrow_mut(); + if let Some(map) = object.as_map_mut() { let deleted = map.remove(key).is_some(); (deleted, map.len()) } else { @@ -134,7 +131,7 @@ impl Map { if let Value::Object(ref object) = this { let object = object.borrow(); if let Some(map) = object.as_map_ref() { - return Ok(if let Some(result) = map.borrow().get(key) { + return Ok(if let Some(result) = map.get(key) { result.clone() } else { Value::Undefined @@ -156,7 +153,7 @@ impl Map { /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.clear /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/clear pub(crate) fn clear(this: &Value, _: &[Value], _: &mut Interpreter) -> ResultValue { - this.set_data(ObjectData::Map(GcCell::new(OrderedMap::new()))); + this.set_data(ObjectData::Map(OrderedMap::new())); Self::set_size(this, 0); @@ -183,7 +180,7 @@ impl Map { if let Value::Object(ref object) = this { let object = object.borrow(); if let Some(map) = object.as_map_ref() { - return Ok(map.borrow().contains_key(key).into()); + return Ok(map.contains_key(key).into()); } } @@ -214,7 +211,7 @@ impl Map { if let Value::Object(ref object) = this { let object = object.borrow(); - if let Some(map) = object.as_map_clone() { + if let Some(map) = object.as_map_ref().cloned() { for (key, value) in map { let arguments = [value, key, this.clone()]; @@ -259,7 +256,7 @@ impl Map { _ => match &args[0] { Value::Object(object) => { let object = object.borrow(); - if let Some(map) = object.as_map_clone() { + if let Some(map) = object.as_map_ref().cloned() { map } else if object.is_array() { let mut map = OrderedMap::new(); @@ -291,7 +288,7 @@ impl Map { // finally create length property Self::set_size(this, data.len()); - this.set_data(ObjectData::Map(GcCell::new(data))); + this.set_data(ObjectData::Map(data)); Ok(this.clone()) } diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index ec0a670a278..4d2b02b1f14 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -24,7 +24,7 @@ use crate::{ exec::Interpreter, BoaProfiler, }; -use gc::{Finalize, GcCell, Trace}; +use gc::{Finalize, Trace}; use rustc_hash::FxHashMap; use std::fmt::{Debug, Display, Error, Formatter}; @@ -68,7 +68,7 @@ pub struct Object { #[derive(Debug, Trace, Finalize, Clone)] pub enum ObjectData { Array, - Map(GcCell>), + Map(OrderedMap), BigInt(RcBigInt), Boolean(bool), Function(Function), @@ -261,17 +261,17 @@ impl Object { } #[inline] - pub fn as_map_clone(&self) -> Option> { + pub fn as_map_ref(&self) -> Option<&OrderedMap> { match self.data { - ObjectData::Map(ref map) => Some(map.borrow().clone()), + ObjectData::Map(ref map) => Some(map), _ => None, } } #[inline] - pub fn as_map_ref(&self) -> Option<&GcCell>> { - match self.data { - ObjectData::Map(ref map) => Some(map), + pub fn as_map_mut(&mut self) -> Option<&mut OrderedMap> { + match &mut self.data { + ObjectData::Map(map) => Some(map), _ => None, } } diff --git a/boa/src/builtins/value/display.rs b/boa/src/builtins/value/display.rs index d0ec4d3fbf0..02241bc1b7f 100644 --- a/boa/src/builtins/value/display.rs +++ b/boa/src/builtins/value/display.rs @@ -123,7 +123,6 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: if print_children { let mappings = map - .borrow() .iter() .map(|(key, value)| { let key = log_string_from(key, print_internals, false); From ae519c3c68ad22088a17b82029911bc919393099 Mon Sep 17 00:00:00 2001 From: joshwd36 Date: Tue, 14 Jul 2020 20:19:40 +0100 Subject: [PATCH 31/31] Remove commented get_map function --- boa/src/builtins/map/mod.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 4d0b3c3aae0..3c04dae4c5c 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -24,17 +24,6 @@ impl Map { pub(crate) const LENGTH: usize = 1; - /// Helper function to get the map object. - /*fn get_map(this: &Value, ctx: &mut Interpreter) -> Result, Value> { - if let Value::Object(ref object) = this { - let object = object.borrow(); - if let Some(map) = object.as_map() { - return Ok(map); - } - } - Err(ctx.construct_type_error("'this' is not a Map")) - }*/ - /// Helper function to set the size property. fn set_size(this: &Value, size: usize) { let size = Property::new()