From 5d4d8fe79446e7b18bd9334995706eba537333f4 Mon Sep 17 00:00:00 2001 From: HalidOdat Date: Tue, 21 Jul 2020 02:26:53 +0200 Subject: [PATCH] Refactor Property Descriptor flags (#553) --- .vscode/launch.json | 2 +- Cargo.lock | 30 ++- boa/src/builtins/array/mod.rs | 21 +-- boa/src/builtins/function/mod.rs | 38 ++-- boa/src/builtins/map/mod.rs | 11 +- boa/src/builtins/object/internal_methods.rs | 87 ++++----- boa/src/builtins/object/mod.rs | 2 +- boa/src/builtins/property/attribute/mod.rs | 145 +++++++++++++++ boa/src/builtins/property/attribute/tests.rs | 167 +++++++++++++++++ boa/src/builtins/property/mod.rs | 172 +++++++++++++----- boa/src/builtins/value/mod.rs | 37 ++-- .../environment/global_environment_record.rs | 33 ++-- .../environment/object_environment_record.rs | 20 +- 13 files changed, 570 insertions(+), 195 deletions(-) create mode 100644 boa/src/builtins/property/attribute/mod.rs create mode 100644 boa/src/builtins/property/attribute/tests.rs diff --git a/.vscode/launch.json b/.vscode/launch.json index 99f2493327..ed59383546 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -47,4 +47,4 @@ "externalConsole": true }, ] -} \ No newline at end of file +} diff --git a/Cargo.lock b/Cargo.lock index a86450f8e0..545b12ce29 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -146,9 +146,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.0.57" +version = "1.0.58" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fde55d2a2bfaa4c9668bbc63f531fbdeee3ffe188f4662511ce2c22b3eedebe" +checksum = "f9a06fb2e53271d7c279ec1efea6ab691c35a2ae67ec0d91d7acec0caf13b518" [[package]] name = "cfg-if" @@ -356,6 +356,15 @@ dependencies = [ "wasi", ] +[[package]] +name = "hashbrown" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34f595585f103464d8d2f6e9864682d74c1601fed5e07d62b1c9058dba8246fb" +dependencies = [ + "autocfg", +] + [[package]] name = "heck" version = "0.3.1" @@ -376,11 +385,12 @@ dependencies = [ [[package]] name = "indexmap" -version = "1.4.0" +version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c398b2b113b55809ceb9ee3e753fcbac793f1956663f3c36549c1346015c2afe" +checksum = "5b88cd59ee5f71fea89a62248fc8f387d44400cefe05ef548466d61ced9029a7" dependencies = [ "autocfg", + "hashbrown", ] [[package]] @@ -436,9 +446,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" -version = "0.2.72" +version = "0.2.73" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9f8082297d534141b30c8d39e9b1773713ab50fdbe4ff30f750d063b3bfd701" +checksum = "bd7d4bd64732af4bf3a67f367c27df8520ad7e230c5817b8ff485864d80242b9" [[package]] name = "lock_api" @@ -639,9 +649,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.18" +version = "1.0.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "beae6331a816b1f65d04c45b078fd8e6c93e8071771f41b8163255bbd8d7c8fa" +checksum = "04f5f085b5d71e2188cb8271e5da0161ad52c3f227a661a3c135fdf28e258b12" dependencies = [ "unicode-xid", ] @@ -925,9 +935,9 @@ dependencies = [ [[package]] name = "syn" -version = "1.0.34" +version = "1.0.35" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "936cae2873c940d92e697597c5eee105fb570cd5689c695806f672883653349b" +checksum = "fb7f4c519df8c117855e19dd8cc851e89eb746fe7a73f0157e0d95fdec5369b0" dependencies = [ "proc-macro2", "quote", diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 6654ec61fb..19650b84ea 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -16,7 +16,7 @@ use super::function::{make_builtin_fn, make_constructor_fn}; use crate::{ builtins::{ object::{ObjectData, PROTOTYPE}, - property::Property, + property::{Attribute, Property}, value::{same_value_zero, ResultValue, Value}, }, exec::Interpreter, @@ -75,12 +75,10 @@ impl Array { } // Create length - let length = Property::new() - .value(Value::from(array_contents.len() as i32)) - .writable(true) - .configurable(false) - .enumerable(false); - + let length = Property::data_descriptor( + array_contents.len().into(), + Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, + ); array_obj_ptr.set_property("length".to_string(), length); for (n, value) in array_contents.iter().enumerate() { @@ -143,11 +141,10 @@ impl Array { } // finally create length property - let length = Property::new() - .value(Value::from(length)) - .writable(true) - .configurable(false) - .enumerable(false); + let length = Property::data_descriptor( + length.into(), + Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, + ); this.set_property("length".to_string(), length); diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index f97e9c480b..d2d53395de 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -13,10 +13,10 @@ use crate::{ builtins::{ - array::Array, object::{Object, ObjectData, PROTOTYPE}, - property::Property, + property::{Attribute, Property}, value::{RcString, ResultValue, Value}, + Array, }, environment::function_environment_record::BindingStatus, environment::lexical_environment::{new_function_environment, Environment}, @@ -411,19 +411,19 @@ pub fn create_unmapped_arguments_object(arguments_list: &[Value]) -> Value { let mut obj = Object::default(); obj.set_internal_slot("ParameterMap", Value::undefined()); // Set length - let mut length = Property::default(); - length = length.writable(true).value(Value::from(len)); + let length = Property::data_descriptor( + len.into(), + Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, + ); // Define length as a property obj.define_own_property("length".to_string(), length); let mut index: usize = 0; while index < len { let val = arguments_list.get(index).expect("Could not get argument"); - let mut prop = Property::default(); - prop = prop - .value(val.clone()) - .enumerable(true) - .writable(true) - .configurable(true); + let prop = Property::data_descriptor( + val.clone(), + Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE, + ); obj.properties_mut() .insert(RcString::from(index.to_string()), prop); @@ -469,18 +469,16 @@ pub fn make_constructor_fn( let mut constructor = Object::function(function, global.get_field("Function").get_field(PROTOTYPE)); - let length = Property::new() - .value(Value::from(length)) - .writable(false) - .configurable(false) - .enumerable(false); + let length = Property::data_descriptor( + length.into(), + Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, + ); constructor.insert_property("length", length); - let name = Property::new() - .value(Value::from(name)) - .writable(false) - .configurable(false) - .enumerable(false); + let name = Property::data_descriptor( + name.into(), + Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, + ); constructor.insert_property("name", name); let constructor = Value::from(constructor); diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 75fb8d8654..cad09ee98e 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -4,7 +4,7 @@ use super::function::{make_builtin_fn, make_constructor_fn}; use crate::{ builtins::{ object::{ObjectData, PROTOTYPE}, - property::Property, + property::{Attribute, Property}, value::{ResultValue, Value}, }, exec::Interpreter, @@ -26,11 +26,10 @@ impl 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); + let size = Property::data_descriptor( + size.into(), + Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, + ); this.set_property("size".to_string(), size); } diff --git a/boa/src/builtins/object/internal_methods.rs b/boa/src/builtins/object/internal_methods.rs index bbd51bcf6c..685c4d0c76 100644 --- a/boa/src/builtins/object/internal_methods.rs +++ b/boa/src/builtins/object/internal_methods.rs @@ -7,7 +7,7 @@ use crate::builtins::{ object::{Object, PROTOTYPE}, - property::Property, + property::{Attribute, Property}, value::{same_value, RcString, Value}, }; use crate::BoaProfiler; @@ -73,7 +73,7 @@ impl Object { { return true; } - if desc.configurable.expect("unable to get value") { + if desc.configurable_or(false) { self.remove_property(&prop_key.to_string()); return true; } @@ -131,14 +131,14 @@ impl Object { if !parent.is_null() { // TODO: come back to this } - own_desc = Property::new() - .writable(true) - .enumerable(true) - .configurable(true); + own_desc = Property::data_descriptor( + Value::undefined(), + Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE, + ); } // [3] if own_desc.is_data_descriptor() { - if !own_desc.writable.unwrap() { + if !own_desc.writable() { return false; } @@ -184,18 +184,12 @@ impl Object { } // 4 - if !current.configurable.unwrap_or(false) { - if desc.configurable.is_some() && desc.configurable.expect("unable to get prop desc") { + if !current.configurable_or(false) { + if desc.configurable_or(false) { return false; } - if desc.enumerable.is_some() - && (desc.enumerable.as_ref().expect("unable to get prop desc") - != current - .enumerable - .as_ref() - .expect("unable to get prop desc")) - { + if desc.enumerable_or(false) != current.enumerable_or(false) { return false; } } @@ -205,14 +199,14 @@ impl Object { // 6 } else if current.is_data_descriptor() != desc.is_data_descriptor() { // a - if !current.configurable.expect("unable to get prop desc") { + if !current.configurable() { return false; } // b if current.is_data_descriptor() { // Convert to accessor current.value = None; - current.writable = None; + current.attribute.remove(Attribute::WRITABLE); } else { // c // convert to data @@ -224,10 +218,8 @@ impl Object { // 7 } else if current.is_data_descriptor() && desc.is_data_descriptor() { // a - if !current.configurable.expect("unable to get prop desc") - && !current.writable.expect("unable to get prop desc") - { - if desc.writable.is_some() && desc.writable.expect("unable to get prop desc") { + if !current.configurable() && !current.writable() { + if desc.writable_or(false) { return false; } @@ -244,7 +236,7 @@ impl Object { } // 8 } else { - if !current.configurable.unwrap() { + if !current.configurable() { if desc.set.is_some() && !same_value(&desc.set.clone().unwrap(), ¤t.set.clone().unwrap()) { @@ -279,43 +271,35 @@ impl Object { debug_assert!(Property::is_property_key(prop)); // Prop could either be a String or Symbol match *prop { - Value::String(ref st) => { - self.properties() - .get(st) - .map_or_else(Property::default, |v| { - let mut d = Property::default(); - if v.is_data_descriptor() { - d.value = v.value.clone(); - d.writable = v.writable; - } else { - debug_assert!(v.is_accessor_descriptor()); - d.get = v.get.clone(); - d.set = v.set.clone(); - } - d.enumerable = v.enumerable; - d.configurable = v.configurable; - d - }) - } + Value::String(ref st) => self.properties().get(st).map_or_else(Property::empty, |v| { + let mut d = Property::empty(); + if v.is_data_descriptor() { + d.value = v.value.clone(); + } else { + debug_assert!(v.is_accessor_descriptor()); + d.get = v.get.clone(); + d.set = v.set.clone(); + } + d.attribute = v.attribute; + d + }), Value::Symbol(ref symbol) => { self.symbol_properties() .get(&symbol.hash()) - .map_or_else(Property::default, |v| { - let mut d = Property::default(); + .map_or_else(Property::empty, |v| { + let mut d = Property::empty(); if v.is_data_descriptor() { d.value = v.value.clone(); - d.writable = v.writable; } else { debug_assert!(v.is_accessor_descriptor()); d.get = v.get.clone(); d.set = v.set.clone(); } - d.enumerable = v.enumerable; - d.configurable = v.configurable; + d.attribute = v.attribute; d }) } - _ => Property::default(), + _ => unreachable!("the field can only be of type string or symbol"), } } @@ -409,11 +393,10 @@ impl Object { { self.properties.insert( name.into(), - Property::default() - .value(value) - .writable(true) - .configurable(true) - .enumerable(true), + Property::data_descriptor( + value, + Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE, + ), ) } diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index 40a5cc98a8..ea614a6666 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -571,7 +571,7 @@ pub fn property_is_enumerable(this: &Value, args: &[Value], ctx: &mut Interprete }); Ok(own_property.map_or(Value::from(false), |own_prop| { - Value::from(own_prop.enumerable.unwrap_or(false)) + Value::from(own_prop.enumerable_or(false)) })) } diff --git a/boa/src/builtins/property/attribute/mod.rs b/boa/src/builtins/property/attribute/mod.rs new file mode 100644 index 0000000000..e909a38ad7 --- /dev/null +++ b/boa/src/builtins/property/attribute/mod.rs @@ -0,0 +1,145 @@ +//! This module implements the `Attribute` struct which contains the attibutes for property descriptors. + +use bitflags::bitflags; +use gc::{unsafe_empty_trace, Finalize, Trace}; + +#[cfg(test)] +mod tests; + +bitflags! { + /// This struct constains the property flags as describen in the ECMAScript specification. + /// + /// It contains the following flags: + /// - `[[Writable]]` (`WRITABLE`) - If `false`, attempts by ECMAScript code to change the property's `[[Value]]` attribute using `[[Set]]` will not succeed. + /// - `[[Enumerable]]` (`ENUMERABLE`) - If the property will be enumerated by a for-in enumeration. + /// - `[[Configurable]]` (`CONFIGURABLE`) - If `false`, attempts to delete the property, change the property to be an `accessor property`, or change its attributes (other than `[[Value]]`, or changing `[[Writable]]` to `false`) will fail. + /// + /// Additionaly there are flags for when the flags are defined. + #[derive(Finalize)] + pub struct Attribute: u8 { + /// None of the flags are present. + const NONE = 0b0000_0000; + + /// The `Writable` attribute decides whether the value associated with the property can be changed or not, from its initial value. + const WRITABLE = 0b0000_0011; + + /// The property is not writable. + const READONLY = 0b0000_0010; + + /// Is the `Writable` flag defined. + const HAS_WRITABLE = 0b0000_0010; + + /// If the property can be enumerated by a `for-in` loop. + const ENUMERABLE = 0b0000_1100; + + /// The property can not be enumerated in a `for-in` loop. + const NON_ENUMERABLE = 0b0000_1000; + + /// Is the `Enumerable` flag defined. + const HAS_ENUMERABLE = 0b0000_1000; + + /// If the property descriptor can be changed later. + const CONFIGURABLE = 0b0011_0000; + + /// The property descriptor cannot be changed. + const PERMANENT = 0b0010_0000; + + /// Is the `Configurable` flag defined. + const HAS_CONFIGURABLE = 0b0010_0000; + } +} + +// We implement `Trace` manualy rather that wih derive, beacuse `rust-gc`, +// derive `Trace` does not allow `Copy` and `Trace` to be both implemented. +// +// SAFETY: The `Attribute` struct only contains an `u8` +// and therefore it should be safe to implement an empty trace. +unsafe impl Trace for Attribute { + unsafe_empty_trace!(); +} + +impl Attribute { + /// Clear all flags. + #[inline] + pub fn clear(&mut self) { + self.bits = 0; + } + + /// Is the `writable` flag defined. + #[inline] + pub fn has_writable(self) -> bool { + self.contains(Self::HAS_WRITABLE) + } + + /// Sets the `writable` flag. + #[inline] + pub fn set_writable(&mut self, value: bool) { + if value { + *self |= Self::WRITABLE; + } else { + *self |= *self & !Self::WRITABLE; + } + } + + /// Gets the `writable` flag. + #[inline] + pub fn writable(self) -> bool { + debug_assert!(self.has_writable()); + self.contains(Self::WRITABLE) + } + + /// Is the `enumerable` flag defined. + #[inline] + pub fn has_enumerable(self) -> bool { + self.contains(Self::HAS_ENUMERABLE) + } + + /// Sets the `enumerable` flag. + #[inline] + pub fn set_enumerable(&mut self, value: bool) { + if value { + *self |= Self::ENUMERABLE; + } else { + *self |= *self & !Self::ENUMERABLE; + } + } + + /// Gets the `enumerable` flag. + #[inline] + pub fn enumerable(self) -> bool { + debug_assert!(self.has_enumerable()); + self.contains(Self::ENUMERABLE) + } + + /// Is the `configurable` flag defined. + #[inline] + pub fn has_configurable(self) -> bool { + self.contains(Self::HAS_CONFIGURABLE) + } + + /// Sets the `configurable` flag. + #[inline] + pub fn set_configurable(&mut self, value: bool) { + if value { + *self |= Self::CONFIGURABLE; + } else { + *self |= *self & !Self::CONFIGURABLE; + } + } + + /// Gets the `configurable` flag. + #[inline] + pub fn configurable(self) -> bool { + debug_assert!(self.has_configurable()); + self.contains(Self::CONFIGURABLE) + } +} + +impl Default for Attribute { + /// Returns the default flags according to the [ECMAScript specification][spec]. + /// + /// [spec]: https://tc39.es/ecma262/#table-default-attribute-values + fn default() -> Self { + Self::READONLY | Self::NON_ENUMERABLE | Self::PERMANENT + } +} diff --git a/boa/src/builtins/property/attribute/tests.rs b/boa/src/builtins/property/attribute/tests.rs new file mode 100644 index 0000000000..e9ced87917 --- /dev/null +++ b/boa/src/builtins/property/attribute/tests.rs @@ -0,0 +1,167 @@ +use super::Attribute; + +#[test] +fn writable() { + let attribute = Attribute::WRITABLE; + + assert!(attribute.has_writable()); + assert!(attribute.writable()); +} + +#[test] +fn enumerable() { + let attribute = Attribute::ENUMERABLE; + + assert!(attribute.has_enumerable()); + assert!(attribute.enumerable()); +} + +#[test] +fn configurable() { + let attribute = Attribute::CONFIGURABLE; + + assert!(attribute.has_configurable()); + assert!(attribute.configurable()); +} + +#[test] +fn writable_and_enumerable() { + let attribute = Attribute::WRITABLE | Attribute::ENUMERABLE; + + assert!(attribute.has_writable()); + assert!(attribute.writable()); + assert!(attribute.has_enumerable()); + assert!(attribute.enumerable()); + + assert!(!attribute.has_configurable()); +} + +#[test] +fn enumerable_configurable() { + let attribute = Attribute::ENUMERABLE | Attribute::CONFIGURABLE; + + assert!(!attribute.has_writable()); + + assert!(attribute.has_enumerable()); + assert!(attribute.enumerable()); + assert!(attribute.has_configurable()); + assert!(attribute.configurable()); +} + +#[test] +fn writable_enumerable_configurable() { + let attribute = Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE; + + assert!(attribute.has_writable()); + assert!(attribute.writable()); + assert!(attribute.has_enumerable()); + assert!(attribute.enumerable()); + assert!(attribute.has_configurable()); + assert!(attribute.configurable()); +} + +#[test] +fn default() { + let attribute = Attribute::default(); + + assert!(attribute.has_writable()); + assert!(attribute.has_enumerable()); + assert!(attribute.has_configurable()); +} + +#[test] +fn clear() { + let mut attribute = Attribute::default(); + + attribute.clear(); + + assert!(!attribute.has_writable()); + assert!(!attribute.has_enumerable()); + assert!(!attribute.has_configurable()); + + assert!(attribute.is_empty()); +} + +#[test] +fn set_writable_to_true() { + let mut attribute = Attribute::default(); + + attribute.set_writable(true); + + assert!(attribute.has_writable()); + assert!(attribute.writable()); + assert!(attribute.has_enumerable()); + assert!(!attribute.enumerable()); + assert!(attribute.has_configurable()); + assert!(!attribute.configurable()); +} + +#[test] +fn set_writable_to_false() { + let mut attribute = Attribute::default(); + + attribute.set_writable(false); + + assert!(attribute.has_writable()); + assert!(!attribute.writable()); + assert!(attribute.has_enumerable()); + assert!(!attribute.enumerable()); + assert!(attribute.has_configurable()); + assert!(!attribute.configurable()); +} + +#[test] +fn set_enumerable_to_true() { + let mut attribute = Attribute::default(); + + attribute.set_enumerable(true); + + assert!(attribute.has_writable()); + assert!(!attribute.writable()); + assert!(attribute.has_enumerable()); + assert!(attribute.enumerable()); + assert!(attribute.has_configurable()); + assert!(!attribute.configurable()); +} + +#[test] +fn set_enumerable_to_false() { + let mut attribute = Attribute::default(); + + attribute.set_enumerable(false); + + assert!(attribute.has_writable()); + assert!(!attribute.writable()); + assert!(attribute.has_enumerable()); + assert!(!attribute.enumerable()); + assert!(attribute.has_configurable()); + assert!(!attribute.configurable()); +} + +#[test] +fn set_configurable_to_true() { + let mut attribute = Attribute::default(); + + attribute.set_configurable(true); + + assert!(attribute.has_writable()); + assert!(!attribute.writable()); + assert!(attribute.has_enumerable()); + assert!(!attribute.enumerable()); + assert!(attribute.has_configurable()); + assert!(attribute.configurable()); +} + +#[test] +fn set_configurable_to_false() { + let mut attribute = Attribute::default(); + + attribute.set_configurable(false); + + assert!(attribute.has_writable()); + assert!(!attribute.writable()); + assert!(attribute.has_enumerable()); + assert!(!attribute.enumerable()); + assert!(attribute.has_configurable()); + assert!(!attribute.configurable()); +} diff --git a/boa/src/builtins/property/mod.rs b/boa/src/builtins/property/mod.rs index 5496e6256c..cfe05472a5 100644 --- a/boa/src/builtins/property/mod.rs +++ b/boa/src/builtins/property/mod.rs @@ -14,9 +14,15 @@ //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty //! [section]: https://tc39.es/ecma262/#sec-property-attributes -use crate::builtins::value::Value; +use crate::builtins::Value; use gc::{Finalize, Trace}; +pub mod attribute; +pub use attribute::Attribute; + +#[cfg(test)] +mod tests; + /// This represents a Javascript Property AKA The Property Descriptor. /// /// Property descriptors present in objects come in two main flavors: @@ -37,12 +43,7 @@ use gc::{Finalize, Trace}; /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty #[derive(Trace, Finalize, Clone, Debug)] pub struct Property { - /// If the type of this can be changed and this can be deleted - pub configurable: Option, - /// If the property shows up in enumeration of the object - pub enumerable: Option, - /// If this property can be changed with an assignment - pub writable: Option, + pub(crate) attribute: Attribute, /// The value associated with the property pub value: Option, /// The function serving as getter @@ -53,8 +54,9 @@ pub struct Property { impl Property { /// Checks if the provided Value can be used as a property key. + #[inline] pub fn is_property_key(value: &Value) -> bool { - value.is_string() || value.is_symbol() // Uncomment this when we are handeling symbols. + value.is_string() || value.is_symbol() } /// Make a new property with the given value @@ -62,62 +64,126 @@ impl Property { /// /// New: zeros everything to make an empty object /// Default: Defaults according to the spec + #[inline] pub fn new() -> Self { Self { - configurable: None, - enumerable: None, - writable: None, + attribute: Default::default(), value: None, get: None, set: None, } } - /// Set configurable - pub fn configurable(mut self, configurable: bool) -> Self { - self.configurable = Some(configurable); - self + #[inline] + pub fn empty() -> Self { + Self { + attribute: Attribute::NONE, + value: None, + get: None, + set: None, + } + } + + #[inline] + pub fn data_descriptor(value: Value, attribute: Attribute) -> Self { + Self { + attribute, + value: Some(value), + get: None, + set: None, + } + } + + /// Get the + #[inline] + pub fn configurable(&self) -> bool { + self.attribute.configurable() + } + + #[inline] + pub fn set_configurable(&mut self, configurable: bool) { + self.attribute.set_configurable(configurable) + } + + #[inline] + pub fn configurable_or(&self, value: bool) -> bool { + if self.attribute.has_configurable() { + self.attribute.configurable() + } else { + value + } } /// Set enumerable - pub fn enumerable(mut self, enumerable: bool) -> Self { - self.enumerable = Some(enumerable); - self + #[inline] + pub fn enumerable(&self) -> bool { + self.attribute.enumerable() + } + + #[inline] + pub fn enumerable_or(&self, value: bool) -> bool { + if self.attribute.has_enumerable() { + self.attribute.enumerable() + } else { + value + } } /// Set writable - pub fn writable(mut self, writable: bool) -> Self { - self.writable = Some(writable); - self + #[inline] + pub fn writable(&self) -> bool { + self.attribute.writable() + } + + #[inline] + pub fn writable_or(&self, value: bool) -> bool { + if self.attribute.has_writable() { + self.attribute.writable() + } else { + value + } } /// Set value + #[inline] pub fn value(mut self, value: Value) -> Self { self.value = Some(value); self } /// Set get + #[inline] pub fn get(mut self, get: Value) -> Self { self.get = Some(get); self } + #[inline] + pub fn has_get(&self) -> bool { + self.get.is_some() + } + /// Set set + #[inline] pub fn set(mut self, set: Value) -> Self { self.set = Some(set); self } + #[inline] + pub fn has_set(&self) -> bool { + self.set.is_some() + } + /// Is this an empty Property? /// /// `true` if all fields are set to none + #[inline] pub fn is_none(&self) -> bool { - self.get.is_none() + self.value.is_none() + && self.attribute.is_empty() + && self.get.is_none() && self.set.is_none() - && self.writable.is_none() - && self.configurable.is_none() - && self.enumerable.is_none() } /// An accessor Property Descriptor is one that includes any fields named either [[Get]] or [[Set]]. @@ -126,6 +192,7 @@ impl Property { /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-isaccessordescriptor + #[inline] pub fn is_accessor_descriptor(&self) -> bool { self.get.is_some() || self.set.is_some() } @@ -136,16 +203,18 @@ impl Property { /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-isdatadescriptor + #[inline] pub fn is_data_descriptor(&self) -> bool { - self.value.is_some() || self.writable.is_some() + self.value.is_some() || self.attribute.has_writable() } - /// Check if a property is generic. + /// Check if a property is generic descriptor. /// /// More information: /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-isgenericdescriptor + #[inline] pub fn is_generic_descriptor(&self) -> bool { !self.is_accessor_descriptor() && !self.is_data_descriptor() } @@ -158,24 +227,27 @@ impl Default for Property { /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#table-default-attribute-values + #[inline] fn default() -> Self { - Self { - configurable: None, - enumerable: None, - writable: None, - value: None, - get: None, - set: None, - } + Self::new() } } impl From<&Property> for Value { fn from(value: &Property) -> Value { let property = Value::new_object(None); - property.set_field("configurable", Value::from(value.configurable)); - property.set_field("enumerable", Value::from(value.enumerable)); - property.set_field("writable", Value::from(value.writable)); + if value.attribute.has_writable() { + property.set_field("writable", value.attribute.writable()); + } + + if value.attribute.has_enumerable() { + property.set_field("enumerable", value.attribute.enumerable()); + } + + if value.attribute.has_configurable() { + property.set_field("configurable", value.attribute.configurable()); + } + property.set_field("value", value.value.clone().unwrap_or_else(Value::null)); property.set_field("get", value.get.clone().unwrap_or_else(Value::null)); property.set_field("set", value.set.clone().unwrap_or_else(Value::null)); @@ -187,16 +259,28 @@ impl<'a> From<&'a Value> for Property { /// Attempt to fetch values "configurable", "enumerable", "writable" from the value, /// if they're not there default to false fn from(value: &Value) -> Self { + let mut attribute = Attribute::empty(); + + let writable = value.get_field("writable"); + if !writable.is_undefined() { + attribute.set_writable(bool::from(&writable)); + } + + let enumerable = value.get_field("enumerable"); + if !enumerable.is_undefined() { + attribute.set_enumerable(bool::from(&enumerable)); + } + + let configurable = value.get_field("configurable"); + if !configurable.is_undefined() { + attribute.set_configurable(bool::from(&configurable)); + } + Self { - configurable: { Some(bool::from(&value.get_field("configurable"))) }, - enumerable: { Some(bool::from(&value.get_field("enumerable"))) }, - writable: { Some(bool::from(&value.get_field("writable"))) }, + attribute, value: Some(value.get_field("value")), get: Some(value.get_field("get")), set: Some(value.get_field("set")), } } } - -#[cfg(test)] -mod tests; diff --git a/boa/src/builtins/value/mod.rs b/boa/src/builtins/value/mod.rs index 2e398ceffa..61520f3dc0 100644 --- a/boa/src/builtins/value/mod.rs +++ b/boa/src/builtins/value/mod.rs @@ -12,7 +12,7 @@ pub use crate::builtins::value::val_type::Type; use crate::builtins::{ function::Function, object::{GcObject, InternalState, InternalStateCell, Object, ObjectData, PROTOTYPE}, - property::Property, + property::{Attribute, Property}, BigInt, Symbol, }; use crate::exec::Interpreter; @@ -201,11 +201,10 @@ impl Value { for (idx, json) in vs.into_iter().enumerate() { new_obj.set_property( idx.to_string(), - Property::default() - .value(Self::from_json(json, interpreter)) - .writable(true) - .configurable(true) - .enumerable(true), + Property::data_descriptor( + Self::from_json(json, interpreter), + Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE, + ), ); } new_obj.set_property( @@ -220,11 +219,10 @@ impl Value { let value = Self::from_json(json, interpreter); new_obj.set_property( key, - Property::default() - .value(value) - .writable(true) - .configurable(true) - .enumerable(true), + Property::data_descriptor( + value, + Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE, + ), ); } new_obj @@ -495,24 +493,15 @@ impl Value { /// update_prop will overwrite individual [Property] fields, unlike /// Set_prop, which will overwrite prop with a new Property + /// /// Mostly used internally for now - pub fn update_property( - &self, - field: &str, - value: Option, - enumerable: Option, - writable: Option, - configurable: Option, - ) { + pub(crate) fn update_property(&self, field: &str, new_property: Property) { let _timer = BoaProfiler::global().start_event("Value::update_property", "value"); if let Some(ref mut object) = self.as_object_mut() { // Use value, or walk up the prototype chain - if let Some(ref mut property) = object.properties_mut().get_mut(field) { - property.value = value; - property.enumerable = enumerable; - property.writable = writable; - property.configurable = configurable; + if let Some(property) = object.properties_mut().get_mut(field) { + *property = new_property; } } } diff --git a/boa/src/environment/global_environment_record.rs b/boa/src/environment/global_environment_record.rs index b04ec34d48..312bb670ae 100644 --- a/boa/src/environment/global_environment_record.rs +++ b/boa/src/environment/global_environment_record.rs @@ -8,7 +8,10 @@ //! More info: use crate::{ - builtins::value::Value, + builtins::{ + property::{Attribute, Property}, + Value, + }, environment::{ declarative_environment_record::DeclarativeEnvironmentRecord, environment_record_trait::EnvironmentRecordTrait, @@ -41,7 +44,7 @@ impl GlobalEnvironmentRecord { let existing_prop = global_object.get_property(name); match existing_prop { Some(prop) => { - if prop.value.is_none() || prop.configurable.unwrap_or(false) { + if prop.value.is_none() || prop.configurable() { return false; } true @@ -70,23 +73,19 @@ impl GlobalEnvironmentRecord { let global_object = &mut self.object_record.bindings; let existing_prop = global_object.get_property(&name); if let Some(prop) = existing_prop { - if prop.value.is_none() || prop.configurable.unwrap_or(false) { - global_object.update_property( - name, - Some(value), - Some(true), - Some(true), - Some(deletion), - ); + if prop.value.is_none() || prop.configurable_or(false) { + let mut property = + Property::data_descriptor(value, Attribute::WRITABLE | Attribute::ENUMERABLE); + property.set_configurable(deletion); + + global_object.update_property(name, property); } } else { - global_object.update_property( - name, - Some(value), - Some(true), - Some(true), - Some(deletion), - ); + let mut property = + Property::data_descriptor(value, Attribute::WRITABLE | Attribute::ENUMERABLE); + property.set_configurable(deletion); + + global_object.update_property(name, property); } } } diff --git a/boa/src/environment/object_environment_record.rs b/boa/src/environment/object_environment_record.rs index aada300d7a..e00f5044db 100644 --- a/boa/src/environment/object_environment_record.rs +++ b/boa/src/environment/object_environment_record.rs @@ -7,7 +7,10 @@ //! More info: [Object Records](https://tc39.es/ecma262/#sec-object-environment-records) use crate::{ - builtins::{property::Property, value::Value}, + builtins::{ + property::{Attribute, Property}, + value::Value, + }, environment::{ environment_record_trait::EnvironmentRecordTrait, lexical_environment::{Environment, EnvironmentType}, @@ -38,11 +41,11 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { // TODO: could save time here and not bother generating a new undefined object, // only for it to be replace with the real value later. We could just add the name to a Vector instead let bindings = &mut self.bindings; - let prop = Property::default() - .value(Value::undefined()) - .writable(true) - .enumerable(true) - .configurable(deletion); + let mut prop = Property::data_descriptor( + Value::undefined(), + Attribute::WRITABLE | Attribute::ENUMERABLE, + ); + prop.set_configurable(deletion); bindings.set_property(name, prop); } @@ -62,8 +65,9 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { fn set_mutable_binding(&mut self, name: &str, value: Value, strict: bool) { debug_assert!(value.is_object() || value.is_function()); - let bindings = &mut self.bindings; - bindings.update_property(name, Some(value), None, None, Some(strict)); + let mut property = Property::data_descriptor(value, Attribute::ENUMERABLE); + property.set_configurable(strict); + self.bindings.update_property(name, property); } fn get_binding_value(&self, name: &str, strict: bool) -> Value {