From 1c3eef531762659ef8acf8160e4f9b55d5644a62 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Tue, 6 Apr 2021 21:39:17 +0200 Subject: [PATCH] LibJS: Use MarkedValueList for internal own properties getter functions Letting these create and return a JS::Array directly is pretty awkward since we then need to go through the indexed properties for iteration. Just use a MarkedValueList (i.e. Vector) for this and add a new Array::create_from() function to turn the Vector into a returnable Array as we did before. This brings it a lot closer to the spec as well, which uses the CreateArrayFromList abstract operation to do exactly this. There's an optimization opportunity for the future here, since we know the Vector's size we could prepare the newly created Array accordingly, e.g. by switching to generic storage upfront if needed. --- Userland/Libraries/LibJS/AST.cpp | 4 +- Userland/Libraries/LibJS/Runtime/Array.cpp | 9 ++++ Userland/Libraries/LibJS/Runtime/Array.h | 1 + Userland/Libraries/LibJS/Runtime/Object.cpp | 41 ++++++++----------- Userland/Libraries/LibJS/Runtime/Object.h | 4 +- .../LibJS/Runtime/ObjectConstructor.cpp | 6 +-- .../Libraries/LibJS/Runtime/ReflectObject.cpp | 3 +- 7 files changed, 37 insertions(+), 31 deletions(-) diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 2eace27df2..0ce0d456b3 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -480,8 +480,8 @@ Value ForInStatement::execute(Interpreter& interpreter, GlobalObject& global_obj auto* object = rhs_result.to_object(global_object); while (object) { auto property_names = object->get_enumerable_own_property_names(Object::PropertyKind::Key); - for (auto& property_name : property_names.as_object().indexed_properties()) { - interpreter.vm().set_variable(variable_name, property_name.value_and_attributes(object).value, global_object, has_declaration); + for (auto& value : property_names) { + interpreter.vm().set_variable(variable_name, value, global_object, has_declaration); if (interpreter.exception()) return {}; last_value = interpreter.execute_statement(global_object, *m_body).value_or(last_value); diff --git a/Userland/Libraries/LibJS/Runtime/Array.cpp b/Userland/Libraries/LibJS/Runtime/Array.cpp index c04a65c1d8..27f6c835af 100644 --- a/Userland/Libraries/LibJS/Runtime/Array.cpp +++ b/Userland/Libraries/LibJS/Runtime/Array.cpp @@ -36,6 +36,15 @@ Array* Array::create(GlobalObject& global_object) return global_object.heap().allocate(global_object, *global_object.array_prototype()); } +// 7.3.17 CreateArrayFromList, https://tc39.es/ecma262/#sec-createarrayfromlist +Array* Array::create_from(GlobalObject& global_object, const Vector& values) +{ + auto* array = Array::create(global_object); + for (size_t i = 0; i < values.size(); ++i) + array->define_property(i, values[i]); + return array; +} + Array::Array(Object& prototype) : Object(prototype) { diff --git a/Userland/Libraries/LibJS/Runtime/Array.h b/Userland/Libraries/LibJS/Runtime/Array.h index 5beeefa809..7167e17e48 100644 --- a/Userland/Libraries/LibJS/Runtime/Array.h +++ b/Userland/Libraries/LibJS/Runtime/Array.h @@ -35,6 +35,7 @@ class Array : public Object { public: static Array* create(GlobalObject&); + static Array* create_from(GlobalObject&, const Vector&); explicit Array(Object& prototype); virtual void initialize(GlobalObject&) override; diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index 57a0e51b34..5bc2f27322 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/Userland/Libraries/LibJS/Runtime/Object.cpp @@ -187,9 +187,9 @@ Value Object::get_own_property(const PropertyName& property_name, Value receiver return value_here; } -Value Object::get_own_properties(PropertyKind kind, bool only_enumerable_properties, GetOwnPropertyReturnType return_type) const +MarkedValueList Object::get_own_properties(PropertyKind kind, bool only_enumerable_properties, GetOwnPropertyReturnType return_type) const { - auto* properties_array = Array::create(global_object()); + MarkedValueList properties(heap()); // FIXME: Support generic iterables if (is(*this)) { @@ -197,54 +197,51 @@ Value Object::get_own_properties(PropertyKind kind, bool only_enumerable_propert for (size_t i = 0; i < str.length(); ++i) { if (kind == PropertyKind::Key) { - properties_array->define_property(i, js_string(vm(), String::number(i))); + properties.append(js_string(vm(), String::number(i))); } else if (kind == PropertyKind::Value) { - properties_array->define_property(i, js_string(vm(), String::formatted("{:c}", str[i]))); + properties.append(js_string(vm(), String::formatted("{:c}", str[i]))); } else { auto* entry_array = Array::create(global_object()); entry_array->define_property(0, js_string(vm(), String::number(i))); entry_array->define_property(1, js_string(vm(), String::formatted("{:c}", str[i]))); - properties_array->define_property(i, entry_array); + properties.append(entry_array); } if (vm().exception()) - return {}; + return MarkedValueList { heap() }; } - return properties_array; + return properties; } - size_t property_index = 0; for (auto& entry : m_indexed_properties) { auto value_and_attributes = entry.value_and_attributes(const_cast(this)); if (only_enumerable_properties && !value_and_attributes.attributes.is_enumerable()) continue; if (kind == PropertyKind::Key) { - properties_array->define_property(property_index, js_string(vm(), String::number(entry.index()))); + properties.append(js_string(vm(), String::number(entry.index()))); } else if (kind == PropertyKind::Value) { - properties_array->define_property(property_index, value_and_attributes.value); + properties.append(value_and_attributes.value); } else { auto* entry_array = Array::create(global_object()); entry_array->define_property(0, js_string(vm(), String::number(entry.index()))); entry_array->define_property(1, value_and_attributes.value); - properties_array->define_property(property_index, entry_array); + properties.append(entry_array); } if (vm().exception()) - return {}; - - ++property_index; + return MarkedValueList { heap() }; } auto add_property_to_results = [&](auto& property) { if (kind == PropertyKind::Key) { - properties_array->define_property(property_index, property.key.to_value(vm())); + properties.append(property.key.to_value(vm())); } else if (kind == PropertyKind::Value) { - properties_array->define_property(property_index, get(property.key)); + properties.append(get(property.key)); } else { auto* entry_array = Array::create(global_object()); entry_array->define_property(0, property.key.to_value(vm())); entry_array->define_property(1, get(property.key)); - properties_array->define_property(property_index, entry_array); + properties.append(entry_array); } }; @@ -258,8 +255,7 @@ Value Object::get_own_properties(PropertyKind kind, bool only_enumerable_propert continue; add_property_to_results(it); if (vm().exception()) - return {}; - ++property_index; + return MarkedValueList { heap() }; } } if (return_type == GetOwnPropertyReturnType::All || return_type == GetOwnPropertyReturnType::SymbolOnly) { @@ -270,16 +266,15 @@ Value Object::get_own_properties(PropertyKind kind, bool only_enumerable_propert continue; add_property_to_results(it); if (vm().exception()) - return {}; - ++property_index; + return MarkedValueList { heap() }; } } - return properties_array; + return properties; } // 7.3.23 EnumerableOwnPropertyNames, https://tc39.es/ecma262/#sec-enumerableownpropertynames -Value Object::get_enumerable_own_property_names(PropertyKind kind) const +MarkedValueList Object::get_enumerable_own_property_names(PropertyKind kind) const { return get_own_properties(kind, true, Object::GetOwnPropertyReturnType::StringOnly); } diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index 9ee5ac4c1e..e72e0cc705 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -97,8 +97,8 @@ public: virtual bool put(const PropertyName&, Value, Value receiver = {}); Value get_own_property(const PropertyName&, Value receiver) const; - Value get_own_properties(PropertyKind, bool only_enumerable_properties = false, GetOwnPropertyReturnType = GetOwnPropertyReturnType::All) const; - Value get_enumerable_own_property_names(PropertyKind) const; + MarkedValueList get_own_properties(PropertyKind, bool only_enumerable_properties = false, GetOwnPropertyReturnType = GetOwnPropertyReturnType::All) const; + MarkedValueList get_enumerable_own_property_names(PropertyKind) const; virtual Optional get_own_property_descriptor(const PropertyName&) const; Value get_own_property_descriptor_object(const PropertyName&) const; diff --git a/Userland/Libraries/LibJS/Runtime/ObjectConstructor.cpp b/Userland/Libraries/LibJS/Runtime/ObjectConstructor.cpp index 2551b10d3c..e9ab856b11 100644 --- a/Userland/Libraries/LibJS/Runtime/ObjectConstructor.cpp +++ b/Userland/Libraries/LibJS/Runtime/ObjectConstructor.cpp @@ -209,7 +209,7 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::keys) if (vm.exception()) return {}; - return obj_arg->get_enumerable_own_property_names(PropertyKind::Key); + return Array::create_from(global_object, obj_arg->get_enumerable_own_property_names(PropertyKind::Key)); } JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::values) @@ -222,7 +222,7 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::values) if (vm.exception()) return {}; - return obj_arg->get_enumerable_own_property_names(PropertyKind::Value); + return Array::create_from(global_object, obj_arg->get_enumerable_own_property_names(PropertyKind::Value)); } JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::entries) @@ -235,7 +235,7 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::entries) if (vm.exception()) return {}; - return obj_arg->get_enumerable_own_property_names(PropertyKind::KeyAndValue); + return Array::create_from(global_object, obj_arg->get_enumerable_own_property_names(PropertyKind::KeyAndValue)); } } diff --git a/Userland/Libraries/LibJS/Runtime/ReflectObject.cpp b/Userland/Libraries/LibJS/Runtime/ReflectObject.cpp index 434fa65fb7..7e936abb7c 100644 --- a/Userland/Libraries/LibJS/Runtime/ReflectObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/ReflectObject.cpp @@ -25,6 +25,7 @@ */ #include +#include #include #include #include @@ -235,7 +236,7 @@ JS_DEFINE_NATIVE_FUNCTION(ReflectObject::own_keys) auto* target = get_target_object_from(global_object, "ownKeys"); if (!target) return {}; - return target->get_own_properties(PropertyKind::Key); + return Array::create_from(global_object, target->get_own_properties(PropertyKind::Key)); } JS_DEFINE_NATIVE_FUNCTION(ReflectObject::prevent_extensions)