From cb67634c40ee4615284c476d7b386bd83da5f96d Mon Sep 17 00:00:00 2001 From: HalidOdat Date: Tue, 7 Jul 2020 23:57:17 +0200 Subject: [PATCH 1/3] Made configurable and enumerable from Option to bool --- boa/src/builtins/object/internal_methods.rs | 22 ++++-------- boa/src/builtins/object/mod.rs | 2 +- boa/src/builtins/property/mod.rs | 34 ++++++++----------- boa/src/builtins/value/mod.rs | 4 +-- .../environment/global_environment_record.rs | 20 +++-------- .../environment/object_environment_record.rs | 2 +- 6 files changed, 30 insertions(+), 54 deletions(-) diff --git a/boa/src/builtins/object/internal_methods.rs b/boa/src/builtins/object/internal_methods.rs index 7b79c0c0544..1dcb5e87721 100644 --- a/boa/src/builtins/object/internal_methods.rs +++ b/boa/src/builtins/object/internal_methods.rs @@ -73,7 +73,7 @@ impl Object { { return true; } - if desc.configurable.expect("unable to get value") { + if desc.configurable { self.remove_property(&prop_key.to_string()); return true; } @@ -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 { + if desc.configurable { 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 != current.enumerable { return false; } } @@ -205,7 +199,7 @@ 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 @@ -224,9 +218,7 @@ 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 !current.configurable && !current.writable.expect("unable to get prop desc") { if desc.writable.is_some() && desc.writable.expect("unable to get prop desc") { 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()) { diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index 45cfbaa65c5..1e1d4067efe 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -528,7 +528,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) })) } diff --git a/boa/src/builtins/property/mod.rs b/boa/src/builtins/property/mod.rs index 5496e6256c2..855eccc6a31 100644 --- a/boa/src/builtins/property/mod.rs +++ b/boa/src/builtins/property/mod.rs @@ -17,6 +17,9 @@ use crate::builtins::value::Value; use gc::{Finalize, Trace}; +#[cfg(test)] +mod tests; + /// This represents a Javascript Property AKA The Property Descriptor. /// /// Property descriptors present in objects come in two main flavors: @@ -38,9 +41,9 @@ use gc::{Finalize, Trace}; #[derive(Trace, Finalize, Clone, Debug)] pub struct Property { /// If the type of this can be changed and this can be deleted - pub configurable: Option, + pub configurable: bool, /// If the property shows up in enumeration of the object - pub enumerable: Option, + pub enumerable: bool, /// If this property can be changed with an assignment pub writable: Option, /// The value associated with the property @@ -64,8 +67,8 @@ impl Property { /// Default: Defaults according to the spec pub fn new() -> Self { Self { - configurable: None, - enumerable: None, + configurable: false, + enumerable: false, writable: None, value: None, get: None, @@ -75,13 +78,13 @@ impl Property { /// Set configurable pub fn configurable(mut self, configurable: bool) -> Self { - self.configurable = Some(configurable); + self.configurable = configurable; self } /// Set enumerable pub fn enumerable(mut self, enumerable: bool) -> Self { - self.enumerable = Some(enumerable); + self.enumerable = enumerable; self } @@ -113,11 +116,7 @@ impl Property { /// /// `true` if all fields are set to none pub fn is_none(&self) -> bool { - self.get.is_none() - && self.set.is_none() - && self.writable.is_none() - && self.configurable.is_none() - && self.enumerable.is_none() + self.get.is_none() && self.set.is_none() && self.writable.is_none() } /// An accessor Property Descriptor is one that includes any fields named either [[Get]] or [[Set]]. @@ -160,8 +159,8 @@ impl Default for Property { /// [spec]: https://tc39.es/ecma262/#table-default-attribute-values fn default() -> Self { Self { - configurable: None, - enumerable: None, + configurable: false, + enumerable: false, writable: None, value: None, get: None, @@ -188,15 +187,12 @@ impl<'a> From<&'a Value> for Property { /// if they're not there default to false fn from(value: &Value) -> Self { 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"))) }, + configurable: bool::from(&value.get_field("configurable")), + enumerable: bool::from(&value.get_field("enumerable")), + writable: Some(bool::from(&value.get_field("writable"))), 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 8d5447dfe15..12f817b6d43 100644 --- a/boa/src/builtins/value/mod.rs +++ b/boa/src/builtins/value/mod.rs @@ -510,9 +510,9 @@ impl Value { &self, field: &str, value: Option, - enumerable: Option, + enumerable: bool, writable: Option, - configurable: Option, + configurable: bool, ) { let _timer = BoaProfiler::global().start_event("Value::update_property", "value"); diff --git a/boa/src/environment/global_environment_record.rs b/boa/src/environment/global_environment_record.rs index b04ec34d482..00068692e15 100644 --- a/boa/src/environment/global_environment_record.rs +++ b/boa/src/environment/global_environment_record.rs @@ -41,7 +41,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 +70,11 @@ 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 { + global_object.update_property(name, Some(value), true, Some(true), deletion); } } else { - global_object.update_property( - name, - Some(value), - Some(true), - Some(true), - Some(deletion), - ); + global_object.update_property(name, Some(value), true, Some(true), deletion); } } } diff --git a/boa/src/environment/object_environment_record.rs b/boa/src/environment/object_environment_record.rs index aada300d7a7..85016f9498b 100644 --- a/boa/src/environment/object_environment_record.rs +++ b/boa/src/environment/object_environment_record.rs @@ -63,7 +63,7 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { debug_assert!(value.is_object() || value.is_function()); let bindings = &mut self.bindings; - bindings.update_property(name, Some(value), None, None, Some(strict)); + bindings.update_property(name, Some(value), false, None, strict); } fn get_binding_value(&self, name: &str, strict: bool) -> Value { From bc0ead16e1a6c4124ded110e21fa86bf17c70396 Mon Sep 17 00:00:00 2001 From: HalidOdat Date: Wed, 8 Jul 2020 23:22:09 +0200 Subject: [PATCH 2/3] Added `Attribute` bitflag structure. --- boa/src/builtins/property/attribute/mod.rs | 145 ++++++++++++++++ boa/src/builtins/property/attribute/tests.rs | 168 +++++++++++++++++++ boa/src/builtins/property/mod.rs | 5 +- 3 files changed, 317 insertions(+), 1 deletion(-) create mode 100644 boa/src/builtins/property/attribute/mod.rs create mode 100644 boa/src/builtins/property/attribute/tests.rs diff --git a/boa/src/builtins/property/attribute/mod.rs b/boa/src/builtins/property/attribute/mod.rs new file mode 100644 index 00000000000..15404cd37bc --- /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`. + +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`][spec]. + /// + /// 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`. + 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 00000000000..0426a4337d5 --- /dev/null +++ b/boa/src/builtins/property/attribute/tests.rs @@ -0,0 +1,168 @@ + +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 855eccc6a31..2cb07213022 100644 --- a/boa/src/builtins/property/mod.rs +++ b/boa/src/builtins/property/mod.rs @@ -17,6 +17,9 @@ use crate::builtins::value::Value; use gc::{Finalize, Trace}; +pub mod attribute; +pub use attribute::Attribute; + #[cfg(test)] mod tests; @@ -57,7 +60,7 @@ pub struct Property { impl Property { /// Checks if the provided Value can be used as a property key. 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 From 4a11560383185261ec46e0c138b9c887c70e0f8b Mon Sep 17 00:00:00 2001 From: HalidOdat Date: Wed, 15 Jul 2020 22:39:05 +0200 Subject: [PATCH 3/3] Added Attribute to Property --- .vscode/launch.json | 8 +- boa/src/builtins/array/mod.rs | 21 +-- boa/src/builtins/function/mod.rs | 36 ++-- boa/src/builtins/object/internal_methods.rs | 79 ++++----- boa/src/builtins/object/mod.rs | 2 +- boa/src/builtins/property/attribute/mod.rs | 6 +- boa/src/builtins/property/attribute/tests.rs | 1 - boa/src/builtins/property/mod.rs | 159 ++++++++++++++---- boa/src/builtins/value/mod.rs | 37 ++-- boa/src/builtins/value/tests.rs | 1 + .../environment/global_environment_record.rs | 21 ++- .../environment/object_environment_record.rs | 20 ++- 12 files changed, 232 insertions(+), 159 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 99f24933278..cf24fef326e 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -11,10 +11,8 @@ "windows": { "program": "${workspaceFolder}/target/debug/boa.exe" }, - "program": "${workspaceFolder}/target/debug/boa", - "args": [ - "${workspaceFolder}/tests/js/test.js" - ], + "program": "target/debug/deps/boa-0e316821f44800c8", + "args": [], "sourceLanguages": [ "rust" ] @@ -47,4 +45,4 @@ "externalConsole": true }, ] -} \ No newline at end of file +} diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index bfe76015d3d..33d48a3bf3c 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, INSTANCE_PROTOTYPE, PROTOTYPE}, - property::Property, + property::{Attribute, Property}, value::{same_value_zero, ResultValue, Value}, }, exec::Interpreter, @@ -76,12 +76,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() { @@ -142,11 +140,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 50f955f928b..d60b6395fd0 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -15,7 +15,7 @@ use crate::{ builtins::{ array::Array, object::{Object, ObjectData, INSTANCE_PROTOTYPE, PROTOTYPE}, - property::Property, + property::{Attribute, Property}, value::{RcString, ResultValue, Value}, }, environment::function_environment_record::BindingStatus, @@ -376,19 +376,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); @@ -437,18 +437,16 @@ pub fn make_constructor_fn( 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/object/internal_methods.rs b/boa/src/builtins/object/internal_methods.rs index 1dcb5e87721..4572411c2eb 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, INSTANCE_PROTOTYPE, 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 { + 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,12 +184,12 @@ impl Object { } // 4 - if !current.configurable { - if desc.configurable { + if !current.configurable_or(false) { + if desc.configurable_or(false) { return false; } - if desc.enumerable != current.enumerable { + if desc.enumerable_or(false) != current.enumerable_or(false) { return false; } } @@ -199,14 +199,14 @@ impl Object { // 6 } else if current.is_data_descriptor() != desc.is_data_descriptor() { // a - if !current.configurable { + 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 @@ -218,8 +218,8 @@ impl Object { // 7 } else if current.is_data_descriptor() && desc.is_data_descriptor() { // a - if !current.configurable && !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; } @@ -236,7 +236,7 @@ impl Object { } // 8 } else { - if !current.configurable { + if !current.configurable() { if desc.set.is_some() && !same_value(&desc.set.clone().unwrap(), ¤t.set.clone().unwrap()) { @@ -271,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!(""), } } @@ -401,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 1e1d4067efe..3a41db3bfbb 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -528,7 +528,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) + 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 index 15404cd37bc..e909a38ad7c 100644 --- a/boa/src/builtins/property/attribute/mod.rs +++ b/boa/src/builtins/property/attribute/mod.rs @@ -1,4 +1,4 @@ -//! This module implements the `Attribute` struct which contains the attibutes for `Property`. +//! This module implements the `Attribute` struct which contains the attibutes for property descriptors. use bitflags::bitflags; use gc::{unsafe_empty_trace, Finalize, Trace}; @@ -7,7 +7,7 @@ use gc::{unsafe_empty_trace, Finalize, Trace}; mod tests; bitflags! { - /// This struct constains the property flags as describen in the [`ECMAScript specification`][spec]. + /// 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. @@ -32,7 +32,7 @@ bitflags! { /// 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`. + /// The property can not be enumerated in a `for-in` loop. const NON_ENUMERABLE = 0b0000_1000; /// Is the `Enumerable` flag defined. diff --git a/boa/src/builtins/property/attribute/tests.rs b/boa/src/builtins/property/attribute/tests.rs index 0426a4337d5..e9ced879177 100644 --- a/boa/src/builtins/property/attribute/tests.rs +++ b/boa/src/builtins/property/attribute/tests.rs @@ -1,4 +1,3 @@ - use super::Attribute; #[test] diff --git a/boa/src/builtins/property/mod.rs b/boa/src/builtins/property/mod.rs index 2cb07213022..cfe05472a57 100644 --- a/boa/src/builtins/property/mod.rs +++ b/boa/src/builtins/property/mod.rs @@ -14,7 +14,7 @@ //! [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; @@ -43,12 +43,7 @@ mod tests; /// [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: bool, - /// If the property shows up in enumeration of the object - pub enumerable: bool, - /// 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 @@ -59,6 +54,7 @@ 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() } @@ -68,58 +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: false, - enumerable: false, - writable: None, + attribute: Default::default(), value: None, get: None, set: None, } } - /// Set configurable - pub fn configurable(mut self, configurable: bool) -> Self { - self.configurable = 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 = 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.set.is_none() && self.writable.is_none() + self.value.is_none() + && self.attribute.is_empty() + && self.get.is_none() + && self.set.is_none() } /// An accessor Property Descriptor is one that includes any fields named either [[Get]] or [[Set]]. @@ -128,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() } @@ -138,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() } @@ -160,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: false, - enumerable: false, - 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)); @@ -189,10 +259,25 @@ 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: bool::from(&value.get_field("configurable")), - enumerable: 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")), diff --git a/boa/src/builtins/value/mod.rs b/boa/src/builtins/value/mod.rs index 12f817b6d43..f85cf8a8cb6 100644 --- a/boa/src/builtins/value/mod.rs +++ b/boa/src/builtins/value/mod.rs @@ -15,7 +15,7 @@ use crate::builtins::{ GcObject, InternalState, InternalStateCell, Object, ObjectData, INSTANCE_PROTOTYPE, PROTOTYPE, }, - property::Property, + property::{Attribute, Property}, BigInt, Symbol, }; use crate::exec::Interpreter; @@ -208,11 +208,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( @@ -227,11 +226,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 @@ -505,24 +503,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: bool, - writable: Option, - configurable: bool, - ) { + 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/builtins/value/tests.rs b/boa/src/builtins/value/tests.rs index 16323fd0e83..ced55ceecec 100644 --- a/boa/src/builtins/value/tests.rs +++ b/boa/src/builtins/value/tests.rs @@ -31,6 +31,7 @@ fn get_set_field() { // Create string and convert it to a Value let s = Value::from("bar"); obj.set_field("foo", s); + println!("{:?}", obj); assert_eq!(obj.get_field("foo").to_string(), "bar"); } diff --git a/boa/src/environment/global_environment_record.rs b/boa/src/environment/global_environment_record.rs index 00068692e15..312bb670ae3 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 { + if prop.value.is_none() || prop.configurable() { return false; } true @@ -70,11 +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 { - global_object.update_property(name, Some(value), true, Some(true), 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), true, Some(true), 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 85016f9498b..e00f5044dbf 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), false, None, 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 {