Skip to content

Commit

Permalink
Define array index as NonMaxU32
Browse files Browse the repository at this point in the history
Based on the definition in spec, [array index][] is only valid from zero
to u32::MAX - 1. It would be helpful to use the NonMaxU32 type to define
it as the property key instead so that contributors won't forget to
check the validity.

[array index]: https://tc39.es/ecma262/#array-index
  • Loading branch information
CYBAI committed Jun 20, 2022
1 parent 6280a95 commit 6559c96
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 55 deletions.
12 changes: 6 additions & 6 deletions boa_engine/src/object/internal_methods/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,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 as usize)
.get(index.get() as usize)
{
// a. Set desc.[[Value]] to Get(map, P).
return Ok(Some(
Expand Down Expand Up @@ -81,8 +81,8 @@ pub(crate) fn arguments_exotic_define_own_property(
obj.borrow()
.as_mapped_arguments()
.expect("arguments exotic method must only be callable from arguments objects")
.get(index as usize)
.map(|value| (index as usize, value))
.get(index.get() as usize)
.map(|value| (index.get() as usize, value))
} else {
None
};
Expand Down Expand Up @@ -173,7 +173,7 @@ pub(crate) fn arguments_exotic_get(
.borrow()
.as_mapped_arguments()
.expect("arguments exotic method must only be callable from arguments objects")
.get(*index as usize)
.get(index.get() as usize)
{
// a. Assert: map contains a formal parameter mapping for P.
// b. Return Get(map, P).
Expand Down Expand Up @@ -212,7 +212,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 as usize, &value);
.set(index.get() as usize, &value);
}
}

Expand Down Expand Up @@ -243,7 +243,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 as usize);
.delete(index.get() as usize);
}
}

Expand Down
8 changes: 6 additions & 2 deletions boa_engine/src/object/internal_methods/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ pub(crate) fn array_exotic_define_own_property(
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, &"length".into(), context)?
.expect("the property descriptor must exist");
Expand Down Expand Up @@ -201,14 +203,16 @@ fn array_set_length(
.borrow()
.properties
.index_property_keys()
.filter(|idx| new_len <= **idx && **idx < u32::MAX)
.filter(|idx| new_len <= idx.get())
.copied()
.collect();
keys.sort_unstable_by(|x, y| y.cmp(x));
keys
};

for index in ordered_keys {
let index = index.get();

// a. Let deleteSucceeded be ! A.[[Delete]](P).
// b. If deleteSucceeded is false, then
if !obj.__delete__(&index.into(), context)? {
Expand Down
33 changes: 18 additions & 15 deletions boa_engine/src/object/internal_methods/integer_indexed.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{
builtins::{array_buffer::SharedMemoryOrder, typed_array::integer_indexed_object::ContentType},
nonmaxu32::NonMaxU32,
object::JsObject,
property::{PropertyDescriptor, PropertyKey},
Context, JsResult, JsValue,
Expand Down Expand Up @@ -44,14 +45,16 @@ pub(crate) fn integer_indexed_exotic_get_own_property(
// i. Let value be ! IntegerIndexedElementGet(O, numericIndex).
// ii. If value is undefined, return undefined.
// iii. Return the PropertyDescriptor { [[Value]]: value, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true }.
Ok(integer_indexed_element_get(obj, *index as usize).map(|v| {
PropertyDescriptor::builder()
.value(v)
.writable(true)
.enumerable(true)
.configurable(true)
.build()
}))
Ok(
integer_indexed_element_get(obj, index.get() as usize).map(|v| {
PropertyDescriptor::builder()
.value(v)
.writable(true)
.enumerable(true)
.configurable(true)
.build()
}),
)
} else {
// 2. Return OrdinaryGetOwnProperty(O, P).
super::ordinary_get_own_property(obj, key, context)
Expand All @@ -74,7 +77,7 @@ pub(crate) fn integer_indexed_exotic_has_property(
// a. Let numericIndex be ! CanonicalNumericIndexString(P).
if let PropertyKey::Index(index) = key {
// b. If numericIndex is not undefined, return ! IsValidIntegerIndex(O, numericIndex).
Ok(is_valid_integer_index(obj, *index as usize))
Ok(is_valid_integer_index(obj, index.get() as usize))
} else {
// 2. Return ? OrdinaryHasProperty(O, P).
super::ordinary_has_property(obj, key, context)
Expand Down Expand Up @@ -103,7 +106,7 @@ pub(crate) fn integer_indexed_exotic_define_own_property(
// iii. If Desc has an [[Enumerable]] field and if Desc.[[Enumerable]] is false, return false.
// v. If Desc has a [[Writable]] field and if Desc.[[Writable]] is false, return false.
// iv. If ! IsAccessorDescriptor(Desc) is true, return false.
if !is_valid_integer_index(obj, index as usize)
if !is_valid_integer_index(obj, index.get() as usize)
|| !desc
.configurable()
.or_else(|| desc.enumerable())
Expand All @@ -116,7 +119,7 @@ pub(crate) fn integer_indexed_exotic_define_own_property(

// vi. If Desc has a [[Value]] field, perform ? IntegerIndexedElementSet(O, numericIndex, Desc.[[Value]]).
if let Some(value) = desc.value() {
integer_indexed_element_set(obj, index as usize, value, context)?;
integer_indexed_element_set(obj, index.get() as usize, value, context)?;
}

// vii. Return true.
Expand Down Expand Up @@ -145,7 +148,7 @@ pub(crate) fn integer_indexed_exotic_get(
// b. If numericIndex is not undefined, then
if let PropertyKey::Index(index) = key {
// i. Return ! IntegerIndexedElementGet(O, numericIndex).
Ok(integer_indexed_element_get(obj, *index as usize).unwrap_or_default())
Ok(integer_indexed_element_get(obj, index.get() as usize).unwrap_or_default())
} else {
// 2. Return ? OrdinaryGet(O, P, Receiver).
super::ordinary_get(obj, key, receiver, context)
Expand All @@ -171,7 +174,7 @@ pub(crate) fn integer_indexed_exotic_set(
// b. If numericIndex is not undefined, then
if let PropertyKey::Index(index) = key {
// i. Perform ? IntegerIndexedElementSet(O, numericIndex, V).
integer_indexed_element_set(obj, index as usize, &value, context)?;
integer_indexed_element_set(obj, index.get() as usize, &value, context)?;

// ii. Return true.
Ok(true)
Expand All @@ -198,7 +201,7 @@ pub(crate) fn integer_indexed_exotic_delete(
// b. If numericIndex is not undefined, then
if let PropertyKey::Index(index) = key {
// i. If ! IsValidIntegerIndex(O, numericIndex) is false, return true; else return false.
Ok(!is_valid_integer_index(obj, *index as usize))
Ok(!is_valid_integer_index(obj, index.get() as usize))
} else {
// 2. Return ? OrdinaryDelete(O, P).
super::ordinary_delete(obj, key, context)
Expand Down Expand Up @@ -231,7 +234,7 @@ pub(crate) fn integer_indexed_exotic_own_property_keys(
// i. Add ! ToString(𝔽(i)) as the last element of keys.
(0..inner.array_length())
.into_iter()
.map(|index| PropertyKey::Index(index as u32))
.filter_map(|index| NonMaxU32::new(index as u32).map(PropertyKey::Index))
.collect()
};

Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/object/internal_methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ pub(crate) fn ordinary_own_property_keys(

// 2. For each own property key P of O such that P is an array index, in ascending numeric index order, do
// a. Add P as the last element of keys.
keys.extend(ordered_indexes.into_iter().map(Into::into));
keys.extend(ordered_indexes.into_iter().map(|idx| idx.get().into()));

// 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
// a. Add P as the last element of keys.
Expand Down
6 changes: 3 additions & 3 deletions boa_engine/src/object/internal_methods/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ pub(crate) fn string_exotic_own_property_keys(
.properties
.index_property_keys()
.copied()
.filter(|idx| (*idx as usize) >= len)
.filter(|idx| (idx.get() as usize) >= len)
.collect();
remaining_indices.sort_unstable();
keys.extend(remaining_indices.into_iter().map(Into::into));
keys.extend(remaining_indices.into_iter().map(|idx| idx.get().into()));

// 7. 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 Expand Up @@ -159,7 +159,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
24 changes: 12 additions & 12 deletions boa_engine/src/object/property_map.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::{PropertyDescriptor, PropertyKey};
use crate::{JsString, JsSymbol};
use crate::{nonmaxu32::NonMaxU32, JsString, JsSymbol};
use boa_gc::{custom_trace, Finalize, Trace};
use indexmap::IndexMap;
use rustc_hash::{FxHashMap, FxHasher};
Expand Down Expand Up @@ -30,7 +30,7 @@ unsafe impl<K: Trace> Trace for OrderedHashMap<K> {

#[derive(Default, Debug, Trace, Finalize)]
pub struct PropertyMap {
indexed_properties: FxHashMap<u32, PropertyDescriptor>,
indexed_properties: FxHashMap<NonMaxU32, PropertyDescriptor>,
/// Properties
string_properties: OrderedHashMap<JsString>,
/// Symbol Properties
Expand Down Expand Up @@ -126,15 +126,15 @@ impl PropertyMap {
SymbolPropertyValues(self.symbol_properties.0.values())
}

/// An iterator visiting all indexed key-value pairs in arbitrary order. The iterator element type is `(&'a u32, &'a Property)`.
/// An iterator visiting all indexed key-value pairs in arbitrary order. The iterator element type is `(&'a NonMaxU32, &'a Property)`.
///
/// This iterator does not recurse down the prototype chain.
#[inline]
pub fn index_properties(&self) -> IndexProperties<'_> {
IndexProperties(self.indexed_properties.iter())
}

/// An iterator visiting all index keys in arbitrary order. The iterator element type is `&'a u32`.
/// An iterator visiting all index keys in arbitrary order. The iterator element type is `&'a NonMaxU32`.
///
/// This iterator does not recurse down the prototype chain.
#[inline]
Expand Down Expand Up @@ -197,7 +197,7 @@ impl PropertyMap {
/// An iterator over the property entries of an `Object`
#[derive(Debug, Clone)]
pub struct Iter<'a> {
indexed_properties: hash_map::Iter<'a, u32, PropertyDescriptor>,
indexed_properties: hash_map::Iter<'a, NonMaxU32, PropertyDescriptor>,
string_properties: indexmap::map::Iter<'a, JsString, PropertyDescriptor>,
symbol_properties: indexmap::map::Iter<'a, JsSymbol, PropertyDescriptor>,
}
Expand All @@ -206,7 +206,7 @@ impl<'a> Iterator for Iter<'a> {
type Item = (PropertyKey, &'a PropertyDescriptor);
fn next(&mut self) -> Option<Self::Item> {
if let Some((key, value)) = self.indexed_properties.next() {
Some(((*key).into(), value))
Some((key.get().into(), value))
} else if let Some((key, value)) = self.string_properties.next() {
Some((key.clone().into(), value))
} else {
Expand Down Expand Up @@ -350,10 +350,10 @@ impl FusedIterator for SymbolPropertyValues<'_> {}

/// An iterator over the indexed property entries of an `Object`
#[derive(Debug, Clone)]
pub struct IndexProperties<'a>(hash_map::Iter<'a, u32, PropertyDescriptor>);
pub struct IndexProperties<'a>(hash_map::Iter<'a, NonMaxU32, PropertyDescriptor>);

impl<'a> Iterator for IndexProperties<'a> {
type Item = (&'a u32, &'a PropertyDescriptor);
type Item = (&'a NonMaxU32, &'a PropertyDescriptor);

#[inline]
fn next(&mut self) -> Option<Self::Item> {
Expand All @@ -375,12 +375,12 @@ impl ExactSizeIterator for IndexProperties<'_> {

impl FusedIterator for IndexProperties<'_> {}

/// An iterator over the index keys (`u32`) of an `Object`.
/// An iterator over the index keys (`NonMaxU32`) of an `Object`.
#[derive(Debug, Clone)]
pub struct IndexPropertyKeys<'a>(hash_map::Keys<'a, u32, PropertyDescriptor>);
pub struct IndexPropertyKeys<'a>(hash_map::Keys<'a, NonMaxU32, PropertyDescriptor>);

impl<'a> Iterator for IndexPropertyKeys<'a> {
type Item = &'a u32;
type Item = &'a NonMaxU32;

#[inline]
fn next(&mut self) -> Option<Self::Item> {
Expand All @@ -404,7 +404,7 @@ impl FusedIterator for IndexPropertyKeys<'_> {}

/// An iterator over the index values (`Property`) of an `Object`.
#[derive(Debug, Clone)]
pub struct IndexPropertyValues<'a>(hash_map::Values<'a, u32, PropertyDescriptor>);
pub struct IndexPropertyValues<'a>(hash_map::Values<'a, NonMaxU32, PropertyDescriptor>);

impl<'a> Iterator for IndexPropertyValues<'a> {
type Item = &'a PropertyDescriptor;
Expand Down
Loading

0 comments on commit 6559c96

Please sign in to comment.