Extract PropertyMap struct from Object (#1425)

boa-dev · Aug 6, 2021 · f331694 · f331694
1 parent fe665db
commit f331694
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 61 deletions.
diff --git 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
@@ -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
@@ -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<u32> = self.borrow().index_property_keys().cloned().collect();
+            let mut keys: Vec<u32> = self
+                .borrow()
+                .properties
+                .index_property_keys()
+                .cloned()
+                .collect();
             keys.sort_unstable();
             let mut arr: Vec<JSONValue> = 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<PropertyKey> = self.borrow().keys().collect();
+            let keys: Vec<PropertyKey> = 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
@@ -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<PropertyDescriptor> {
         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<PropertyKey> {
-        self.borrow().keys().collect()
+        self.borrow().properties.keys().collect()
     }
 
     /// The abstract operation ObjectDefineProperties
@@ -895,26 +896,13 @@ impl Object {
         K: Into<PropertyKey>,
         P: Into<PropertyDescriptor>,
     {
-        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<PropertyDescriptor> {
-        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
@@ -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<T: Any + Debug + Trace> NativeObject for T {
 pub struct Object {
     /// The type of the object.
     pub data: ObjectData,
-    indexed_properties: FxHashMap<u32, PropertyDescriptor>,
-    /// Properties
-    string_properties: FxHashMap<JsString, PropertyDescriptor>,
-    /// Symbol Properties
-    symbol_properties: FxHashMap<JsSymbol, PropertyDescriptor>,
+    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 → boa/src/object/property_map.rs b/boa/src/object/iter.rs → 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<u32, PropertyDescriptor>,
+    /// Properties
+    string_properties: FxHashMap<JsString, PropertyDescriptor>,
+    /// Symbol Properties
+    symbol_properties: FxHashMap<JsSymbol, PropertyDescriptor>,
+}
+
+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<PropertyDescriptor> {
+        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<PropertyDescriptor> {
+        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
@@ -73,6 +73,7 @@ macro_rules! print_obj_value {
     (impl $v:expr, $f:expr) => {
         $v
             .borrow()
+            .properties()
             .iter()
             .map($f)
             .collect::<Vec<String>>()