From f331694962656ad427d8bcc8a556915984fa0005 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Fri, 6 Aug 2021 14:38:18 -0500 Subject: [PATCH] Extract `PropertyMap` struct from `Object` (#1425) --- boa/src/builtins/json/mod.rs | 6 +-- boa/src/builtins/object/mod.rs | 2 +- boa/src/object/gcobject.rs | 9 +++- boa/src/object/internal_methods.rs | 34 ++++-------- boa/src/object/mod.rs | 44 ++++++--------- boa/src/object/{iter.rs => property_map.rs} | 59 +++++++++++++++++++-- boa/src/value/display.rs | 1 + 7 files changed, 94 insertions(+), 61 deletions(-) rename boa/src/object/{iter.rs => property_map.rs} (84%) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 89bead0fa3..f5918a6513 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -106,7 +106,7 @@ impl Json { let value = holder.get_field(key.clone(), context)?; if let Value::Object(ref object) = value { - let keys: Vec<_> = object.borrow().keys().collect(); + let keys: Vec<_> = object.borrow().properties().keys().collect(); for key in keys { let v = Self::walk(reviver, context, &mut value.clone(), &key); @@ -196,7 +196,7 @@ impl Json { .as_object() .map(|obj| { let object_to_return = Value::object(Object::default()); - for key in obj.borrow().keys() { + for key in obj.borrow().properties().keys() { let val = obj.__get__(&key, obj.clone().into(), context)?; let this_arg = object.clone(); object_to_return.set_property( @@ -222,7 +222,7 @@ impl Json { } else if replacer_as_object.is_array() { let mut obj_to_return = serde_json::Map::new(); let replacer_as_object = replacer_as_object.borrow(); - let fields = replacer_as_object.keys().filter_map(|key| { + let fields = replacer_as_object.properties().keys().filter_map(|key| { if key == "length" { None } else { diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index e677bb4377..4d4279ff54 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -192,7 +192,7 @@ impl Object { .to_object(context)?; let descriptors = context.construct_object(); - for key in object.borrow().keys() { + for key in object.borrow().properties().keys() { let descriptor = { let desc = object .__get_own_property__(&key) diff --git a/boa/src/object/gcobject.rs b/boa/src/object/gcobject.rs index 1e74ec9173..2841399d06 100644 --- a/boa/src/object/gcobject.rs +++ b/boa/src/object/gcobject.rs @@ -428,7 +428,12 @@ impl GcObject { if rec_limiter.live { Err(context.construct_type_error("cyclic object value")) } else if self.is_array() { - let mut keys: Vec = self.borrow().index_property_keys().cloned().collect(); + let mut keys: Vec = self + .borrow() + .properties + .index_property_keys() + .cloned() + .collect(); keys.sort_unstable(); let mut arr: Vec = Vec::with_capacity(keys.len()); let this = Value::from(self.clone()); @@ -444,7 +449,7 @@ impl GcObject { } else { let mut new_obj = Map::new(); let this = Value::from(self.clone()); - let keys: Vec = self.borrow().keys().collect(); + let keys: Vec = self.borrow().properties.keys().collect(); for k in keys { let key = k.clone(); let value = this.get_field(k.to_string(), context)?; diff --git a/boa/src/object/internal_methods.rs b/boa/src/object/internal_methods.rs index 8244235c75..8d5314ea49 100644 --- a/boa/src/object/internal_methods.rs +++ b/boa/src/object/internal_methods.rs @@ -526,11 +526,16 @@ impl GcObject { return Ok(false); } - let max_value = self.borrow().index_property_keys().max().copied(); + let max_value = self + .borrow() + .properties + .index_property_keys() + .max() + .copied(); if let Some(mut index) = max_value { while index >= new_len { - let contains_index = self.borrow().indexed_properties.contains_key(&index); + let contains_index = self.borrow().properties.contains_key(&index.into()); if contains_index && !self.__delete__(&index.into()) { new_len_desc = new_len_desc.value(index + 1); if !new_writable { @@ -661,11 +666,7 @@ impl GcObject { #[inline] pub fn ordinary_get_own_property(&self, key: &PropertyKey) -> Option { let object = self.borrow(); - let property = match key { - PropertyKey::Index(index) => object.indexed_properties.get(index), - PropertyKey::String(ref st) => object.string_properties.get(st), - PropertyKey::Symbol(ref symbol) => object.symbol_properties.get(symbol), - }; + let property = object.properties.get(key); property.cloned() } @@ -679,7 +680,7 @@ impl GcObject { #[inline] #[track_caller] pub fn own_property_keys(&self) -> Vec { - self.borrow().keys().collect() + self.borrow().properties.keys().collect() } /// The abstract operation ObjectDefineProperties @@ -895,26 +896,13 @@ impl Object { K: Into, P: Into, { - let property = property.into(); - match key.into() { - PropertyKey::Index(index) => self.indexed_properties.insert(index, property), - PropertyKey::String(ref string) => { - self.string_properties.insert(string.clone(), property) - } - PropertyKey::Symbol(ref symbol) => { - self.symbol_properties.insert(symbol.clone(), property) - } - } + self.properties.insert(key.into(), property.into()) } /// Helper function for property removal. #[inline] pub(crate) fn remove(&mut self, key: &PropertyKey) -> Option { - match key { - PropertyKey::Index(index) => self.indexed_properties.remove(index), - PropertyKey::String(ref string) => self.string_properties.remove(string), - PropertyKey::Symbol(ref symbol) => self.symbol_properties.remove(symbol), - } + self.properties.remove(key) } /// Inserts a field in the object `properties` without checking if it's writable. diff --git a/boa/src/object/mod.rs b/boa/src/object/mod.rs index a09a555190..15bb613765 100644 --- a/boa/src/object/mod.rs +++ b/boa/src/object/mod.rs @@ -17,7 +17,6 @@ use crate::{ property::{Attribute, PropertyDescriptor, PropertyKey}, BoaProfiler, Context, JsBigInt, JsString, JsSymbol, Value, }; -use rustc_hash::FxHashMap; use std::{ any::Any, fmt::{self, Debug, Display}, @@ -30,11 +29,11 @@ mod tests; mod gcobject; mod internal_methods; -mod iter; +mod property_map; use crate::builtins::object::for_in_iterator::ForInIterator; pub use gcobject::{GcObject, RecursionLimiter, Ref, RefMut}; -pub use iter::*; +pub use property_map::*; /// Static `prototype`, usually set on constructors as a key to point to their respective prototype object. pub static PROTOTYPE: &str = "prototype"; @@ -67,11 +66,7 @@ impl NativeObject for T { pub struct Object { /// The type of the object. pub data: ObjectData, - indexed_properties: FxHashMap, - /// Properties - string_properties: FxHashMap, - /// Symbol Properties - symbol_properties: FxHashMap, + properties: PropertyMap, /// Instance prototype `__proto__`. prototype: Value, /// Whether it can have new properties added to it. @@ -142,9 +137,7 @@ impl Default for Object { fn default() -> Self { Self { data: ObjectData::Ordinary, - indexed_properties: FxHashMap::default(), - string_properties: FxHashMap::default(), - symbol_properties: FxHashMap::default(), + properties: PropertyMap::default(), prototype: Value::null(), extensible: true, } @@ -164,9 +157,7 @@ impl Object { Self { data: ObjectData::Function(function), - indexed_properties: FxHashMap::default(), - string_properties: FxHashMap::default(), - symbol_properties: FxHashMap::default(), + properties: PropertyMap::default(), prototype, extensible: true, } @@ -191,9 +182,7 @@ impl Object { pub fn boolean(value: bool) -> Self { Self { data: ObjectData::Boolean(value), - indexed_properties: FxHashMap::default(), - string_properties: FxHashMap::default(), - symbol_properties: FxHashMap::default(), + properties: PropertyMap::default(), prototype: Value::null(), extensible: true, } @@ -204,9 +193,7 @@ impl Object { pub fn number(value: f64) -> Self { Self { data: ObjectData::Number(value), - indexed_properties: FxHashMap::default(), - string_properties: FxHashMap::default(), - symbol_properties: FxHashMap::default(), + properties: PropertyMap::default(), prototype: Value::null(), extensible: true, } @@ -220,9 +207,7 @@ impl Object { { Self { data: ObjectData::String(value.into()), - indexed_properties: FxHashMap::default(), - string_properties: FxHashMap::default(), - symbol_properties: FxHashMap::default(), + properties: PropertyMap::default(), prototype: Value::null(), extensible: true, } @@ -233,9 +218,7 @@ impl Object { pub fn bigint(value: JsBigInt) -> Self { Self { data: ObjectData::BigInt(value), - indexed_properties: FxHashMap::default(), - string_properties: FxHashMap::default(), - symbol_properties: FxHashMap::default(), + properties: PropertyMap::default(), prototype: Value::null(), extensible: true, } @@ -249,9 +232,7 @@ impl Object { { Self { data: ObjectData::NativeObject(Box::new(value)), - indexed_properties: FxHashMap::default(), - string_properties: FxHashMap::default(), - symbol_properties: FxHashMap::default(), + properties: PropertyMap::default(), prototype: Value::null(), extensible: true, } @@ -610,6 +591,11 @@ impl Object { _ => None, } } + + #[inline] + pub fn properties(&self) -> &PropertyMap { + &self.properties + } } /// The functions binding. diff --git a/boa/src/object/iter.rs b/boa/src/object/property_map.rs similarity index 84% rename from boa/src/object/iter.rs rename to boa/src/object/property_map.rs index f5b2f10ced..04c3a0edbe 100644 --- a/boa/src/object/iter.rs +++ b/boa/src/object/property_map.rs @@ -1,8 +1,52 @@ -use super::{Object, PropertyDescriptor, PropertyKey}; -use crate::{JsString, JsSymbol}; +use super::{PropertyDescriptor, PropertyKey}; +use crate::{ + gc::{Finalize, Trace}, + JsString, JsSymbol, +}; +use rustc_hash::FxHashMap; use std::{collections::hash_map, iter::FusedIterator}; -impl Object { +#[derive(Default, Debug, Trace, Finalize)] +pub struct PropertyMap { + indexed_properties: FxHashMap, + /// Properties + string_properties: FxHashMap, + /// Symbol Properties + symbol_properties: FxHashMap, +} + +impl PropertyMap { + pub fn new() -> Self { + Self::default() + } + pub fn get(&self, key: &PropertyKey) -> Option<&PropertyDescriptor> { + match key { + PropertyKey::Index(index) => self.indexed_properties.get(index), + PropertyKey::String(string) => self.string_properties.get(string), + PropertyKey::Symbol(symbol) => self.symbol_properties.get(symbol), + } + } + + pub fn insert( + &mut self, + key: PropertyKey, + property: PropertyDescriptor, + ) -> Option { + match &key { + PropertyKey::Index(index) => self.indexed_properties.insert(*index, property), + PropertyKey::String(string) => self.string_properties.insert(string.clone(), property), + PropertyKey::Symbol(symbol) => self.symbol_properties.insert(symbol.clone(), property), + } + } + + pub fn remove(&mut self, key: &PropertyKey) -> Option { + match key { + PropertyKey::Index(index) => self.indexed_properties.remove(index), + PropertyKey::String(string) => self.string_properties.remove(string), + PropertyKey::Symbol(symbol) => self.symbol_properties.remove(symbol), + } + } + /// An iterator visiting all key-value pairs in arbitrary order. The iterator element type is `(PropertyKey, &'a Property)`. /// /// This iterator does not recurse down the prototype chain. @@ -103,6 +147,15 @@ impl Object { pub fn string_property_values(&self) -> StringPropertyValues<'_> { StringPropertyValues(self.string_properties.values()) } + + #[inline] + pub fn contains_key(&self, key: &PropertyKey) -> bool { + match key { + PropertyKey::Index(index) => self.indexed_properties.contains_key(index), + PropertyKey::String(string) => self.string_properties.contains_key(string), + PropertyKey::Symbol(symbol) => self.symbol_properties.contains_key(symbol), + } + } } /// An iterator over the property entries of an `Object` diff --git a/boa/src/value/display.rs b/boa/src/value/display.rs index af18c8a79b..e7e657c547 100644 --- a/boa/src/value/display.rs +++ b/boa/src/value/display.rs @@ -73,6 +73,7 @@ macro_rules! print_obj_value { (impl $v:expr, $f:expr) => { $v .borrow() + .properties() .iter() .map($f) .collect::>()