Skip to content

Commit

Permalink
Add NonMaxU32 as integer variant for PropertyKey (#3321)
Browse files Browse the repository at this point in the history
* Add max array index

* Apply review

* Switch to u32

* Mark function as unsafe
  • Loading branch information
raskad committed Sep 30, 2023
1 parent b03aa36 commit ca37aa2
Show file tree
Hide file tree
Showing 16 changed files with 117 additions and 61 deletions.
2 changes: 1 addition & 1 deletion boa_engine/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,7 @@ pub(crate) fn set_function_name(
)
}
PropertyKey::String(string) => string.clone(),
PropertyKey::Index(index) => js_string!(format!("{index}")),
PropertyKey::Index(index) => js_string!(format!("{}", index.get())),
};

// 3. Else if name is a Private Name, then
Expand Down
4 changes: 3 additions & 1 deletion boa_engine/src/builtins/object/for_in_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ impl ForInIterator {
iterator.remaining_keys.push_back(k.clone());
}
PropertyKey::Index(i) => {
iterator.remaining_keys.push_back(i.to_string().into());
iterator
.remaining_keys
.push_back(i.get().to_string().into());
}
PropertyKey::Symbol(_) => {}
}
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/builtins/object/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1420,7 +1420,7 @@ fn get_own_property_keys(
(PropertyKeyType::String, PropertyKey::String(_))
| (PropertyKeyType::Symbol, PropertyKey::Symbol(_)) => Some(next_key.into()),
(PropertyKeyType::String, PropertyKey::Index(index)) => {
Some(js_string!(index.to_string()).into())
Some(js_string!(index.get().to_string()).into())
}
_ => None,
}
Expand Down
20 changes: 10 additions & 10 deletions boa_engine/src/object/internal_methods/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub(crate) fn arguments_exotic_get_own_property(
.borrow()
.as_mapped_arguments()
.expect("arguments exotic method must only be callable from arguments objects")
.get(*index)
.get(index.get())
{
// a. Set desc.[[Value]] to Get(map, P).
return Ok(Some(
Expand Down Expand Up @@ -73,12 +73,12 @@ pub(crate) fn arguments_exotic_define_own_property(
context: &mut Context<'_>,
) -> JsResult<bool> {
// 2. Let isMapped be HasOwnProperty(map, P).
let mapped = if let &PropertyKey::Index(index) = key {
let mapped = if let &PropertyKey::Index(index) = &key {
// 1. Let map be args.[[ParameterMap]].
obj.borrow()
.as_mapped_arguments()
.expect("arguments exotic method must only be callable from arguments objects")
.get(index)
.get(index.get())
.map(|value| (index, value))
} else {
None
Expand Down Expand Up @@ -128,21 +128,21 @@ pub(crate) fn arguments_exotic_define_own_property(
// a. If IsAccessorDescriptor(Desc) is true, then
if desc.is_accessor_descriptor() {
// i. Call map.[[Delete]](P).
map.delete(index);
map.delete(index.get());
}
// b. Else,
else {
// i. If Desc.[[Value]] is present, then
if let Some(value) = desc.value() {
// 1. Let setStatus be Set(map, P, Desc.[[Value]], false).
// 2. Assert: setStatus is true because formal parameters mapped by argument objects are always writable.
map.set(index, value);
map.set(index.get(), value);
}

// ii. If Desc.[[Writable]] is present and its value is false, then
if desc.writable() == Some(false) {
// 1. Call map.[[Delete]](P).
map.delete(index);
map.delete(index.get());
}
}
}
Expand Down Expand Up @@ -170,7 +170,7 @@ pub(crate) fn arguments_exotic_get(
.borrow()
.as_mapped_arguments()
.expect("arguments exotic method must only be callable from arguments objects")
.get(*index)
.get(index.get())
{
// a. Assert: map contains a formal parameter mapping for P.
// b. Return Get(map, P).
Expand Down Expand Up @@ -199,7 +199,7 @@ pub(crate) fn arguments_exotic_set(
// 1. If SameValue(args, Receiver) is false, then
// a. Let isMapped be false.
// 2. Else,
if let PropertyKey::Index(index) = key {
if let PropertyKey::Index(index) = &key {
if JsValue::same_value(&obj.clone().into(), &receiver) {
// a. Let map be args.[[ParameterMap]].
// b. Let isMapped be ! HasOwnProperty(map, P).
Expand All @@ -209,7 +209,7 @@ pub(crate) fn arguments_exotic_set(
obj.borrow_mut()
.as_mapped_arguments_mut()
.expect("arguments exotic method must only be callable from arguments objects")
.set(index, &value);
.set(index.get(), &value);
}
}

Expand Down Expand Up @@ -240,7 +240,7 @@ pub(crate) fn arguments_exotic_delete(
obj.borrow_mut()
.as_mapped_arguments_mut()
.expect("arguments exotic method must only be callable from arguments objects")
.delete(*index);
.delete(index.get());
}
}

Expand Down
6 changes: 4 additions & 2 deletions boa_engine/src/object/internal_methods/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,17 @@ pub(crate) fn array_exotic_define_own_property(
context: &mut Context<'_>,
) -> JsResult<bool> {
// 1. Assert: IsPropertyKey(P) is true.
match *key {
match key {
// 2. If P is "length", then
PropertyKey::String(ref s) if s == utf16!("length") => {
// a. Return ? ArraySetLength(A, Desc).

array_set_length(obj, desc, context)
}
// 3. Else if P is an array index, then
PropertyKey::Index(index) if index < u32::MAX => {
PropertyKey::Index(index) => {
let index = index.get();

// a. Let oldLenDesc be OrdinaryGetOwnProperty(A, "length").
let old_len_desc =
super::ordinary_get_own_property(obj, &utf16!("length").into(), context)?
Expand Down
16 changes: 7 additions & 9 deletions boa_engine/src/object/internal_methods/integer_indexed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ pub(crate) fn integer_indexed_exotic_get_own_property(
// 1.a. Let numericIndex be CanonicalNumericIndexString(P).
canonical_numeric_index_string(key)
}
PropertyKey::Index(index) => Some((*index).into()),
PropertyKey::Index(index) => Some(index.get().into()),
PropertyKey::Symbol(_) => None,
};

Expand Down Expand Up @@ -111,7 +111,7 @@ pub(crate) fn integer_indexed_exotic_has_property(
// 1.a. Let numericIndex be CanonicalNumericIndexString(P).
canonical_numeric_index_string(key)
}
PropertyKey::Index(index) => Some((*index).into()),
PropertyKey::Index(index) => Some(index.get().into()),
PropertyKey::Symbol(_) => None,
};

Expand Down Expand Up @@ -142,7 +142,7 @@ pub(crate) fn integer_indexed_exotic_define_own_property(
// 1.a. Let numericIndex be CanonicalNumericIndexString(P).
canonical_numeric_index_string(key)
}
PropertyKey::Index(index) => Some((*index).into()),
PropertyKey::Index(index) => Some(index.get().into()),
PropertyKey::Symbol(_) => None,
};

Expand Down Expand Up @@ -204,7 +204,7 @@ pub(crate) fn integer_indexed_exotic_get(
// 1.a. Let numericIndex be CanonicalNumericIndexString(P).
canonical_numeric_index_string(key)
}
PropertyKey::Index(index) => Some((*index).into()),
PropertyKey::Index(index) => Some(index.get().into()),
PropertyKey::Symbol(_) => None,
};

Expand Down Expand Up @@ -237,7 +237,7 @@ pub(crate) fn integer_indexed_exotic_set(
// 1.a. Let numericIndex be CanonicalNumericIndexString(P).
canonical_numeric_index_string(key)
}
PropertyKey::Index(index) => Some((*index).into()),
PropertyKey::Index(index) => Some(index.get().into()),
PropertyKey::Symbol(_) => None,
};

Expand Down Expand Up @@ -279,7 +279,7 @@ pub(crate) fn integer_indexed_exotic_delete(
// 1.a. Let numericIndex be CanonicalNumericIndexString(P).
canonical_numeric_index_string(key)
}
PropertyKey::Index(index) => Some((*index).into()),
PropertyKey::Index(index) => Some(index.get().into()),
PropertyKey::Symbol(_) => None,
};

Expand Down Expand Up @@ -317,9 +317,7 @@ pub(crate) fn integer_indexed_exotic_own_property_keys(
// 2. If IsDetachedBuffer(O.[[ViewedArrayBuffer]]) is false, then
// a. For each integer i starting with 0 such that i < O.[[ArrayLength]], in ascending order, do
// i. Add ! ToString(𝔽(i)) as the last element of keys.
(0..inner.array_length())
.map(|index| PropertyKey::Index(index as u32))
.collect()
(0..inner.array_length()).map(PropertyKey::from).collect()
};

// 3. For each own property key P of O such that Type(P) is String and P is not an array index, in ascending chronological order of property creation, do
Expand Down
8 changes: 4 additions & 4 deletions boa_engine/src/object/internal_methods/module_namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ fn module_namespace_exotic_get_own_property(
// 1. If P is a Symbol, return OrdinaryGetOwnProperty(O, P).
let key = match key {
PropertyKey::Symbol(_) => return ordinary_get_own_property(obj, key, context),
PropertyKey::Index(idx) => js_string!(format!("{idx}")),
PropertyKey::Index(idx) => js_string!(format!("{}", idx.get())),
PropertyKey::String(s) => s.clone(),
};

Expand Down Expand Up @@ -169,7 +169,7 @@ fn module_namespace_exotic_has_property(
// 1. If P is a Symbol, return ! OrdinaryHasProperty(O, P).
let key = match key {
PropertyKey::Symbol(_) => return ordinary_has_property(obj, key, context),
PropertyKey::Index(idx) => js_string!(format!("{idx}")),
PropertyKey::Index(idx) => js_string!(format!("{}", idx.get())),
PropertyKey::String(s) => s.clone(),
};

Expand Down Expand Up @@ -199,7 +199,7 @@ fn module_namespace_exotic_get(
// a. Return ! OrdinaryGet(O, P, Receiver).
let key = match key {
PropertyKey::Symbol(_) => return ordinary_get(obj, key, receiver, context),
PropertyKey::Index(idx) => js_string!(format!("{idx}")),
PropertyKey::Index(idx) => js_string!(format!("{}", idx.get())),
PropertyKey::String(s) => s.clone(),
};

Expand Down Expand Up @@ -287,7 +287,7 @@ fn module_namespace_exotic_delete(
// a. Return ! OrdinaryDelete(O, P).
let key = match key {
PropertyKey::Symbol(_) => return ordinary_delete(obj, key, context),
PropertyKey::Index(idx) => js_string!(format!("{idx}")),
PropertyKey::Index(idx) => js_string!(format!("{}", idx.get())),
PropertyKey::String(s) => s.clone(),
};

Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/object/internal_methods/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ fn string_get_own_property(obj: &JsObject, key: &PropertyKey) -> Option<Property
// 6. If IsIntegralNumber(index) is false, return undefined.
// 7. If index is -0𝔽, return undefined.
let pos = match key {
PropertyKey::Index(index) => *index as usize,
PropertyKey::Index(index) => index.get() as usize,
_ => return None,
};

Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/object/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ impl JsObject {
// a. If Type(key) is String, then
let key_str = match &key {
PropertyKey::String(s) => Some(s.clone()),
PropertyKey::Index(i) => Some(i.to_string().into()),
PropertyKey::Index(i) => Some(i.get().to_string().into()),
PropertyKey::Symbol(_) => None,
};

Expand Down
8 changes: 4 additions & 4 deletions boa_engine/src/object/property_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ impl PropertyMap {
#[must_use]
pub fn get(&self, key: &PropertyKey) -> Option<PropertyDescriptor> {
if let PropertyKey::Index(index) = key {
return self.indexed_properties.get(*index);
return self.indexed_properties.get(index.get());
}
if let Some(slot) = self.shape.lookup(key) {
return Some(self.get_storage(slot));
Expand Down Expand Up @@ -301,7 +301,7 @@ impl PropertyMap {
/// Insert the given property descriptor with the given key [`PropertyMap`].
pub fn insert(&mut self, key: &PropertyKey, property: PropertyDescriptor) -> bool {
if let PropertyKey::Index(index) = key {
return self.indexed_properties.insert(*index, property);
return self.indexed_properties.insert(index.get(), property);
}

let attributes = property.to_slot_attributes();
Expand Down Expand Up @@ -390,7 +390,7 @@ impl PropertyMap {
/// Remove the property with the given key from the [`PropertyMap`].
pub fn remove(&mut self, key: &PropertyKey) -> bool {
if let PropertyKey::Index(index) = key {
return self.indexed_properties.remove(*index);
return self.indexed_properties.remove(index.get());
}
if let Some(slot) = self.shape.lookup(key) {
// shift all elements when removing.
Expand Down Expand Up @@ -461,7 +461,7 @@ impl PropertyMap {
#[must_use]
pub fn contains_key(&self, key: &PropertyKey) -> bool {
if let PropertyKey::Index(index) = key {
return self.indexed_properties.contains_key(*index);
return self.indexed_properties.contains_key(index.get());
}
if self.shape.lookup(key).is_some() {
return true;
Expand Down

0 comments on commit ca37aa2

Please sign in to comment.