Skip to content

Add completed PropertyDescriptor type #5284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
HalidOdat wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add completed PropertyDescriptor type #5284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 36 additions & 18 deletions core/engine/src/builtins/array/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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);
}

Expand All @@ -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(
Expand Down Expand Up @@ -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)? {
Expand All @@ -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);
}

Expand Down
94 changes: 49 additions & 45 deletions core/engine/src/builtins/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -67,23 +67,25 @@ 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,
},
);

{
let mut prototype = self.prototype.borrow_mut();
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(),
},
);
}
}
Expand All @@ -109,19 +111,21 @@ impl<S: ApplyToObject + IsConstructor> ApplyToObject for Callable<S> {
}
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);
Expand Down Expand Up @@ -641,11 +645,12 @@ impl<T> 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
}
Expand All @@ -656,12 +661,15 @@ impl<T> BuiltInBuilder<'_, T> {
K: Into<PropertyKey>,
V: Into<JsValue>,
{
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
}

Expand All @@ -676,21 +684,17 @@ impl<T> BuiltInBuilder<'_, T> {
where
K: Into<PropertyKey>,
{
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
}
Expand Down
18 changes: 8 additions & 10 deletions core/engine/src/builtins/function/arguments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -283,7 +283,7 @@ pub(crate) fn arguments_exotic_get_own_property(
obj: &JsObject,
key: &PropertyKey,
context: &mut InternalMethodPropertyContext<'_>,
) -> JsResult<Option<PropertyDescriptor>> {
) -> JsResult<Option<CompletePropertyDescriptor>> {
// 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 {
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/object/for_in_iterator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand Down
27 changes: 13 additions & 14 deletions core/engine/src/builtins/object/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)]
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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 )`
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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::<JsValue>()
.pipe(Ok)
.is_some_and(CompletePropertyDescriptor::enumerable)
.into())
}

/// `Object.assign( target, ...sources )`
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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)?;
Expand Down
Loading
Loading