diff --git a/boa_engine/src/builtins/map/ordered_map.rs b/boa_engine/src/builtins/map/ordered_map.rs index 8c1f3de0c34..626e12ddfa9 100644 --- a/boa_engine/src/builtins/map/ordered_map.rs +++ b/boa_engine/src/builtins/map/ordered_map.rs @@ -10,7 +10,7 @@ use std::{ }; #[derive(PartialEq, Eq, Clone, Debug)] -enum MapKey { +pub(crate) enum MapKey { Key(JsValue), Empty(usize), // Necessary to ensure empty keys are still unique. } diff --git a/boa_engine/src/builtins/set/mod.rs b/boa_engine/src/builtins/set/mod.rs index 2ba7aa0579b..19418ebe73b 100644 --- a/boa_engine/src/builtins/set/mod.rs +++ b/boa_engine/src/builtins/set/mod.rs @@ -10,8 +10,16 @@ //! [spec]: https://tc39.es/ecma262/#sec-set-objects //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set +mod set_iterator; + +#[cfg(test)] +mod tests; + +pub mod ordered_set; + +use self::ordered_set::OrderedSet; use crate::{ - builtins::BuiltInObject, + builtins::{BuiltInBuilder, BuiltInConstructor, BuiltInObject, IntrinsicObject}, context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors}, error::JsNativeError, object::{internal_methods::get_prototype_from_constructor, JsObject, ObjectData}, @@ -21,16 +29,9 @@ use crate::{ Context, JsArgs, JsResult, JsValue, }; use boa_profiler::Profiler; +use num_traits::Zero; -use super::{BuiltInBuilder, BuiltInConstructor, IntrinsicObject}; - -pub mod ordered_set; -use self::ordered_set::OrderedSet; - -mod set_iterator; pub(crate) use set_iterator::SetIterator; -#[cfg(test)] -mod tests; #[derive(Debug, Clone)] pub(crate) struct Set; @@ -52,10 +53,6 @@ impl IntrinsicObject for Set { .name("get size") .build(); - let iterator_symbol = JsSymbol::iterator(); - - let to_string_tag = JsSymbol::to_string_tag(); - let values_function = BuiltInBuilder::new(intrinsics) .callable(Self::values) .name("values") @@ -91,12 +88,12 @@ impl IntrinsicObject for Set { Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, ) .property( - iterator_symbol, + JsSymbol::iterator(), values_function, Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, ) .property( - to_string_tag, + JsSymbol::to_string_tag(), Self::NAME, Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, ) @@ -221,27 +218,36 @@ impl Set { /// [spec]: https://tc39.es/ecma262/#sec-set.prototype.add /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/add pub(crate) fn add(this: &JsValue, args: &[JsValue], _: &mut Context<'_>) -> JsResult { - let value = args.get_or_undefined(0); + const JS_ZERO: &JsValue = &JsValue::Integer(0); - if let Some(object) = this.as_object() { - if let Some(set) = object.borrow_mut().as_set_mut() { - set.add(if value.as_number().map_or(false, |n| n == -0f64) { - JsValue::Integer(0) - } else { - value.clone() - }); - } else { - return Err(JsNativeError::typ() - .with_message("'this' is not a Set") - .into()); - } - } else { + // 1. Let S be the this value. + // 2. Perform ? RequireInternalSlot(S, [[SetData]]). + let Some(mut object) = this.as_object().map(JsObject::borrow_mut) else { return Err(JsNativeError::typ() - .with_message("'this' is not a Set") + .with_message("Method Set.prototype.add called on incompatible receiver") .into()); }; + let Some(s) = object.as_set_mut() else { + return Err(JsNativeError::typ() + .with_message("Method Set.prototype.add called on incompatible receiver") + .into()); + }; + + // 3. For each element e of S.[[SetData]], do + // a. If e is not empty and SameValueZero(e, value) is true, then + // i. Return S. + // 4. If value is -0𝔽, set value to +0𝔽. + let value = args.get_or_undefined(0); + let value = match value.as_number() { + Some(n) if n.is_zero() => JS_ZERO, + _ => value, + }; + + // 5. Append value to S.[[SetData]]. + s.add(value.clone()); Ok(this.clone()) + // 6. Return S. } /// `Set.prototype.clear( )` @@ -285,18 +291,33 @@ impl Set { args: &[JsValue], _: &mut Context<'_>, ) -> JsResult { - let value = args.get_or_undefined(0); + const JS_ZERO: &JsValue = &JsValue::Integer(0); - let mut object = this - .as_object() - .map(JsObject::borrow_mut) - .ok_or_else(|| JsNativeError::typ().with_message("'this' is not a Set"))?; + // 1. Let S be the this value. + // 2. Perform ? RequireInternalSlot(S, [[SetData]]). + let Some(mut object) = this.as_object().map(JsObject::borrow_mut) else { + return Err(JsNativeError::typ() + .with_message("Method Set.prototype.delete called on incompatible receiver") + .into()); + }; + let Some(s) = object.as_set_mut() else { + return Err(JsNativeError::typ() + .with_message("Method Set.prototype.delete called on incompatible receiver") + .into()); + }; - let set = object - .as_set_mut() - .ok_or_else(|| JsNativeError::typ().with_message("'this' is not a Set"))?; + let value = args.get_or_undefined(0); + let value = match value.as_number() { + Some(n) if n.is_zero() => JS_ZERO, + _ => value, + }; - Ok(set.delete(value).into()) + // 3. For each element e of S.[[SetData]], do + // a. If e is not empty and SameValueZero(e, value) is true, then + // i. Replace the element of S.[[SetData]] whose value is e with an element whose value is empty. + // ii. Return true. + // 4. Return false. + Ok(s.delete(value).into()) } /// `Set.prototype.entries( )` @@ -314,22 +335,16 @@ impl Set { _: &[JsValue], context: &mut Context<'_>, ) -> JsResult { - if let Some(object) = this.as_object() { - let object = object.borrow(); - if !object.is_set() { - return Err(JsNativeError::typ() - .with_message("Method Set.prototype.entries called on incompatible receiver") - .into()); - } - } else { + let Some(lock) = this.as_object().and_then(|o| o.borrow_mut().as_set_mut().map(|set| set.lock(o.clone()))) else { return Err(JsNativeError::typ() .with_message("Method Set.prototype.entries called on incompatible receiver") .into()); - } + }; Ok(SetIterator::create_set_iterator( this.clone(), PropertyNameKind::KeyAndValue, + lock, context, )) } @@ -349,42 +364,54 @@ impl Set { args: &[JsValue], context: &mut Context<'_>, ) -> JsResult { - if args.is_empty() { + // 1. Let S be the this value. + // 2. Perform ? RequireInternalSlot(S, [[SetData]]). + let Some(lock) = this.as_object().and_then(|o| o.borrow_mut().as_set_mut().map(|set| set.lock(o.clone()))) else { return Err(JsNativeError::typ() - .with_message("Missing argument for Set.prototype.forEach") + .with_message("Method Set.prototype.forEach called on incompatible receiver") .into()); - } - - let callback_arg = &args[0]; - let this_arg = args.get_or_undefined(1); + }; - // TODO: if condition should also check that we are not in strict mode - let this_arg = if this_arg.is_undefined() { - context.global_object().clone().into() - } else { - this_arg.clone() + // 3. If IsCallable(callbackfn) is false, throw a TypeError exception. + let Some(callback_fn) = args.get_or_undefined(0).as_callable() else { + return Err(JsNativeError::typ() + .with_message("Method Set.prototype.forEach called with non-callable callback function") + .into()); }; + // 4. Let entries be S.[[SetData]]. + // 5. Let numEntries be the number of elements in entries. + // 6. Let index be 0. let mut index = 0; - while index < Self::get_size(this)? { - let arguments = this - .as_object() - .and_then(|obj| { - obj.borrow().as_set().map(|set| { - set.get_index(index) - .map(|value| [value.clone(), value.clone(), this.clone()]) - }) - }) - .ok_or_else(|| JsNativeError::typ().with_message("'this' is not a Set"))?; - - if let Some(arguments) = arguments { - callback_arg.call(&this_arg, &arguments, context)?; - } + // 7. Repeat, while index < numEntries, + while index < Self::get_size_full(this)? { + // a. Let e be entries[index]. + let Some(e) = this.as_object().and_then(|o| o.borrow().as_set().map(|s| s.get_index(index).cloned())) else { + return Err(JsNativeError::typ() + .with_message("Method Set.prototype.forEach called on incompatible receiver") + .into()); + }; + // b. Set index to index + 1. index += 1; + + // c. If e is not empty, then + if let Some(e) = e { + // i. Perform ? Call(callbackfn, thisArg, « e, e, S »). + // ii. NOTE: The number of elements in entries may have increased during execution of callbackfn. + // iii. Set numEntries to the number of elements in entries. + callback_fn.call( + args.get_or_undefined(1), + &[e.clone(), e.clone(), this.clone()], + context, + )?; + } } + drop(lock); + + // 8. Return undefined. Ok(JsValue::Undefined) } @@ -399,15 +426,31 @@ impl Set { /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.has /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/has pub(crate) fn has(this: &JsValue, args: &[JsValue], _: &mut Context<'_>) -> JsResult { + const JS_ZERO: &JsValue = &JsValue::Integer(0); + + // 1. Let S be the this value. + // 2. Perform ? RequireInternalSlot(S, [[SetData]]). + let Some(mut object) = this.as_object().map(JsObject::borrow_mut) else { + return Err(JsNativeError::typ() + .with_message("Method Set.prototype.has called on incompatible receiver") + .into()); + }; + let Some(s) = object.as_set_mut() else { + return Err(JsNativeError::typ() + .with_message("Method Set.prototype.has called on incompatible receiver") + .into()); + }; + let value = args.get_or_undefined(0); + let value = match value.as_number() { + Some(n) if n.is_zero() => JS_ZERO, + _ => value, + }; - this.as_object() - .and_then(|obj| obj.borrow().as_set().map(|set| set.contains(value).into())) - .ok_or_else(|| { - JsNativeError::typ() - .with_message("'this' is not a Set") - .into() - }) + // 3. For each element e of S.[[SetData]], do + // a. If e is not empty and SameValueZero(e, value) is true, return true. + // 4. Return false. + Ok(s.contains(value).into()) } /// `Set.prototype.values( )` @@ -425,22 +468,16 @@ impl Set { _: &[JsValue], context: &mut Context<'_>, ) -> JsResult { - if let Some(object) = this.as_object() { - let object = object.borrow(); - if !object.is_set() { - return Err(JsNativeError::typ() - .with_message("Method Set.prototype.values called on incompatible receiver") - .into()); - } - } else { + let Some(lock) = this.as_object().and_then(|o| o.borrow_mut().as_set_mut().map(|set| set.lock(o.clone()))) else { return Err(JsNativeError::typ() .with_message("Method Set.prototype.values called on incompatible receiver") .into()); - } + }; Ok(SetIterator::create_set_iterator( this.clone(), PropertyNameKind::Value, + lock, context, )) } @@ -452,7 +489,18 @@ impl Set { /// Helper function to get the size of the `Set` object. pub(crate) fn get_size(set: &JsValue) -> JsResult { set.as_object() - .and_then(|obj| obj.borrow().as_set().map(OrderedSet::size)) + .and_then(|obj| obj.borrow().as_set().map(OrderedSet::len)) + .ok_or_else(|| { + JsNativeError::typ() + .with_message("'this' is not a Set") + .into() + }) + } + + /// Helper function to get the full size of the `Set` object. + pub(crate) fn get_size_full(set: &JsValue) -> JsResult { + set.as_object() + .and_then(|obj| obj.borrow().as_set().map(OrderedSet::full_len)) .ok_or_else(|| { JsNativeError::typ() .with_message("'this' is not a Set") diff --git a/boa_engine/src/builtins/set/ordered_set.rs b/boa_engine/src/builtins/set/ordered_set.rs index 77754fdaf62..6eb5ae78c3a 100644 --- a/boa_engine/src/builtins/set/ordered_set.rs +++ b/boa_engine/src/builtins/set/ordered_set.rs @@ -1,55 +1,48 @@ //! Implements a set type that preserves insertion order. +use crate::{builtins::map::ordered_map::MapKey, object::JsObject, JsValue}; use boa_gc::{custom_trace, Finalize, Trace}; -use indexmap::{ - set::{IntoIter, Iter}, - IndexSet, -}; -use std::{ - collections::hash_map::RandomState, - fmt::Debug, - hash::{BuildHasher, Hash}, -}; +use indexmap::IndexSet; +use std::{collections::hash_map::RandomState, fmt::Debug, hash::BuildHasher}; /// A type wrapping `indexmap::IndexSet` -#[derive(Clone)] -pub struct OrderedSet -where - V: Hash + Eq, -{ - inner: IndexSet, +#[derive(Clone, Finalize)] +pub struct OrderedSet { + inner: IndexSet, + lock: u32, + empty_count: usize, } -impl Finalize for OrderedSet {} -unsafe impl Trace for OrderedSet { +unsafe impl Trace for OrderedSet { custom_trace!(this, { for v in this.inner.iter() { - mark(v); + if let MapKey::Key(v) = v { + mark(v); + } } }); } -impl Debug for OrderedSet { +impl Debug for OrderedSet { fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { self.inner.fmt(formatter) } } -impl Default for OrderedSet { +impl Default for OrderedSet { fn default() -> Self { Self::new() } } -impl OrderedSet -where - V: Hash + Eq, -{ +impl OrderedSet { /// Creates a new empty `OrderedSet`. #[must_use] pub fn new() -> Self { Self { inner: IndexSet::new(), + lock: 0, + empty_count: 0, } } @@ -58,18 +51,28 @@ where pub fn with_capacity(capacity: usize) -> Self { Self { inner: IndexSet::with_capacity(capacity), + lock: 0, + empty_count: 0, } } - /// Return the number of key-value pairs in the map. + /// Return the number of elements in the set, including empty elements. /// /// Computes in **O(1)** time. #[must_use] - pub fn size(&self) -> usize { + pub fn full_len(&self) -> usize { self.inner.len() } - /// Returns true if the map contains no elements. + /// Return the number of elements in the set. + /// + /// Computes in **O(1)** time. + #[must_use] + pub fn len(&self) -> usize { + self.inner.len() - self.empty_count + } + + /// Returns true if the set contains no elements. /// /// Computes in **O(1)** time. #[must_use] @@ -85,23 +88,33 @@ where /// inserted, last in order, and false /// /// Computes in **O(1)** time (amortized average). - pub fn add(&mut self, value: V) -> bool { - self.inner.insert(value) + pub fn add(&mut self, value: JsValue) -> bool { + self.inner.insert(MapKey::Key(value)) } /// Delete the `value` from the set and return true if successful /// - /// Return `false` if `value` is not in map. + /// Return `false` if `value` is not in set. /// /// Computes in **O(n)** time (average). - pub fn delete(&mut self, value: &V) -> bool { - self.inner.shift_remove(value) + pub fn delete(&mut self, value: &JsValue) -> bool { + if self.lock == 0 { + self.inner.shift_remove(value) + } else if self.inner.contains(value) { + self.inner.insert(MapKey::Empty(self.empty_count)); + self.empty_count += 1; + self.inner.swap_remove(value) + } else { + false + } } /// Removes all elements in the set, while preserving its capacity. #[inline] pub fn clear(&mut self) { self.inner.clear(); + self.inner.shrink_to_fit(); + self.empty_count = 0; } /// Checks if a given value is present in the set @@ -109,7 +122,7 @@ where /// Return `true` if `value` is present in set, false otherwise. /// /// Computes in **O(n)** time (average). - pub fn contains(&self, value: &V) -> bool { + pub fn contains(&self, value: &JsValue) -> bool { self.inner.contains(value) } @@ -117,37 +130,60 @@ where /// Valid indices are 0 <= index < self.len() /// Computes in O(1) time. #[must_use] - pub fn get_index(&self, index: usize) -> Option<&V> { - self.inner.get_index(index) + pub fn get_index(&self, index: usize) -> Option<&JsValue> { + if let MapKey::Key(value) = self.inner.get_index(index)? { + Some(value) + } else { + None + } } /// Return an iterator over the values of the set, in their order - #[must_use] - pub fn iter(&self) -> Iter<'_, V> { - self.inner.iter() + pub fn iter(&self) -> impl Iterator { + self.inner.iter().filter_map(|v| { + if let MapKey::Key(v) = v { + Some(v) + } else { + None + } + }) + } + + /// Increases the lock counter and returns a lock object that will decrement the counter when dropped. + /// + /// This allows objects to be removed from the set during iteration without affecting the indexes until the iteration has completed. + pub(crate) fn lock(&mut self, set: JsObject) -> SetLock { + self.lock += 1; + SetLock(set) + } + + /// Decreases the lock counter and, if 0, removes all empty entries. + fn unlock(&mut self) { + self.lock -= 1; + if self.lock == 0 { + self.inner.retain(|k| matches!(k, MapKey::Key(_))); + self.empty_count = 0; + } } } -impl<'a, V, S> IntoIterator for &'a OrderedSet -where - V: Hash + Eq, - S: BuildHasher, -{ - type Item = &'a V; - type IntoIter = Iter<'a, V>; - fn into_iter(self) -> Self::IntoIter { - self.inner.iter() +/// Increases the lock count of the set for the lifetime of the guard. +/// This should not be dropped until iteration has completed. +#[derive(Debug, Trace)] +pub(crate) struct SetLock(JsObject); + +impl Clone for SetLock { + fn clone(&self) -> Self { + let mut set = self.0.borrow_mut(); + let set = set.as_set_mut().expect("SetLock does not point to a set"); + set.lock(self.0.clone()) } } -impl IntoIterator for OrderedSet -where - V: Hash + Eq, - S: BuildHasher, -{ - type Item = V; - type IntoIter = IntoIter; - fn into_iter(self) -> IntoIter { - self.inner.into_iter() +impl Finalize for SetLock { + fn finalize(&self) { + let mut set = self.0.borrow_mut(); + let set = set.as_set_mut().expect("SetLock does not point to a set"); + set.unlock(); } } diff --git a/boa_engine/src/builtins/set/set_iterator.rs b/boa_engine/src/builtins/set/set_iterator.rs index d0cf928a44f..4fcf1812fa3 100644 --- a/boa_engine/src/builtins/set/set_iterator.rs +++ b/boa_engine/src/builtins/set/set_iterator.rs @@ -5,6 +5,7 @@ //! //! [spec]: https://tc39.es/ecma262/#sec-set-iterator-objects +use super::ordered_set::SetLock; use crate::{ builtins::{ iterable::create_iter_result_object, Array, BuiltInBuilder, IntrinsicObject, JsValue, @@ -31,6 +32,7 @@ pub struct SetIterator { next_index: usize, #[unsafe_ignore_trace] iteration_kind: PropertyNameKind, + lock: SetLock, } impl IntrinsicObject for SetIterator { @@ -55,11 +57,12 @@ impl IntrinsicObject for SetIterator { impl SetIterator { /// Constructs a new `SetIterator`, that will iterate over `set`, starting at index 0 - const fn new(set: JsValue, kind: PropertyNameKind) -> Self { + const fn new(set: JsValue, kind: PropertyNameKind, lock: SetLock) -> Self { Self { iterated_set: set, next_index: 0, iteration_kind: kind, + lock, } } @@ -74,11 +77,12 @@ impl SetIterator { pub(crate) fn create_set_iterator( set: JsValue, kind: PropertyNameKind, + lock: SetLock, context: &Context<'_>, ) -> JsValue { let set_iterator = JsObject::from_proto_and_data( context.intrinsics().objects().iterator_prototypes().set(), - ObjectData::set_iterator(Self::new(set, kind)), + ObjectData::set_iterator(Self::new(set, kind, lock)), ); set_iterator.into() } @@ -121,7 +125,7 @@ impl SetIterator { .and_then(|obj| obj.as_set()) .ok_or_else(|| JsNativeError::typ().with_message("'this' is not a Set"))?; - let num_entries = entries.size(); + let num_entries = entries.full_len(); while index < num_entries { let e = entries.get_index(index); index += 1; diff --git a/boa_engine/src/object/mod.rs b/boa_engine/src/object/mod.rs index fa6c780527e..86bfef4b7c3 100644 --- a/boa_engine/src/object/mod.rs +++ b/boa_engine/src/object/mod.rs @@ -226,7 +226,7 @@ pub enum ObjectKind { GeneratorFunction(Function), /// The `Set` object kind. - Set(OrderedSet), + Set(OrderedSet), /// The `SetIterator` object kind. SetIterator(SetIterator), @@ -519,7 +519,7 @@ impl ObjectData { /// Create the `Set` object data #[must_use] - pub fn set(set: OrderedSet) -> Self { + pub fn set(set: OrderedSet) -> Self { Self { kind: ObjectKind::Set(set), internal_methods: &ORDINARY_INTERNAL_METHODS, @@ -1089,7 +1089,7 @@ impl Object { /// Gets the set data if the object is a `Set`. #[inline] - pub const fn as_set(&self) -> Option<&OrderedSet> { + pub const fn as_set(&self) -> Option<&OrderedSet> { match self.data { ObjectData { kind: ObjectKind::Set(ref set), @@ -1101,7 +1101,7 @@ impl Object { /// Gets the mutable set data if the object is a `Set`. #[inline] - pub fn as_set_mut(&mut self) -> Option<&mut OrderedSet> { + pub fn as_set_mut(&mut self) -> Option<&mut OrderedSet> { match &mut self.data { ObjectData { kind: ObjectKind::Set(set), diff --git a/boa_engine/src/value/display.rs b/boa_engine/src/value/display.rs index 639d3ddcf81..ea54b0ffc1c 100644 --- a/boa_engine/src/value/display.rs +++ b/boa_engine/src/value/display.rs @@ -183,7 +183,7 @@ pub(crate) fn log_string_from(x: &JsValue, print_internals: bool, print_children } } ObjectKind::Set(ref set) => { - let size = set.size(); + let size = set.len(); if size == 0 { return String::from("Set(0)");