diff --git a/Libraries/LibJS/Runtime/AbstractOperations.cpp b/Libraries/LibJS/Runtime/AbstractOperations.cpp index 0202414441f..b410d6ed7fb 100644 --- a/Libraries/LibJS/Runtime/AbstractOperations.cpp +++ b/Libraries/LibJS/Runtime/AbstractOperations.cpp @@ -1132,8 +1132,17 @@ Object* create_mapped_arguments_object(VM& vm, FunctionObject& function, Nonnull // 16. Perform ! DefinePropertyOrThrow(obj, "length", PropertyDescriptor { [[Value]]: 𝔽(len), [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true }). object->put_direct(realm.intrinsics().mapped_arguments_object_length_offset(), Value(length)); + // OPTIMIZATION: We take a different route here than what the spec suggests. + // The spec would have us allocate a new object for the parameter map, + // and then populate it with getters and setters for each mapped parameter. + // That would be 1 GC allocation for the parameter map and 2 more for each + // parameter's getter/setter pair. + // Instead, we allocate the ArgumentsObject and let it implement the parameter map + // and getter/setter behavior itself without extra GC allocations. + // 17. Let mappedNames be a new empty List. - HashTable mapped_names; + HashTable seen_names; + Vector mapped_names; // 18. Set index to numberOfParameters - 1. // 19. Repeat, while index ≥ 0, @@ -1143,30 +1152,26 @@ Object* create_mapped_arguments_object(VM& vm, FunctionObject& function, Nonnull auto const& name = formals->parameters()[index].binding.get>()->string(); // b. If name is not an element of mappedNames, then - if (mapped_names.contains(name)) + if (seen_names.contains(name)) continue; // i. Add name as an element of the list mappedNames. - mapped_names.set(name); + seen_names.set(name); // ii. If index < len, then if (index < length) { // 1. Let g be MakeArgGetter(name, env). // 2. Let p be MakeArgSetter(name, env). // 3. Perform ! map.[[DefineOwnProperty]](! ToString(𝔽(index)), PropertyDescriptor { [[Set]]: p, [[Get]]: g, [[Enumerable]]: false, [[Configurable]]: true }). - object->parameter_map().indexed_properties().put( - index, - Accessor::create(vm, - NativeFunction::create(realm, FlyString {}, [&environment, name](VM& vm) -> ThrowCompletionOr { - return MUST(environment.get_binding_value(vm, name, false)); - }), - NativeFunction::create(realm, FlyString {}, [&environment, name](VM& vm) { - MUST(environment.set_mutable_binding(vm, name, vm.argument(0), false)); - return js_undefined(); - }))); + if (index >= static_cast(mapped_names.size())) + mapped_names.resize(index + 1); + + mapped_names[index] = name; } } + object->set_mapped_names(move(mapped_names)); + // 20. Perform ! DefinePropertyOrThrow(obj, @@iterator, PropertyDescriptor { [[Value]]: %Array.prototype.values%, [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true }). auto array_prototype_values = realm.intrinsics().array_prototype_values_function(); object->put_direct(realm.intrinsics().mapped_arguments_object_well_known_symbol_iterator_offset(), array_prototype_values); diff --git a/Libraries/LibJS/Runtime/ArgumentsObject.cpp b/Libraries/LibJS/Runtime/ArgumentsObject.cpp index e60a124ea5e..642200585d1 100644 --- a/Libraries/LibJS/Runtime/ArgumentsObject.cpp +++ b/Libraries/LibJS/Runtime/ArgumentsObject.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Andreas Kling + * Copyright (c) 2021-2025, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ @@ -22,24 +22,27 @@ void ArgumentsObject::initialize(Realm& realm) { Base::initialize(realm); set_has_parameter_map(); - m_parameter_map = Object::create(realm, nullptr); } void ArgumentsObject::visit_edges(Cell::Visitor& visitor) { Base::visit_edges(visitor); visitor.visit(m_environment); - visitor.visit(m_parameter_map); +} + +bool ArgumentsObject::parameter_map_has(PropertyKey const& key) const +{ + return key.is_number() + && key.as_number() < m_mapped_names.size() + && !m_mapped_names[key.as_number()].is_empty(); } // 10.4.4.3 [[Get]] ( P, Receiver ), https://tc39.es/ecma262/#sec-arguments-exotic-objects-get-p-receiver ThrowCompletionOr ArgumentsObject::internal_get(PropertyKey const& property_key, Value receiver, CacheablePropertyMetadata* cacheable_metadata, PropertyLookupPhase phase) const { // 1. Let map be args.[[ParameterMap]]. - auto& map = *m_parameter_map; - // 2. Let isMapped be ! HasOwnProperty(map, P). - bool is_mapped = MUST(m_parameter_map->has_own_property(property_key)); + bool is_mapped = parameter_map_has(property_key); // 3. If isMapped is false, then if (!is_mapped) { @@ -47,10 +50,9 @@ ThrowCompletionOr ArgumentsObject::internal_get(PropertyKey const& proper return Object::internal_get(property_key, receiver, cacheable_metadata, phase); } - // FIXME: a. Assert: map contains a formal parameter mapping for P. - + // a. Assert: map contains a formal parameter mapping for P. // b. Return ! Get(map, P). - return MUST(map.get(property_key)); + return get_from_parameter_map(property_key); } // 10.4.4.4 [[Set]] ( P, V, Receiver ), https://tc39.es/ecma262/#sec-arguments-exotic-objects-set-p-v-receiver @@ -65,15 +67,14 @@ ThrowCompletionOr ArgumentsObject::internal_set(PropertyKey const& propert } else { // a. Let map be args.[[ParameterMap]]. // b. Let isMapped be ! HasOwnProperty(map, P). - is_mapped = MUST(parameter_map().has_own_property(property_key)); + is_mapped = parameter_map_has(property_key); } // 3. If isMapped is true, then if (is_mapped) { // a. Assert: The following Set will succeed, since formal parameters mapped by arguments objects are always writable. - // b. Perform ! Set(map, P, V, false). - MUST(m_parameter_map->set(property_key, value, Object::ShouldThrowExceptions::No)); + set_in_parameter_map(property_key, value); } // 4. Return ? OrdinarySet(args, P, V, Receiver). @@ -84,10 +85,8 @@ ThrowCompletionOr ArgumentsObject::internal_set(PropertyKey const& propert ThrowCompletionOr ArgumentsObject::internal_delete(PropertyKey const& property_key) { // 1. Let map be args.[[ParameterMap]]. - auto& map = parameter_map(); - // 2. Let isMapped be ! HasOwnProperty(map, P). - bool is_mapped = MUST(map.has_own_property(property_key)); + bool is_mapped = parameter_map_has(property_key); // 3. Let result be ? OrdinaryDelete(args, P). bool result = TRY(Object::internal_delete(property_key)); @@ -95,7 +94,7 @@ ThrowCompletionOr ArgumentsObject::internal_delete(PropertyKey const& prop // 4. If result is true and isMapped is true, then if (result && is_mapped) { // a. Perform ! map.[[Delete]](P). - MUST(map.internal_delete(property_key)); + delete_from_parameter_map(property_key); } // 5. Return result. @@ -114,12 +113,12 @@ ThrowCompletionOr> ArgumentsObject::internal_get_ow // 3. Let map be args.[[ParameterMap]]. // 4. Let isMapped be ! HasOwnProperty(map, P). - bool is_mapped = MUST(m_parameter_map->has_own_property(property_key)); + bool is_mapped = parameter_map_has(property_key); // 5. If isMapped is true, then if (is_mapped) { // a. Set desc.[[Value]] to ! Get(map, P). - desc->value = MUST(m_parameter_map->get(property_key)); + desc->value = get_from_parameter_map(property_key); } // 6. Return desc. @@ -130,10 +129,8 @@ ThrowCompletionOr> ArgumentsObject::internal_get_ow ThrowCompletionOr ArgumentsObject::internal_define_own_property(PropertyKey const& property_key, PropertyDescriptor const& descriptor, Optional* precomputed_get_own_property) { // 1. Let map be args.[[ParameterMap]]. - auto& map = parameter_map(); - // 2. Let isMapped be ! HasOwnProperty(map, P). - bool is_mapped = MUST(map.has_own_property(property_key)); + bool is_mapped = parameter_map_has(property_key); // 3. Let newArgDesc be Desc. auto new_arg_desc = descriptor; @@ -145,7 +142,7 @@ ThrowCompletionOr ArgumentsObject::internal_define_own_property(PropertyKe // i. Set newArgDesc to a copy of Desc. new_arg_desc = descriptor; // ii. Set newArgDesc.[[Value]] to ! Get(map, P). - new_arg_desc.value = MUST(map.get(property_key)); + new_arg_desc.value = get_from_parameter_map(property_key); } } @@ -161,19 +158,19 @@ ThrowCompletionOr ArgumentsObject::internal_define_own_property(PropertyKe // a. If IsAccessorDescriptor(Desc) is true, then if (descriptor.is_accessor_descriptor()) { // i. Perform ! map.[[Delete]](P). - MUST(map.internal_delete(property_key)); + delete_from_parameter_map(property_key); } else { // i. If Desc has a [[Value]] field, then if (descriptor.value.has_value()) { // 1. Assert: The following Set will succeed, since formal parameters mapped by arguments objects are always writable. // 2. Perform ! Set(map, P, Desc.[[Value]], false). - MUST(map.set(property_key, descriptor.value.value(), Object::ShouldThrowExceptions::No)); + set_in_parameter_map(property_key, descriptor.value.value()); } // ii. If Desc has a [[Writable]] field and Desc.[[Writable]] is false, then if (descriptor.writable == false) { // 1. Perform ! map.[[Delete]](P). - MUST(map.internal_delete(property_key)); + delete_from_parameter_map(property_key); } } } @@ -182,4 +179,19 @@ ThrowCompletionOr ArgumentsObject::internal_define_own_property(PropertyKe return true; } +void ArgumentsObject::delete_from_parameter_map(PropertyKey const& property_key) +{ + m_mapped_names[property_key.as_number()] = FlyString {}; +} + +Value ArgumentsObject::get_from_parameter_map(PropertyKey const& property_key) const +{ + return MUST(m_environment->get_binding_value(vm(), m_mapped_names[property_key.as_number()], false)); +} + +void ArgumentsObject::set_in_parameter_map(PropertyKey const& property_key, Value value) +{ + MUST(m_environment->set_mutable_binding(vm(), m_mapped_names[property_key.as_number()], value, false)); +} + } diff --git a/Libraries/LibJS/Runtime/ArgumentsObject.h b/Libraries/LibJS/Runtime/ArgumentsObject.h index 4c7e34df4fd..b6cafebec41 100644 --- a/Libraries/LibJS/Runtime/ArgumentsObject.h +++ b/Libraries/LibJS/Runtime/ArgumentsObject.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Andreas Kling + * Copyright (c) 2021-2025, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ @@ -20,24 +20,26 @@ public: virtual void initialize(Realm&) override; virtual ~ArgumentsObject() override = default; - Environment& environment() { return m_environment; } - virtual ThrowCompletionOr> internal_get_own_property(PropertyKey const&) const override; virtual ThrowCompletionOr internal_define_own_property(PropertyKey const&, PropertyDescriptor const&, Optional* precomputed_get_own_property = nullptr) override; virtual ThrowCompletionOr internal_get(PropertyKey const&, Value receiver, CacheablePropertyMetadata*, PropertyLookupPhase) const override; virtual ThrowCompletionOr internal_set(PropertyKey const&, Value value, Value receiver, CacheablePropertyMetadata*, PropertyLookupPhase) override; virtual ThrowCompletionOr internal_delete(PropertyKey const&) override; - // [[ParameterMap]] - Object& parameter_map() { return *m_parameter_map; } + void set_mapped_names(Vector mapped_names) { m_mapped_names = move(mapped_names); } private: ArgumentsObject(Realm&, Environment&); + [[nodiscard]] bool parameter_map_has(PropertyKey const&) const; + [[nodiscard]] Value get_from_parameter_map(PropertyKey const&) const; + void set_in_parameter_map(PropertyKey const&, Value); + void delete_from_parameter_map(PropertyKey const&); + virtual void visit_edges(Cell::Visitor&) override; GC::Ref m_environment; - GC::Ptr m_parameter_map; + Vector m_mapped_names; }; }