diff --git a/boa/src/builtins/function/arguments.rs b/boa/src/builtins/function/arguments.rs index a27c30ba80c..4de082a9a5d 100644 --- a/boa/src/builtins/function/arguments.rs +++ b/boa/src/builtins/function/arguments.rs @@ -2,8 +2,8 @@ use crate::{ builtins::Array, environments::DeclarativeEnvironment, gc::{Finalize, Trace}, - object::{FunctionBuilder, JsObject, ObjectData}, - property::{PropertyDescriptor, PropertyKey}, + object::{JsObject, ObjectData}, + property::PropertyDescriptor, symbol::{self, WellKnownSymbols}, syntax::ast::node::FormalParameter, Context, JsValue, @@ -11,19 +11,57 @@ use crate::{ use gc::Gc; use rustc_hash::FxHashMap; +/// `ParameterMap` represents the `[[ParameterMap]]` internal slot on a Arguments exotic object. +/// +/// This struct stores all the data to access mapped function parameters in their environment. #[derive(Debug, Clone, Trace, Finalize)] -pub struct MappedArguments(JsObject); +pub struct ParameterMap { + binding_indices: Vec>, + environment: Gc, +} -impl MappedArguments { - pub(crate) fn parameter_map(&self) -> JsObject { - self.0.clone() +impl ParameterMap { + /// Deletes the binding with the given index from the parameter map. + pub(crate) fn delete(&mut self, index: usize) { + if let Some(binding) = self.binding_indices.get_mut(index) { + *binding = None; + } + } + + /// Get the value of the binding at the given index from the function environment. + /// + /// Note: This function is the abstract getter closure described in 10.4.4.7.1 `MakeArgGetter ( name, env )` + /// + /// More information: + /// - [ECMAScript reference][spec] + /// + /// [spec]: https://tc39.es/ecma262/#sec-makearggetter + pub(crate) fn get(&self, index: usize) -> Option { + if let Some(Some(binding_index)) = self.binding_indices.get(index) { + return Some(self.environment.get(*binding_index)); + } + None + } + + /// Set the value of the binding at the given index in the function environment. + /// + /// Note: This function is the abstract setter closure described in 10.4.4.7.2 `MakeArgSetter ( name, env )` + /// + /// More information: + /// - [ECMAScript reference][spec] + /// + /// [spec]: https://tc39.es/ecma262/#sec-makeargsetter + pub(crate) fn set(&self, index: usize, value: &JsValue) { + if let Some(Some(binding_index)) = self.binding_indices.get(index) { + self.environment.set(*binding_index, value.clone()); + } } } #[derive(Debug, Clone, Trace, Finalize)] pub enum Arguments { Unmapped, - Mapped(MappedArguments), + Mapped(ParameterMap), } impl Arguments { @@ -128,38 +166,8 @@ impl Arguments { // 8. Set obj.[[Delete]] as specified in 10.4.4.5. // 9. Set obj.[[Prototype]] to %Object.prototype%. - // 10. Let map be ! OrdinaryObjectCreate(null). - let map = JsObject::empty(); - - // 11. Set obj.[[ParameterMap]] to map. - let obj = JsObject::from_proto_and_data( - context.standard_objects().object_object().prototype(), - ObjectData::arguments(Self::Mapped(MappedArguments(map.clone()))), - ); - - // 14. Let index be 0. - // 15. Repeat, while index < len, - for (index, val) in arguments_list.iter().cloned().enumerate() { - // a. Let val be argumentsList[index]. - // b. Perform ! CreateDataPropertyOrThrow(obj, ! ToString(𝔽(index)), val). - obj.create_data_property_or_throw(index, val, context) - .expect("Defining new own properties for a new ordinary object cannot fail"); - // c. Set index to index + 1. - } - - // 16. Perform ! DefinePropertyOrThrow(obj, "length", PropertyDescriptor { [[Value]]: 𝔽(len), - // [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true }). - obj.define_property_or_throw( - "length", - PropertyDescriptor::builder() - .value(len) - .writable(true) - .enumerable(false) - .configurable(true), - context, - ) - .expect("Defining new own properties for a new ordinary object cannot fail"); - + // Section 17-19 are done first, for easier object creation in 11. + // // The section 17-19 differs from the spec, due to the way the runtime environments work. // // This section creates getters and setters for all mapped arguments. @@ -205,59 +213,54 @@ impl Arguments { property_index += 1; } } + + let mut map = ParameterMap { + binding_indices: vec![None; property_index], + environment: env.clone(), + }; + for (binding_index, property_index) in bindings.values() { - // 19.b.ii.1. Let g be MakeArgGetter(name, env). - // https://tc39.es/ecma262/#sec-makearggetter - let g = { - // 2. Let getter be ! CreateBuiltinFunction(getterClosure, 0, "", « »). - // 3. NOTE: getter is never directly accessible to ECMAScript code. - // 4. Return getter. - FunctionBuilder::closure_with_captures( - context, - // 1. Let getterClosure be a new Abstract Closure with no parameters that captures - // name and env and performs the following steps when called: - |_, _, captures, _| Ok(captures.0.get(captures.1)), - (env.clone(), *binding_index), - ) - .length(0) - .build() - }; - // 19.b.ii.2. Let p be MakeArgSetter(name, env). - // https://tc39.es/ecma262/#sec-makeargsetter - let p = { - // 2. Let setter be ! CreateBuiltinFunction(setterClosure, 1, "", « »). - // 3. NOTE: setter is never directly accessible to ECMAScript code. - // 4. Return setter. - FunctionBuilder::closure_with_captures( - context, - // 1. Let setterClosure be a new Abstract Closure with parameters (value) that captures - // name and env and performs the following steps when called: - |_, args, captures, _| { - let value = args.get(0).cloned().unwrap_or_default(); - captures.0.set(captures.1, value); - Ok(JsValue::undefined()) - }, - (env.clone(), *binding_index), - ) - .length(1) - .build() - }; + map.binding_indices[*property_index] = Some(*binding_index); + } - // 19.b.ii.3. Perform map.[[DefineOwnProperty]](! ToString(𝔽(index)), PropertyDescriptor { - // [[Set]]: p, [[Get]]: g, [[Enumerable]]: false, [[Configurable]]: true }). - map.__define_own_property__( - PropertyKey::from(*property_index), + // 11. Set obj.[[ParameterMap]] to map. + let obj = JsObject::from_proto_and_data( + context.standard_objects().object_object().prototype(), + ObjectData::arguments(Self::Mapped(map)), + ); + + // 14. Let index be 0. + // 15. Repeat, while index < len, + for (index, val) in arguments_list.iter().cloned().enumerate() { + // a. Let val be argumentsList[index]. + // b. Perform ! CreateDataPropertyOrThrow(obj, ! ToString(𝔽(index)), val). + // Note: Insert is used here because `CreateDataPropertyOrThrow` would cause a panic while executing + // exotic argument object set methods before the variables in the environment are initialized. + obj.insert( + index, PropertyDescriptor::builder() - .set(p) - .get(g) - .enumerable(false) + .value(val) + .writable(true) + .enumerable(true) .configurable(true) .build(), - context, - ) - .expect("Defining new own properties for a new ordinary object cannot fail"); + ); + // c. Set index to index + 1. } + // 16. Perform ! DefinePropertyOrThrow(obj, "length", PropertyDescriptor { [[Value]]: 𝔽(len), + // [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true }). + obj.define_property_or_throw( + "length", + PropertyDescriptor::builder() + .value(len) + .writable(true) + .enumerable(false) + .configurable(true), + context, + ) + .expect("Defining new own properties for a new ordinary object cannot fail"); + // 20. Perform ! DefinePropertyOrThrow(obj, @@iterator, PropertyDescriptor { // [[Value]]: %Array.prototype.values%, [[Writable]]: true, [[Enumerable]]: false, // [[Configurable]]: true }). diff --git a/boa/src/object/internal_methods/arguments.rs b/boa/src/object/internal_methods/arguments.rs index 46fac77f4e9..7ade852d3f3 100644 --- a/boa/src/object/internal_methods/arguments.rs +++ b/boa/src/object/internal_methods/arguments.rs @@ -37,31 +37,29 @@ pub(crate) fn arguments_exotic_get_own_property( }; // 3. Let map be args.[[ParameterMap]]. - let map = obj - .borrow() - .as_mapped_arguments() - .expect("arguments exotic method must only be callable from arguments objects") - .parameter_map(); - - Ok(Some( - // 4. Let isMapped be ! HasOwnProperty(map, P). - // 5. If isMapped is true, then - if map - .has_own_property(key.clone(), context) - .expect("HasOwnProperty must not fail per the spec") + // 4. Let isMapped be ! HasOwnProperty(map, P). + // 5. If isMapped is true, then + if let PropertyKey::Index(index) = key { + if let Some(value) = obj + .borrow() + .as_mapped_arguments() + .expect("arguments exotic method must only be callable from arguments objects") + .get(*index as usize) { // a. Set desc.[[Value]] to Get(map, P). - PropertyDescriptor::builder() - .value(map.get(key.clone(), context)?) - .maybe_writable(desc.writable()) - .maybe_enumerable(desc.enumerable()) - .maybe_configurable(desc.configurable()) - .build() - } else { - // 6. Return desc. - desc - }, - )) + return Ok(Some( + PropertyDescriptor::builder() + .value(value) + .maybe_writable(desc.writable()) + .maybe_enumerable(desc.enumerable()) + .maybe_configurable(desc.configurable()) + .build(), + )); + } + } + + // 6. Return desc. + Ok(Some(desc)) } /// `[[DefineOwnProperty]]` for arguments exotic objects. @@ -77,15 +75,17 @@ pub(crate) fn arguments_exotic_define_own_property( desc: PropertyDescriptor, context: &mut Context, ) -> JsResult { - // 1. Let map be args.[[ParameterMap]]. - let map = obj - .borrow() - .as_mapped_arguments() - .expect("arguments exotic method must only be callable from arguments objects") - .parameter_map(); - // 2. Let isMapped be HasOwnProperty(map, P). - let is_mapped = map.has_own_property(key.clone(), context)?; + 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 as usize) + .map(|value| (index as usize, value)) + } else { + None + }; let new_arg_desc = match desc.kind() { // 4. If isMapped is true and IsDataDescriptor(Desc) is true, then @@ -94,16 +94,20 @@ pub(crate) fn arguments_exotic_define_own_property( DescriptorKind::Data { writable: Some(false), value: None, - } if is_mapped => + } => // i. Set newArgDesc to a copy of Desc. // ii. Set newArgDesc.[[Value]] to Get(map, P). { - PropertyDescriptor::builder() - .value(map.get(key.clone(), context)?) - .writable(false) - .maybe_enumerable(desc.enumerable()) - .maybe_configurable(desc.configurable()) - .build() + if let Some((_, value)) = &mapped { + PropertyDescriptor::builder() + .value(value) + .writable(false) + .maybe_enumerable(desc.enumerable()) + .maybe_configurable(desc.configurable()) + .build() + } else { + desc.clone() + } } // 3. Let newArgDesc be Desc. @@ -112,32 +116,36 @@ pub(crate) fn arguments_exotic_define_own_property( // 5. Let allowed be ? OrdinaryDefineOwnProperty(args, P, newArgDesc). // 6. If allowed is false, return false. - if !super::ordinary_define_own_property(obj, key.clone(), new_arg_desc, context)? { + if !super::ordinary_define_own_property(obj, key, new_arg_desc, context)? { return Ok(false); } // 7. If isMapped is true, then - if is_mapped { + if let Some((index, _)) = mapped { + // 1. Let map be args.[[ParameterMap]]. + let mut obj_mut = obj.borrow_mut(); + let map = obj_mut + .as_mapped_arguments_mut() + .expect("arguments exotic method must only be callable from arguments objects"); + // a. If IsAccessorDescriptor(Desc) is true, then if desc.is_accessor_descriptor() { // i. Call map.[[Delete]](P). - map.__delete__(&key, context)?; + map.delete(index); } // 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). - let set_status = map.set(key.clone(), value, false, context); - // 2. Assert: setStatus is true because formal parameters mapped by argument objects are always writable. - assert_eq!(set_status, Ok(true)); + map.set(index, value); } // ii. If Desc.[[Writable]] is present and its value is false, then if let Some(false) = desc.writable() { // 1. Call map.[[Delete]](P). - map.__delete__(&key, context)?; + map.delete(index); } } } @@ -158,28 +166,24 @@ pub(crate) fn arguments_exotic_get( receiver: JsValue, context: &mut Context, ) -> JsResult { - // 1. Let map be args.[[ParameterMap]]. - let map = obj - .borrow() - .as_mapped_arguments() - .expect("arguments exotic method must only be callable from arguments objects") - .parameter_map(); - - // 2. Let isMapped be ! HasOwnProperty(map, P). - // 4. Else, - if map - .has_own_property(key.clone(), context) - .expect("HasOwnProperty must not fail per the spec") - { - // a. Assert: map contains a formal parameter mapping for P. - // b. Return Get(map, P). - map.get(key.clone(), context) + if let PropertyKey::Index(index) = key { + // 1. Let map be args.[[ParameterMap]]. + // 2. Let isMapped be ! HasOwnProperty(map, P). + if let Some(value) = obj + .borrow() + .as_mapped_arguments() + .expect("arguments exotic method must only be callable from arguments objects") + .get(*index as usize) + { + // a. Assert: map contains a formal parameter mapping for P. + // b. Return Get(map, P). + return Ok(value); + } + } // 3. If isMapped is false, then - } else { - // a. Return ? OrdinaryGet(args, P, Receiver). - super::ordinary_get(obj, key, receiver, context) - } + // a. Return ? OrdinaryGet(args, P, Receiver). + super::ordinary_get(obj, key, receiver, context) } /// `[[Set]]` for arguments exotic objects. @@ -198,25 +202,17 @@ pub(crate) fn arguments_exotic_set( // 1. If SameValue(args, Receiver) is false, then // a. Let isMapped be false. // 2. Else, - if JsValue::same_value(&obj.clone().into(), &receiver) { - // a. Let map be args.[[ParameterMap]]. - let map = obj - .borrow() - .as_mapped_arguments() - .expect("arguments exotic method must only be callable from arguments objects") - .parameter_map(); - - // b. Let isMapped be ! HasOwnProperty(map, P). - // 3. If isMapped is true, then - if map - .has_own_property(key.clone(), context) - .expect("HasOwnProperty must not fail per the spec") - { + 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). + // 3. If isMapped is true, then // a. Let setStatus be Set(map, P, V, false). - let set_status = map.set(key.clone(), value.clone(), false, context); - // b. Assert: setStatus is true because formal parameters mapped by argument objects are always writable. - assert_eq!(set_status, Ok(true)); + obj.borrow_mut() + .as_mapped_arguments_mut() + .expect("arguments exotic method must only be callable from arguments objects") + .set(index as usize, &value); } } @@ -235,25 +231,20 @@ pub(crate) fn arguments_exotic_delete( key: &PropertyKey, context: &mut Context, ) -> JsResult { - // 1. Let map be args.[[ParameterMap]]. - let map = obj - .borrow() - .as_mapped_arguments() - .expect("arguments exotic method must only be callable from arguments objects") - .parameter_map(); - - // 2. Let isMapped be ! HasOwnProperty(map, P). - let is_mapped = map - .has_own_property(key.clone(), context) - .expect("HasOwnProperty must not fail per the spec"); - // 3. Let result be ? OrdinaryDelete(args, P). let result = super::ordinary_delete(obj, key, context)?; - // 4. If result is true and isMapped is true, then - if is_mapped && result { - // a. Call map.[[Delete]](P). - map.__delete__(key, context)?; + if result { + if let PropertyKey::Index(index) = key { + // 1. Let map be args.[[ParameterMap]]. + // 2. Let isMapped be ! HasOwnProperty(map, P). + // 4. If result is true and isMapped is true, then + // a. Call map.[[Delete]](P). + obj.borrow_mut() + .as_mapped_arguments_mut() + .expect("arguments exotic method must only be callable from arguments objects") + .delete(*index as usize); + } } // 5. Return result. diff --git a/boa/src/object/mod.rs b/boa/src/object/mod.rs index 2ea26899d96..f8faa2880be 100644 --- a/boa/src/object/mod.rs +++ b/boa/src/object/mod.rs @@ -4,7 +4,7 @@ use crate::{ builtins::{ array::array_iterator::ArrayIterator, array_buffer::ArrayBuffer, - function::arguments::{Arguments, MappedArguments}, + function::arguments::{Arguments, ParameterMap}, function::{BoundFunction, Captures, Function, NativeFunctionSignature}, map::map_iterator::MapIterator, map::ordered_map::OrderedMap, @@ -958,7 +958,7 @@ impl Object { /// Gets the mapped arguments data if this is a mapped arguments object. #[inline] - pub fn as_mapped_arguments(&self) -> Option<&MappedArguments> { + pub fn as_mapped_arguments(&self) -> Option<&ParameterMap> { match self.data { ObjectData { kind: ObjectKind::Arguments(Arguments::Mapped(ref args)), @@ -968,6 +968,18 @@ impl Object { } } + /// Gets the mutable mapped arguments data if this is a mapped arguments object. + #[inline] + pub fn as_mapped_arguments_mut(&mut self) -> Option<&mut ParameterMap> { + match self.data { + ObjectData { + kind: ObjectKind::Arguments(Arguments::Mapped(ref mut args)), + .. + } => Some(args), + _ => None, + } + } + /// Gets the typed array data (integer indexed object) if this is a typed array. #[inline] pub fn as_typed_array(&self) -> Option<&IntegerIndexed> {