From 1594d05822d69d4bbe61769fe1bcab8ace1374d9 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Wed, 1 Apr 2026 18:17:21 +0200 Subject: [PATCH] Add completed PropertyDescriptor type --- core/engine/src/builtins/array/mod.rs | 54 +- core/engine/src/builtins/builder.rs | 94 ++-- .../engine/src/builtins/function/arguments.rs | 18 +- .../src/builtins/object/for_in_iterator.rs | 2 +- core/engine/src/builtins/object/mod.rs | 27 +- core/engine/src/builtins/promise/mod.rs | 2 +- core/engine/src/builtins/proxy/mod.rs | 111 ++-- .../engine/src/builtins/typed_array/object.rs | 16 +- core/engine/src/class.rs | 6 +- core/engine/src/context/mod.rs | 17 +- core/engine/src/module/namespace.rs | 24 +- .../engine/src/object/internal_methods/mod.rs | 496 ++++++++++++------ .../src/object/internal_methods/string.rs | 24 +- core/engine/src/object/jsobject.rs | 8 +- core/engine/src/object/mod.rs | 171 +++--- core/engine/src/object/operations.rs | 14 +- core/engine/src/object/property_map.rs | 228 ++++---- core/engine/src/property/mod.rs | 394 +++++++++++--- core/engine/src/property/nonmaxu32.rs | 2 + .../src/value/conversions/serde_json.rs | 13 +- core/engine/src/value/display/arguments.rs | 29 +- core/engine/src/value/display/array.rs | 44 +- core/engine/src/value/display/object.rs | 63 ++- core/engine/src/value/display/value.rs | 7 +- core/engine/src/value/tests.rs | 5 +- .../src/vm/opcode/define/class/getter.rs | 10 +- .../src/vm/opcode/define/class/setter.rs | 10 +- core/engine/src/vm/opcode/set/property.rs | 9 +- 28 files changed, 1180 insertions(+), 718 deletions(-) diff --git a/core/engine/src/builtins/array/mod.rs b/core/engine/src/builtins/array/mod.rs index acd695878ea..cbb94c43684 100644 --- a/core/engine/src/builtins/array/mod.rs +++ b/core/engine/src/builtins/array/mod.rs @@ -26,7 +26,9 @@ use crate::{ ordinary_get_own_property, }, }, - property::{Attribute, PropertyDescriptor, PropertyKey, PropertyNameKind}, + property::{ + Attribute, CompletePropertyDescriptor, PropertyDescriptor, PropertyKey, PropertyNameKind, + }, realm::Realm, string::StaticJsStrings, symbol::JsSymbol, @@ -3286,11 +3288,12 @@ impl Array { pub(crate) fn unscopables_object() -> JsObject { // 1. Let unscopableList be OrdinaryObjectCreate(null). let unscopable_list = JsObject::with_null_proto(); - let true_prop = PropertyDescriptor::builder() - .value(true) - .writable(true) - .enumerable(true) - .configurable(true); + let true_prop = CompletePropertyDescriptor::Data { + value: JsValue::new(true), + writable: true, + configurable: true, + enumerable: true, + }; { let mut obj = unscopable_list.borrow_mut(); // 2. Perform ! CreateDataPropertyOrThrow(unscopableList, "at", true). @@ -3493,21 +3496,28 @@ fn array_exotic_define_own_property( .expect("the property descriptor must exist"); // b. Assert: ! IsDataDescriptor(oldLenDesc) is true. - debug_assert!(old_len_desc.is_data_descriptor()); + let CompletePropertyDescriptor::Data { + value, + writable, + enumerable, + configurable, + } = old_len_desc + else { + panic!("length is always a data property descriptor"); + }; // c. Assert: oldLenDesc.[[Configurable]] is false. - debug_assert!(!old_len_desc.expect_configurable()); + debug_assert!(!configurable); // d. Let oldLen be oldLenDesc.[[Value]]. // e. Assert: oldLen is a non-negative integral Number. // f. Let index be ! ToUint32(P). - let old_len = old_len_desc - .expect_value() + let old_len = value .to_u32(context) .js_expect("this ToUint32 call must not fail")?; // g. If index ≥ oldLen and oldLenDesc.[[Writable]] is false, return false. - if index >= old_len && !old_len_desc.expect_writable() { + if index >= old_len && !writable { return Ok(false); } @@ -3518,9 +3528,9 @@ fn array_exotic_define_own_property( // i. Set oldLenDesc.[[Value]] to index + 1𝔽. let old_len_desc = PropertyDescriptor::builder() .value(new_len) - .maybe_writable(old_len_desc.writable()) - .maybe_enumerable(old_len_desc.enumerable()) - .maybe_configurable(old_len_desc.configurable()); + .writable(writable) + .enumerable(enumerable) + .configurable(configurable); // ii. Set succeeded to OrdinaryDefineOwnProperty(A, "length", oldLenDesc). let succeeded = ordinary_define_own_property( @@ -3590,13 +3600,21 @@ fn array_set_length( .expect("the property descriptor must exist"); // 8. Assert: ! IsDataDescriptor(oldLenDesc) is true. - debug_assert!(old_len_desc.is_data_descriptor()); + let CompletePropertyDescriptor::Data { + value, + writable, + configurable, + .. + } = old_len_desc + else { + panic!("length is always a data property descriptor"); + }; // 9. Assert: oldLenDesc.[[Configurable]] is false. - debug_assert!(!old_len_desc.expect_configurable()); + debug_assert!(!configurable); // 10. Let oldLen be oldLenDesc.[[Value]]. - let old_len = old_len_desc.expect_value(); + let old_len = value; // 11. If newLen ≥ oldLen, then if new_len >= old_len.to_u32(context)? { @@ -3610,7 +3628,7 @@ fn array_set_length( } // 12. If oldLenDesc.[[Writable]] is false, return false. - if !old_len_desc.expect_writable() { + if !writable { return Ok(false); } diff --git a/core/engine/src/builtins/builder.rs b/core/engine/src/builtins/builder.rs index ded9726ac82..fcae970d684 100644 --- a/core/engine/src/builtins/builder.rs +++ b/core/engine/src/builtins/builder.rs @@ -5,7 +5,7 @@ use crate::{ CONSTRUCTOR, FunctionBinding, JsFunction, JsPrototype, PROTOTYPE, shape::{property_table::PropertyTableInner, slot::SlotAttributes}, }, - property::{Attribute, PropertyDescriptor, PropertyKey}, + property::{Attribute, CompletePropertyDescriptor, PropertyKey}, realm::Realm, string::StaticJsStrings, }; @@ -67,11 +67,12 @@ impl ApplyToObject for Constructor { fn apply_to(self, object: &JsObject) { object.insert( PROTOTYPE, - PropertyDescriptor::builder() - .value(self.prototype.clone()) - .writable(false) - .enumerable(false) - .configurable(false), + CompletePropertyDescriptor::Data { + value: self.prototype.clone().into(), + writable: false, + enumerable: false, + configurable: false, + }, ); { @@ -79,11 +80,12 @@ impl ApplyToObject for Constructor { prototype.set_prototype(self.inherits); prototype.insert( CONSTRUCTOR, - PropertyDescriptor::builder() - .value(object.clone()) - .writable(self.attributes.writable()) - .enumerable(self.attributes.enumerable()) - .configurable(self.attributes.configurable()), + CompletePropertyDescriptor::Data { + value: object.clone().into(), + writable: self.attributes.writable(), + enumerable: self.attributes.enumerable(), + configurable: self.attributes.configurable(), + }, ); } } @@ -109,19 +111,21 @@ impl ApplyToObject for Callable { } object.insert( StaticJsStrings::LENGTH, - PropertyDescriptor::builder() - .value(self.length) - .writable(false) - .enumerable(false) - .configurable(true), + CompletePropertyDescriptor::Data { + value: self.length.into(), + writable: false, + enumerable: false, + configurable: true, + }, ); object.insert( js_string!("name"), - PropertyDescriptor::builder() - .value(self.name) - .writable(false) - .enumerable(false) - .configurable(true), + CompletePropertyDescriptor::Data { + value: self.name.into(), + writable: false, + enumerable: false, + configurable: true, + }, ); self.kind.apply_to(object); @@ -641,11 +645,12 @@ impl BuiltInBuilder<'_, T> { self.object.insert( binding.binding, - PropertyDescriptor::builder() - .value(function) - .writable(true) - .enumerable(false) - .configurable(true), + CompletePropertyDescriptor::Data { + value: function.into(), + writable: true, + enumerable: false, + configurable: true, + }, ); self } @@ -656,12 +661,15 @@ impl BuiltInBuilder<'_, T> { K: Into, V: Into, { - let property = PropertyDescriptor::builder() - .value(value) - .writable(attribute.writable()) - .enumerable(attribute.enumerable()) - .configurable(attribute.configurable()); - self.object.insert(key, property); + self.object.insert( + key, + CompletePropertyDescriptor::Data { + value: value.into(), + writable: attribute.writable(), + enumerable: attribute.enumerable(), + configurable: attribute.configurable(), + }, + ); self } @@ -676,21 +684,17 @@ impl BuiltInBuilder<'_, T> { where K: Into, { - let mut property = PropertyDescriptor::builder() - .enumerable(attribute.enumerable()) - .configurable(attribute.configurable()); - - if let Some(get) = get { - property = property.get(get); - } - - if let Some(set) = set { - property = property.set(set); - } - let key = key.into(); - self.object.insert(key, property); + self.object.insert( + key, + CompletePropertyDescriptor::Accessor { + get, + set, + enumerable: attribute.enumerable(), + configurable: attribute.configurable(), + }, + ); self } diff --git a/core/engine/src/builtins/function/arguments.rs b/core/engine/src/builtins/function/arguments.rs index 0341f83b8b4..b50e97aaa64 100644 --- a/core/engine/src/builtins/function/arguments.rs +++ b/core/engine/src/builtins/function/arguments.rs @@ -10,7 +10,7 @@ use crate::{ ordinary_set, ordinary_try_get, }, }, - property::{DescriptorKind, PropertyDescriptor, PropertyKey}, + property::{CompletePropertyDescriptor, DescriptorKind, PropertyDescriptor, PropertyKey}, }; use boa_ast::{function::FormalParameterList, operations::bound_names, scope::Scope}; use boa_gc::{Finalize, Gc, Trace}; @@ -283,7 +283,7 @@ pub(crate) fn arguments_exotic_get_own_property( obj: &JsObject, key: &PropertyKey, context: &mut InternalMethodPropertyContext<'_>, -) -> JsResult> { +) -> JsResult> { // 1. Let desc be OrdinaryGetOwnProperty(args, P). // 2. If desc is undefined, return desc. let Some(desc) = ordinary_get_own_property(obj, key, context)? else { @@ -300,14 +300,12 @@ pub(crate) fn arguments_exotic_get_own_property( .get(index.get()) { // a. Set desc.[[Value]] to Get(map, P). - return Ok(Some( - PropertyDescriptor::builder() - .value(value) - .maybe_writable(desc.writable()) - .maybe_enumerable(desc.enumerable()) - .maybe_configurable(desc.configurable()) - .build(), - )); + return Ok(Some(CompletePropertyDescriptor::Data { + value, + writable: desc.writable().unwrap_or(false), + enumerable: desc.enumerable(), + configurable: desc.configurable(), + })); } // 6. Return desc. diff --git a/core/engine/src/builtins/object/for_in_iterator.rs b/core/engine/src/builtins/object/for_in_iterator.rs index 980d772c0c4..bd88312e050 100644 --- a/core/engine/src/builtins/object/for_in_iterator.rs +++ b/core/engine/src/builtins/object/for_in_iterator.rs @@ -113,7 +113,7 @@ impl ForInIterator { )? { iterator.visited_keys.insert(r.clone()); - if desc.expect_enumerable() { + if desc.enumerable() { return Ok(create_iter_result_object(JsValue::new(r), false, context)); } } diff --git a/core/engine/src/builtins/object/mod.rs b/core/engine/src/builtins/object/mod.rs index 1947101550a..ed0a3601bc2 100644 --- a/core/engine/src/builtins/object/mod.rs +++ b/core/engine/src/builtins/object/mod.rs @@ -17,6 +17,7 @@ use super::{ Array, BuiltInBuilder, BuiltInConstructor, Date, IntrinsicObject, RegExp, error::Error, }; use crate::builtins::function::arguments::{MappedArguments, UnmappedArguments}; +use crate::property::CompletePropertyDescriptor; use crate::value::JsVariant; use crate::{ Context, JsArgs, JsData, JsExpect, JsResult, JsString, @@ -36,7 +37,6 @@ use crate::{ }; use boa_gc::{Finalize, Trace}; use boa_macros::js_str; -use tap::{Conv, Pipe}; pub(crate) mod for_in_iterator; #[cfg(test)] @@ -378,8 +378,8 @@ impl OrdinaryObject { // b. If desc is not undefined, then if let Some(current_desc) = desc { // i. If IsAccessorDescriptor(desc) is true, return desc.[[Get]]. - return if current_desc.is_accessor_descriptor() { - Ok(current_desc.expect_get().clone()) + return if let CompletePropertyDescriptor::Accessor { get, .. } = current_desc { + Ok(get.map(JsValue::new).unwrap_or_default()) } else { // ii. Return undefined. Ok(JsValue::undefined()) @@ -424,8 +424,8 @@ impl OrdinaryObject { // b. If desc is not undefined, then if let Some(current_desc) = desc { // i. If IsAccessorDescriptor(desc) is true, return desc.[[Set]]. - return if current_desc.is_accessor_descriptor() { - Ok(current_desc.expect_set().clone()) + return if let CompletePropertyDescriptor::Accessor { set, .. } = current_desc { + Ok(set.map(JsValue::new).unwrap_or_default()) } else { // ii. Return undefined. Ok(JsValue::undefined()) @@ -506,7 +506,7 @@ impl OrdinaryObject { obj.__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))?; // 4. Return FromPropertyDescriptor(desc). - Self::from_property_descriptor(desc, context) + Self::from_property_descriptor(desc.map(Into::into), context) } /// `Object.getOwnPropertyDescriptors( object )` @@ -542,7 +542,8 @@ impl OrdinaryObject { obj.__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))?; // b. Let descriptor be FromPropertyDescriptor(desc). - let descriptor = Self::from_property_descriptor(desc, context)?; + let descriptor = Self::from_property_descriptor(desc.map(Into::into), context) + .expect("should never fail"); // c. If descriptor is not undefined, // perform ! CreateDataPropertyOrThrow(descriptors, key, descriptor). @@ -942,12 +943,10 @@ impl OrdinaryObject { .to_object(context)? .__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))?; - own_prop + Ok(own_prop .as_ref() - .and_then(PropertyDescriptor::enumerable) - .unwrap_or_default() - .conv::() - .pipe(Ok) + .is_some_and(CompletePropertyDescriptor::enumerable) + .into()) } /// `Object.assign( target, ...sources )` @@ -990,7 +989,7 @@ impl OrdinaryObject { &mut InternalMethodPropertyContext::new(context), )? { // 3.a.iii.2. If desc is not undefined and desc.[[Enumerable]] is true, then - if desc.expect_enumerable() { + if desc.enumerable() { // 3.a.iii.2.a. Let propValue be ? Get(from, nextKey). let property = from.get(key.clone(), context)?; // 3.a.iii.2.b. Perform ? Set(to, nextKey, propValue, true). @@ -1458,7 +1457,7 @@ fn object_define_properties( if let Some(prop_desc) = props .__get_own_property__(&next_key, &mut InternalMethodPropertyContext::new(context))? - && prop_desc.expect_enumerable() + && prop_desc.enumerable() { // i. Let descObj be ? Get(props, nextKey). let desc_obj = props.get(next_key.clone(), context)?; diff --git a/core/engine/src/builtins/promise/mod.rs b/core/engine/src/builtins/promise/mod.rs index 3893ce8eeaa..47f8b275b3b 100644 --- a/core/engine/src/builtins/promise/mod.rs +++ b/core/engine/src/builtins/promise/mod.rs @@ -1251,7 +1251,7 @@ impl Promise { // b. If desc is not undefined and desc.[[Enumerable]] is true, then if let Some(desc) = desc - && desc.expect_enumerable() + && desc.enumerable() { // i. Let value be ? Get(promises, key). let value = promises.get(key.clone(), context)?; diff --git a/core/engine/src/builtins/proxy/mod.rs b/core/engine/src/builtins/proxy/mod.rs index a86155e2421..9dc026e1cb1 100644 --- a/core/engine/src/builtins/proxy/mod.rs +++ b/core/engine/src/builtins/proxy/mod.rs @@ -13,6 +13,7 @@ use super::{BuiltInBuilder, BuiltInConstructor, IntrinsicObject, OrdinaryObject}; use crate::JsExpect; use crate::object::internal_methods::InternalMethodCallContext; +use crate::property::CompletePropertyDescriptor; use crate::value::JsVariant; use crate::{ Context, JsArgs, JsResult, JsString, JsValue, @@ -468,7 +469,7 @@ pub(crate) fn proxy_exotic_get_own_property( obj: &JsObject, key: &PropertyKey, context: &mut InternalMethodPropertyContext<'_>, -) -> JsResult> { +) -> JsResult> { context.slot().attributes |= SlotAttributes::NOT_CACHEABLE; // 1. Let handler be O.[[ProxyHandler]]. @@ -508,7 +509,7 @@ pub(crate) fn proxy_exotic_get_own_property( if trap_result_obj.is_undefined() { if let Some(desc) = target_desc { // b. If targetDesc.[[Configurable]] is false, throw a TypeError exception. - if !desc.expect_configurable() { + if !desc.configurable() { return Err(JsNativeError::typ() .with_message( "Proxy trap result is undefined and target result is not configurable", @@ -543,8 +544,9 @@ pub(crate) fn proxy_exotic_get_own_property( // 14. Let valid be IsCompatiblePropertyDescriptor(extensibleTarget, resultDesc, targetDesc). // 15. If valid is false, throw a TypeError exception. if !is_compatible_property_descriptor( + obj, extensible_target, - result_desc.clone(), + PropertyDescriptor::from(result_desc.clone()), target_desc.clone(), ) { return Err(JsNativeError::typ() @@ -553,14 +555,17 @@ pub(crate) fn proxy_exotic_get_own_property( } // 16. If resultDesc.[[Configurable]] is false, then - if !result_desc.expect_configurable() { + if !result_desc.configurable() { // a. If targetDesc is undefined or targetDesc.[[Configurable]] is true, then match &target_desc { - Some(desc) if !desc.expect_configurable() => { + Some(desc) if !desc.configurable() => { // b. If resultDesc has a [[Writable]] field and resultDesc.[[Writable]] is false, then - if result_desc.writable() == Some(false) { + if let CompletePropertyDescriptor::Data { + writable: false, .. + } = result_desc + { // i. If targetDesc.[[Writable]] is true, throw a TypeError exception. - if desc.expect_writable() { + if let CompletePropertyDescriptor::Data { writable: true, .. } = desc { return Err(JsNativeError::typ().with_message("Proxy trap result is writable and not configurable while target result is not configurable").into()) ; @@ -658,6 +663,7 @@ pub(crate) fn proxy_exotic_define_own_property( Some(target_desc) => { // a. If IsCompatiblePropertyDescriptor(extensibleTarget, Desc, targetDesc) is false, throw a TypeError exception. if !is_compatible_property_descriptor( + obj, extensible_target, desc.clone(), Some(target_desc.clone()), @@ -668,16 +674,18 @@ pub(crate) fn proxy_exotic_define_own_property( } // b. If settingConfigFalse is true and targetDesc.[[Configurable]] is true, throw a TypeError exception. - if setting_config_false && target_desc.expect_configurable() { + if setting_config_false && target_desc.configurable() { return Err(JsNativeError::typ() .with_message("Proxy trap set property with unexpected configurable field") .into()); } // c. If IsDataDescriptor(targetDesc) is true, targetDesc.[[Configurable]] is false, and targetDesc.[[Writable]] is true, then - if target_desc.is_data_descriptor() - && !target_desc.expect_configurable() - && target_desc.expect_writable() + if let CompletePropertyDescriptor::Data { + configurable: false, + writable: true, + .. + } = target_desc { // i. If Desc has a [[Writable]] field and Desc.[[Writable]] is false, throw a TypeError exception. if let Some(writable) = desc.writable() @@ -741,7 +749,7 @@ pub(crate) fn proxy_exotic_has_property( // b. If targetDesc is not undefined, then if let Some(target_desc) = target_desc { // i. If targetDesc.[[Configurable]] is false, throw a TypeError exception. - if !target_desc.expect_configurable() { + if !target_desc.configurable() { return Err(JsNativeError::typ() .with_message("Proxy trap returned unexpected property") .into()); @@ -828,26 +836,32 @@ pub(crate) fn proxy_exotic_get( // 9. If targetDesc is not undefined and targetDesc.[[Configurable]] is false, then if let Some(target_desc) = target_desc - && !target_desc.expect_configurable() + && !target_desc.configurable() { - // a. If IsDataDescriptor(targetDesc) is true and targetDesc.[[Writable]] is false, then - if target_desc.is_data_descriptor() && !target_desc.expect_writable() { - // i. If SameValue(trapResult, targetDesc.[[Value]]) is false, throw a TypeError exception. - if !JsValue::same_value(&trap_result, target_desc.expect_value()) { - return Err(JsNativeError::typ() - .with_message("Proxy trap returned unexpected data descriptor") - .into()); + match target_desc { + // a. If IsDataDescriptor(targetDesc) is true and targetDesc.[[Writable]] is false, then + CompletePropertyDescriptor::Data { + writable: false, + value: target_value, + .. + } => { + // i. If SameValue(trapResult, targetDesc.[[Value]]) is false, throw a TypeError exception. + if !JsValue::same_value(&trap_result, &target_value) { + return Err(JsNativeError::typ() + .with_message("Proxy trap returned unexpected data descriptor") + .into()); + } } - } - - // b. If IsAccessorDescriptor(targetDesc) is true and targetDesc.[[Get]] is undefined, then - if target_desc.is_accessor_descriptor() && target_desc.expect_get().is_undefined() { - // i. If trapResult is not undefined, throw a TypeError exception. - if !trap_result.is_undefined() { - return Err(JsNativeError::typ() - .with_message("Proxy trap returned unexpected accessor descriptor") - .into()); + // b. If IsAccessorDescriptor(targetDesc) is true and targetDesc.[[Get]] is undefined, then + CompletePropertyDescriptor::Accessor { get: None, .. } => { + // i. If trapResult is not undefined, throw a TypeError exception. + if !trap_result.is_undefined() { + return Err(JsNativeError::typ() + .with_message("Proxy trap returned unexpected accessor descriptor") + .into()); + } } + _ => {} } } @@ -909,29 +923,32 @@ pub(crate) fn proxy_exotic_set( // 10. If targetDesc is not undefined and targetDesc.[[Configurable]] is false, then if let Some(target_desc) = target_desc - && !target_desc.expect_configurable() + && !target_desc.configurable() { - // a. If IsDataDescriptor(targetDesc) is true and targetDesc.[[Writable]] is false, then - if target_desc.is_data_descriptor() && !target_desc.expect_writable() { - // i. If SameValue(V, targetDesc.[[Value]]) is false, throw a TypeError exception. - if !JsValue::same_value(&value, target_desc.expect_value()) { - return Err(JsNativeError::typ() - .with_message("Proxy trap set unexpected data descriptor") - .into()); + match target_desc { + // a. If IsDataDescriptor(targetDesc) is true and targetDesc.[[Writable]] is false, then + CompletePropertyDescriptor::Data { + writable: false, + value: target_value, + .. + } => { + // i. If SameValue(V, targetDesc.[[Value]]) is false, throw a TypeError exception. + if !JsValue::same_value(&value, &target_value) { + return Err(JsNativeError::typ() + .with_message("Proxy trap set unexpected data descriptor") + .into()); + } } - } - - // b. If IsAccessorDescriptor(targetDesc) is true, then - if target_desc.is_accessor_descriptor() { - // i. If targetDesc.[[Set]] is undefined, throw a TypeError exception. - match target_desc.set().map(JsValue::is_undefined) { - None | Some(true) => { + // b. If IsAccessorDescriptor(targetDesc) is true, then + CompletePropertyDescriptor::Accessor { set, .. } => { + // i. If targetDesc.[[Set]] is undefined, throw a TypeError exception. + if set.is_none() { return Err(JsNativeError::typ() .with_message("Proxy trap set unexpected accessor descriptor") .into()); } - _ => {} } + CompletePropertyDescriptor::Data { .. } => {} } } @@ -985,7 +1002,7 @@ pub(crate) fn proxy_exotic_delete( None => return Ok(true), // 11. If targetDesc.[[Configurable]] is false, throw a TypeError exception. Some(target_desc) => { - if !target_desc.expect_configurable() { + if !target_desc.configurable() { return Err(JsNativeError::typ() .with_message("Proxy trap failed to delete property") .into()); @@ -1081,7 +1098,7 @@ pub(crate) fn proxy_exotic_own_property_keys( // a. Let desc be ? target.[[GetOwnProperty]](key). match target.__get_own_property__(&key, &mut context.into())? { // b. If desc is not undefined and desc.[[Configurable]] is false, then - Some(desc) if !desc.expect_configurable() => { + Some(desc) if !desc.configurable() => { // i. Append key as an element of targetNonconfigurableKeys. target_nonconfigurable_keys.push(key); } diff --git a/core/engine/src/builtins/typed_array/object.rs b/core/engine/src/builtins/typed_array/object.rs index 57087cc1350..e0642302421 100644 --- a/core/engine/src/builtins/typed_array/object.rs +++ b/core/engine/src/builtins/typed_array/object.rs @@ -13,7 +13,7 @@ use crate::{ ordinary_has_property, ordinary_prevent_extensions, ordinary_set, ordinary_try_get, }, }, - property::{PropertyDescriptor, PropertyKey}, + property::{CompletePropertyDescriptor, PropertyDescriptor, PropertyKey}, }; use boa_gc::{Finalize, Trace}; use boa_macros::js_str; @@ -362,7 +362,7 @@ pub(crate) fn typed_array_exotic_get_own_property( obj: &JsObject, key: &PropertyKey, context: &mut InternalMethodPropertyContext<'_>, -) -> JsResult> { +) -> JsResult> { let p = match key { PropertyKey::String(key) => { // 1.a. Let numericIndex be CanonicalNumericIndexString(P). @@ -380,13 +380,11 @@ pub(crate) fn typed_array_exotic_get_own_property( // ii. If value is undefined, return undefined. // iii. Return the PropertyDescriptor { [[Value]]: value, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true }. - return Ok(value.map(|v| { - PropertyDescriptor::builder() - .value(v) - .writable(true) - .enumerable(true) - .configurable(true) - .build() + return Ok(value.map(|value| CompletePropertyDescriptor::Data { + value, + writable: true, + enumerable: true, + configurable: true, })); } diff --git a/core/engine/src/class.rs b/core/engine/src/class.rs index 91ac60edb3e..2083e07111e 100644 --- a/core/engine/src/class.rs +++ b/core/engine/src/class.rs @@ -109,7 +109,7 @@ use crate::{ error::JsNativeError, native_function::NativeFunction, object::{ConstructorBuilder, FunctionBinding, JsFunction, JsObject, NativeObject, PROTOTYPE}, - property::{Attribute, PropertyDescriptor, PropertyKey}, + property::{Attribute, CompletePropertyDescriptor, PropertyKey}, }; /// Native class. @@ -355,7 +355,7 @@ impl<'ctx> ClassBuilder<'ctx> { pub fn property_descriptor(&mut self, key: K, property: P) -> &mut Self where K: Into, - P: Into, + P: Into, { self.builder.property_descriptor(key, property); self @@ -367,7 +367,7 @@ impl<'ctx> ClassBuilder<'ctx> { pub fn static_property_descriptor(&mut self, key: K, property: P) -> &mut Self where K: Into, - P: Into, + P: Into, { self.builder.static_property_descriptor(key, property); self diff --git a/core/engine/src/context/mod.rs b/core/engine/src/context/mod.rs index b9791334283..379e4c6a988 100644 --- a/core/engine/src/context/mod.rs +++ b/core/engine/src/context/mod.rs @@ -17,6 +17,7 @@ use timezone_provider::experimental_tzif::ZeroCompiledTzdbProvider; use crate::job::Job; use crate::js_error; use crate::module::DynModuleLoader; +use crate::property::CompletePropertyDescriptor; use crate::vm::{CodeBlock, RuntimeLimits, create_function_object_fast}; use crate::{ HostDefined, JsNativeError, JsResult, JsString, JsValue, NativeObject, Source, builtins, @@ -673,14 +674,16 @@ impl Context { }; // 5. If existingProp.[[Configurable]] is true, return true. - if existing_prop.configurable() == Some(true) { + if existing_prop.configurable() { return Ok(true); } // 6. If IsDataDescriptor(existingProp) is true and existingProp has attribute values { [[Writable]]: true, [[Enumerable]]: true }, return true. - if existing_prop.is_data_descriptor() - && existing_prop.writable() == Some(true) - && existing_prop.enumerable() == Some(true) + if let CompletePropertyDescriptor::Data { + writable: true, + enumerable: true, + .. + } = existing_prop { return Ok(true); } @@ -776,9 +779,7 @@ impl Context { global_object.__get_own_property__(&name.clone().into(), &mut self.into())?; // 4. If existingProp is undefined or existingProp.[[Configurable]] is true, then - let desc = if existing_prop.is_none() - || existing_prop.and_then(|p| p.configurable()) == Some(true) - { + let desc = if existing_prop.is_none() || existing_prop.is_some_and(|p| p.configurable()) { // a. Let desc be the PropertyDescriptor { [[Value]]: V, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: D }. PropertyDescriptor::builder() .value(function.clone()) @@ -828,7 +829,7 @@ impl Context { }; // 5. If existingProp.[[Configurable]] is true, return false. - if existing_prop.configurable() == Some(true) { + if existing_prop.configurable() { return Ok(false); } diff --git a/core/engine/src/module/namespace.rs b/core/engine/src/module/namespace.rs index f42692c9f1d..c686802a671 100644 --- a/core/engine/src/module/namespace.rs +++ b/core/engine/src/module/namespace.rs @@ -12,7 +12,7 @@ use crate::object::internal_methods::{ ordinary_has_property, ordinary_own_property_keys, ordinary_try_get, }; use crate::object::{JsData, JsPrototype}; -use crate::property::{PropertyDescriptor, PropertyKey}; +use crate::property::{CompletePropertyDescriptor, PropertyDescriptor, PropertyKey}; use crate::{Context, JsExpect, JsResult, JsString, JsValue, js_string, object::JsObject}; use crate::{JsNativeError, Module}; @@ -177,7 +177,7 @@ fn module_namespace_exotic_get_own_property( obj: &JsObject, key: &PropertyKey, context: &mut InternalMethodPropertyContext<'_>, -) -> JsResult> { +) -> JsResult> { // 1. If P is a Symbol, return OrdinaryGetOwnProperty(O, P). let key = match key { PropertyKey::Symbol(_) => return ordinary_get_own_property(obj, key, context), @@ -202,14 +202,12 @@ fn module_namespace_exotic_get_own_property( let value = obj.get(key, context)?; // 5. Return PropertyDescriptor { [[Value]]: value, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: false }. - Ok(Some( - PropertyDescriptor::builder() - .value(value) - .writable(true) - .enumerable(true) - .configurable(false) - .build(), - )) + Ok(Some(CompletePropertyDescriptor::Data { + value, + writable: true, + enumerable: true, + configurable: false, + })) } /// [`[[DefineOwnProperty]] ( P, Desc )`][spec] @@ -246,7 +244,11 @@ fn module_namespace_exotic_define_own_property( // 8. If Desc has a [[Value]] field, return SameValue(Desc.[[Value]], current.[[Value]]). // 9. Return true. - Ok(desc.value().is_none_or(|v| v == current.expect_value())) + Ok(desc.value().is_none_or(|v| { + v == current + .value() + .expect("should always be a data property descriptor") + })) } /// [`[[HasProperty]] ( P )`][spec] diff --git a/core/engine/src/object/internal_methods/mod.rs b/core/engine/src/object/internal_methods/mod.rs index 8e474b5dde5..5b80674ba11 100644 --- a/core/engine/src/object/internal_methods/mod.rs +++ b/core/engine/src/object/internal_methods/mod.rs @@ -14,8 +14,10 @@ use super::{ use crate::{ Context, JsNativeError, JsResult, context::intrinsics::{StandardConstructor, StandardConstructors}, - object::JsObject, - property::{DescriptorKind, PropertyDescriptor, PropertyKey}, + object::{JsFunction, JsObject}, + property::{ + CompletePropertyDescriptor, DescriptorKind, NonMaxU32, PropertyDescriptor, PropertyKey, + }, value::JsValue, vm::source_info::NativeSourceInfo, }; @@ -199,7 +201,7 @@ impl JsObject { &self, key: &PropertyKey, context: &mut InternalMethodPropertyContext<'_>, - ) -> JsResult> { + ) -> JsResult> { (self.vtable().__get_own_property__)(self, key, context) } @@ -409,7 +411,7 @@ pub struct InternalObjectMethods { &JsObject, &PropertyKey, &mut InternalMethodPropertyContext<'_>, - ) -> JsResult>, + ) -> JsResult>, pub(crate) __define_own_property__: fn( &JsObject, &PropertyKey, @@ -620,7 +622,7 @@ pub(crate) fn ordinary_get_own_property( obj: &JsObject, key: &PropertyKey, context: &mut InternalMethodPropertyContext<'_>, -) -> JsResult> { +) -> JsResult> { // 1. Assert: IsPropertyKey(P) is true. // 2. If O does not have an own property with key P, return undefined. // 3. Let D be a newly created Property Descriptor with no fields. @@ -657,8 +659,8 @@ pub(crate) fn ordinary_define_own_property( let extensible = obj.__is_extensible__(context)?; // 3. Return ValidateAndApplyPropertyDescriptor(O, P, extensible, Desc, current). - Ok(validate_and_apply_property_descriptor( - Some((obj, key)), + Ok(validate_and_apply_property_descriptor::( + (obj, key), extensible, desc, current, @@ -727,20 +729,20 @@ pub(crate) fn ordinary_get( Ok(JsValue::undefined()) } } - Some(ref desc) => { - match desc.kind() { + Some(desc) => { + match desc { // 4. If IsDataDescriptor(desc) is true, return desc.[[Value]]. - DescriptorKind::Data { - value: Some(value), .. - } => Ok(value.clone()), + CompletePropertyDescriptor::Data { value, .. } => Ok(value), // 5. Assert: IsAccessorDescriptor(desc) is true. // 6. Let getter be desc.[[Get]]. - DescriptorKind::Accessor { get: Some(get), .. } if !get.is_undefined() => { + CompletePropertyDescriptor::Accessor { get, .. } => { + // 7. If getter is undefined, return undefined. + let Some(get) = get else { + return Ok(JsValue::undefined()); + }; // 8. Return ? Call(getter, Receiver). get.call(&receiver, &[], context) } - // 7. If getter is undefined, return undefined. - _ => Ok(JsValue::undefined()), } } } @@ -780,20 +782,20 @@ pub(crate) fn ordinary_try_get( Ok(None) } } - Some(ref desc) => { - match desc.kind() { + Some(desc) => { + match desc { // 4. If IsDataDescriptor(desc) is true, return desc.[[Value]]. - DescriptorKind::Data { - value: Some(value), .. - } => Ok(Some(value.clone())), + CompletePropertyDescriptor::Data { value, .. } => Ok(Some(value)), // 5. Assert: IsAccessorDescriptor(desc) is true. // 6. Let getter be desc.[[Get]]. - DescriptorKind::Accessor { get: Some(get), .. } if !get.is_undefined() => { + CompletePropertyDescriptor::Accessor { get, .. } => { + // 7. If getter is undefined, return undefined. + let Some(get) = get else { + return Ok(Some(JsValue::undefined())); + }; // 8. Return ? Call(getter, Receiver). get.call(&receiver, &[], context).map(Some) } - // 7. If getter is undefined, return undefined. - _ => Ok(Some(JsValue::undefined())), } } } @@ -846,84 +848,85 @@ pub(crate) fn ordinary_set( // i. Set ownDesc to the PropertyDescriptor { [[Value]]: undefined, [[Writable]]: true, // [[Enumerable]]: true, [[Configurable]]: true }. - PropertyDescriptor::builder() - .value(JsValue::undefined()) - .writable(true) - .enumerable(true) - .configurable(true) - .build() - }; - - // 3. If IsDataDescriptor(ownDesc) is true, then - if own_desc.is_data_descriptor() { - // a. If ownDesc.[[Writable]] is false, return false. - if !own_desc.expect_writable() { - return Ok(false); + CompletePropertyDescriptor::Data { + value: JsValue::undefined(), + writable: true, + enumerable: true, + configurable: true, } + }; - // b. If Type(Receiver) is not Object, return false. - let Some(receiver) = receiver.as_object() else { - return Ok(false); - }; - - let obj_is_receiver = JsObject::equals(obj, &receiver); - - // NOTE(HaledOdat): If the object and receiver are not the same then it's not inline cacheable for now. - context - .slot() - .attributes - .set(SlotAttributes::NOT_CACHEABLE, !obj_is_receiver); - - // OPTIMIZATION: If obj and receiver are the same, there's no need to call [[GetOwnProperty]](P) - // again because it was already performed above. - let existing_descriptor = if has_own_desc && obj_is_receiver { - Some(own_desc) - } else { - // c. Let existingDescriptor be ? Receiver.[[GetOwnProperty]](P). - receiver.__get_own_property__(&key, context)? - }; - - // d. If existingDescriptor is not undefined, then - if let Some(ref existing_desc) = existing_descriptor { - // i. If IsAccessorDescriptor(existingDescriptor) is true, return false. - if existing_desc.is_accessor_descriptor() { + match own_desc { + // 3. If IsDataDescriptor(ownDesc) is true, then + CompletePropertyDescriptor::Data { writable, .. } => { + // a. If ownDesc.[[Writable]] is false, return false. + if !writable { return Ok(false); } - // ii. If existingDescriptor.[[Writable]] is false, return false. - if !existing_desc.expect_writable() { + // b. If Type(Receiver) is not Object, return false. + let Some(receiver) = receiver.as_object() else { return Ok(false); - } + }; - // iii. Let valueDesc be the PropertyDescriptor { [[Value]]: V }. - // iv. Return ? Receiver.[[DefineOwnProperty]](P, valueDesc). - return receiver.__define_own_property__( - &key, - PropertyDescriptor::builder().value(value).build(), - context, - ); - } + let obj_is_receiver = JsObject::equals(obj, &receiver); - // e. Else - // i. Assert: Receiver does not currently have a property P. - // ii. Return ? CreateDataProperty(Receiver, P, V). - return receiver.create_data_property_with_slot(key, value, context); - } + // NOTE(HaledOdat): If the object and receiver are not the same then it's not inline cacheable for now. + context + .slot() + .attributes + .set(SlotAttributes::NOT_CACHEABLE, !obj_is_receiver); - // 4. Assert: IsAccessorDescriptor(ownDesc) is true. - debug_assert!(own_desc.is_accessor_descriptor()); + // OPTIMIZATION: If obj and receiver are the same, there's no need to call [[GetOwnProperty]](P) + // again because it was already performed above. + let existing_descriptor = if has_own_desc && obj_is_receiver { + Some(own_desc) + } else { + // c. Let existingDescriptor be ? Receiver.[[GetOwnProperty]](P). + receiver.__get_own_property__(&key, context)? + }; - // 5. Let setter be ownDesc.[[Set]]. - match own_desc.set() { - Some(set) if !set.is_undefined() => { - // 7. Perform ? Call(setter, Receiver, « V »). - set.call(&receiver, &[value], context)?; + // c. Let existingDescriptor be ? Receiver.[[GetOwnProperty]](P). + // d. If existingDescriptor is not undefined, then + if let Some(existing_desc) = existing_descriptor { + // i. If IsAccessorDescriptor(existingDescriptor) is true, return false. + let CompletePropertyDescriptor::Data { writable, .. } = existing_desc else { + return Ok(false); + }; + + // ii. If existingDescriptor.[[Writable]] is false, return false. + if !writable { + return Ok(false); + } - // 8. Return true. - Ok(true) + // iii. Let valueDesc be the PropertyDescriptor { [[Value]]: V }. + // iv. Return ? Receiver.[[DefineOwnProperty]](P, valueDesc). + return receiver.__define_own_property__( + &key, + PropertyDescriptor::builder().value(value).build(), + context, + ); + } + + // e. Else + // i. Assert: Receiver does not currently have a property P. + // ii. Return ? CreateDataProperty(Receiver, P, V). + receiver.create_data_property_with_slot(key, value, context) + } + // 4. Assert: IsAccessorDescriptor(ownDesc) is true. + CompletePropertyDescriptor::Accessor { set, .. } => { + // 5. Let setter be ownDesc.[[Set]]. + if let Some(set) = set { + // 7. Perform ? Call(setter, Receiver, « V »). + set.call(&receiver, &[value], context)?; + + // 8. Return true. + Ok(true) + } else { + // 6. If setter is undefined, return false. + Ok(false) + } } - // 6. If setter is undefined, return false. - _ => Ok(false), } } @@ -943,7 +946,7 @@ pub(crate) fn ordinary_delete( // 2. Let desc be ? O.[[GetOwnProperty]](P). match obj.__get_own_property__(key, context)? { // 4. If desc.[[Configurable]] is true, then - Some(desc) if desc.expect_configurable() => { + Some(desc) if desc.configurable() => { // a. Remove the own property with name P from O. obj.borrow_mut().remove(key); // b. Return true. @@ -999,12 +1002,19 @@ pub(crate) fn ordinary_own_property_keys( /// /// [spec]: https://tc39.es/ecma262/#sec-iscompatiblepropertydescriptor pub(crate) fn is_compatible_property_descriptor( + obj: &JsObject, extensible: bool, desc: PropertyDescriptor, - current: Option, + current: Option, ) -> bool { // 1. Return ValidateAndApplyPropertyDescriptor(undefined, undefined, Extensible, Desc, Current). - validate_and_apply_property_descriptor(None, extensible, desc, current, &mut Slot::new()) + validate_and_apply_property_descriptor::( + (obj, &PropertyKey::Index(NonMaxU32::ZERO)), + extensible, + desc, + current, + &mut Slot::new(), + ) } /// Abstract operation `ValidateAndApplyPropertyDescriptor` @@ -1013,16 +1023,17 @@ pub(crate) fn is_compatible_property_descriptor( /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-validateandapplypropertydescriptor -pub(crate) fn validate_and_apply_property_descriptor( - obj_and_key: Option<(&JsObject, &PropertyKey)>, +pub(crate) fn validate_and_apply_property_descriptor( + (obj, key): (&JsObject, &PropertyKey), extensible: bool, desc: PropertyDescriptor, - current: Option, + current: Option, slot: &mut Slot, ) -> bool { - // 1. Assert: If O is not undefined, then IsPropertyKey(P) is true. + // SKIP: 1. Assert: If O is not undefined, then IsPropertyKey(P) is true. + // NOTE: This is guaranteed by the Rust type-system. - let Some(mut current) = current else { + let Some(current) = current else { // 2. If current is undefined, then // a. If extensible is false, return false. if !extensible { @@ -1031,28 +1042,44 @@ pub(crate) fn validate_and_apply_property_descriptor( // b. Assert: extensible is true. - if let Some((obj, key)) = obj_and_key { + if APPLY { + let enumerable = desc.enumerable().unwrap_or(false); + let configurable = desc.configurable().unwrap_or(false); obj.borrow_mut().properties.insert_with_slot( key, - // c. If IsGenericDescriptor(Desc) is true or IsDataDescriptor(Desc) is true, then - if desc.is_generic_descriptor() || desc.is_data_descriptor() { - // i. If O is not undefined, create an own data property named P of - // object O whose [[Value]], [[Writable]], [[Enumerable]], and - // [[Configurable]] attribute values are described by Desc. - // If the value of an attribute field of Desc is absent, the attribute - // of the newly created property is set to its default value. - desc.into_data_defaulted() - } - // d. Else, - else { - // i. Assert: ! IsAccessorDescriptor(Desc) is true. - - // ii. If O is not undefined, create an own accessor property named P - // of object O whose [[Get]], [[Set]], [[Enumerable]], and [[Configurable]] - // attribute values are described by Desc. If the value of an attribute field - // of Desc is absent, the attribute of the newly created property is set to - // its default value. - desc.into_accessor_defaulted() + match desc.kind { + // c. If IsAccessorDescriptor(Desc) is true, then + // i. Create an own accessor property named P of object O whose [[Get]], [[Set]], [[Enumerable]], + // and [[Configurable]] attributes are set to the value of the corresponding field in Desc if Desc has that field, + // or to the attribute's default value otherwise. + DescriptorKind::Accessor { get, set } => CompletePropertyDescriptor::Accessor { + get: get + .as_ref() + .and_then(JsValue::as_object) + .map(JsFunction::from_object_unchecked), + set: set + .as_ref() + .and_then(JsValue::as_object) + .map(JsFunction::from_object_unchecked), + enumerable, + configurable, + }, + // d. Else, + // i. Create an own data property named P of object O whose [[Value]], [[Writable]], [[Enumerable]], + // and [[Configurable]] attributes are set to the value of the corresponding field in Desc if Desc has that field, + // or to the attribute's default value otherwise. + DescriptorKind::Data { value, writable } => CompletePropertyDescriptor::Data { + value: value.unwrap_or_default(), + writable: writable.unwrap_or(false), + enumerable, + configurable, + }, + DescriptorKind::Generic => CompletePropertyDescriptor::Data { + value: JsValue::undefined(), + writable: false, + enumerable, + configurable, + }, }, slot, ); @@ -1062,13 +1089,16 @@ pub(crate) fn validate_and_apply_property_descriptor( return true; }; - // 3. If every field in Desc is absent, return true. + // SKIP: 3. Assert: current is a fully populated Property Descriptor. + // NOTE: This is guaranteed by the Rust type-system. + + // 4. If Desc does not have any fields, return true. if desc.is_empty() { return true; } // 4. If current.[[Configurable]] is false, then - if !current.expect_configurable() { + if !current.configurable() { // a. If Desc.[[Configurable]] is present and its value is true, return false. if matches!(desc.configurable(), Some(true)) { return false; @@ -1076,81 +1106,207 @@ pub(crate) fn validate_and_apply_property_descriptor( // b. If Desc.[[Enumerable]] is present and ! SameValue(Desc.[[Enumerable]], current.[[Enumerable]]) // is false, return false. - if matches!(desc.enumerable(), Some(desc_enum) if desc_enum != current.expect_enumerable()) - { + if matches!(desc.enumerable(), Some(desc_enum) if desc_enum != current.enumerable()) { return false; } } - // 5. If ! IsGenericDescriptor(Desc) is true, then - if desc.is_generic_descriptor() { - // a. NOTE: No further validation is required. - } - // 6. Else if ! SameValue(! IsDataDescriptor(current), ! IsDataDescriptor(Desc)) is false, then - else if current.is_data_descriptor() != desc.is_data_descriptor() { - // a. If current.[[Configurable]] is false, return false. - if !current.expect_configurable() { - return false; + let current = match (current, desc.kind()) { + // 5. If ! IsGenericDescriptor(Desc) is true, then + (mut current, DescriptorKind::Generic) => { + // a. NOTE: No further validation is required. + + // 9. If O is not undefined, then + if APPLY { + let (enumerable, configurable) = match &mut current { + CompletePropertyDescriptor::Data { + enumerable, + configurable, + .. + } + | CompletePropertyDescriptor::Accessor { + enumerable, + configurable, + .. + } => (enumerable, configurable), + }; + + *enumerable = desc.enumerable().unwrap_or(*enumerable); + *configurable = desc.configurable().unwrap_or(*configurable); + + // SKIP: a. For each field of Desc that is present, set the corresponding attribute of the + // property named P of object O to the value of the field. + // let current = current.fill_with(desc); + obj.borrow_mut() + .properties + .insert_with_slot(key, current, slot); + slot.attributes |= SlotAttributes::FOUND; + } + + // 10. Return true. + return true; } + // 7. Else if IsDataDescriptor(current) and IsDataDescriptor(Desc) are both true, then + ( + CompletePropertyDescriptor::Data { + value: current_value, + writable: current_writable, + configurable: current_configurable, + enumerable: current_enumerable, + }, + DescriptorKind::Data { + value: desc_value, + writable: desc_writable, + }, + ) => { + // a. If current.[[Configurable]] is false and current.[[Writable]] is false, then + if !current_configurable && !current_writable { + // i. If Desc.[[Writable]] is present and Desc.[[Writable]] is true, return false. + if *desc_writable == Some(true) { + return false; + } + // ii. If Desc.[[Value]] is present and SameValue(Desc.[[Value]], current.[[Value]]) is false, return false. + if let Some(value) = desc_value + && !JsValue::same_value(value, ¤t_value) + { + return false; + } + // iii. Return true. + return true; + } - if obj_and_key.is_some() { - // b. If IsDataDescriptor(current) is true, then - if current.is_data_descriptor() { - // i. If O is not undefined, convert the property named P of object O from a data - // property to an accessor property. Preserve the existing values of the converted - // property's [[Configurable]] and [[Enumerable]] attributes and set the rest of - // the property's attributes to their default values. + // 9. If O is not undefined, then + if APPLY { + // a. For each field of Desc that is present, set the corresponding attribute of the + // property named P of object O to the value of the field. + let current = CompletePropertyDescriptor::Data { + value: desc_value.as_ref().unwrap_or(¤t_value).clone(), + writable: desc_writable.unwrap_or(current_writable), + enumerable: desc.enumerable().unwrap_or(current_enumerable), + configurable: desc.configurable().unwrap_or(current_configurable), + }; + obj.borrow_mut() + .properties + .insert_with_slot(key, current, slot); + slot.attributes |= SlotAttributes::FOUND; + } + + return true; + } + // 6. Else if ! SameValue(! IsDataDescriptor(current), ! IsDataDescriptor(Desc)) is false, then + ( + mut current @ CompletePropertyDescriptor::Data { + configurable: current_configurable, + .. + }, + DescriptorKind::Accessor { .. }, + ) => { + // a. If current.[[Configurable]] is false, return false. + if !current_configurable { + return false; + } + + if APPLY { + // b. If IsDataDescriptor(current) is true, then + // i. If O is not undefined, convert the property named P of object O from a data + // property to an accessor property. Preserve the existing values of the converted + // property's [[Configurable]] and [[Enumerable]] attributes and set the rest of + // the property's attributes to their default values. current = current.into_accessor_defaulted(); + // SKIP: c. Else, + // SKIP: i. If O is not undefined, convert the property named P of object O from an + // accessor property to a data property. Preserve the existing values of the + // converted property's [[Configurable]] and [[Enumerable]] attributes and set + // the rest of the property's attributes to their default values. + + // a. For each field of Desc that is present, set the corresponding attribute of the + // property named P of object O to the value of the field. + let current = current.fill_with(desc); + obj.borrow_mut() + .properties + .insert_with_slot(key, current, slot); + slot.attributes |= SlotAttributes::FOUND; } - // c. Else, - else { + + return true; + } + // 6. Else if ! SameValue(! IsDataDescriptor(current), ! IsDataDescriptor(Desc)) is false, then + ( + mut current @ CompletePropertyDescriptor::Accessor { + configurable: current_configurable, + .. + }, + DescriptorKind::Data { .. }, + ) => { + // a. If current.[[Configurable]] is false, return false. + if !current_configurable { + return false; + } + + if APPLY { + // SKIP: b. If IsDataDescriptor(current) is true, then + // SKIP: i. If O is not undefined, convert the property named P of object O from a data + // property to an accessor property. Preserve the existing values of the converted + // property's [[Configurable]] and [[Enumerable]] attributes and set the rest of + // the property's attributes to their default values. + // c. Else, // i. If O is not undefined, convert the property named P of object O from an - // accessor property to a data property. Preserve the existing values of the - // converted property's [[Configurable]] and [[Enumerable]] attributes and set - // the rest of the property's attributes to their default values. + // accessor property to a data property. Preserve the existing values of the + // converted property's [[Configurable]] and [[Enumerable]] attributes and set + // the rest of the property's attributes to their default values. current = current.into_data_defaulted(); + + // 9. If O is not undefined, then + // a. For each field of Desc that is present, set the corresponding attribute of the + // property named P of object O to the value of the field. + let current = current.fill_with(desc); + obj.borrow_mut() + .properties + .insert_with_slot(key, current, slot); + slot.attributes |= SlotAttributes::FOUND; } + + return true; } - } - // 7. Else if IsDataDescriptor(current) and IsDataDescriptor(Desc) are both true, then - else if current.is_data_descriptor() && desc.is_data_descriptor() { - // a. If current.[[Configurable]] is false and current.[[Writable]] is false, then - if !current.expect_configurable() && !current.expect_writable() { - // i. If Desc.[[Writable]] is present and Desc.[[Writable]] is true, return false. - if matches!(desc.writable(), Some(true)) { + // 8. Else, + // a. Assert: ! IsAccessorDescriptor(current) and ! IsAccessorDescriptor(Desc) are both true. + ( + CompletePropertyDescriptor::Accessor { + get: current_get, + set: current_set, + configurable: current_configurable, + .. + }, + DescriptorKind::Accessor { + get: desc_get, + set: desc_set, + }, + // b. If current.[[Configurable]] is false, then + ) if !current_configurable => { + // i. If Desc.[[Set]] is present and SameValue(Desc.[[Set]], current.[[Set]]) is false, return false. + if let Some(set) = desc_set + && set.as_object() != current_set.map(Into::into) + { return false; } - // ii. If Desc.[[Value]] is present and SameValue(Desc.[[Value]], current.[[Value]]) is false, return false. - if matches!(desc.value(), Some(value) if !JsValue::same_value(value, current.expect_value())) + + // ii. If Desc.[[Get]] is present and SameValue(Desc.[[Get]], current.[[Get]]) is false, return false. + if let Some(get) = desc_get + && get.as_object() != current_get.map(Into::into) { return false; } // iii. Return true. return true; } - } - // 8. Else, - // a. Assert: ! IsAccessorDescriptor(current) and ! IsAccessorDescriptor(Desc) are both true. - // b. If current.[[Configurable]] is false, then - else if !current.expect_configurable() { - // i. If Desc.[[Set]] is present and SameValue(Desc.[[Set]], current.[[Set]]) is false, return false. - if matches!(desc.set(), Some(set) if !JsValue::same_value(set, current.expect_set())) { - return false; - } - - // ii. If Desc.[[Get]] is present and SameValue(Desc.[[Get]], current.[[Get]]) is false, return false. - if matches!(desc.get(), Some(get) if !JsValue::same_value(get, current.expect_get())) { - return false; - } - // iii. Return true. - return true; - } + (current, _) => current, + }; // 9. If O is not undefined, then - if let Some((obj, key)) = obj_and_key { + if APPLY { // a. For each field of Desc that is present, set the corresponding attribute of the // property named P of object O to the value of the field. - current.fill_with(desc); + let current = current.fill_with(desc); obj.borrow_mut() .properties .insert_with_slot(key, current, slot); diff --git a/core/engine/src/object/internal_methods/string.rs b/core/engine/src/object/internal_methods/string.rs index 11a60219bd3..f0d4212d49d 100644 --- a/core/engine/src/object/internal_methods/string.rs +++ b/core/engine/src/object/internal_methods/string.rs @@ -1,7 +1,7 @@ use crate::{ Context, JsExpect, JsResult, JsString, object::{JsData, JsObject}, - property::{PropertyDescriptor, PropertyKey}, + property::{CompletePropertyDescriptor, PropertyDescriptor, PropertyKey}, }; use super::{InternalMethodPropertyContext, InternalObjectMethods, ORDINARY_INTERNAL_METHODS}; @@ -29,7 +29,7 @@ pub(crate) fn string_exotic_get_own_property( obj: &JsObject, key: &PropertyKey, context: &mut InternalMethodPropertyContext<'_>, -) -> JsResult> { +) -> JsResult> { // 1. Assert: IsPropertyKey(P) is true. // 2. Let desc be OrdinaryGetOwnProperty(S, P). let desc = super::ordinary_get_own_property(obj, key, context)?; @@ -65,6 +65,7 @@ pub(crate) fn string_exotic_define_own_property( let extensible = obj.borrow().extensible; // b. Return ! IsCompatiblePropertyDescriptor(extensible, Desc, stringDesc). Ok(super::is_compatible_property_descriptor( + obj, extensible, desc, Some(string_desc), @@ -133,7 +134,10 @@ pub(crate) fn string_exotic_own_property_keys( /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-stringgetownproperty -fn string_get_own_property(obj: &JsObject, key: &PropertyKey) -> Option { +fn string_get_own_property( + obj: &JsObject, + key: &PropertyKey, +) -> Option { // 1. Assert: S is an Object that has a [[StringData]] internal slot. // 2. Assert: IsPropertyKey(P) is true. // 3. If Type(P) is not String, return undefined. @@ -159,12 +163,10 @@ fn string_get_own_property(obj: &JsObject, key: &PropertyKey) -> Option Option { + pub(crate) fn get_property(&self, key: &PropertyKey) -> Option { let mut obj = Some(self.clone()); while let Some(o) = obj { @@ -987,7 +987,7 @@ impl JsObject { pub(crate) fn insert(&self, key: K, property: P) -> bool where K: Into, - P: Into, + P: Into, { self.borrow_mut().insert(key, property) } @@ -999,7 +999,7 @@ impl JsObject { pub fn insert_property(&self, key: K, property: P) -> bool where K: Into, - P: Into, + P: Into, { self.insert(key.into(), property) } diff --git a/core/engine/src/object/mod.rs b/core/engine/src/object/mod.rs index 258da691647..a4d8c11d71c 100644 --- a/core/engine/src/object/mod.rs +++ b/core/engine/src/object/mod.rs @@ -15,7 +15,7 @@ use crate::{ context::intrinsics::StandardConstructor, js_string, native_function::{NativeFunction, NativeFunctionObject}, - property::{Attribute, PropertyDescriptor, PropertyKey}, + property::{Attribute, CompletePropertyDescriptor, PropertyKey}, realm::Realm, string::StaticJsStrings, }; @@ -293,7 +293,7 @@ impl Object { pub(crate) fn insert(&mut self, key: K, property: P) -> bool where K: Into, - P: Into, + P: Into, { self.properties.insert(&key.into(), property.into()) } @@ -541,11 +541,12 @@ impl<'ctx> ObjectInitializer<'ctx> { self.object.borrow_mut().insert( binding.binding, - PropertyDescriptor::builder() - .value(function) - .writable(true) - .enumerable(false) - .configurable(true), + CompletePropertyDescriptor::Data { + value: function.into(), + writable: true, + enumerable: false, + configurable: true, + }, ); self } @@ -556,12 +557,15 @@ impl<'ctx> ObjectInitializer<'ctx> { K: Into, V: Into, { - let property = PropertyDescriptor::builder() - .value(value) - .writable(attribute.writable()) - .enumerable(attribute.enumerable()) - .configurable(attribute.configurable()); - self.object.borrow_mut().insert(key, property); + self.object.borrow_mut().insert( + key, + CompletePropertyDescriptor::Data { + value: value.into(), + writable: attribute.writable(), + enumerable: attribute.enumerable(), + configurable: attribute.configurable(), + }, + ); self } @@ -583,11 +587,12 @@ impl<'ctx> ObjectInitializer<'ctx> { // Accessors should have at least one function. assert!(set.is_some() || get.is_some()); - let property = PropertyDescriptor::builder() - .maybe_get(get) - .maybe_set(set) - .enumerable(attribute.enumerable()) - .configurable(attribute.configurable()); + let property = CompletePropertyDescriptor::Accessor { + get, + set, + enumerable: attribute.enumerable(), + configurable: attribute.configurable(), + }; self.object.borrow_mut().insert(key, property); self } @@ -664,11 +669,12 @@ impl<'ctx> ConstructorBuilder<'ctx> { self.prototype.insert( binding.binding, - PropertyDescriptor::builder() - .value(function) - .writable(true) - .enumerable(false) - .configurable(true), + CompletePropertyDescriptor::Data { + value: function.into(), + writable: true, + enumerable: false, + configurable: true, + }, ); self } @@ -692,11 +698,12 @@ impl<'ctx> ConstructorBuilder<'ctx> { self.constructor_object.insert( binding.binding, - PropertyDescriptor::builder() - .value(function) - .writable(true) - .enumerable(false) - .configurable(true), + CompletePropertyDescriptor::Data { + value: function.into(), + writable: true, + enumerable: false, + configurable: true, + }, ); self } @@ -707,12 +714,15 @@ impl<'ctx> ConstructorBuilder<'ctx> { K: Into, V: Into, { - let property = PropertyDescriptor::builder() - .value(value) - .writable(attribute.writable()) - .enumerable(attribute.enumerable()) - .configurable(attribute.configurable()); - self.prototype.insert(key, property); + self.prototype.insert( + key, + CompletePropertyDescriptor::Data { + value: value.into(), + writable: attribute.writable(), + enumerable: attribute.enumerable(), + configurable: attribute.configurable(), + }, + ); self } @@ -722,12 +732,15 @@ impl<'ctx> ConstructorBuilder<'ctx> { K: Into, V: Into, { - let property = PropertyDescriptor::builder() - .value(value) - .writable(attribute.writable()) - .enumerable(attribute.enumerable()) - .configurable(attribute.configurable()); - self.constructor_object.insert(key, property); + self.constructor_object.insert( + key, + CompletePropertyDescriptor::Data { + value: value.into(), + writable: attribute.writable(), + enumerable: attribute.enumerable(), + configurable: attribute.configurable(), + }, + ); self } @@ -742,12 +755,15 @@ impl<'ctx> ConstructorBuilder<'ctx> { where K: Into, { - let property = PropertyDescriptor::builder() - .maybe_get(get) - .maybe_set(set) - .enumerable(attribute.enumerable()) - .configurable(attribute.configurable()); - self.prototype.insert(key, property); + self.prototype.insert( + key, + CompletePropertyDescriptor::Accessor { + get, + set, + enumerable: attribute.enumerable(), + configurable: attribute.configurable(), + }, + ); self } @@ -762,12 +778,15 @@ impl<'ctx> ConstructorBuilder<'ctx> { where K: Into, { - let property = PropertyDescriptor::builder() - .maybe_get(get) - .maybe_set(set) - .enumerable(attribute.enumerable()) - .configurable(attribute.configurable()); - self.constructor_object.insert(key, property); + self.constructor_object.insert( + key, + CompletePropertyDescriptor::Accessor { + get, + set, + enumerable: attribute.enumerable(), + configurable: attribute.configurable(), + }, + ); self } @@ -775,7 +794,7 @@ impl<'ctx> ConstructorBuilder<'ctx> { pub fn property_descriptor(&mut self, key: K, property: P) -> &mut Self where K: Into, - P: Into, + P: Into, { let property = property.into(); self.prototype.insert(key, property); @@ -786,7 +805,7 @@ impl<'ctx> ConstructorBuilder<'ctx> { pub fn static_property_descriptor(&mut self, key: K, property: P) -> &mut Self where K: Into, - P: Into, + P: Into, { let property = property.into(); self.constructor_object.insert(key, property); @@ -866,16 +885,18 @@ impl<'ctx> ConstructorBuilder<'ctx> { /// Build the constructor function object. #[must_use] pub fn build(mut self) -> StandardConstructor { - let length = PropertyDescriptor::builder() - .value(self.length) - .writable(false) - .enumerable(false) - .configurable(true); - let name = PropertyDescriptor::builder() - .value(self.name.clone()) - .writable(false) - .enumerable(false) - .configurable(true); + let length = CompletePropertyDescriptor::Data { + value: self.length.into(), + writable: false, + enumerable: false, + configurable: true, + }; + let name = CompletePropertyDescriptor::Data { + value: self.name.clone().into(), + writable: false, + enumerable: false, + configurable: true, + }; let prototype = { if let Some(proto) = self.inherit.take() { @@ -926,11 +947,12 @@ impl<'ctx> ConstructorBuilder<'ctx> { if self.has_prototype_property { constructor.insert( PROTOTYPE, - PropertyDescriptor::builder() - .value(prototype.clone()) - .writable(false) - .enumerable(false) - .configurable(false), + CompletePropertyDescriptor::Data { + value: prototype.clone().into(), + writable: false, + enumerable: false, + configurable: false, + }, ); } @@ -941,11 +963,12 @@ impl<'ctx> ConstructorBuilder<'ctx> { let mut prototype = prototype.borrow_mut(); prototype.insert( CONSTRUCTOR, - PropertyDescriptor::builder() - .value(constructor.clone()) - .writable(true) - .enumerable(false) - .configurable(true), + CompletePropertyDescriptor::Data { + value: constructor.clone().into(), + writable: true, + enumerable: false, + configurable: true, + }, ); } diff --git a/core/engine/src/object/operations.rs b/core/engine/src/object/operations.rs index 9ddb438070a..54c7819e43b 100644 --- a/core/engine/src/object/operations.rs +++ b/core/engine/src/object/operations.rs @@ -1,5 +1,6 @@ use super::internal_methods::InternalMethodPropertyContext; use crate::js_error; +use crate::property::CompletePropertyDescriptor; use crate::value::JsVariant; use crate::{ Context, JsExpect, JsResult, JsSymbol, JsValue, @@ -629,13 +630,15 @@ impl JsObject { // b. If currentDesc is not undefined, then if let Some(current_desc) = current_desc { // i. If currentDesc.[[Configurable]] is true, return false. - if current_desc.expect_configurable() { + if current_desc.configurable() { return Ok(false); } // ii. If level is frozen and IsDataDescriptor(currentDesc) is true, then - if level.is_frozen() && current_desc.is_data_descriptor() { + if level.is_frozen() + && let CompletePropertyDescriptor::Data { writable, .. } = current_desc + { // 1. If currentDesc.[[Writable]] is true, return false. - if current_desc.expect_writable() { + if writable { return Ok(false); } } @@ -755,7 +758,7 @@ impl JsObject { .__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))?; // ii. If desc is not undefined and desc.[[Enumerable]] is true, then if let Some(desc) = desc - && desc.expect_enumerable() + && desc.enumerable() { match kind { // 1. If kind is key, append key to properties. @@ -931,8 +934,7 @@ impl JsObject { // ii. If desc is not undefined and desc.[[Enumerable]] is true, then if let Some(desc) = desc - && let Some(enumerable) = desc.enumerable() - && enumerable + && desc.enumerable() { // 1. Let propValue be ? Get(from, nextKey). let prop_value = from.__get__(&key, from.clone().into(), context)?; diff --git a/core/engine/src/object/property_map.rs b/core/engine/src/object/property_map.rs index 80cf4408272..3b51b7b14b2 100644 --- a/core/engine/src/object/property_map.rs +++ b/core/engine/src/object/property_map.rs @@ -1,5 +1,5 @@ use super::{ - JsPrototype, ObjectStorage, PropertyDescriptor, PropertyKey, + JsPrototype, ObjectStorage, PropertyKey, shape::{ ChangeTransitionAction, RootShape, Shape, UniqueShape, property_table::PropertyTableInner, @@ -7,8 +7,8 @@ use super::{ slot::{Slot, SlotAttributes}, }, }; -use crate::value::JsVariant; -use crate::{JsValue, property::PropertyDescriptorBuilder}; +use crate::{JsValue, object::JsFunction}; +use crate::{property::CompletePropertyDescriptor, value::JsVariant}; use boa_gc::{Finalize, Trace}; use rustc_hash::FxHashMap; use std::{collections::hash_map, iter::FusedIterator}; @@ -48,7 +48,7 @@ pub enum IndexedProperties { SparseElement(Box>), /// Sparse storage that keeps full property descriptors. - SparseProperty(Box>), + SparseProperty(Box>), } impl Default for IndexedProperties { @@ -67,19 +67,21 @@ impl IndexedProperties { } #[inline] - fn property_simple_value(property: &PropertyDescriptor) -> Option<&JsValue> { - if property.writable().unwrap_or(false) - && property.enumerable().unwrap_or(false) - && property.configurable().unwrap_or(false) + fn property_simple_value(property: &CompletePropertyDescriptor) -> Option<&JsValue> { + if let CompletePropertyDescriptor::Data { + value, + writable: true, + enumerable: true, + configurable: true, + } = property { - property.value() - } else { - None + return Some(value); } + None } /// Get a property descriptor if it exists. - fn get(&self, key: u32) -> Option { + fn get(&self, key: u32) -> Option { let value = match self { Self::DenseI32(vec) => vec.get(key as usize).copied()?.into(), Self::DenseF64(vec) => vec.get(key as usize).copied()?.into(), @@ -88,18 +90,18 @@ impl IndexedProperties { Self::SparseProperty(map) => return map.get(&key).cloned(), }; - Some( - PropertyDescriptorBuilder::new() - .writable(true) - .enumerable(true) - .configurable(true) - .value(value) - .build(), - ) + Some(CompletePropertyDescriptor::Data { + value, + writable: true, + enumerable: true, + configurable: true, + }) } /// Helper function for converting from a dense storage type to sparse storage type. - fn convert_dense_to_sparse(vec: &mut ThinVec) -> FxHashMap + fn convert_dense_to_sparse( + vec: &mut ThinVec, + ) -> FxHashMap where T: Into, { @@ -110,12 +112,12 @@ impl IndexedProperties { .map(|(index, value)| { ( index as u32, - PropertyDescriptorBuilder::new() - .writable(true) - .enumerable(true) - .configurable(true) - .value(value.into()) - .build(), + CompletePropertyDescriptor::Data { + value: value.into(), + writable: true, + enumerable: true, + configurable: true, + }, ) }) .collect() @@ -133,7 +135,11 @@ impl IndexedProperties { .collect() } - fn convert_to_sparse_and_insert(&mut self, key: u32, property: PropertyDescriptor) -> bool { + fn convert_to_sparse_and_insert( + &mut self, + key: u32, + property: CompletePropertyDescriptor, + ) -> bool { let mut descriptors = match self { Self::DenseI32(vec) => Self::convert_dense_to_sparse(vec), Self::DenseF64(vec) => Self::convert_dense_to_sparse(vec), @@ -143,12 +149,12 @@ impl IndexedProperties { .map(|(&index, value)| { ( index, - PropertyDescriptorBuilder::new() - .writable(true) - .enumerable(true) - .configurable(true) - .value(value.clone()) - .build(), + CompletePropertyDescriptor::Data { + value: value.clone(), + writable: true, + enumerable: true, + configurable: true, + }, ) }) .collect(), @@ -163,7 +169,7 @@ impl IndexedProperties { } /// Inserts a property descriptor with the specified key. - fn insert(&mut self, key: u32, property: PropertyDescriptor) -> bool { + fn insert(&mut self, key: u32, property: CompletePropertyDescriptor) -> bool { let Some(value) = Self::property_simple_value(&property) else { return self.convert_to_sparse_and_insert(key, property); }; @@ -297,15 +303,17 @@ impl IndexedProperties { replaced } Self::SparseElement(map) => map.insert(key, value.clone()).is_some(), - Self::SparseProperty(map) => { - let descriptor = PropertyDescriptorBuilder::new() - .value(value.clone()) - .writable(true) - .enumerable(true) - .configurable(true) - .build(); - map.insert(key, descriptor).is_some() - } + Self::SparseProperty(map) => map + .insert( + key, + CompletePropertyDescriptor::Data { + value: value.clone(), + writable: true, + enumerable: true, + configurable: true, + }, + ) + .is_some(), } } @@ -473,7 +481,7 @@ impl IndexedProperties { impl<'a> IntoIterator for &'a IndexedProperties { type IntoIter = IndexProperties<'a>; - type Item = (u32, PropertyDescriptor); + type Item = (u32, CompletePropertyDescriptor); fn into_iter(self) -> Self::IntoIter { self.iter() } @@ -531,7 +539,7 @@ impl PropertyMap { /// Get the property with the given key from the [`PropertyMap`]. #[must_use] - pub fn get(&self, key: &PropertyKey) -> Option { + pub fn get(&self, key: &PropertyKey) -> Option { if let PropertyKey::Index(index) = key { return self.indexed_properties.get(index.get()); } @@ -548,7 +556,7 @@ impl PropertyMap { &self, key: &PropertyKey, out_slot: &mut Slot, - ) -> Option { + ) -> Option { if let PropertyKey::Index(index) = key { return self.indexed_properties.get(index.get()); } @@ -567,27 +575,34 @@ impl PropertyMap { /// Get the property with the given key from the [`PropertyMap`]. #[must_use] - pub(crate) fn get_storage(&self, Slot { index, attributes }: Slot) -> PropertyDescriptor { + pub(crate) fn get_storage( + &self, + Slot { index, attributes }: Slot, + ) -> CompletePropertyDescriptor { let index = index as usize; - let mut builder = PropertyDescriptor::builder() - .configurable(attributes.contains(SlotAttributes::CONFIGURABLE)) - .enumerable(attributes.contains(SlotAttributes::ENUMERABLE)); if attributes.is_accessor_descriptor() { - if attributes.has_get() { - builder = builder.get(self.storage[index].clone()); - } - if attributes.has_set() { - builder = builder.set(self.storage[index + 1].clone()); + CompletePropertyDescriptor::Accessor { + set: self.storage[index + 1] + .as_object() + .map(JsFunction::from_object_unchecked), + get: self.storage[index] + .as_object() + .map(JsFunction::from_object_unchecked), + configurable: attributes.contains(SlotAttributes::CONFIGURABLE), + enumerable: attributes.contains(SlotAttributes::ENUMERABLE), } } else { - builder = builder.writable(attributes.contains(SlotAttributes::WRITABLE)); - builder = builder.value(self.storage[index].clone()); + CompletePropertyDescriptor::Data { + value: self.storage[index].clone(), + writable: attributes.contains(SlotAttributes::WRITABLE), + configurable: attributes.contains(SlotAttributes::CONFIGURABLE), + enumerable: attributes.contains(SlotAttributes::ENUMERABLE), + } } - builder.build() } /// Insert the given property descriptor with the given key [`PropertyMap`]. - pub fn insert(&mut self, key: &PropertyKey, property: PropertyDescriptor) -> bool { + pub fn insert(&mut self, key: &PropertyKey, property: CompletePropertyDescriptor) -> bool { let mut dummy_slot = Slot::new(); self.insert_with_slot(key, property, &mut dummy_slot) } @@ -596,7 +611,7 @@ impl PropertyMap { pub(crate) fn insert_with_slot( &mut self, key: &PropertyKey, - property: PropertyDescriptor, + property: CompletePropertyDescriptor, out_slot: &mut Slot, ) -> bool { if let PropertyKey::Index(index) = key { @@ -627,23 +642,14 @@ impl PropertyMap { } } - if attributes.is_accessor_descriptor() { - if attributes.has_get() { - self.storage[index] = property - .get() - .cloned() - .map(JsValue::new) - .unwrap_or_default(); + match property { + CompletePropertyDescriptor::Data { value, .. } => { + self.storage[index] = value; } - if attributes.has_set() { - self.storage[index + 1] = property - .set() - .cloned() - .map(JsValue::new) - .unwrap_or_default(); + CompletePropertyDescriptor::Accessor { get, set, .. } => { + self.storage[index + 1] = set.map(JsValue::new).unwrap_or_default(); + self.storage[index] = get.map(JsValue::new).unwrap_or_default(); } - } else { - self.storage[index] = property.expect_value().clone(); } out_slot.index = slot.index; out_slot.attributes = @@ -670,24 +676,14 @@ impl PropertyMap { out_slot.attributes = (out_slot.attributes & SlotAttributes::INLINE_CACHE_BITS) | attributes; - if attributes.is_accessor_descriptor() { - self.storage.push( - property - .get() - .cloned() - .map(JsValue::new) - .unwrap_or_default(), - ); - self.storage.push( - property - .set() - .cloned() - .map(JsValue::new) - .unwrap_or_default(), - ); - } else { - self.storage - .push(property.value().cloned().unwrap_or_default()); + match property { + CompletePropertyDescriptor::Data { value, .. } => { + self.storage.push(value); + } + CompletePropertyDescriptor::Accessor { get, set, .. } => { + self.storage.push(get.map(JsValue::new).unwrap_or_default()); + self.storage.push(set.map(JsValue::new).unwrap_or_default()); + } } false @@ -885,11 +881,11 @@ pub enum IndexProperties<'a> { SparseElement(hash_map::Iter<'a, u32, JsValue>), /// An iterator over sparse, `HashMap` backed indexed property entries storing descriptors. - SparseProperty(hash_map::Iter<'a, u32, PropertyDescriptor>), + SparseProperty(hash_map::Iter<'a, u32, CompletePropertyDescriptor>), } impl Iterator for IndexProperties<'_> { - type Item = (u32, PropertyDescriptor); + type Item = (u32, CompletePropertyDescriptor); fn next(&mut self) -> Option { let (index, value) = match self { @@ -910,12 +906,12 @@ impl Iterator for IndexProperties<'_> { Some(( index as u32, - PropertyDescriptorBuilder::new() - .writable(true) - .configurable(true) - .enumerable(true) - .value(value.clone()) - .build(), + CompletePropertyDescriptor::Data { + value, + writable: true, + enumerable: true, + configurable: true, + }, )) } @@ -952,11 +948,11 @@ pub enum IndexPropertyKeys<'a> { /// An iterator over dense, Vec backed indexed property entries of an `Object`. Dense(std::ops::Range), - /// An iterator over sparse, `HashMap` backed indexed property entries of an `Object`. - SparseProperty(hash_map::Keys<'a, u32, PropertyDescriptor>), - /// An iterator over sparse, `HashMap` backed indexed property entries storing values. SparseElement(hash_map::Keys<'a, u32, JsValue>), + + /// An iterator over sparse, `HashMap` backed indexed property entries of an `Object`. + SparseProperty(hash_map::Keys<'a, u32, CompletePropertyDescriptor>), } impl Iterator for IndexPropertyKeys<'_> { @@ -1007,15 +1003,15 @@ pub enum IndexPropertyValues<'a> { /// An iterator over dense, Vec backed indexed property entries of an `Object`. DenseElement(std::slice::Iter<'a, JsValue>), - /// An iterator over sparse, `HashMap` backed indexed property entries of an `Object`. - SparseProperty(hash_map::Values<'a, u32, PropertyDescriptor>), - /// An iterator over sparse, `HashMap` backed indexed property entries storing values. SparseElement(hash_map::Values<'a, u32, JsValue>), + + /// An iterator over sparse, `HashMap` backed indexed property entries of an `Object`. + SparseProperty(hash_map::Values<'a, u32, CompletePropertyDescriptor>), } impl Iterator for IndexPropertyValues<'_> { - type Item = PropertyDescriptor; + type Item = CompletePropertyDescriptor; fn next(&mut self) -> Option { let value = match self { @@ -1026,14 +1022,12 @@ impl Iterator for IndexPropertyValues<'_> { Self::SparseElement(map) => map.next().cloned()?, }; - Some( - PropertyDescriptorBuilder::new() - .writable(true) - .configurable(true) - .enumerable(true) - .value(value) - .build(), - ) + Some(CompletePropertyDescriptor::Data { + value, + writable: true, + enumerable: true, + configurable: true, + }) } #[inline] diff --git a/core/engine/src/property/mod.rs b/core/engine/src/property/mod.rs index 4bad2a263f6..a34f783d6db 100644 --- a/core/engine/src/property/mod.rs +++ b/core/engine/src/property/mod.rs @@ -19,7 +19,9 @@ mod attribute; mod nonmaxu32; use crate::{ - JsString, JsSymbol, JsValue, js_string, object::shape::slot::SlotAttributes, string::JsStr, + JsString, JsSymbol, JsValue, js_string, + object::{JsFunction, shape::slot::SlotAttributes}, + string::JsStr, }; use boa_gc::{Finalize, Trace}; use std::{fmt, iter::FusedIterator}; @@ -49,14 +51,16 @@ pub use {attribute::Attribute, nonmaxu32::NonMaxU32}; /// [spec]: https://tc39.es/ecma262/#sec-property-descriptor-specification-type /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty #[derive(Default, Debug, Clone, Trace, Finalize)] +#[boa_gc(unsafe_no_drop)] pub struct PropertyDescriptor { enumerable: Option, configurable: Option, - kind: DescriptorKind, + pub(crate) kind: DescriptorKind, } /// `DescriptorKind` represents the different kinds of property descriptors. #[derive(Debug, Default, Clone, Trace, Finalize)] +#[boa_gc(unsafe_no_drop)] pub enum DescriptorKind { /// A data property descriptor. Data { @@ -296,6 +300,32 @@ impl PropertyDescriptor { self } + /// Creates an accessor property descriptor with default values. + #[inline] + #[must_use] + pub fn into_concrete_accessor_defaulted(self) -> CompletePropertyDescriptor { + match self.kind { + DescriptorKind::Accessor { set, get } => CompletePropertyDescriptor::Accessor { + get: get + .as_ref() + .and_then(JsValue::as_object) + .map(JsFunction::from_object_unchecked), + set: set + .as_ref() + .and_then(JsValue::as_object) + .map(JsFunction::from_object_unchecked), + enumerable: self.enumerable.unwrap_or(false), + configurable: self.configurable.unwrap_or(false), + }, + _ => CompletePropertyDescriptor::Accessor { + get: None, + set: None, + enumerable: self.enumerable.unwrap_or(false), + configurable: self.configurable.unwrap_or(false), + }, + } + } + /// Creates a data property descriptor with default values. #[must_use] pub fn into_data_defaulted(mut self) -> Self { @@ -320,75 +350,55 @@ impl PropertyDescriptor { self } - /// Creates an generic property descriptor with default values. - #[inline] + /// Creates a data property descriptor with default values. #[must_use] - pub fn complete_property_descriptor(self) -> Self { - PropertyDescriptorBuilder { inner: self } - .complete_with_defaults() - .build() - } - - /// Fills the fields of the `PropertyDescriptor` that are not set - /// with fields from the given `PropertyDescriptor`. - /// - /// # Panics - /// - /// Panics if the given `PropertyDescriptor` is not compatible with this one. - #[inline] - pub fn fill_with(&mut self, mut desc: Self) { - match (&mut self.kind, &mut desc.kind) { - ( - DescriptorKind::Data { value, writable }, - DescriptorKind::Data { - value: desc_value, - writable: desc_writable, - }, - ) => { - if desc_value.is_some() { - std::mem::swap(value, desc_value); - } - if desc_writable.is_some() { - std::mem::swap(writable, desc_writable); - } - } - ( - DescriptorKind::Accessor { get, set }, - DescriptorKind::Accessor { - get: desc_get, - set: desc_set, - }, - ) => { - if desc_get.is_some() { - std::mem::swap(get, desc_get); - } - if desc_set.is_some() { - std::mem::swap(set, desc_set); - } - } - (_, DescriptorKind::Generic) => {} - _ => panic!("Tried to fill a descriptor with an incompatible descriptor"), - } - - if let Some(enumerable) = desc.enumerable { - self.enumerable = Some(enumerable); - } - if let Some(configurable) = desc.configurable { - self.configurable = Some(configurable); + pub fn into_concrete_data_defaulted(self) -> CompletePropertyDescriptor { + match self.kind { + DescriptorKind::Data { value, writable } => CompletePropertyDescriptor::Data { + value: value.unwrap_or_default(), + writable: writable.unwrap_or(false), + enumerable: self.enumerable.unwrap_or(false), + configurable: self.configurable.unwrap_or(false), + }, + _ => CompletePropertyDescriptor::Data { + value: JsValue::undefined(), + writable: false, + enumerable: self.enumerable.unwrap_or(false), + configurable: self.configurable.unwrap_or(false), + }, } } - pub(crate) fn to_slot_attributes(&self) -> SlotAttributes { - let mut attributes = SlotAttributes::empty(); - attributes.set(SlotAttributes::CONFIGURABLE, self.expect_configurable()); - attributes.set(SlotAttributes::ENUMERABLE, self.expect_enumerable()); - if self.is_data_descriptor() { - attributes.set(SlotAttributes::WRITABLE, self.expect_writable()); - } else { - attributes.set(SlotAttributes::GET, self.get().is_some()); - attributes.set(SlotAttributes::SET, self.set().is_some()); + /// Creates an generic property descriptor with default values. + #[inline] + #[must_use] + pub fn complete_property_descriptor(self) -> CompletePropertyDescriptor { + match self.kind { + DescriptorKind::Generic => CompletePropertyDescriptor::Data { + value: JsValue::undefined(), + writable: false, + enumerable: self.enumerable.unwrap_or(false), + configurable: self.configurable.unwrap_or(false), + }, + DescriptorKind::Data { value, writable } => CompletePropertyDescriptor::Data { + value: value.unwrap_or_default(), + writable: writable.unwrap_or(false), + enumerable: self.enumerable.unwrap_or(false), + configurable: self.configurable.unwrap_or(false), + }, + DescriptorKind::Accessor { set, get } => CompletePropertyDescriptor::Accessor { + get: get + .as_ref() + .and_then(JsValue::as_object) + .map(JsFunction::from_object_unchecked), + set: set + .as_ref() + .and_then(JsValue::as_object) + .map(JsFunction::from_object_unchecked), + enumerable: self.enumerable.unwrap_or(false), + configurable: self.configurable.unwrap_or(false), + }, } - attributes } } @@ -604,6 +614,258 @@ impl From for PropertyDescriptor { } } +/// A completed [`PropertyDescriptor`]. +#[derive(Debug, Clone, Trace, Finalize)] +#[boa_gc(unsafe_no_drop)] +pub enum CompletePropertyDescriptor { + /// A data property descriptor. + Data { + /// The value of the property. + value: JsValue, + /// A boolean indicating if the property can be changed. + writable: bool, + /// A boolean indicating if the property is enumerable. + enumerable: bool, + /// A boolean indicating if the property descriptor can be changed or deleted. + configurable: bool, + }, + /// A accessor property descriptor. + Accessor { + /// The getter function for the property. + get: Option, + /// The setter function for the property. + set: Option, + /// A boolean indicating if the property is enumerable. + enumerable: bool, + /// A boolean indicating if the property descriptor can be changed or deleted. + configurable: bool, + }, +} + +impl From for PropertyDescriptor { + fn from(value: CompletePropertyDescriptor) -> Self { + match value { + CompletePropertyDescriptor::Data { + value, + writable, + configurable, + enumerable, + } => PropertyDescriptor::builder() + .value(value) + .writable(writable) + .configurable(configurable) + .enumerable(enumerable) + .build(), + CompletePropertyDescriptor::Accessor { + get, + set, + configurable, + enumerable, + } => PropertyDescriptor::builder() + .get(get.map(JsValue::new).unwrap_or_default()) + .set(set.map(JsValue::new).unwrap_or_default()) + .configurable(configurable) + .enumerable(enumerable) + .build(), + } + } +} + +impl CompletePropertyDescriptor { + /// Return the value of the property. + #[must_use] + pub fn value(&self) -> Option<&JsValue> { + if let Self::Data { value, .. } = self { + return Some(value); + } + None + } + + /// Returns if the property descriptor is writable. + #[must_use] + pub fn writable(&self) -> Option { + match self { + Self::Data { writable, .. } => Some(*writable), + Self::Accessor { .. } => None, + } + } + + /// Returns if the property descriptor is enumerable. + #[must_use] + pub fn enumerable(&self) -> bool { + match self { + Self::Data { enumerable, .. } | Self::Accessor { enumerable, .. } => *enumerable, + } + } + + /// Returns if the property descriptor is configurable. + #[must_use] + pub fn configurable(&self) -> bool { + match self { + Self::Data { configurable, .. } | Self::Accessor { configurable, .. } => *configurable, + } + } + + /// Returns the getter of the property descriptor. + #[must_use] + pub fn get(&self) -> Option<&JsFunction> { + if let Self::Accessor { get, .. } = self { + return get.as_ref(); + } + None + } + + /// Returns the setter of the property descriptor. + #[must_use] + pub fn set(&self) -> Option<&JsFunction> { + if let Self::Accessor { set, .. } = self { + return set.as_ref(); + } + None + } + + /// Returns `true` if this property descriptor is a data descriptor. + #[must_use] + pub fn is_data_descriptor(&self) -> bool { + matches!(self, Self::Data { .. }) + } + + /// Returns `true` if this property descriptor is a accessor descriptor. + #[must_use] + pub fn is_accessor_descriptor(&self) -> bool { + matches!(self, Self::Accessor { .. }) + } + + pub(crate) fn to_slot_attributes(&self) -> SlotAttributes { + let mut attributes = SlotAttributes::empty(); + match self { + Self::Data { + writable, + configurable, + enumerable, + .. + } => { + attributes.set(SlotAttributes::WRITABLE, *writable); + attributes.set(SlotAttributes::CONFIGURABLE, *configurable); + attributes.set(SlotAttributes::ENUMERABLE, *enumerable); + } + Self::Accessor { + configurable, + enumerable, + .. + } => { + attributes.set(SlotAttributes::GET, true); + attributes.set(SlotAttributes::SET, true); + attributes.set(SlotAttributes::CONFIGURABLE, *configurable); + attributes.set(SlotAttributes::ENUMERABLE, *enumerable); + } + } + attributes + } + + /// Creates an accessor property descriptor with default values. + #[inline] + #[must_use] + pub fn into_accessor_defaulted(self) -> Self { + match self { + Self::Data { + enumerable, + configurable, + .. + } => Self::Accessor { + get: None, + set: None, + enumerable, + configurable, + }, + Self::Accessor { .. } => self, + } + } + + /// Creates a data property descriptor with default values. + #[must_use] + pub fn into_data_defaulted(self) -> Self { + match self { + Self::Data { .. } => self, + Self::Accessor { + enumerable, + configurable, + .. + } => Self::Data { + value: JsValue::undefined(), + writable: false, + enumerable, + configurable, + }, + } + } + + /// Fills the fields of the `PropertyDescriptor` that are not set + /// with fields from the given `PropertyDescriptor`. + /// + /// # Panics + /// + /// Panics if the given `PropertyDescriptor` is not compatible with this one. + #[inline] + #[must_use] + pub fn fill_with(mut self, desc: PropertyDescriptor) -> Self { + match (&mut self, desc.kind) { + ( + Self::Data { + value, + writable, + enumerable, + configurable, + }, + DescriptorKind::Data { + value: desc_value, + writable: desc_writable, + }, + ) => { + if let Some(desc_value) = desc_value { + *value = desc_value; + } + if let Some(desc_writable) = desc_writable { + *writable = desc_writable; + } + if let Some(desc_enumerable) = desc.enumerable { + *enumerable = desc_enumerable; + } + if let Some(desc_configurable) = desc.configurable { + *configurable = desc_configurable; + } + } + ( + Self::Accessor { + get, + set, + enumerable, + configurable, + }, + DescriptorKind::Accessor { + get: desc_get, + set: desc_set, + }, + ) => { + if let Some(desc_get) = desc_get { + *get = desc_get.as_object().map(JsFunction::from_object_unchecked); + } + if let Some(desc_set) = desc_set { + *set = desc_set.as_object().map(JsFunction::from_object_unchecked); + } + if let Some(desc_enumerable) = desc.enumerable { + *enumerable = desc_enumerable; + } + if let Some(desc_configurable) = desc.configurable { + *configurable = desc_configurable; + } + } + _ => panic!("Tried to fill a descriptor with an incompatible descriptor"), + } + self + } +} + /// This abstracts away the need for `IsPropertyKey` by transforming the `PropertyKey` /// values into an enum with both valid types: String and Symbol /// diff --git a/core/engine/src/property/nonmaxu32.rs b/core/engine/src/property/nonmaxu32.rs index c3a7fddcb25..262a7c3904c 100644 --- a/core/engine/src/property/nonmaxu32.rs +++ b/core/engine/src/property/nonmaxu32.rs @@ -5,6 +5,8 @@ pub struct NonMaxU32 { } impl NonMaxU32 { + pub(crate) const ZERO: Self = Self { inner: 0 }; + /// Creates a non-max `u32`. /// /// # Safety diff --git a/core/engine/src/value/conversions/serde_json.rs b/core/engine/src/value/conversions/serde_json.rs index 4350b4c30a6..10259bbdcf5 100644 --- a/core/engine/src/value/conversions/serde_json.rs +++ b/core/engine/src/value/conversions/serde_json.rs @@ -7,7 +7,7 @@ use crate::{ error::JsNativeError, js_string, object::JsObject, - property::{PropertyDescriptor, PropertyKey}, + property::{CompletePropertyDescriptor, PropertyKey}, }; use serde_json::{Map, Value}; use std::collections::HashSet; @@ -68,11 +68,12 @@ impl JsValue { Value::Object(obj) => { let js_obj = JsObject::with_object_proto(context.intrinsics()); for (key, value) in obj { - let property = PropertyDescriptor::builder() - .value(Self::from_json(value, context)?) - .writable(true) - .enumerable(true) - .configurable(true); + let property = CompletePropertyDescriptor::Data { + value: Self::from_json(value, context)?, + writable: true, + enumerable: true, + configurable: true, + }; js_obj .borrow_mut() .insert(js_string!(key.clone()), property); diff --git a/core/engine/src/value/display/arguments.rs b/core/engine/src/value/display/arguments.rs index 42bcb9ba16a..ff78f519481 100644 --- a/core/engine/src/value/display/arguments.rs +++ b/core/engine/src/value/display/arguments.rs @@ -1,5 +1,5 @@ use crate::builtins::function::arguments::MappedArguments; -use crate::property::DescriptorKind; +use crate::property::CompletePropertyDescriptor; use crate::value::display::value; use crate::{JsObject, JsValue, js_string}; use std::collections::HashSet; @@ -61,22 +61,18 @@ pub(super) fn log_arguments_to( } else { let borrow = x.borrow(); if let Some(d) = borrow.properties().get(&i.into()) { - match d.kind() { - DescriptorKind::Data { value, .. } => { - if let Some(v) = value { - write!( - f, - "{}", - CompactValue { - value: v, - print_internals - } - )?; - } else { - f.write_str("undefined")?; - } + match &d { + CompletePropertyDescriptor::Data { value, .. } => { + write!( + f, + "{}", + CompactValue { + value, + print_internals + } + )?; } - DescriptorKind::Accessor { get, set } => { + CompletePropertyDescriptor::Accessor { get, set, .. } => { let label = match (get.is_some(), set.is_some()) { (true, true) => "[Getter/Setter]", (true, false) => "[Getter]", @@ -85,7 +81,6 @@ pub(super) fn log_arguments_to( }; f.write_str(label)?; } - DescriptorKind::Generic => f.write_str("undefined")?, } } else { f.write_str("")?; diff --git a/core/engine/src/value/display/array.rs b/core/engine/src/value/display/array.rs index 8ac39697ca3..df253910046 100644 --- a/core/engine/src/value/display/array.rs +++ b/core/engine/src/value/display/array.rs @@ -1,7 +1,7 @@ use std::collections::HashSet; use std::fmt; -use crate::{JsObject, JsValue, js_string, property::DescriptorKind}; +use crate::{JsObject, JsValue, js_string, property::CompletePropertyDescriptor}; pub(super) fn log_array_to( f: &mut fmt::Formatter<'_>, @@ -39,15 +39,11 @@ pub(super) fn log_array_to( } if let Some(desc) = x.borrow().properties().get(&i.into()) { - match desc.kind() { - DescriptorKind::Data { value, .. } => { - if let Some(value) = value { - super::value::log_value_to(f, value, print_internals, false)?; - } else { - f.write_str("undefined")?; - } + match desc { + CompletePropertyDescriptor::Data { value, .. } => { + super::value::log_value_to(f, &value, print_internals, false)?; } - DescriptorKind::Accessor { get, set } => { + CompletePropertyDescriptor::Accessor { get, set, .. } => { let display = match (get.is_some(), set.is_some()) { (true, true) => "[Getter/Setter]", (true, false) => "[Getter]", @@ -56,9 +52,6 @@ pub(super) fn log_array_to( }; f.write_str(display)?; } - DescriptorKind::Generic => { - unreachable!("found generic descriptor in array") - } } } else { f.write_str("")?; @@ -107,21 +100,17 @@ pub(super) fn log_array_compact( f.write_str(", ")?; } if let Some(desc) = x.borrow().properties().get(&i.into()) { - match desc.kind() { - DescriptorKind::Data { value, .. } => { - if let Some(value) = value { - super::value::log_value_compact( - f, - value, - depth + 1, - print_internals, - encounters, - )?; - } else { - f.write_str("undefined")?; - } + match &desc { + CompletePropertyDescriptor::Data { value, .. } => { + super::value::log_value_compact( + f, + value, + depth + 1, + print_internals, + encounters, + )?; } - DescriptorKind::Accessor { get, set } => { + CompletePropertyDescriptor::Accessor { get, set, .. } => { let display = match (get.is_some(), set.is_some()) { (true, true) => "[Getter/Setter]", (true, false) => "[Getter]", @@ -130,9 +119,6 @@ pub(super) fn log_array_compact( }; f.write_str(display)?; } - DescriptorKind::Generic => { - unreachable!("found generic descriptor in array") - } } } else { f.write_str("")?; diff --git a/core/engine/src/value/display/object.rs b/core/engine/src/value/display/object.rs index e69a7ce3ac2..64a7d222f0d 100644 --- a/core/engine/src/value/display/object.rs +++ b/core/engine/src/value/display/object.rs @@ -1,10 +1,8 @@ use std::collections::HashSet; use std::fmt::{self, Display, Write}; -use crate::{ - JsObject, JsString, JsValue, js_string, - property::{DescriptorKind, PropertyKey}, -}; +use crate::property::CompletePropertyDescriptor; +use crate::{JsObject, JsString, JsValue, js_string, property::PropertyKey}; fn print_obj_value_internals( f: &mut fmt::Formatter<'_>, @@ -59,17 +57,25 @@ fn print_obj_value_props( .expect("There should be a value"); write!(f, "{:>width$}{}: ", "", key, width = indent)?; - if val.is_data_descriptor() { - let v = val.expect_value(); - log_object_to_internal(f, v, encounters, indent.wrapping_add(4), print_internals)?; - } else { - let display = match (val.set().is_some(), val.get().is_some()) { - (true, true) => "Getter & Setter", - (true, false) => "Setter", - (false, true) => "Getter", - _ => "No Getter/Setter", - }; - write!(f, "{display}")?; + match &val { + CompletePropertyDescriptor::Data { value, .. } => { + log_object_to_internal( + f, + value, + encounters, + indent.wrapping_add(4), + print_internals, + )?; + } + CompletePropertyDescriptor::Accessor { get, set, .. } => { + let display = match (set.is_some(), get.is_some()) { + (true, true) => "Getter & Setter", + (true, false) => "Setter", + (false, true) => "Getter", + _ => "No Getter/Setter", + }; + write!(f, "{display}")?; + } } } f.write_char('\n')?; @@ -194,21 +200,17 @@ pub(super) fn log_plain_object_compact( write!(f, "{key}: ")?; if let Some(val) = obj.borrow().properties().get(key) { - match val.kind() { - DescriptorKind::Data { value, .. } => { - if let Some(value) = value { - super::value::log_value_compact( - f, - value, - depth + 1, - print_internals, - encounters, - )?; - } else { - f.write_str("undefined")?; - } + match &val { + CompletePropertyDescriptor::Data { value, .. } => { + super::value::log_value_compact( + f, + value, + depth + 1, + print_internals, + encounters, + )?; } - DescriptorKind::Accessor { get, set } => { + CompletePropertyDescriptor::Accessor { get, set, .. } => { let display = match (get.is_some(), set.is_some()) { (true, true) => "[Getter/Setter]", (true, false) => "[Getter]", @@ -217,9 +219,6 @@ pub(super) fn log_plain_object_compact( }; f.write_str(display)?; } - DescriptorKind::Generic => { - f.write_str("undefined")?; - } } } } diff --git a/core/engine/src/value/display/value.rs b/core/engine/src/value/display/value.rs index 448f999667b..d466cce4df9 100644 --- a/core/engine/src/value/display/value.rs +++ b/core/engine/src/value/display/value.rs @@ -1,6 +1,7 @@ use std::collections::HashSet; use std::fmt::{self, Display}; +use crate::property::CompletePropertyDescriptor; use crate::{ JsError, JsString, JsValue, JsVariant, builtins::{ @@ -18,7 +19,7 @@ use crate::{ weak_set::NativeWeakSet, }, js_string, - property::{PropertyDescriptor, PropertyKey}, + property::PropertyKey, }; /// Maximum nesting depth before objects/arrays are collapsed @@ -59,7 +60,7 @@ pub(crate) fn log_value_to( let name: std::borrow::Cow<'static, str> = v .get_property(&js_string!("name").into()) .as_ref() - .and_then(PropertyDescriptor::value) + .and_then(CompletePropertyDescriptor::value) .map_or_else( || "".into(), |v| { @@ -75,7 +76,7 @@ pub(crate) fn log_value_to( let message = v .get_property(&js_string!("message").into()) .as_ref() - .and_then(PropertyDescriptor::value) + .and_then(CompletePropertyDescriptor::value) .map(|v| { v.as_string().as_ref().map_or_else( || v.display().to_string(), diff --git a/core/engine/src/value/tests.rs b/core/engine/src/value/tests.rs index ee21a6e967d..bf5cba14cd3 100644 --- a/core/engine/src/value/tests.rs +++ b/core/engine/src/value/tests.rs @@ -210,7 +210,7 @@ fn string_length_is_not_enumerable() { ) .unwrap() .unwrap(); - !length_desc.expect_enumerable() + !length_desc.enumerable() })]); } @@ -228,7 +228,8 @@ fn string_length_is_in_utf16_codeunits() { .unwrap() .unwrap(); length_desc - .expect_value() + .value() + .expect("length should be data property descriptor") .to_integer_or_infinity(context) .unwrap() == IntegerOrInfinity::Integer(2) diff --git a/core/engine/src/vm/opcode/define/class/getter.rs b/core/engine/src/vm/opcode/define/class/getter.rs index 11432391c08..656193e14ae 100644 --- a/core/engine/src/vm/opcode/define/class/getter.rs +++ b/core/engine/src/vm/opcode/define/class/getter.rs @@ -4,7 +4,7 @@ use crate::{ Context, JsResult, builtins::function::{OrdinaryFunction, set_function_name}, object::internal_methods::InternalMethodPropertyContext, - property::PropertyDescriptor, + property::{CompletePropertyDescriptor, PropertyDescriptor}, vm::opcode::{IndexOperand, Operation, RegisterOperand}, }; @@ -43,7 +43,7 @@ impl DefineClassStaticGetterByName { let set = class .__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))? .as_ref() - .and_then(PropertyDescriptor::set) + .and_then(CompletePropertyDescriptor::set) .cloned(); class.__define_own_property__( &key, @@ -100,7 +100,7 @@ impl DefineClassGetterByName { let set = class_proto .__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))? .as_ref() - .and_then(PropertyDescriptor::set) + .and_then(CompletePropertyDescriptor::set) .cloned(); class_proto.__define_own_property__( &key, @@ -156,7 +156,7 @@ impl DefineClassStaticGetterByValue { let set = class .__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))? .as_ref() - .and_then(PropertyDescriptor::set) + .and_then(CompletePropertyDescriptor::set) .cloned(); class.define_property_or_throw( key, @@ -211,7 +211,7 @@ impl DefineClassGetterByValue { let set = class_proto .__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))? .as_ref() - .and_then(PropertyDescriptor::set) + .and_then(CompletePropertyDescriptor::set) .cloned(); class_proto.__define_own_property__( &key, diff --git a/core/engine/src/vm/opcode/define/class/setter.rs b/core/engine/src/vm/opcode/define/class/setter.rs index 431b5937fa5..67d9e1ab224 100644 --- a/core/engine/src/vm/opcode/define/class/setter.rs +++ b/core/engine/src/vm/opcode/define/class/setter.rs @@ -4,7 +4,7 @@ use crate::{ Context, JsResult, builtins::function::{OrdinaryFunction, set_function_name}, object::internal_methods::InternalMethodPropertyContext, - property::PropertyDescriptor, + property::{CompletePropertyDescriptor, PropertyDescriptor}, vm::opcode::{IndexOperand, Operation, RegisterOperand}, }; @@ -43,7 +43,7 @@ impl DefineClassStaticSetterByName { let get = class .__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))? .as_ref() - .and_then(PropertyDescriptor::get) + .and_then(CompletePropertyDescriptor::get) .cloned(); class.__define_own_property__( @@ -101,7 +101,7 @@ impl DefineClassSetterByName { let get = class_proto .__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))? .as_ref() - .and_then(PropertyDescriptor::get) + .and_then(CompletePropertyDescriptor::get) .cloned(); class_proto.__define_own_property__( @@ -158,7 +158,7 @@ impl DefineClassStaticSetterByValue { let get = class .__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))? .as_ref() - .and_then(PropertyDescriptor::get) + .and_then(CompletePropertyDescriptor::get) .cloned(); class.define_property_or_throw( @@ -215,7 +215,7 @@ impl DefineClassSetterByValue { let get = class_proto .__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))? .as_ref() - .and_then(PropertyDescriptor::get) + .and_then(CompletePropertyDescriptor::get) .cloned(); class_proto.__define_own_property__( diff --git a/core/engine/src/vm/opcode/set/property.rs b/core/engine/src/vm/opcode/set/property.rs index 6bf6d66c1f6..97029c4191d 100644 --- a/core/engine/src/vm/opcode/set/property.rs +++ b/core/engine/src/vm/opcode/set/property.rs @@ -1,5 +1,6 @@ use crate::JsExpect; use crate::JsValue; +use crate::property::CompletePropertyDescriptor; use crate::value::JsVariant; use crate::vm::opcode::{IndexOperand, RegisterOperand}; use crate::{ @@ -232,7 +233,7 @@ impl SetPropertyGetterByName { let set = object .__get_own_property__(&name, &mut InternalMethodPropertyContext::new(context))? .as_ref() - .and_then(PropertyDescriptor::set) + .and_then(CompletePropertyDescriptor::set) .cloned(); object.__define_own_property__( &name, @@ -276,7 +277,7 @@ impl SetPropertyGetterByValue { let set = object .__get_own_property__(&name, &mut InternalMethodPropertyContext::new(context))? .as_ref() - .and_then(PropertyDescriptor::set) + .and_then(CompletePropertyDescriptor::set) .cloned(); object.__define_own_property__( &name, @@ -325,7 +326,7 @@ impl SetPropertySetterByName { let get = object .__get_own_property__(&name, &mut InternalMethodPropertyContext::new(context))? .as_ref() - .and_then(PropertyDescriptor::get) + .and_then(CompletePropertyDescriptor::get) .cloned(); object.__define_own_property__( &name, @@ -370,7 +371,7 @@ impl SetPropertySetterByValue { let get = object .__get_own_property__(&name, &mut InternalMethodPropertyContext::new(context))? .as_ref() - .and_then(PropertyDescriptor::get) + .and_then(CompletePropertyDescriptor::get) .cloned(); object.__define_own_property__( &name,