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<Value>) 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

@@ -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);

Userland/Libraries/LibJS/Runtime/Array.cpp

@@ -36,6 +36,15 @@ Array* Array::create(GlobalObject& global_object)
     return global_object.heap().allocate<Array>(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<Value>& 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)
 {

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<Value>&);
 
     explicit Array(Object& prototype);
     virtual void initialize(GlobalObject&) override;

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<StringObject>(*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<Object*>(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);
 }

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<PropertyDescriptor> get_own_property_descriptor(const PropertyName&) const;
     Value get_own_property_descriptor_object(const PropertyName&) const;
 

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));
 }
 
 }

Userland/Libraries/LibJS/Runtime/ReflectObject.cpp

@@ -25,6 +25,7 @@
  */
 
 #include <AK/Function.h>
+#include <LibJS/Runtime/Array.h>
 #include <LibJS/Runtime/Error.h>
 #include <LibJS/Runtime/Function.h>
 #include <LibJS/Runtime/GlobalObject.h>
@@ -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)